eudaimos opened a new issue, #42580:
URL: https://github.com/apache/superset/issues/42580
### Bug
`LoggingMiddleware._is_error_response` in
`superset/mcp_service/middleware.py` decides whether a tool call succeeded by
substring-matching the serialized response:
```python
def _is_error_response(self, result: ToolResult) -> bool:
"""Check if a tool result contains an error schema response.
MCP tools return error schemas (ChartError, DashboardError, etc.)
instead of raising exceptions. These serialize to JSON containing
an "error_type" field.
"""
try:
return '"error_type"' in result.content[0].text
except (AttributeError, IndexError):
return False
```
This is used to set the `success` field on the `mcp_tool_call` audit event
and log line:
```python
result = await call_next(context)
success = not self._is_error_response(result)
```
The check tests for the *presence of the key*, not for a non-null value.
`ExecuteSqlResponse` includes `error` and `error_type` as nullable fields that
are always serialized, so a fully successful `execute_sql` response contains
`"error_type":null` — and is therefore logged as a failure.
### Reproduction
Four tool calls in a single authenticated session against Superset 6.1.0,
all of which returned correct results to the client:
| Tool | Response contains `"error_type"` | Logged `success=` |
|---|---|---|
| `health_check` | no | `True` |
| `get_instance_info` | no | `True` |
| `list_databases` | no | `True` |
| `execute_sql` | yes (`"error_type":null`) | **`False`** |
The `execute_sql` call returned real data:
```json
{"success":true,"rows":[{"database":"bronze_layer","tables":161}, ...],
"row_count":12,"error":null,"error_type":null}
```
while the server logged:
```
INFO:superset.mcp_service.middleware:MCP tool call: tool=execute_sql,
user_id=2, method=tools/call, duration_ms=195, success=False
```
Note the tool's own payload says `"success":true` and `"error_type":null` in
the same response the middleware classifies as an error.
It also contradicts the tool's own progress logging in the same request:
```
DEBUG Sending INFO to client: SQL execution completed successfully:
rows_returned=1, execution_time=7.04
INFO:superset.mcp_service.middleware:MCP tool call: tool=execute_sql, ...,
duration_ms=7098, success=False
```
### Impact
`success` is wrong for every tool whose response schema declares a nullable
`error_type`, which includes `execute_sql` — likely the single most-used tool
in the service. Every successful SQL execution is recorded as a failure in the
`logs` table and the Action Log UI.
Combined with #42579 (error events never written at all), MCP observability
is inverted end to end: real failures are absent, and successes are recorded as
failures. Any dashboard or alert built on `mcp_tool_call.success` reports a
~100% failure rate for `execute_sql`.
This actively misleads operators triaging a report. While diagnosing an
unrelated user issue we used `success=` to distinguish working calls from
broken ones, and it disagreed with both the tool's own response payload and its
progress log.
### Suggested fix
Inspect the parsed value rather than substring-matching the key:
```python
def _is_error_response(self, result: ToolResult) -> bool:
try:
payload = json.loads(result.content[0].text)
except (AttributeError, IndexError, ValueError):
return False
if not isinstance(payload, dict):
return False
return payload.get("error_type") is not None
```
A more robust option is to have the tools signal errors structurally — e.g.
check `isinstance` against the error schema types before serialization, or set
`ToolResult.meta` — rather than inferring status from response text at all.
### Environment
- Superset 6.1.0 (`apache/superset:6.1.0`)
- fastmcp 3.4.3
- Python 3.10.20, Linux
- Transport: streamable-http
--
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]