Skip to content

Commit d53d47b

Browse files
fix(client): surface non-2xx HTTP responses to the waiting caller
A 4xx/5xx response to a message POST hit a bare `response.raise_for_status()` inside the POST's background task. Nothing caught the exception and nothing resolved the request, so the caller sat until its read timeout fired: a 401 was indistinguishable from a slow server, and the escaping error tore down the transport's task group with it. Non-2xx statuses are now answered rather than raised, generalising the handling 404 already had: a `JSONRPCError` stamped with the original request's id, sent as a `SessionMessage` over the read stream, so it resolves through the normal response-correlation path. A spec-compliant JSON-RPC error in the body is preferred over the status-derived stand-in. The connection stays usable. This matches the behaviour already shipped on the 2.0 line, with one deliberate difference: 404 is answered ahead of the body check. A terminated session 404s *with* a JSON-RPC error body, so preferring it would replace this line's `Session terminated` / code 32600 — which callers match on for session expiry — with the server's own wording. Callers still cannot tell a terminal 401 from a retryable 503; both remain `-32603 Server returned an error response`, as on 2.0. Carrying the status would be a follow-up on the 2.0 line. Github-Issue: #3091 Reported-by: afterrburn
1 parent ba33472 commit d53d47b

2 files changed

Lines changed: 234 additions & 16 deletions

File tree

src/mcp/client/streamable_http.py

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from anyio.abc import TaskGroup
2121
from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
2222
from httpx_sse import EventSource, ServerSentEvent, aconnect_sse
23+
from pydantic import ValidationError
2324
from typing_extensions import deprecated
2425

