eudaimos opened a new issue, #42579:
URL: https://github.com/apache/superset/issues/42579
### Bug
`GlobalErrorHandlerMiddleware._handle_error` in
`superset/mcp_service/middleware.py` calls `event_logger.log()` without three
arguments that `DBEventLogger.log()` requires:
```python
try:
event_logger.log(
user_id=user_id,
action="mcp_tool_error",
duration_ms=duration_ms,
curated_payload={
"tool": tool_name,
"error_type": type(error).__name__,
"error_message": str(error),
"method": context.method,
},
)
except Exception as log_error:
logger.warning("Failed to log error event: %s", log_error)
```
`DBEventLogger.log` signature (confirmed via `inspect.signature` on 6.1.0):
```
(self, user_id: 'int | None', action: 'str', dashboard_id: 'int | None',
duration_ms: 'int | None', slice_id: 'int | None', referrer: 'str | None',
*args: 'Any', **kwargs: 'Any') -> 'None'
```
`dashboard_id`, `slice_id` and `referrer` have no defaults, so the call
raises `TypeError` every time. The surrounding `except` swallows it into a
`WARNING`, so it fails silently.
`LoggingMiddleware.on_call_tool` in the same file does it correctly and
shows the intended call shape:
```python
event_logger.log(
user_id=user_id,
action="mcp_tool_call",
dashboard_id=dashboard_id,
duration_ms=duration_ms,
slice_id=slice_id,
referrer=None,
curated_payload={...},
)
```
### Reproduction
Trigger any MCP tool error (e.g. call any tool with invalid arguments) on a
deployment using the default `DBEventLogger`. Observed on every error in our
logs:
```
WARNING:superset.mcp_service.middleware:Failed to log error event:
DBEventLogger.log() missing 3 required positional arguments: 'dashboard_id',
'slice_id', and 'referrer'
```
### Impact
No `mcp_tool_error` event is ever written to the `logs` table / Action Log
UI on a default configuration. MCP tool failures are invisible to Superset's
own audit and analytics surfaces — you only find them by reading container
logs. This makes it impossible to monitor MCP error rates through the mechanism
Superset provides for exactly that, and it defeats the stated purpose of the
middleware ("proper error logging for all MCP tool calls").
It also means the one code path that fires whenever something has *already*
gone wrong is itself broken, so failures are least observable exactly when
observability matters most.
### Suggested fix
Pass the three required arguments, mirroring `LoggingMiddleware`:
```python
event_logger.log(
user_id=user_id,
action="mcp_tool_error",
dashboard_id=None,
duration_ms=duration_ms,
slice_id=None,
referrer=None,
curated_payload={...},
)
```
`_extract_context_info` already derives `dashboard_id` / `slice_id` from the
tool params, so those could be passed through instead of `None` to make error
events as useful as call events.
Separately, `logger.warning` for a swallowed `TypeError` in the logging path
is easy to miss — this bug appears to have gone unnoticed despite firing on
every single MCP error. Raising or logging at `error` level would surface
programming errors here rather than hiding them.
### Environment
- Superset 6.1.0 (`apache/superset:6.1.0`)
- fastmcp 3.4.3
- Python 3.10.20, Linux
- Default `DBEventLogger` (no custom `EVENT_LOGGER` configured)
--
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]
