eschutho opened a new pull request, #42441: URL: https://github.com/apache/superset/pull/42441
### SUMMARY Fixes two Sentry issues in org `preset-inc` caused by the same underlying incident: - [SUPERSET-PYTHON-XVB](https://preset-inc.sentry.io/issues/SUPERSET-PYTHON-XVB) — 2413 events, first seen 2025-10-16, still firing daily, 0 users impacted - [SUPERSET-PYTHON-YYS](https://preset-inc.sentry.io/issues/SUPERSET-PYTHON-YYS) — 2428 events, first seen 2026-01-18, still firing daily, 0 users impacted **Root cause:** `superset/mcp_service/` runs a FastMCP server (the `/mcp` service, port 5008) on top of the third-party `mcp` SDK. When an MCP client disconnects mid-request — a cancelled or timed-out tool call, normal client behavior and not a Superset bug — the SDK's own code logs it at ERROR with a full traceback, from a single incident, twice: 1. `mcp/server/streamable_http.py:_handle_post_request` catches `starlette.requests.ClientDisconnect` (raised from `await request.body()`) in a broad `except Exception as err:` and calls `logger.exception("Error handling POST request")` on logger `mcp.server.streamable_http`. 2. That same except block also does `await writer.send(Exception(err))`, pushing a bare-wrapped exception onto the session's read stream. The low-level server's message loop (`mcp/server/lowlevel/server.py:_handle_message`) receives it and logs `logger.error(f"Received exception from stream: {message}")` on logger `mcp.server.lowlevel.server`. Since `ClientDisconnect` is always raised with zero args, `str(ClientDisconnect())` is always `""`, so this line always renders as the literal string `"Received exception from stream: "` — matching the observed Sentry event text exactly. Sampled events for both issues share the same trace_id/request_id/session_id, confirming a single root incident producing two separate Sentry issues. This is the same bug class already fixed once in this file — `FastMCPValidationFilter` downgrades FastMCP's own "Error validating tool" ERROR logs to WARNING for the same reason (expected client conditions logged too loudly by a third-party SDK, captured by Sentry's default `LoggingIntegration(event_level=ERROR)`). **Fix:** Add `MCPTransportDisconnectFilter`, following the same pattern, registered on both `mcp.server.streamable_http` and `mcp.server.lowlevel.server` loggers. It downgrades ERROR → WARNING only for these two exact patterns: - `mcp.server.streamable_http`: `record.exc_info[1]` is a `ClientDisconnect` instance - `mcp.server.lowlevel.server`: `record.getMessage() == "Received exception from stream: "` (exact match) Every other ERROR on these loggers is left untouched, so genuine transport bugs still surface and page. ### TRADEOFFS This changes failure-mode **log severity only**. The WARNING is still logged and remains fully visible in Datadog/log aggregation — it just no longer creates or reopens a Sentry issue. There is no change to request-handling behavior, no raise→warn/suppress/degrade semantics change, and no functional behavior change of any kind to disclose. ### TESTING INSTRUCTIONS New unit test file `tests/unit_tests/mcp_service/test_logging_filters.py` covers: - `mcp.server.streamable_http` ERROR record with `exc_info` set to a real `ClientDisconnect` → downgraded to WARNING - same logger with an unrelated exception (e.g. `ValueError`) → stays ERROR - `mcp.server.lowlevel.server` ERROR record with message exactly `"Received exception from stream: "` → downgraded to WARNING - same logger with a different/unrelated message → stays ERROR - records below ERROR level pass through unchanged regardless of logger name/message ``` $ pytest tests/unit_tests/mcp_service/test_logging_filters.py -v ... tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterStreamableHttp::test_client_disconnect_downgraded_to_warning PASSED [ 12%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterStreamableHttp::test_other_exception_stays_at_error PASSED [ 25%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterStreamableHttp::test_no_exc_info_stays_at_error PASSED [ 37%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterLowlevelServer::test_empty_exception_message_downgraded_to_warning PASSED [ 50%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterLowlevelServer::test_different_message_stays_at_error PASSED [ 62%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterLowlevelServer::test_unrelated_message_stays_at_error PASSED [ 75%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterPassthrough::test_warning_level_passes_through_unchanged PASSED [ 87%] tests/unit_tests/mcp_service/test_logging_filters.py::TestMCPTransportDisconnectFilterPassthrough::test_info_level_passes_through_unchanged PASSED [100%] ========================= 8 passed, 1 warning in 0.89s ========================= ``` Also verified: full `test_middleware_logging.py` + `test_mcp_server.py` suites still pass (40 passed), `ruff check`/`ruff format --check` clean on both changed files (pinned ruff==0.9.7, matching `requirements/development.txt`), and `mypy --check-untyped-defs` on `superset/mcp_service/server.py` introduces zero new errors (1489 pre-existing errors, identical count before/after this change — verified via `git stash`). ### ADDITIONAL INFORMATION - [x] Has associated issue: Shortcut story https://app.shortcut.com/preset/story/115264 - [ ] 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]