2526
from mcp.shared._httpx_utils import (
@@ -28,6 +29,7 @@
2829
)
2930
from mcp.shared.message import ClientMessageMetadata, SessionMessage
3031
from mcp.types import (
32+
INTERNAL_ERROR,
3133
ErrorData,
3234
InitializeResult,
3335
JSONRPCError,
@@ -347,15 +349,14 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
347349
logger.debug("Received 202 Accepted")
348350
return
349351

350-
if response.status_code == 404: # pragma: no branch
352+
if response.status_code >= 400:
353+
# Raising here would only throw into this POST's background task, where nothing
354+
# catches it: the waiting caller would never be told, and would sit until its
355+
# read timeout fired. Answer it instead, on the id it is waiting for.
351356
if isinstance(message.root, JSONRPCRequest):
352-
await self._send_session_terminated_error( # pragma: no cover
353-
ctx.read_stream_writer, # pragma: no cover
354-
message.root.id, # pragma: no cover
355-
) # pragma: no cover
356-
return # pragma: no cover
357+
await self._send_http_status_error(response, ctx.read_stream_writer, message.root.id)
358+
return
357359

358-
response.raise_for_status()
359360
if is_initialization:
360361
self._maybe_extract_session_id_from_response(response)
361362

@@ -507,19 +508,46 @@ async def _handle_unexpected_content_type(
507508
logger.error(error_msg) # pragma: no cover
508509
await read_stream_writer.send(ValueError(error_msg)) # pragma: no cover
509510

510-
async def _send_session_terminated_error(
511+
async def _send_http_status_error(
511512
self,
513+
response: httpx.Response,
512514
read_stream_writer: StreamWriter,
513515
request_id: RequestId,
514516
) -> None:
515-
"""Send a session terminated error response."""
516-
jsonrpc_error = JSONRPCError(
517-
jsonrpc="2.0",
518-
id=request_id,
519-
error=ErrorData(code=32600, message="Session terminated"),
520-
)
521-
session_message = SessionMessage(JSONRPCMessage(jsonrpc_error))
522-
await read_stream_writer.send(session_message)
517+
"""Surface a non-2xx response to the caller that is waiting on `request_id`.
518+
519+
The error is routed as a `JSONRPCError` stamped with the original request's id, because
520+
that id is what the session correlates a reply against; a bare exception carries none and
521+
so can never resolve the caller.
522+
"""
523+
if response.status_code == 404:
524+
# Answered ahead of the body, unlike every other status: a terminated session 404s
525+
# *with* a JSON-RPC error body, and preferring it would rewrite the message and code
526+
# this release line already reports. Callers match on both, so 404 stays verbatim.
527+
error_data = ErrorData(code=32600, message="Session terminated")
528+
await read_stream_writer.send(
529+
SessionMessage(JSONRPCMessage(JSONRPCError(jsonrpc="2.0", id=request_id, error=error_data)))
530+
)
531+
return
532+
533+
# A spec-correct server may put the JSON-RPC error in the body of a non-2xx (e.g. 400 for
534+
# INVALID_PARAMS). Prefer it: the server's account of why it refused beats our stand-in.
535+
if response.headers.get(CONTENT_TYPE, "").lower().startswith(JSON):
536+
try:
537+
body = await response.aread()
538+
parsed = JSONRPCMessage.model_validate_json(body)
539+
if isinstance(parsed.root, JSONRPCError):
540+
# Re-stamped with this request's id rather than trusting the body's: a server
541+
# echoing someone else's id would otherwise resolve the wrong caller.
542+
reply = JSONRPCError(jsonrpc="2.0", id=request_id, error=parsed.root.error)
543+
await read_stream_writer.send(SessionMessage(JSONRPCMessage(reply)))
544+
return
545+
except (httpx.StreamError, ValidationError):
546+
logger.debug("Non-2xx body was not a JSON-RPC error; using the status-derived fallback")
547+
548+
error_data = ErrorData(code=INTERNAL_ERROR, message="Server returned an error response")
549+
jsonrpc_error = JSONRPCError(jsonrpc="2.0", id=request_id, error=error_data)
550+
await read_stream_writer.send(SessionMessage(JSONRPCMessage(jsonrpc_error)))
523551

524552
async def post_writer(
525553
self,

tests/shared/test_streamable_http.py

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,3 +2353,193 @@ async def test_streamablehttp_client_deprecation_warning(basic_server: None, bas
23532353
await session.initialize()
23542354
tools = await session.list_tools()
23552355
assert len(tools.tools) > 0
2356+
2357+
2358+
@pytest.mark.anyio
2359+
@pytest.mark.parametrize("status", [401, 403, 500, 503])
2360+
async def test_non_2xx_status_reaches_the_waiting_caller(status: int) -> None:
2361+
"""A 4xx/5xx to a request is answered as a JSON-RPC error rather than raised into the void.
2362+
2363+
Regression test for #3091: the status used to escape `raise_for_status()` inside the POST's
2364+
background task, where nothing caught it and nothing resolved the caller, so the request hung
2365+
until its read timeout — a 401 was indistinguishable from a slow server. The `fail_after` below
2366+
is what pins that: without the fix this test hangs rather than failing an assertion.
2367+
"""
2368+
2369+
def handler(request: httpx.Request) -> httpx.Response:
2370+
return httpx.Response(status)
2371+
2372+
with anyio.fail_after(5):
2373+
async with (
2374+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2375+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2376+
read,
2377+
write,
2378+
):
2379+
request = types.JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})
2380+
await write.send(SessionMessage(types.JSONRPCMessage(request)))
2381+
reply = await read.receive()
2382+
2383+
assert isinstance(reply, SessionMessage)
2384+
assert isinstance(reply.message.root, types.JSONRPCError)
2385+
assert reply.message.root.id == 1
2386+
assert reply.message.root.error == types.ErrorData(
2387+
code=types.INTERNAL_ERROR, message="Server returned an error response"
2388+
)
2389+
2390+
2391+
@pytest.mark.anyio
2392+
async def test_404_still_reports_session_terminated() -> None:
2393+
"""The 404 spelling is unchanged by #3091: same message, same (positive) code callers match on."""
2394+
2395+
def handler(request: httpx.Request) -> httpx.Response:
2396+
return httpx.Response(404)
2397+
2398+
with anyio.fail_after(5):
2399+
async with (
2400+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2401+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2402+
read,
2403+
write,
2404+
):
2405+
request = types.JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})
2406+
await write.send(SessionMessage(types.JSONRPCMessage(request)))
2407+
reply = await read.receive()
2408+
2409+
assert isinstance(reply, SessionMessage)
2410+
assert isinstance(reply.message.root, types.JSONRPCError)
2411+
assert reply.message.root.error == types.ErrorData(code=32600, message="Session terminated")
2412+
2413+
2414+
@pytest.mark.anyio
2415+
async def test_non_2xx_prefers_the_servers_own_jsonrpc_error_body() -> None:
2416+
"""When the server explains itself in the body, the caller gets that, not our stand-in.
2417+
2418+
The reply is re-stamped with the request's id rather than trusting the body's, so a server that
2419+
echoes a different id cannot resolve the wrong caller (or, echoing none, no caller at all).
2420+
"""
2421+
2422+
def handler(request: httpx.Request) -> httpx.Response:
2423+
error = {"code": types.INVALID_PARAMS, "message": "Unknown tool: nope"}
2424+
return httpx.Response(400, json={"jsonrpc": "2.0", "id": 99, "error": error})
2425+
2426+
with anyio.fail_after(5):
2427+
async with (
2428+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2429+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2430+
read,
2431+
write,
2432+
):
2433+
request = types.JSONRPCRequest(jsonrpc="2.0", id=7, method="tools/call", params={})
2434+
await write.send(SessionMessage(types.JSONRPCMessage(request)))
2435+
reply = await read.receive()
2436+
2437+
assert isinstance(reply, SessionMessage)
2438+
assert isinstance(reply.message.root, types.JSONRPCError)
2439+
assert reply.message.root.id == 7
2440+
assert reply.message.root.error == types.ErrorData(code=types.INVALID_PARAMS, message="Unknown tool: nope")
2441+
2442+
2443+
@pytest.mark.anyio
2444+
async def test_a_failed_request_leaves_the_session_usable() -> None:
2445+
"""The connection survives a 500: the error resolves one call, it does not tear down the transport.
2446+
2447+
In v1 the escaping `HTTPStatusError` cancelled the transport's task group, so a single 500 took
2448+
the whole session with it.
2449+
"""
2450+
2451+
def handler(request: httpx.Request) -> httpx.Response:
2452+
request_id = json.loads(request.content)["id"]
2453+
if request_id == 1:
2454+
return httpx.Response(500)
2455+
return httpx.Response(200, json={"jsonrpc": "2.0", "id": request_id, "result": {}})
2456+
2457+
with anyio.fail_after(5):
2458+
async with (
2459+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2460+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2461+
read,
2462+
write,
2463+
):
2464+
failing = types.JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})
2465+
await write.send(SessionMessage(types.JSONRPCMessage(failing)))
2466+
first = await read.receive()
2467+
2468+
following = types.JSONRPCRequest(jsonrpc="2.0", id=2, method="tools/list", params={})
2469+
await write.send(SessionMessage(types.JSONRPCMessage(following)))
2470+
second = await read.receive()
2471+
2472+
assert isinstance(first, SessionMessage)
2473+
assert isinstance(first.message.root, types.JSONRPCError)
2474+
assert isinstance(second, SessionMessage)
2475+
assert isinstance(second.message.root, types.JSONRPCResponse)
2476+
2477+
2478+
@pytest.mark.anyio
2479+
async def test_non_2xx_to_a_notification_resolves_nobody_and_keeps_the_session_alive() -> None:
2480+
"""A notification has no id and no waiter, so a 4xx to one cannot be surfaced — only survived.
2481+
2482+
The following request still gets its answer: the rejected notification must not take the
2483+
transport down with it.
2484+
"""
2485+
2486+
def handler(request: httpx.Request) -> httpx.Response:
2487+
body = json.loads(request.content)
2488+
if "id" not in body:
2489+
return httpx.Response(500)
2490+
return httpx.Response(200, json={"jsonrpc": "2.0", "id": body["id"], "result": {}})
2491+
2492+
with anyio.fail_after(5):
2493+
async with (
2494+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2495+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2496+
read,
2497+
write,
2498+
):
2499+
notification = types.JSONRPCNotification(jsonrpc="2.0", method="notifications/progress")
2500+
await write.send(SessionMessage(types.JSONRPCMessage(notification)))
2501+
2502+
request = types.JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})
2503+
await write.send(SessionMessage(types.JSONRPCMessage(request)))
2504+
reply = await read.receive()
2505+
2506+
assert isinstance(reply, SessionMessage)
2507+
assert isinstance(reply.message.root, types.JSONRPCResponse)
2508+
2509+
2510+
@pytest.mark.anyio
2511+
@pytest.mark.parametrize(
2512+
("body", "case"),
2513+
[
2514+
({"jsonrpc": "2.0", "id": 1, "result": {}}, "valid JSON-RPC, but not an error"),
2515+
({"detail": "gateway rejected the request"}, "JSON, but not JSON-RPC at all"),
2516+
],
2517+
)
2518+
async def test_a_json_body_that_is_not_a_jsonrpc_error_falls_back_to_the_stand_in(
2519+
body: dict[str, Any], case: str
2520+
) -> None:
2521+
"""`Content-Type: application/json` is not a promise of a JSON-RPC error body.
2522+
2523+
Proxies and gateways send JSON that is nothing of the sort. Neither shape may be handed to the
2524+
caller as their answer, and neither may be allowed to escape: both fall back to the stand-in.
2525+
"""
2526+
2527+
def handler(request: httpx.Request) -> httpx.Response:
2528+
return httpx.Response(500, json=body)
2529+
2530+
with anyio.fail_after(5):
2531+
async with (
2532+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2533+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2534+
read,
2535+
write,
2536+
):
2537+
request = types.JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})
2538+
await write.send(SessionMessage(types.JSONRPCMessage(request)))
2539+
reply = await read.receive()
2540+
2541+
assert isinstance(reply, SessionMessage)
2542+
assert isinstance(reply.message.root, types.JSONRPCError)
2543+
assert reply.message.root.error == types.ErrorData(
2544+
code=types.INTERNAL_ERROR, message="Server returned an error response"
2545+
)

0 commit comments

Comments
 (0)