Skip to content

Commit 6e667f8

Browse files
fix(client): drain POST response bodies so legacy streamable HTTP reuses 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 #3281
1 parent a4f4ccd commit 6e667f8

2 files changed

Lines changed: 112 additions & 1 deletion

File tree

src/mcp/client/streamable_http.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
337337
"server answered a request with 202 Accepted",
338338
code=INVALID_REQUEST,
339339
)
340+
await self._drain_response(response)
340341
return
341342

342343
if response.status_code >= 400:
@@ -388,6 +389,10 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
388389
error_data = ErrorData(code=INVALID_REQUEST, message=f"Unexpected content type: {content_type}")
389390
error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data))
390391
await ctx.read_stream_writer.send(error_msg)
392+
else:
393+
# A notification POST has no response body; drain it so the
394+
# connection returns to the pool instead of being discarded.
395+
await self._drain_response(response)
391396

392397
async def _handle_json_response(
393398
self,
@@ -408,6 +413,24 @@ async def _handle_json_response(
408413
error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=request_id, error=error_data))
409414
await read_stream_writer.send(error_msg)
410415

416+
async def _drain_response(self, response: httpx2.Response) -> None:
417+
"""Consume a response body to EOF so httpx can return the TCP connection
418+
to its pool instead of discarding it. In streamable-HTTP legacy mode
419+
every POST owns a response stream; abandoning it mid-body (the previous
420+
``aclose()`` call) costs one TCP connection per JSON-RPC exchange. A 202
421+
or notification body drains instantly; an SSE body the EventSource
422+
already iterated is drained via the raw stream because ``aiter_raw``
423+
raises ``StreamConsumed`` once iteration has started.
424+
"""
425+
try:
426+
await response.aread()
427+
except httpx2.StreamConsumed:
428+
try:
429+
async for _ in response.stream: # type: ignore[attr-defined]
430+
pass
431+
except Exception: # pragma: lax no cover
432+
logger.debug("failed to drain response stream", exc_info=True)
433+
411434
async def _handle_sse_response(
412435
self,
413436
response: httpx2.Response,
@@ -442,7 +465,7 @@ async def _handle_sse_response(
442465
# If the SSE event indicates completion, like returning response/error
443466
# break the loop
444467
if is_complete:
445-
await response.aclose()
468+
await self._drain_response(response)
446469
return # Normal completion, no reconnect needed
447470
except Exception:
448471
logger.debug("SSE stream ended", exc_info=True) # pragma: lax no cover

tests/client/test_streamable_http.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,3 +748,91 @@ async def test_resolving_an_abandoned_request_after_the_reader_closed_is_contain
748748
_abandoned_request_context(http, send), "evt-7", None, MAX_RECONNECTION_ATTEMPTS
749749
)
750750
send.close()
751+
752+
753+
@pytest.mark.anyio
754+
async def test_legacy_mode_reuses_tcp_connections_across_exchanges() -> None:
755+
"""Regression test for #3281: in streamable-HTTP legacy mode the client must
756+
drain each POST's response body to EOF so httpx returns the TCP connection
757+
to its pool, instead of `aclose()`-ing an unread stream and opening one
758+
connection per JSON-RPC exchange.
759+
760+
Without the drain, every exchange (initialize, initialized notification,
761+
tools/list, DELETE) opens a fresh connection. With it, at least one POST
762+
reuses the previous exchange's connection, so distinct connections < posts.
763+
"""
764+
import json
765+
import socket
766+
import threading
767+
import time
768+
769+
import uvicorn
770+
771+
from mcp.client.client import Client
772+
from mcp.server.mcpserver import MCPServer
773+
774+
server = MCPServer(name="conn-reuse", version="1.0.0")
775+
776+
@server.tool()
777+
def echo(text: str) -> str:
778+
"""Echo a message back verbatim."""
779+
return text
780+
781+
with socket.socket() as s:
782+
s.bind(("127.0.0.1", 0))
783+
port = s.getsockname()[1]
784+
785+
uvicorn_srv = uvicorn.Server(
786+
uvicorn.Config(server.streamable_http_app(), host="127.0.0.1", port=port, log_level="error")
787+
)
788+
thread = threading.Thread(target=uvicorn_srv.run, daemon=True)
789+
thread.start()
790+
try:
791+
# Wait for the server to accept connections.
792+
deadline = time.monotonic() + 10
793+
while time.monotonic() < deadline:
794+
try:
795+
with socket.create_connection(("127.0.0.1", port), timeout=0.5):
796+
break
797+
except OSError:
798+
time.sleep(0.05)
799+
800+
class _TrackingTransport(httpx2.AsyncBaseTransport):
801+
def __init__(self) -> None:
802+
self.inner = httpx2.AsyncHTTPTransport()
803+
self.log: list[tuple[str, int]] = []
804+
805+
async def handle_async_request(self, request: httpx2.Request) -> httpx2.Response:
806+
resp = await self.inner.handle_async_request(request)
807+
try:
808+
method = json.loads(request.content).get("method", request.method)
809+
except Exception:
810+
method = request.method
811+
stream = resp.extensions.get("network_stream")
812+
self.log.append((method, id(stream) if stream is not None else -1))
813+
return resp
814+
815+
async def aclose(self) -> None:
816+
await self.inner.aclose()
817+
818+
transport = _TrackingTransport()
819+
async with httpx2.AsyncClient(transport=transport, timeout=30) as http:
820+
async with Client(
821+
streamable_http_client(f"http://127.0.0.1:{port}/mcp", http_client=http),
822+
mode="legacy",
823+
) as client:
824+
await client.list_tools()
825+
826+
exchanges = transport.log
827+
distinct = len({conn_id for _, conn_id in exchanges})
828+
# The POST exchanges (initialize, notifications/initialized, tools/list,
829+
# DELETE) must share fewer TCP connections than the number of exchanges.
830+
# A long-lived GET resumption stream is expected to hold its own
831+
# connection, so we only require strict sharing overall.
832+
assert distinct < len(exchanges), (
833+
f"legacy mode opened one connection per exchange ({distinct} distinct "
834+
f"for {len(exchanges)} exchanges): response bodies are not being drained"
835+
)
836+
finally:
837+
uvicorn_srv.should_exit = True
838+
thread.join(timeout=3)

0 commit comments

Comments
 (0)