Skip to content

Commit 8d85e18

Browse files
committed
Fix two waiter-resolution gaps in the streamable HTTP client
- A stale POST whose SSE stream ends responselessly no longer synthesizes CONNECTION_CLOSED for a request id that a same-id successor has re-issued; the pending waiter belongs to the successor, whose real response still arrives. Resolution is now identity-guarded the same way the in-flight registry's cleanup already was. - A server that answers a JSON-RPC request with 202 Accepted now resolves the waiter with CONNECTION_CLOSED instead of leaving a timeout-free caller parked forever, including client.listen() enter. - Rewrite the subscriptions story README around the client.listen() context manager the example now uses; it still described the old escape-hatch flow. - Extend the docs import note to cover the watch.py snippet.
1 parent 769d561 commit 8d85e18

4 files changed

Lines changed: 128 additions & 28 deletions

File tree

docs/handlers/subscriptions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ Two more properties of the handle:
8383

8484
Open the subscription first, then start the watcher and get on with your work.
8585

86-
`app.py` imports `BOARD` and `read_board` from the previous example, which this repo stores as `tutorial003.py`. If you save the rendered files side by side as `client.py` and `app.py`, write `from client import BOARD, read_board` instead.
86+
`app.py` imports `BOARD` and `read_board` from the previous example, which this repo stores as `tutorial003.py`. If you save the rendered files side by side as `client.py` and `app.py`, write `from client import BOARD, read_board` instead. The `watch.py` example further down imports `read_board` the same way.
8787

8888
=== "asyncio"
8989

examples/stories/subscriptions/README.md

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ server publishes with `ctx.notify_resource_updated(uri)` /
77
per-stream filtering, subscription-id tagging). Replaces the handshake-era
88
`resources/subscribe` + standalone-GET notification path.
99

10-
The client edits a note it did not subscribe to (silence), edits the one it
11-
did (a tagged `notifications/resources/updated`), registers a tool at runtime
12-
(`notifications/tools/list_changed`, then re-lists and calls it), and finally
13-
stops listening - cancelling the parked request releases the local task, and
14-
closing the connection ends the stream server-side.
10+
The client opens the stream with `client.listen(...)`, edits a note it did
11+
not subscribe to (silence), edits the one it did (a typed `ResourceUpdated`),
12+
registers a tool at runtime (a typed `ToolsListChanged`, then re-lists and
13+
calls it), and finally leaves the `async with` block, which ends the
14+
subscription while the connection lives on.
1515

1616
## Run it
1717

