Uh oh!
There was an error while loading. Please reload this page.
fix: send error on SSE disconnect without resumption token - #1838
fix: send error on SSE disconnect without resumption token#1838jayhemnani9910 wants to merge 5 commits into
Conversation
When SSE stream disconnects before receiving any events with IDs, the client would hang forever waiting for a response. This adds an else branch to send a JSONRPCError to the session layer when reconnection is not possible. Fixesmodelcontextprotocol#1811
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #1811 where SSE stream disconnections without receiving a resumption token would cause the client to hang indefinitely. The fix adds an else branch to send a JSONRPCError to the session layer when no event ID has been received before disconnection, preventing deadlock.
Key Changes:
- Added error handling for SSE disconnections without resumption tokens in
_handle_sse_response() - Sends a JSONRPCError with code -32000 (CONNECTION_CLOSED) to notify the session layer
- Includes comprehensive test coverage for the new error path
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/mcp/client/streamable_http.py | Adds else branch in _handle_sse_response() to send JSONRPCError when SSE stream disconnects without receiving an event ID, preventing indefinite hangs |
| tests/shared/test_streamable_http.py | Adds regression test that verifies error is sent to session layer when SSE stream disconnects before receiving resumption token |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| jsonrpc="2.0", | ||
| id=request_id, | ||
| error=ErrorData( | ||
| code=-32000, |
There was a problem hiding this comment.
The error code -32000 should use the CONNECTION_CLOSED constant defined in mcp.types instead of a magic number. The constant is already imported and available. Using the constant improves maintainability and makes the error code's meaning clearer.
| code=-32000, | |
| code=CONNECTION_CLOSED, |
| jsonrpc="2.0", | ||
| id=request_id, | ||
| error=ErrorData( | ||
| code=-32000, |
There was a problem hiding this comment.
The error code -32000 should use the CONNECTION_CLOSED constant defined in mcp.types instead of a magic number. Using the constant improves test maintainability and ensures consistency with the production code.
| assert isinstance(received, SessionMessage) | ||
| assert isinstance(received.message.root, JSONRPCError) | ||
| assert received.message.root.id == "test-request-123" | ||
| assert received.message.root.error.code == -32000 |
There was a problem hiding this comment.
The error code assertion should use the CONNECTION_CLOSED constant from mcp.types instead of the magic number -32000. This would make the test more maintainable and ensure consistency with the constant definition.
maxisbey
commented
Jan 7, 2026
As there's no linked issue or description I've set this PR to draft for now. |
Rewrite test to actually call _handle_sse_response() instead of duplicating the implementation logic inline. This ensures lines 440-451 in streamable_http.py are properly covered. The test mocks EventSource to yield no events, simulating a stream that closes immediately without sending any resumption tokens. Github-Issue: modelcontextprotocol#1811
The else branch on the isinstance check is unreachable in practice since POST SSE responses always contain JSONRPCRequest messages. Github-Issue: modelcontextprotocol#1811
jayhemnani9910
commented
Jan 8, 2026
Yea my bad, my setup crashed. lol |
dsfaccini
commented
Jan 20, 2026
we're bumping dependencies on our side and needed to add some pragmas because of the hang pydantic/pydantic-ai#4047 so just a +1 on "hopefully this merges soon" |
maxisbey
commented
Jul 29, 2026
Thanks for the PR. This has since landed via #3047. Closing as part of a general backlog cleanup following the v2 release. If this is still relevant against v2, feel free to reopen. |
Summary
Fixes#1811
When an SSE stream disconnects before the server sends any events with IDs (no resumption token), the client now sends a JSONRPCError to the session layer instead of hanging indefinitely.
Changes
_handle_sse_responsefor streams that close without sending resumption tokensCONNECTION_CLOSED(-32000) error code with descriptive messageTest Plan
test_sse_disconnect_without_resumption_token_sends_errorunit test