Skip to content

fix(client): drain POST response bodies so legacy streamable HTTP reuses connections - #3284

Closed
guillaume-flambard wants to merge 1 commit into
modelcontextprotocol:mainfrom
guillaume-flambard:fix/streamable-http-drain-response-connections
Closed

fix(client): drain POST response bodies so legacy streamable HTTP reuses connections#3284
guillaume-flambard wants to merge 1 commit into
modelcontextprotocol:mainfrom
guillaume-flambard:fix/streamable-http-drain-response-connections

Conversation

@guillaume-flambard

Copy link
Copy Markdown

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:

  • SSE request response (_handle_sse_response, is_complete): drain the body to EOF. aiter_raw raises StreamConsumed once the EventSource has started iterating, so the remaining bytes are drained via the raw response.stream.
  • 202 response: await response.aread() (empty body, instant).
  • notification POST (no response body): await response.aread().

Verification

Real server (the SDK's own streamable_http_app()) + a tracking transport that records network_stream identity per request:

before: initialize, notifications/initialized, tools/list, DELETE
        -> 4 distinct connections
after:  all four share one connection (only the long-lived GET
        resumption stream holds its own)

New regression test test_legacy_mode_reuses_tcp_connections_across_exchanges asserts distinct connections < exchanges; it fails without the fix and passes with it.

tests/client suite: 711 passed, 1 skipped, 1 xfailed. Ruff + format clean.

…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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Aug 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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 missing-issue-link label, or adding bypass-issue-check bypasses the check.

@github-actions github-actions Bot closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streamable_http (legacy mode): early response.aclose() discards a TCP connection per JSON-RPC exchange — deterministic reproduction for #2707

1 participant