1818
```bash
19-
# HTTP the client self-hosts the server on a free port, runs, then tears it
19+
# HTTP: the client self-hosts the server on a free port, runs, then tears it
2020
# down (subscriptions/listen is 2026-era only)
2121
uv run python -m stories.subscriptions.client --http
2222
# same, against the lowlevel-API server variant
@@ -25,17 +25,18 @@ uv run python -m stories.subscriptions.client --http --server server_lowlevel
2525

2626
## What to look at
2727

28-
- `client.py` — stream frames arrive as ordinary server notifications via the
29-
constructor-only `message_handler=`. There is no client-side listen API yet,
30-
so opening the stream drops to the `client.session` escape hatch; the request
31-
parks for the stream's lifetime. Cancelling it releases the local task; over
32-
HTTP the server-side stream ends when the connection closes. Every frame's
33-
`_meta["io.modelcontextprotocol/subscriptionId"]` is the listen request's
34-
JSON-RPC id.
35-
- `server.py` — publishing is one `await ctx.notify_*()` line per change; the
28+
- `client.py`: the whole subscription is one context manager,
29+
`async with client.listen(...) as sub`. Entering waits for the server's
30+
acknowledgment, so `sub.honored` is already in hand on the first line of the
31+
block. Events arrive as typed values from `anext(sub)`; the edit to the
32+
unsubscribed note never shows up, because the filter is enforced
33+
server-side. Leaving the block ends the subscription (over HTTP the SDK
34+
closes that request's response stream) and the session carries on, which the
35+
final `search` call proves.
36+
- `server.py`: publishing is one `await ctx.notify_*()` line per change; the
3637
filter, the tagging, and the ack ordering are the SDK's job. Publishing with
3738
no subscribers is a no-op.
38-
- `server_lowlevel.py` the same machinery held by hand: an
39+
- `server_lowlevel.py`: the same machinery held by hand: an
3940
`InMemorySubscriptionBus`, handlers that `await bus.publish(...)`, and
4041
`ListenHandler(bus)` passed as `on_subscriptions_listen=`. A multi-replica
4142
deployment swaps the bus for one backed by its own pub/sub
@@ -51,7 +52,7 @@ uv run python -m stories.subscriptions.client --http --server server_lowlevel
5152

5253
## Spec
5354

54-
[Subscriptions basic utilities](https://modelcontextprotocol.io/specification/draft/basic/utilities/subscriptions)
55+
[Subscriptions, basic utilities](https://modelcontextprotocol.io/specification/draft/basic/utilities/subscriptions)
5556

5657
## See also
5758

src/mcp/client/streamable_http.py

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ 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."""
7476

7577

7678
@dataclass(slots=True)
@@ -328,6 +330,12 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
328330
) as response:
329331
if response.status_code == 202:
330332
logger.debug("Received 202 Accepted")
333+
if isinstance(message, JSONRPCRequest):
334+
# A request's response arrives on this POST's body; 202 says
335+
# none will follow. Resolve rather than park the caller forever.
336+
await self._resolve_abandoned_request(
337+
ctx, message.id, "server answered a request with 202 Accepted"
338+
)
331339
return
332340

333341
if response.status_code >= 400:
@@ -445,21 +453,24 @@ async def _handle_sse_response(
445453
else:
446454
# Not resumable: resolve the waiter, else a listen stream's consumer
447455
# would hang forever instead of learning the subscription is lost.
448-
await self._resolve_abandoned_request(
449-
ctx.read_stream_writer, original_request_id, "SSE stream ended without a response"
450-
)
456+
await self._resolve_abandoned_request(ctx, original_request_id, "SSE stream ended without a response")
451457

452-
async def _resolve_abandoned_request(
453-
self, read_stream_writer: StreamWriter, request_id: RequestId, message: str
454-
) -> None:
458+
async def _resolve_abandoned_request(self, ctx: RequestContext, request_id: RequestId, message: str) -> None:
455459
"""Resolve a request whose response can never arrive with a synthesized error.
456460
457-
Best-effort: a closed read stream means the session is tearing down.
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.
458466
"""
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
459470
error_data = ErrorData(code=CONNECTION_CLOSED, message=message)
460471
error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=request_id, error=error_data))
461472
try:
462-
await read_stream_writer.send(error_msg)
473+
await ctx.read_stream_writer.send(error_msg)
463474
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
464475
logger.debug("read stream closed before request %r could be resolved", request_id)
465476

@@ -480,9 +491,7 @@ async def _handle_reconnection(
480491
# stream) would otherwise hang its caller forever.
481492
logger.debug(f"Max reconnection attempts ({MAX_RECONNECTION_ATTEMPTS}) exceeded")
482493
await self._resolve_abandoned_request(
483-
ctx.read_stream_writer,
484-
original_request_id,
485-
"SSE stream ended and reconnection attempts were exhausted",
494+
ctx, original_request_id, "SSE stream ended and reconnection attempts were exhausted"
486495
)
487496
return
488497

@@ -591,6 +600,7 @@ async def handle_request_async():
591600
modern=self._protocol_version_header in MODERN_PROTOCOL_VERSIONS,
592601
)
593602
self._in_flight_posts[message.id] = post
603+
ctx.in_flight_post = post
594604
tg.start_soon(self._run_request_post, handle_request_async, post, message.id)
595605
else:
596606
await handle_request_async()

tests/client/test_streamable_http.py

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,95 @@ def handler(request: httpx.Request) -> httpx.Response:
631631
assert reply.message.error.code == CONNECTION_CLOSED
632632

633633

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+
651+
class _DeliverOnCommandSSEStream(httpx.AsyncByteStream):
652+
"""Parks after opening, then delivers one JSON-RPC response when told."""
653+
654+
def __init__(self, response_body: dict[str, Any]) -> None:
655+
self._event = f"data: {json.dumps(response_body)}\n\n".encode()
656+
self.opened = anyio.Event()
657+
self.deliver = anyio.Event()
658+
659+
async def __aiter__(self) -> AsyncIterator[bytes]:
660+
self.opened.set()
661+
await self.deliver.wait()
662+
yield self._event
663+
664+
async def aclose(self) -> None:
665+
pass
666+
667+
668+
@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]
676+
677+
def handler(request: httpx.Request) -> httpx.Response:
678+
return httpx.Response(200, headers={"content-type": "text/event-stream"}, stream=streams.pop(0))
679+
680+
with anyio.fail_after(5):
681+
async with (
682+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http,
683+
streamable_http_client("http://test/mcp", http_client=http) as (read, write),
684+
):
685+
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.
689+
await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id="dup-1", method="tools/call", params={})))
690+
await succeeding.opened.wait()
691+
doomed.die.set()
692+
await anyio.wait_all_tasks_blocked()
693+
succeeding.deliver.set()
694+
reply = await read.receive()
695+
assert isinstance(reply, SessionMessage)
696+
assert isinstance(reply.message, JSONRPCResponse), reply.message
697+
assert reply.message.id == "dup-1"
698+
699+
700+
@pytest.mark.anyio
701+
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."""
704+
705+
def handler(request: httpx.Request) -> httpx.Response:
706+
return httpx.Response(202)
707+
708+
with anyio.fail_after(5):
709+
async with (
710+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http,
711+
streamable_http_client("http://test/mcp", http_client=http) as (read, write),
712+
):
713+
await write.send(
714+
SessionMessage(JSONRPCRequest(jsonrpc="2.0", id="listen-1", method="subscriptions/listen", params={}))
715+
)
716+
reply = await read.receive()
717+
assert isinstance(reply, SessionMessage)
718+
assert isinstance(reply.message, JSONRPCError)
719+
assert reply.message.id == "listen-1"
720+
assert reply.message.error.code == CONNECTION_CLOSED
721+
722+
634723
def _abandoned_request_context(
635724
http: httpx.AsyncClient, send: ContextSendStream[SessionMessage | Exception]
636725
) -> RequestContext:

0 commit comments

Comments
 (0)