Skip to content

Commit dcbbf6d

Browse files
committed
fix(client): prevent tool hang by failing pending requests on transport exception (Closes #1401)
1 parent 6affe5c commit dcbbf6d

3 files changed

Lines changed: 123 additions & 5 deletions

File tree

src/mcp/client/session.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,15 @@ async def __call__(self, message: IncomingMessage) -> None: ... # pragma: no br
243243

244244

245245
async def _default_message_handler(message: IncomingMessage) -> None:
246+
# Transport-level faults (e.g. an SSE read timeout) reach the handler as the
247+
# `Exception` arm of `IncomingMessage`. The default used to only checkpoint,
248+
# so these vanished with no trace. Log them at ERROR: in-flight requests are
249+
# already failed by the dispatcher, but a fault with no request in flight
250+
# (an idle SSE stream dropping) would otherwise leave no signal at all.
251+
# Server notifications remain a no-op - a handler is optional for those.
252+
if isinstance(message, Exception):
253+
logger.error("transport error surfaced to message handler: %r", message)
254+
return
246255
await anyio.lowlevel.checkpoint()
247256

248257

src/mcp/shared/jsonrpc_dispatcher.py

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,11 @@ async def _dispatch(
536536
are awaited; any other `await` would head-of-line block the read loop.
537537
"""
538538
if isinstance(item, Exception):
539+
# A transport fault fails every in-flight request now, rather than
540+
# leaving each waiter to block until its own `opts["timeout"]`
541+
# elapses (or forever, if it has none). Done before the observer
542+
# runs so a slow observer can't delay freeing the waiters.
543+
self._fail_pending(item)
539544
if self.on_stream_exception is None:
540545
logger.debug("transport yielded exception: %r", item)
541546
return
@@ -685,19 +690,36 @@ def _spawn(
685690
else:
686691
self._tg.start_soon(fn, *args)
687692

688-
def _fan_out_closed(self) -> None:
689-
"""Wake every pending `send_raw_request` waiter with `CONNECTION_CLOSED`.
693+
def _fan_out_error(self, error: ErrorData) -> None:
694+
"""Wake every pending `send_raw_request` waiter with `error`.
690695
691-
Synchronous: callers may be inside a cancelled scope. Idempotent.
696+
Synchronous: callers may be inside a cancelled scope. Idempotent - the
697+
`_pending` table is cleared, so a second call is a no-op. A waiter that
698+
already holds an outcome (buffer-of-1 full) keeps it: the `WouldBlock`
699+
is swallowed rather than clobbering the real response with the signal.
692700
"""
693-
closed = ErrorData(code=CONNECTION_CLOSED, message="Connection closed")
694701
for pending in self._pending.values():
695702
try:
696-
pending.send.send_nowait(closed)
703+
pending.send.send_nowait(error)
697704
except (anyio.WouldBlock, anyio.BrokenResourceError, anyio.ClosedResourceError):
698705
pass
699706
self._pending.clear()
700707

708+
def _fan_out_closed(self) -> None:
709+
"""Wake every pending `send_raw_request` waiter with `CONNECTION_CLOSED` (EOF/shutdown)."""
710+
self._fan_out_error(ErrorData(code=CONNECTION_CLOSED, message="Connection closed"))
711+
712+
def _fail_pending(self, exc: Exception) -> None:
713+
"""Wake every pending waiter because the read stream yielded a transport fault.
714+
715+
Reported as `CONNECTION_CLOSED` for parity with the EOF path (`send_raw_request`
716+
raises `MCPError` either way), but the message carries `exc` so callers can see
717+
what actually broke - e.g. an `httpx.ReadTimeout` from a low `sse_read_timeout` -
718+
instead of the request hanging until its own timeout. The raw `exc` still reaches
719+
any `on_stream_exception` observer untouched.
720+
"""
721+
self._fan_out_error(ErrorData(code=CONNECTION_CLOSED, message=f"Transport error: {exc!r}"))
722+
701723
async def _handle_request(
702724
self,
703725
req: JSONRPCRequest,

tests/shared/test_jsonrpc_dispatcher.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2514,3 +2514,90 @@ async def call() -> None:
25142514
for stream in (c2s_send, c2s_recv, s2c_send, s2c_recv):
25152515
stream.close()
25162516
assert result_box == [{"ok": True}]
2517+
2518+
2519+
@pytest.mark.anyio
2520+
async def test_transport_exception_fails_pending_request_without_hanging():
2521+
"""A read-stream fault wakes an in-flight `send_raw_request` with `CONNECTION_CLOSED`.
2522+
2523+
Regression for the streamable-http hang: before this, a transport error (e.g. an SSE
2524+
read timeout) reached only the observer, so a request already waiting on its response
2525+
sat until its own `opts["timeout"]` elapsed. Now the dispatcher fails the waiter at once,
2526+
and the error message carries the transport exception so the caller can see the cause.
2527+
"""
2528+
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](4)
2529+
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](4)
2530+
2531+
client: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(c2s_recv, s2c_send)
2532+
on_request, on_notify = echo_handlers(Recorder())
2533+
outcome: dict[str, BaseException] = {}
2534+
boom = TimeoutError("sse read timed out")
2535+
try:
2536+
async with anyio.create_task_group() as tg:
2537+
await tg.start(client.run, on_request, on_notify)
2538+
2539+
async def call() -> None:
2540+
# No `timeout` opt: without the fix this would block forever.
2541+
try:
2542+
await client.send_raw_request("tools/call", {"name": "slow"})
2543+
except BaseException as exc: # noqa: BLE001 - capture whatever the waiter raises
2544+
outcome["exc"] = exc
2545+
2546+
tg.start_soon(call)
2547+
# Let the request register in `_pending` and park on its response.
2548+
with anyio.fail_after(5):
2549+
sent = await s2c_recv.receive()
2550+
assert isinstance(sent, SessionMessage)
2551+
assert isinstance(sent.message, JSONRPCRequest)
2552+
2553+
# The transport now yields an exception instead of a response.
2554+
await c2s_send.send(boom)
2555+
with anyio.fail_after(5):
2556+
while "exc" not in outcome:
2557+
await anyio.sleep(0)
2558+
tg.cancel_scope.cancel()
2559+
finally:
2560+
for s in (c2s_send, c2s_recv, s2c_send, s2c_recv):
2561+
s.close()
2562+
2563+
raised = outcome["exc"]
2564+
assert isinstance(raised, MCPError)
2565+
assert raised.error.code == CONNECTION_CLOSED
2566+
# The original transport exception is preserved in the message for debugging.
2567+
assert "sse read timed out" in raised.error.message
2568+
2569+
2570+
def test_fail_pending_reports_transport_exception_and_clears_pending():
2571+
"""White-box: `_fail_pending` wakes waiters with the exception detail, then empties `_pending`."""
2572+
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](1)
2573+
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](1)
2574+
d: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(s2c_recv, c2s_send)
2575+
send, recv = anyio.create_memory_object_stream[dict[str, Any] | ErrorData](1)
2576+
d._pending[1] = _Pending(send=send, receive=recv) # pyright: ignore[reportPrivateUsage]
2577+
2578+
d._fail_pending(TimeoutError("sse read timed out")) # pyright: ignore[reportPrivateUsage]
2579+
2580+
signalled = recv.receive_nowait()
2581+
assert isinstance(signalled, ErrorData)
2582+
assert signalled.code == CONNECTION_CLOSED
2583+
assert "sse read timed out" in signalled.message
2584+
assert d._pending == {} # pyright: ignore[reportPrivateUsage]
2585+
for s in (c2s_send, c2s_recv, s2c_send, s2c_recv, send, recv):
2586+
s.close()
2587+
2588+
2589+
def test_fail_pending_keeps_existing_outcome_when_waiter_already_resolved():
2590+
"""White-box: a waiter that already holds a real result is not clobbered by the fault signal."""
2591+
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](1)
2592+
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](1)
2593+
d: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(s2c_recv, c2s_send)
2594+
send, recv = anyio.create_memory_object_stream[dict[str, Any] | ErrorData](1)
2595+
d._pending[1] = _Pending(send=send, receive=recv) # pyright: ignore[reportPrivateUsage]
2596+
send.send_nowait({"real": "result"})
2597+
2598+
d._fail_pending(TimeoutError("sse read timed out")) # pyright: ignore[reportPrivateUsage]
2599+
2600+
assert recv.receive_nowait() == {"real": "result"}
2601+
assert d._pending == {} # pyright: ignore[reportPrivateUsage]
2602+
for s in (c2s_send, c2s_recv, s2c_send, s2c_recv, send, recv):
2603+
s.close()

0 commit comments

Comments
 (0)