Skip to content

fix: handle ClosedResourceError when logging errors to disconnected clients#2177

Open
giulio-leone wants to merge 2 commits intomodelcontextprotocol:mainfrom
giulio-leone:fix/handle-closed-resource-on-client-disconnect
Open

fix: handle ClosedResourceError when logging errors to disconnected clients#2177
giulio-leone wants to merge 2 commits intomodelcontextprotocol:mainfrom
giulio-leone:fix/handle-closed-resource-on-client-disconnect

Conversation

@giulio-leone
Copy link

Problem

When a client disconnects mid-request in a stateless streamable-HTTP session, _handle_message catches the resulting Exception (e.g. ClientDisconnect) and tries to call session.send_log_message() to notify the client about the error. Since the session write stream is already closed at this point, this raises ClosedResourceError, which is unhandled inside the TaskGroup and crashes the entire stateless session with an ExceptionGroup.

Root Cause

In lowlevel/server.py, the Exception branch of _handle_message (line ~418) unconditionally calls send_log_message(). This works fine when the client is still connected, but when the error is a client disconnect, the write stream is already closed, so send_log_messagesend_notification_write_stream.send() raises ClosedResourceError.

This is a different code path from what PR #1384 fixed (message router loop). This bug is in the error recovery path:

catch exception → try to log it to client → write stream closed → crash

Fix

Wrap the send_log_message() call in a try-except that catches anyio.ClosedResourceError and anyio.BrokenResourceError, since failing to notify a disconnected client is expected and harmless. A warning is logged instead.

Testing

Added 3 tests:

  • test_exception_handling_tolerates_closed_write_stream — ClosedResourceError is caught
  • test_exception_handling_tolerates_broken_write_stream — BrokenResourceError is caught
  • test_exception_handling_closed_stream_with_raise_exceptions — original exception is still re-raised when raise_exceptions=True

All 9 exception handling tests pass (2 consecutive clean runs).

Related Issues

Fixes #2064

…lients

When a client disconnects mid-request, _handle_message catches the
resulting Exception and tries to send_log_message() back to the client.
Since the session write stream is already closed, this raises
ClosedResourceError (or BrokenResourceError), which is unhandled and
crashes the stateless session with an ExceptionGroup.

This is a different code path from PR modelcontextprotocol#1384, which fixed the message
router loop. This bug is in the error recovery path:
catch exception → try to log it to client → write stream closed → crash.

Fix: catch ClosedResourceError and BrokenResourceError around the
send_log_message call in _handle_message, since failing to notify a
disconnected client is expected and harmless.

Fixes modelcontextprotocol#2064
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.

ClientDisconnect during _handle_post_request crashes stateless session with ClosedResourceError

1 participant