| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…lently swallowing The default _default_message_handler in ClientSession silently discarded all messages including exceptions, making transport errors (e.g. SSE read timeouts) impossible to diagnose. Callers had no indication that an error occurred, leading to silent hangs that were extremely difficult to debug. The handler now logs exceptions at WARNING level with full traceback via exc_info. This provides observability while preserving backward compatibility — callers who want to re-raise exceptions or implement custom error handling can still pass their own message_handler. Github-Issue: modelcontextprotocol#1401 Reported-by: Unshure
The suppressing_handler in test_custom_message_handler_can_suppress_exceptions only ever receives Exception messages in the test, so coverage.py reports the False branch (792->exit) as uncovered. This is a known coverage.py quirk with async functions where one branch path is never taken by design. Adding # pragma: no branch restores 100% coverage. Github-Issue: modelcontextprotocol#1401
|
Thanks for the PR. Tracking this in #3087 instead. Closing this as part of a wider backlog cleanup following the v2 launch. Feel free to reopen if this is still relevant. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Fixes #1401.
The default _default_message_handler in ClientSession silently discarded all messages — including exceptions. When a transport error (e.g. SSE read timeout) was delivered through the read stream, the handler did nothing but await anyio.lowlevel.checkpoint(), making it impossible to diagnose why requests were hanging.
This change adds a logger.warning() call with full exception traceback (exc_info) when the default handler receives an Exception. This provides immediate observability for transport errors without breaking backward compatibility — callers who want custom behavior (re-raising, suppression, etc.) can still pass their own message_handler.
What changed
Why not re-raise?
Re-raising in the default handler would terminate the receive loop and send CONNECTION_CLOSED errors to all pending requests. While this prevents hangs, it also affects non-critical exceptions routed through _handle_incoming (e.g. responses with unknown IDs). Logging provides observability without this side effect. Transport-level error propagation to response streams is addressed separately in #2122.
Test plan