eschutho opened a new pull request, #44171:
URL: https://github.com/apache/superset/pull/44171

   ### SUMMARY
   
   **Sentry issue**: https://preset-inc.sentry.io/issues/7450117832/ — 552 
events, firstSeen 2026-04-29, lastSeen 2026-09-04.
   **Shortcut story**: https://app.shortcut.com/preset/story/120340
   
   **Root cause**: The `call_tool` proxy in `_make_normalizing_call_tool` 
(`superset/mcp_service/server.py`) raises `ToolError` when an LLM/MCP client 
attempts to call synthetic search-tool names (`search_tools`, `call_tool`) via 
the proxy. This is a user/LLM misuse pattern (400-class), not a system failure.
   
   However, `ToolError` inherits from `FastMCPError`, which defaults 
`log_level=logging.ERROR`. FastMCP's own `call_tool` handler catches 
`FastMCPError` and calls `logger.log(e.log_level, ...)` — logging at ERROR — 
*before* Superset's `GlobalErrorHandlerMiddleware` ever sees the exception. 
Sentry's `LoggingIntegration(event_level=logging.ERROR)` then promotes this log 
line to a Sentry event, bypassing Superset's middleware classification that 
correctly identifies `ToolError` as a user error (WARNING, no Sentry forwarding 
via `MCP_ERROR_HOOK`).
   
   **Fix**: Pass `log_level=logging.WARNING` explicitly on this one `ToolError` 
raise. This is FastMCP's first-class, documented mechanism for per-exception 
log severity. It aligns with Superset's existing `_USER_ERROR_TYPES` 
classification in `middleware.py` and requires no changes to Sentry config or 
logger suppression.
   
   ### Tradeoffs
   
   No behavior change. The MCP response returned to the client (the `ToolError` 
message and error semantics) is completely unchanged. Only the log severity 
changes — from ERROR to WARNING — which prevents this 
expected-in-normal-operation error from reaching Sentry via the 
`LoggingIntegration`. Nothing else in the codebase keys off this `ToolError`'s 
log level or relies on it reaching Sentry.
   
   ### TESTING INSTRUCTIONS
   
   ```bash
   python -m pytest 
tests/unit_tests/mcp_service/test_tool_search_transform.py::test_call_tool_proxy_rejects_synthetic_names_with_warning_log_level
 -xvs
   ```
   
   The test:
   - Builds the BM25 search transform via `_apply_tool_search_transform`
   - Gets the `call_tool` Tool from `transform._make_call_tool()`
   - Calls it with `name="search_tools"` and `name="call_tool"` (both synthetic 
names)
   - Asserts the raised `ToolError` has `log_level == logging.WARNING`
   - Verified non-vacuous: test fails without the fix (asserts ERROR ≠ WARNING)
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: Fixes SUPERSET-PYTHON-12QC
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


-- 
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]

Reply via email to