Fail fast on server-to-client requests in JSON-response mode instead of hanging by maxisbey · Pull Request #3195 · modelcontextprotocol/python-sdk · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions docs/migration.md
2 changes: 1 addition & 1 deletion docs/run/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Each transport has its own keyword arguments, all on `run()`:

* `host` / `port`: where to listen. Defaults `127.0.0.1` and `8000`.
* `streamable_http_path`: where the MCP endpoint lives. Default `/mcp`.
* `json_response=True`: answer with plain JSON instead of an SSE stream.
* `json_response=True`: answer each POST with a single JSON body instead of an SSE stream. That body has room for the response and nothing else, so a tool that calls back into the client mid-request (`ctx.elicit()`, sampling) raises `NoBackChannelError` on this leg, and notifications tied to the in-flight call (progress from `ctx.report_progress()`, per-call log messages) are dropped; the standalone `GET` stream still carries unrelated ones.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 The new json_response=True bullet's parenthetical "(ctx.elicit(), sampling)" over-claims for sampling: the default sampling API (ctx.session.create_message()) sends with related_request_id=None, so it rides the connection's standalone channel — which never consults the can_send_request flag this PR wires up — and still routes to the GET stream (or silently hangs if none is attached, the out-of-scope case the PR description acknowledges) rather than raising NoBackChannelError. Only request-scoped calls (ctx.elicit(), Resolve-driven sampling, or an explicit related_request_id) get the new fail-fast; consider dropping "sampling" or qualifying it as request-scoped sampling.

Extended reasoning...

What the doc claims vs. what the code does

The new bullet in docs/run/index.md says that under json_response=True "a tool that calls back into the client mid-request (ctx.elicit(), sampling) raises NoBackChannelError on this leg". That is accurate for ctx.elicit() / ctx.elicit_url() and for Resolve-driven sampling, because those stamp related_request_id=ctx.request_id and therefore travel the request-scoped DispatchContext, whose can_send_request this PR sets to False in JSON-response mode. It is not accurate for the default sampling API.

The code path

ServerSession.create_message() defaults related_request_id=None, and ServerSession.send_request() selects the channel with channel = self._request_outbound if related is not None else self._connection.outbound (src/mcp/server/session.py:88-89). With no related id, sampling rides the connection's standalone channel. For a stateful loop session, Connection.for_loop installs the JSONRPCDispatcher itself as that outbound, and its send_raw_request never consults can_send_request — only the per-message DispatchContext does (src/mcp/shared/jsonrpc_dispatcher.py:167). The mcpserver Context has no sampling helper that stamps a related id (only elicit/elicit_url do, src/mcp/server/mcpserver/context.py:185,218), so the ordinary way a tool samples — ctx.session.create_message(...) — goes standalone by default.

Why the new guards don't fire on this path

Both new mechanisms in this PR are keyed to the request-scoped leg. The transport_context_for fail-fast is only checked in DispatchContext.send_raw_request, which the standalone path bypasses. And the new message_router drop branch in src/mcp/server/streamable_http.py requires related_request_id is not None before it fires; a sampling request with no related id falls through to GET_STREAM_KEY as before.

Step-by-step proof

  1. Deploy a stateful server with json_response=True and a tool that calls await ctx.session.create_message([...], max_tokens=10) mid-request.
  2. Connect a legacy (2025-11-25) client that has not opened the standalone GET stream, and call the tool.
  3. create_message builds ServerMessageMetadata(related_request_id=None) (session.py:266) → send_request picks self._connection.outbound → the dispatcher writes the request and parks a waiter, with no can_send_request check anywhere on that path.
  4. message_router sees a JSONRPCRequest with no related id → target_request_id stays None → routes to GET_STREAM_KEY. No GET stream is registered, so the message is logged and dropped.
  5. The waiter never wakes; the tool call and the POST stall indefinitely — the exact standalone-channel hang the PR description explicitly lists as out of scope. The developer who trusted the doc line expected an immediate NoBackChannelError.

(If the client has a GET stream attached, the request is delivered there instead — also not the documented NoBackChannelError.)

Why this is a nit

Nothing in the code is wrong — the PR's actual behavior change is correct, tested, and matches its description. The inaccuracy is one word in one new doc bullet; the neighboring legacy-clients.md note and troubleshooting.md entries are correctly scoped to the request-scoped channel. All verifiers who traced this agreed.

Suggested fix

Drop "sampling" from the parenthetical, or qualify it, e.g.: "a tool that calls back into the client on the request-scoped channel (ctx.elicit(), sampling with a related request id)".

* `stateless_http=True`: a fresh transport per request, no session tracking.
* `max_request_body_size`: largest accepted POST body in bytes. Defaults to 4 MiB; larger requests
receive HTTP 413 before parsing or session creation. Raise it only when legitimate MCP messages
Expand Down
7 changes: 7 additions & 0 deletions docs/run/legacy-clients.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ Two things about it matter more than what it does.

**It costs both server-to-client channels on that leg.** A session that lives for one `POST` has no stream for the server to push a request down and no standalone stream for it to push notifications down. Every server-initiated request raises `NoBackChannelError`: `ctx.elicit()`, the retired sampling and roots calls (**[Deprecated features](../deprecated.md)**), and, yes, `Resolve` asking a *legacy* client its question. Notifications don't even get an error; they are silently dropped.

!!! note
`json_response=True` is not that knob, but it takes half the same cost on *every* legacy
session: a `POST` answered with one JSON body has no stream for the request-scoped channel,
so a mid-request `ctx.elicit()` raises the same `NoBackChannelError` and notifications tied to
the request are dropped. The session's standalone stream is untouched: unrelated notifications
still arrive.

!!! check
Do the wrong thing. `reserve` is the exact tool that just served both clients. Deploy it with
`stateless_http=True`, connect the same two clients over HTTP, and call it from each.
Expand Down
15 changes: 9 additions & 6 deletions docs/troubleshooting.md
Loading
Loading