eudaimos opened a new issue, #42578:
URL: https://github.com/apache/superset/issues/42578
### Bug
`GlobalErrorHandlerMiddleware` in `superset/mcp_service/middleware.py`
imports pydantic's `ValidationError`:
```python
from pydantic import ValidationError # line 29
```
and dispatches on it:
```python
elif isinstance(error, ValidationError):
# Pydantic validation errors
validation_details = []
for err in error.errors():
field = " -> ".join(str(loc) for loc in err["loc"])
validation_details.append(f"{field}: {err['msg']}")
raise ToolError(
f"Validation error in {tool_name}: {'; '.join(validation_details)}"
) from error
```
But FastMCP 3.x raises its **own** `fastmcp.exceptions.ValidationError` when
tool arguments fail to validate — it is not a subclass of pydantic's, and not a
subclass of `ToolError`:
```
fastmcp.ValidationError MRO: ['ValidationError', 'FastMCPError',
'Exception', 'BaseException', 'object']
is subclass of ToolError: False
is subclass of pydantic.ValidationError: False
```
So the `isinstance` check never matches. Every malformed-arguments call
falls through to the generic branch:
```python
else:
# Generic internal errors
error_id = f"err_{int(time.time())}"
logger.error("Unexpected error [%s] in %s: %s", error_id, tool_name,
error)
raise ToolError(
f"Internal error in {tool_name}: An unexpected error occurred. "
f"Error ID: {error_id}. Please contact support if this persists."
) from error
```
A client-side 400-class error is reported to the client as an opaque
internal server error.
### Reproduction
Confirmed in-process against a running MCP service (Superset 6.1.0, fastmcp
3.4.3, pydantic 2.11.7). Calling `execute_sql` with arguments that omit the
required `request` wrapper:
```python
t = await mcp.get_tool('execute_sql')
try:
await t.run({'database_id': '1', 'sql': 'SHOW TABLES'})
except Exception as e:
print(type(e).__module__ + '.' + type(e).__name__)
print(isinstance(e, PydanticValidationError))
print(isinstance(e, FastMCPValidationError))
```
Output:
```
fastmcp.exceptions.ValidationError
False
True
```
Resulting server log — note `error_type=ValidationError` is logged, then the
*same* `_handle_error` invocation falls through to the generic branch:
```
ERROR:superset.mcp_service.middleware:MCP tool error: tool=execute_sql,
user_id=17, duration_ms=2, error_type=ValidationError, error=3 validation
errors for call[execute_sql]
request
Missing required argument [type=missing_argument,
input_value={'database_id': '1', 'sql': 'SHOW TABLES'}, input_type=dict]
ERROR:superset.mcp_service.middleware:Unexpected error [err_1785349449] in
execute_sql: 3 validation errors for call[execute_sql]
```
What the client receives:
```
Internal error in execute_sql: An unexpected error occurred. Error ID:
err_1785349449. Please contact support if this persists.
```
### Impact
This is actively harmful with LLM clients, which are the primary consumers
of this service. In our case a user's agent made three malformed calls
(`execute_sql`, `list_datasets`, `list_dashboards`), received three "Internal
error… contact support" responses, observed that `health_check` still passed
(it is the only tool taking no arguments), and concluded:
> Health check passes, but every data endpoint still returns internal server
errors — it looks like a backend outage on the Superset MCP's data layer, not
something I can work around from here.
It then stopped retrying and the user escalated it as an outage. The service
was entirely healthy.
The `ValidationError` branch that already exists would have returned:
```
Validation error in execute_sql: request: Missing required argument
```
which is self-correcting — an LLM client reading that will retry with the
wrapper. Correct error classification is the difference between a client
recovering on its own and a false outage report.
### Suggested fix
Match both exception types:
```python
from fastmcp.exceptions import ToolError, ValidationError as
FastMCPValidationError
from pydantic import ValidationError as PydanticValidationError
...
elif isinstance(error, (PydanticValidationError, FastMCPValidationError)):
```
`error.errors()` only exists on the pydantic exception, so the
detail-formatting loop needs to be guarded —
`fastmcp.exceptions.ValidationError` carries its message in `str(error)`, which
already contains the per-field pydantic detail (as visible in the log above).
Since `fastmcp.exceptions.ValidationError` derives from `FastMCPError`, it
may be worth handling `FastMCPError` generally so future FastMCP exception
types don't silently land in the internal-error branch too.
### Environment
- Superset 6.1.0 (`apache/superset:6.1.0`)
- fastmcp 3.4.3
- pydantic 2.11.7
- 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]
