fix(mcp): successful execute_sql calls logged as success=False — _is_error_response substring-matches "error_type" including when null #42580

Bug

LoggingMiddleware._is_error_response in superset/mcp_service/middleware.py decides whether a tool call succeeded by substring-matching the serialized response:

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:

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:

The execute_sql call returned real data:

{"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:

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