aminghadersohi commented on code in PR #41921:
URL: https://github.com/apache/superset/pull/41921#discussion_r3649029429
##########
superset/mcp_service/middleware.py:
##########
@@ -456,8 +615,38 @@ async def on_call_tool(
# GlobalErrorHandlerMiddleware, ValueError, TypeError, etc. —
# will cause encoding failures on the wire.
mcp_call_id = _mcp_call_id_var.get(None)
+ # This is the documented "must never propagate" point, but
+ # formatting/sanitizing both call str(e) — a pathological
+ # __str__ would make this handler itself raise past the
+ # middleware chain. Fall back to the exception class name.
+ try:
+ error_text = f"Error: {e}"
+ sanitized_message = _sanitize_error_for_logging(e)
Review Comment:
Good catch — fixed in 38dbdbd80b8e029e6eb27d88a4d26d68df8d19cd. The
last-resort client-facing text now runs through `_sanitize_error_for_logging`
(the same sanitizer `GlobalErrorHandlerMiddleware` and every other error path
already use), so an exception that bypasses the primary handler can't leak
connection strings/tokens/SQL to the caller. Kept the guarded fallback to
`type(e).__name__` for a pathological `__str__`. Added a regression test
asserting an embedded `postgresql://user:pass@host/db` is redacted from the
response.
--
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]
