Skip to content

Commit 07b5c9a

Browse files
committed
Sever a superseded POST instead of guarding one resolution path
Re-issuing a request id (legal once its waiter was popped) now cancels the superseded POST's scope at registration, so a zombie stream cannot answer, fail, or resolve the successor's request through any exit: a late real response, a status-derived error, a parse error, a reconnection replay, or the synthesized stream-death resolution. This replaces the narrower identity guard inside _resolve_abandoned_request, which covered only its own callers; the guard, the RequestContext.in_flight_post threading, and the ctx-taking signature all revert. Also give the synthesized-error helper a code parameter: a 202 to a request is a server-shape violation, not a connection loss, so it now resolves with INVALID_REQUEST, matching the unexpected-content-type precedent and the CONNECTION_CLOSED constant's own contract.
1 parent f6b5295 commit 07b5c9a

2 files changed

Lines changed: 35 additions & 49 deletions

File tree

src/mcp/client/streamable_http.py

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ class RequestContext:
7171
session_message: SessionMessage
7272
metadata: ClientMessageMetadata | None
7373
read_stream_writer: StreamWriter
74-
in_flight_post: _InFlightPost | None = None
75-
"""This request's `_in_flight_posts` registration; None for notifications."""
7674

7775

7876
@dataclass(slots=True)
@@ -334,7 +332,10 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
334332
# A request's response arrives on this POST's body; 202 says
335333
# none will follow. Resolve rather than park the caller forever.
336334
await self._resolve_abandoned_request(
337-
ctx, message.id, "server answered a request with 202 Accepted"
335+
ctx.read_stream_writer,
336+
message.id,
337+
"server answered a request with 202 Accepted",
338+
code=INVALID_REQUEST,
338339
)
339340
return
340341

@@ -453,24 +454,21 @@ async def _handle_sse_response(
453454
else:
454455
# Not resumable: resolve the waiter, else a listen stream's consumer
455456
# would hang forever instead of learning the subscription is lost.
456-
await self._resolve_abandoned_request(ctx, original_request_id, "SSE stream ended without a response")
457+
await self._resolve_abandoned_request(
458+
ctx.read_stream_writer, original_request_id, "SSE stream ended without a response"
459+
)
457460

458-
async def _resolve_abandoned_request(self, ctx: RequestContext, request_id: RequestId, message: str) -> None:
461+
async def _resolve_abandoned_request(
462+
self, read_stream_writer: StreamWriter, request_id: RequestId, message: str, *, code: int = CONNECTION_CLOSED
463+
) -> None:
459464
"""Resolve a request whose response can never arrive with a synthesized error.
460465
461-
Skipped when a same-id successor has taken over the `_in_flight_posts`
462-
registration: ids are reusable once their waiter is popped, so the
463-
pending waiter now belongs to the successor and a stale stream's death
464-
must not fail it. Best-effort: a closed read stream means the session
465-
is tearing down.
466+
Best-effort: a closed read stream means the session is tearing down.
466467
"""
467-
if ctx.in_flight_post is not None and self._in_flight_posts.get(request_id) is not ctx.in_flight_post:
468-
logger.debug("request %r was re-issued; leaving its waiter to the successor", request_id)
469-
return
470-
error_data = ErrorData(code=CONNECTION_CLOSED, message=message)
468+
error_data = ErrorData(code=code, message=message)
471469
error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=request_id, error=error_data))
472470
try:
473-
await ctx.read_stream_writer.send(error_msg)
471+
await read_stream_writer.send(error_msg)
474472
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
475473
logger.debug("read stream closed before request %r could be resolved", request_id)
476474

@@ -491,7 +489,7 @@ async def _handle_reconnection(
491489
# stream) would otherwise hang its caller forever.
492490
logger.debug(f"Max reconnection attempts ({MAX_RECONNECTION_ATTEMPTS}) exceeded")
493491
await self._resolve_abandoned_request(
494-
ctx, original_request_id, "SSE stream ended and reconnection attempts were exhausted"
492+
ctx.read_stream_writer, original_request_id, "SSE stream ended and reconnection attempts were exhausted"
495493
)
496494
return
497495

@@ -599,8 +597,13 @@ async def handle_request_async():
599597
scope=anyio.CancelScope(),
600598
modern=self._protocol_version_header in MODERN_PROTOCOL_VERSIONS,
601599
)
600+
superseded = self._in_flight_posts.get(message.id)
601+
if superseded is not None:
602+
# A reused id means the waiter belongs to this attempt now:
603+
# sever the old POST so its zombie stream cannot answer,
604+
# fail, or resolve the successor's request.
605+
superseded.scope.cancel()
602606
self._in_flight_posts[message.id] = post
603-
ctx.in_flight_post = post
604607
tg.start_soon(self._run_request_post, handle_request_async, post, message.id)
605608
else:
606609
await handle_request_async()

