Skip to content

fix(session): log exceptions in default message_handler instead of silently swallowing#2374

Open
Aboudjem wants to merge 1 commit intomodelcontextprotocol:mainfrom
Aboudjem:fix/reraise-message-handler-exceptions
Open

fix(session): log exceptions in default message_handler instead of silently swallowing#2374
Aboudjem wants to merge 1 commit intomodelcontextprotocol:mainfrom
Aboudjem:fix/reraise-message-handler-exceptions

Conversation

@Aboudjem
Copy link
Copy Markdown

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

  • _default_message_handler now logs a warning with the exception traceback when it receives an Exception, instead of silently discarding it
  • Added test verifying the default handler logs exceptions with proper exc_info
  • Added test verifying custom handlers can still suppress exceptions if desired

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

  • New test: test_default_message_handler_logs_exceptions — verifies the exception is logged at WARNING level with correct exc_info
  • New test: test_custom_message_handler_can_suppress_exceptions — verifies custom handlers can still suppress exceptions
  • All 13 client session tests pass
  • All 8 shared session tests pass
  • ruff check clean
  • pyright clean

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClientSession Error Handling

1 participant