Copilot commented on code in PR #44387:
URL: https://github.com/apache/superset/pull/44387#discussion_r4036337529
##########
superset/mcp_service/middleware.py:
##########
@@ -165,6 +177,105 @@ def _invoke_error_hook(error: Exception, hook_context:
dict[str, Any]) -> None:
logger.warning("MCP_ERROR_HOOK raised an exception: %s", hook_error)
+def _unwrap_tool_error(error: Exception) -> Exception:
+ """Return the original exception behind FastMCP's ``ToolError`` wrapper.
+
+ FastMCP catches every non-``FastMCPError`` raised inside a tool body and
+ re-raises it as ``ToolError(f"Error calling tool {name!r}: {e}") from e``
+ *before* any middleware error hook runs (see ``FastMCP._call_tool``). By
+ the time :class:`GlobalErrorHandlerMiddleware` sees a tool failure, the
+ concrete type — ``MCPPermissionDeniedError``, ``SupersetException``,
+ ``OperationalError`` — is no longer the exception itself, only its
+ ``__cause__``. Classifying the wrapper instead of the cause collapses
+ every distinct failure into one undifferentiated message.
+
+ A ``ToolError`` raised deliberately by tool code is re-raised by FastMCP
+ untouched and therefore carries no ``__cause__``; it is already formatted
+ for MCP and is returned as-is.
+ """
+ if isinstance(error, ToolError) and error.__cause__ is not None:
+ cause = error.__cause__
+ if isinstance(cause, Exception):
+ return cause
+ return error
+
+
+# Exception classes that mean "the query behind this tool failed", not "the
+# caller used the tool wrong". The tool name and arguments were valid; the
+# datasource, table, column, or connection it reads is broken or gone.
+_DATASOURCE_ERROR_EXCEPTIONS = (
+ ColumnNotFoundException,
+ DatabaseNotFound,
+ QueryObjectValidationError,
+ SupersetGenericDBErrorException,
Review Comment:
`QueryObjectValidationError` is not specific to a datasource: Superset
raises it for missing/invalid query fields and invalid result types (for
example, `common/query_actions.py:144-147` and `:405-408`). Because this tuple
is checked before the generic `SupersetException` branch, those
caller/configuration errors will be reported as “the tool name and arguments
were valid” and blamed on the datasource. Remove this class from the datasource
exception tuple; only classify it by an explicit datasource error type if that
distinction is needed.
This issue also appears in the following locations of the same file:
- line 214
- line 223
- line 1219
--
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]
