Skip to content

Commit a674d2a

Browse files
fix(client): catch read failures on the non-2xx body
`httpx.StreamError` derives from `RuntimeError`, not `httpx.HTTPError`, so it does not cover a body that stalls or truncates while being read: `aread()` raises `ReadTimeout`/`RemoteProtocolError` there. Those escaped into the POST's background task and stranded the caller — the bug this path exists to prevent. Github-Issue: #3091
1 parent d53d47b commit a674d2a

2 files changed

Lines changed: 43 additions & 3 deletions

File tree

src/mcp/client/streamable_http.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,12 @@ async def _send_http_status_error(
542542
reply = JSONRPCError(jsonrpc="2.0", id=request_id, error=parsed.root.error)
543543
await read_stream_writer.send(SessionMessage(JSONRPCMessage(reply)))
544544
return
545-
except (httpx.StreamError, ValidationError):
546-
logger.debug("Non-2xx body was not a JSON-RPC error; using the status-derived fallback")
545+
# `httpx.HTTPError` covers reading the body as much as parsing it: a stalled or
546+
# truncated error body raises `ReadTimeout`/`RemoteProtocolError` here, and letting
547+
# that escape would strand the caller exactly as the bug this method exists to fix.
548+
# `StreamError` is not one of its subclasses (it derives from `RuntimeError`).
549+
except (httpx.HTTPError, httpx.StreamError, ValidationError):
550+
logger.debug("Could not read a JSON-RPC error from the non-2xx body; using the fallback")
547551

548552
error_data = ErrorData(code=INTERNAL_ERROR, message="Server returned an error response")
549553
jsonrpc_error = JSONRPCError(jsonrpc="2.0", id=request_id, error=error_data)

tests/shared/test_streamable_http.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import multiprocessing
99
import socket
1010
import time
11-
from collections.abc import Generator
11+
from collections.abc import AsyncIterator, Generator
1212
from datetime import timedelta
1313
from typing import Any
1414
from unittest.mock import MagicMock
@@ -2543,3 +2543,39 @@ def handler(request: httpx.Request) -> httpx.Response:
25432543
assert reply.message.root.error == types.ErrorData(
25442544
code=types.INTERNAL_ERROR, message="Server returned an error response"
25452545
)
2546+
2547+
2548+
@pytest.mark.anyio
2549+
async def test_a_non_2xx_body_that_fails_to_read_still_resolves_the_caller() -> None:
2550+
"""A stalled or truncated error body must not strand the caller.
2551+
2552+
`aread()` on the non-2xx body raises `ReadTimeout`/`RemoteProtocolError` when the connection
2553+
dies mid-body. Those derive from `httpx.HTTPError`, not `httpx.StreamError`, so letting the
2554+
body-parsing `except` miss them would put the escaping exception right back into the POST's
2555+
background task — the very bug this path exists to fix.
2556+
"""
2557+
2558+
class _DyingStream(httpx.AsyncByteStream):
2559+
async def __aiter__(self) -> AsyncIterator[bytes]:
2560+
raise httpx.ReadTimeout("connection died mid-body")
2561+
yield b"" # pragma: no cover
2562+
2563+
def handler(request: httpx.Request) -> httpx.Response:
2564+
return httpx.Response(500, headers={"content-type": "application/json"}, stream=_DyingStream())
2565+
2566+
with anyio.fail_after(5):
2567+
async with (
2568+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client,
2569+
streamable_http_client("http://test/mcp", http_client=http_client) as (read, write, _),
2570+
read,
2571+
write,
2572+
):
2573+
request = types.JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})
2574+
await write.send(SessionMessage(types.JSONRPCMessage(request)))
2575+
reply = await read.receive()
2576+
2577+
assert isinstance(reply, SessionMessage)
2578+
assert isinstance(reply.message.root, types.JSONRPCError)
2579+
assert reply.message.root.error == types.ErrorData(
2580+
code=types.INTERNAL_ERROR, message="Server returned an error response"
2581+
)

0 commit comments

Comments
 (0)