fix: send JSONRPCError instead of bare exceptions in streamable HTTP client#2005
fix: send JSONRPCError instead of bare exceptions in streamable HTTP client#2005
Conversation
…client When the streamable HTTP client encountered errors (unexpected content type, JSON parse failure, SSE parse failure), it sent bare Exception objects that never resolved the pending send_request() — causing the caller to hang indefinitely until timeout. Send JSONRPCError with the request's id so the response stream unblocks immediately, following the pattern already used by the 404 handler.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a hang in the Streamable HTTP client by ensuring transport-layer failures (unexpected Content-Type, JSON parse errors, SSE parse errors) are surfaced to the session as JSONRPCError responses tied to the original request id, allowing send_request() to resolve immediately instead of waiting for a timeout.
Changes:
- Emit
JSONRPCError(with requestid) for unexpected responseContent-Typeand JSON body parse failures in the Streamable HTTP client. - Emit
JSONRPCError(with requestid) when an SSEmessageevent can’t be parsed, unblocking the pending request. - Add tests covering unexpected content types and invalid JSON responses for non-SDK servers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/mcp/client/streamable_http.py |
Converts previously “bare exception sent to stream” cases into JSONRPCError responses keyed by request id. |
src/mcp/types/jsonrpc.py |
Aligns JSONRPCError.id typing with RequestId. |
tests/client/test_notification_response.py |
Reworks test server setup to ASGITransport and adds regression tests for immediate MCPError on bad responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Catch (httpx.StreamError, ValidationError) instead of bare Exception.
There was a problem hiding this comment.
Thanks for the fix!
nit: no test for the SSE parse failure path (lines 167-175), though it's pragma: no cover and follows the same pattern as the two tested paths. Not blocking.
looks like CI failures are just about coverage checks. ready to stamp once that's fixed
Summary
Exceptionobjects that never resolve the pendingsend_request()— causing the caller to hang until timeout.JSONRPCErrorwith the request'sidso the response stream unblocks immediately, following the pattern already used by the 404 handler.Unexpected content typewhen initializing MCP session #1382, fixes streamable_http client call_tool hangs when receiving invalid JSONRPCMessage #1144.Test plan
test_unexpected_content_type_sends_jsonrpc_error— server returnstext/plain, client getsMCPErrorimmediately (no timeout)test_invalid_json_response_sends_jsonrpc_error— server returns invalid JSON body, client getsMCPErrorimmediatelyuv run --frozen ruff checkpassesuv run --frozen pyrightpasses