fix(client): drain POST response bodies so legacy streamable HTTP reuses connections - #3284
Conversation
…ses connections
In streamable-HTTP legacy mode every JSON-RPC exchange owns its POST
response stream. The client called `response.aclose()` the moment the
reply SSE event arrived, abandoning the body unread; httpx cannot return
an undrained streaming response's TCP connection to its pool, so each
exchange opened a fresh connection (plus a TCP handshake + TLS + slow
start) even against the same host.
Drain the body to EOF instead:
- SSE request responses are drained via the raw stream, because aiter_raw
raises StreamConsumed once the EventSource has started iterating;
- 202 and notification POST bodies (empty) drain instantly via aread().
Observed with the SDK's own server + a tracking transport:
before: initialize, notifications/initialized, tools/list, DELETE each
on its own connection
after: all four share one connection (the GET resumption stream is the
only separate one)
Fixes modelcontextprotocol#3281
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/client/streamable_http.py">
<violation number="1" location="src/mcp/client/streamable_http.py:340">
P1: A server that keeps an SSE response stream open after sending the final JSON-RPC reply can now leave this POST task blocked indefinitely, because draining waits for EOF even though stream termination is only a protocol SHOULD. This is particularly problematic for long-lived response streams and can retain the connection/task until a timeout or client shutdown. Consider retaining the previous close-on-completion behavior for streams without a known finite body, and only drain responses whose framing guarantees EOF is immediately available.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "server answered a request with 202 Accepted", | ||
| code=INVALID_REQUEST, | ||
| ) | ||
| await self._drain_response(response) |
There was a problem hiding this comment.
P1: A server that keeps an SSE response stream open after sending the final JSON-RPC reply can now leave this POST task blocked indefinitely, because draining waits for EOF even though stream termination is only a protocol SHOULD. This is particularly problematic for long-lived response streams and can retain the connection/task until a timeout or client shutdown. Consider retaining the previous close-on-completion behavior for streams without a known finite body, and only drain responses whose framing guarantees EOF is immediately available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/streamable_http.py, line 340:
<comment>A server that keeps an SSE response stream open after sending the final JSON-RPC reply can now leave this POST task blocked indefinitely, because draining waits for EOF even though stream termination is only a protocol SHOULD. This is particularly problematic for long-lived response streams and can retain the connection/task until a timeout or client shutdown. Consider retaining the previous close-on-completion behavior for streams without a known finite body, and only drain responses whose framing guarantees EOF is immediately available.</comment>
<file context>
@@ -337,6 +337,7 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
"server answered a request with 202 Accepted",
code=INVALID_REQUEST,
)
+ await self._drain_response(response)
return
</file context>
|
Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — CONTRIBUTING.md explains why and how we work. This PR has been closed for now because you aren't currently assigned to #3281. If a maintainer would like this change as a PR from you, they'll assign you to #3281 and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.) There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten. Maintainers: reopening this PR, removing the |
Fixes #3281
Problem
In streamable-HTTP legacy mode, every JSON-RPC exchange owns its POST response stream. The client called
await response.aclose()the moment the reply SSE event arrived (_handle_sse_response), abandoning the body unread. httpx cannot return an undrained streaming response's TCP connection to its pool, so each exchange opened a fresh TCP connection — handshake + TLS + slow start — even against the same host. A notification POST (202, empty body) and the DELETE teardown hit the same path and discarded their connections too.The issue's own reproduction shows a plain httpx control reusing 1 connection for the same three exchanges where the SDK used one per exchange.
Fix
Add
_drain_response()and use it on the three POST paths that previously discarded their connection:_handle_sse_response,is_complete): drain the body to EOF.aiter_rawraisesStreamConsumedonce theEventSourcehas started iterating, so the remaining bytes are drained via the rawresponse.stream.await response.aread()(empty body, instant).await response.aread().Verification
Real server (the SDK's own
streamable_http_app()) + a tracking transport that recordsnetwork_streamidentity per request:New regression test
test_legacy_mode_reuses_tcp_connections_across_exchangesassertsdistinct connections < exchanges; it fails without the fix and passes with it.tests/clientsuite: 711 passed, 1 skipped, 1 xfailed. Ruff + format clean.