tests/client/test_streamable_http.py

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
CLIENT_CAPABILITIES_META_KEY,
2020
CLIENT_INFO_META_KEY,
2121
CONNECTION_CLOSED,
22+
INVALID_REQUEST,
2223
METHOD_NOT_FOUND,
2324
PROTOCOL_VERSION_META_KEY,
2425
JSONRPCError,
@@ -631,23 +632,6 @@ def handler(request: httpx.Request) -> httpx.Response:
631632
assert reply.message.error.code == CONNECTION_CLOSED
632633

633634

634-
class _DoomedSSEStream(httpx.AsyncByteStream):
635-
"""Parks after opening, then breaks on command without ever carrying an event id."""
636-
637-
def __init__(self) -> None:
638-
self.opened = anyio.Event()
639-
self.die = anyio.Event()
640-
641-
async def __aiter__(self) -> AsyncIterator[bytes]:
642-
self.opened.set()
643-
yield b": stalling\n\n"
644-
await self.die.wait()
645-
raise httpx.ReadError("connection reset")
646-
647-
async def aclose(self) -> None:
648-
pass
649-
650-
651635
class _DeliverOnCommandSSEStream(httpx.AsyncByteStream):
652636
"""Parks after opening, then delivers one JSON-RPC response when told."""
653637

@@ -666,13 +650,13 @@ async def aclose(self) -> None:
666650

667651

668652
@pytest.mark.anyio
669-
async def test_a_stale_streams_death_does_not_resolve_a_reused_ids_successor() -> None:
670-
"""Once a same-id successor is registered, an abandoned earlier stream ending
671-
responselessly must not synthesize an error for the id: the pending waiter
672-
belongs to the successor, whose real response must still arrive."""
673-
doomed = _DoomedSSEStream()
674-
succeeding = _DeliverOnCommandSSEStream({"jsonrpc": "2.0", "id": "dup-1", "result": {}})
675-
streams: list[httpx.AsyncByteStream] = [doomed, succeeding]
653+
async def test_a_superseded_posts_late_real_response_cannot_answer_the_successor() -> None:
654+
"""SDK-defined: re-issuing an id severs the superseded POST, so nothing from its
655+
stream (a late real response, or a synthesized error for its death) can resolve
656+
the reused id's waiter; only the successor's own response arrives."""
657+
stale = _DeliverOnCommandSSEStream({"jsonrpc": "2.0", "id": "dup-1", "result": {"origin": "stale"}})
658+
succeeding = _DeliverOnCommandSSEStream({"jsonrpc": "2.0", "id": "dup-1", "result": {"origin": "fresh"}})
659+
streams: list[httpx.AsyncByteStream] = [stale, succeeding]
676660

677661
def handler(request: httpx.Request) -> httpx.Response:
678662
return httpx.Response(200, headers={"content-type": "text/event-stream"}, stream=streams.pop(0))
@@ -683,24 +667,23 @@ def handler(request: httpx.Request) -> httpx.Response:
683667
streamable_http_client("http://test/mcp", http_client=http) as (read, write),
684668
):
685669
await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id="dup-1", method="tools/call", params={})))
686-
await doomed.opened.wait()
687-
# The caller abandoned the first attempt and re-issued the id while
688-
# the first stream lingers server-side.
670+
await stale.opened.wait()
689671
await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id="dup-1", method="tools/call", params={})))
690672
await succeeding.opened.wait()
691-
doomed.die.set()
673+
stale.deliver.set()
692674
await anyio.wait_all_tasks_blocked()
693675
succeeding.deliver.set()
694676
reply = await read.receive()
695677
assert isinstance(reply, SessionMessage)
696678
assert isinstance(reply.message, JSONRPCResponse), reply.message
697-
assert reply.message.id == "dup-1"
679+
assert reply.message.result == {"origin": "fresh"}
698680

699681

700682
@pytest.mark.anyio
701683
async def test_a_202_to_a_request_resolves_the_waiter_with_an_error() -> None:
702-
"""A server that answers a request with 202 Accepted has declared no response
703-
will follow; the transport resolves the waiter instead of parking the caller forever."""
684+
"""SDK-defined: a server that answers a request with 202 Accepted has declared no
685+
response will follow (the spec requires SSE or JSON for requests); the transport
686+
resolves the waiter with INVALID_REQUEST instead of parking the caller forever."""
704687

705688
def handler(request: httpx.Request) -> httpx.Response:
706689
return httpx.Response(202)
@@ -717,7 +700,7 @@ def handler(request: httpx.Request) -> httpx.Response:
717700
assert isinstance(reply, SessionMessage)
718701
assert isinstance(reply.message, JSONRPCError)
719702
assert reply.message.id == "listen-1"
720-
assert reply.message.error.code == CONNECTION_CLOSED
703+
assert reply.message.error.code == INVALID_REQUEST
721704

722705

723706
def _abandoned_request_context(

0 commit comments

Comments
 (0)