rusackas opened a new pull request, #42730:
URL: https://github.com/apache/superset/pull/42730
### SUMMARY
TDD PR for #42579. `GlobalErrorHandlerMiddleware._handle_error` in
`superset/mcp_service/middleware.py` calls:
```python
event_logger.log(
user_id=user_id,
action="mcp_tool_error",
duration_ms=duration_ms,
curated_payload={...},
)
```
`DBEventLogger.log()` requires `dashboard_id`, `slice_id`, and `referrer` as
well — none have defaults. So this call raises `TypeError` on every single MCP
tool error, silently swallowed by the surrounding `except Exception:
logger.warning(...)`. No `mcp_tool_error` event is ever written to the `logs`
table on a default configuration — the error-observability path is itself
broken.
Every existing test for this middleware patches `event_logger` with a bare
`MagicMock`, which accepts any kwargs and can't catch a real signature
mismatch. This test uses the real `DBEventLogger` instead (with the DB session
calls stubbed out, so it doesn't need a live database) to actually exercise the
broken argument binding.
This PR is test-only, encoding the expected behavior (the error event should
reach the event logger). It's currently **red**:
```
FAILED
tests/unit_tests/mcp_service/test_middleware.py::TestGlobalErrorHandlerLogLevels::test_error_event_reaches_the_real_event_logger
```
reproducing the exact error from the issue report:
```
Failed to log error event: DBEventLogger.log() missing 3 required positional
arguments: 'dashboard_id', 'slice_id', and 'referrer'
```
Confirms the bug is live; does not fix it.
### TESTING INSTRUCTIONS
`pytest
tests/unit_tests/mcp_service/test_middleware.py::TestGlobalErrorHandlerLogLevels::test_error_event_reaches_the_real_event_logger
-v`
### ADDITIONAL INFORMATION
- [x] Has associated issue: #42579
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
