Skip to content

Commit b237e91

Browse files
author
Jianke LIN
committed
fix(streamable-http): fail request when resumption can't complete
1 parent 53117cb commit b237e91

2 files changed

Lines changed: 96 additions & 5 deletions

File tree

src/mcp/client/streamable_http.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from anyio.abc import TaskGroup
1414
from httpx_sse import EventSource, ServerSentEvent, aconnect_sse
1515
from mcp_types import (
16+
CONNECTION_CLOSED,
1617
INTERNAL_ERROR,
1718
INVALID_REQUEST,
1819
METHOD_NOT_FOUND,
@@ -437,10 +438,16 @@ async def _handle_sse_response(
437438
except Exception:
438439
logger.debug("SSE stream ended", exc_info=True) # pragma: lax no cover
439440

440-
# Stream ended without response - reconnect if we received an event with ID
441-
if last_event_id is not None: # pragma: no branch
442-
logger.info("SSE stream disconnected, reconnecting...")
443-
await self._handle_reconnection(ctx, last_event_id, retry_interval_ms)
441+
# Stream ended without a terminal response/error. If the server provided an event id,
442+
# try resuming; otherwise fail the request instead of hanging forever.
443+
if last_event_id is None:
444+
error_data = ErrorData(code=CONNECTION_CLOSED, message="SSE stream disconnected before response completed")
445+
error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=original_request_id, error=error_data))
446+
await ctx.read_stream_writer.send(error_msg)
447+
return
448+
449+
logger.info("SSE stream disconnected, reconnecting...")
450+
await self._handle_reconnection(ctx, last_event_id, retry_interval_ms)
444451

445452
async def _handle_reconnection(
446453
self,
@@ -451,7 +458,19 @@ async def _handle_reconnection(
451458
) -> None:
452459
"""Reconnect with Last-Event-ID to resume stream after server disconnect."""
453460
# Bail if max retries exceeded
454-
if attempt >= MAX_RECONNECTION_ATTEMPTS: # pragma: no cover
461+
if attempt >= MAX_RECONNECTION_ATTEMPTS:
462+
original_request_id = None
463+
if isinstance(ctx.session_message.message, JSONRPCRequest): # pragma: no branch
464+
original_request_id = ctx.session_message.message.id
465+
466+
if original_request_id is not None:
467+
error_data = ErrorData(
468+
code=CONNECTION_CLOSED,
469+
message="SSE stream disconnected and could not be resumed",
470+
data={"last_event_id": last_event_id},
471+
)
472+
error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=original_request_id, error=error_data))
473+
await ctx.read_stream_writer.send(error_msg)
455474
logger.debug(f"Max reconnection attempts ({MAX_RECONNECTION_ATTEMPTS}) exceeded")
456475
return
457476

tests/interaction/transports/test_hosting_resume.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,78 @@ async def call() -> None:
373373
assert received == snapshot(["before close", "after close"])
374374

375375

376+
async def test_a_call_whose_stream_closes_and_cannot_be_resumed_fails_instead_of_hanging() -> None:
377+
"""If a resumable response stream disconnects and the server session is gone, the client fails
378+
the request instead of hanging forever.
379+
380+
The server closes the call's SSE stream after emitting one related notification. The test then
381+
deletes the active server-side session to force the client's reconnect GET to return 404.
382+
Without a terminal response/error on the read stream, ClientSession.send_request waits forever
383+
(read timeout defaults to None). The transport must surface a request-scoped error when it
384+
gives up reconnecting.
385+
"""
386+
reconnect_attempted = anyio.Event()
387+
allow_exit = anyio.Event()
388+
done = anyio.Event()
389+
raised: list[BaseException] = []
390+
manager_ref = None
391+
deleted_session = False
392+
393+
mcp = MCPServer("resumable")
394+
395+
@mcp.tool()
396+
async def interrupt(ctx: Context) -> str:
397+
await ctx.info("before close")
398+
await ctx.close_sse_stream()
399+
await allow_exit.wait()
400+
return "unreachable"
401+
402+
async def record_request(request: httpx.Request) -> None:
403+
nonlocal deleted_session
404+
if request.method != "GET":
405+
return
406+
if request.headers.get("last-event-id") is None:
407+
return
408+
reconnect_attempted.set()
409+
if deleted_session or manager_ref is None:
410+
return
411+
session_ids = list(manager_ref._server_instances.keys())
412+
if session_ids: # pragma: no branch
413+
del manager_ref._server_instances[session_ids[0]]
414+
deleted_session = True
415+
416+
async with mounted_app(mcp, event_store=SequencedEventStore(), retry_interval=0, on_request=record_request) as (
417+
http,
418+
manager,
419+
):
420+
manager_ref = manager
421+
with anyio.fail_after(5): # pragma: no branch
422+
async with (
423+
streamable_http_client(f"{BASE_URL}/mcp", http_client=http, terminate_on_close=False) as (r, w),
424+
ClientSession(r, w) as session,
425+
anyio.create_task_group() as tg,
426+
):
427+
await session.initialize()
428+
429+
async def call() -> None:
430+
try:
431+
await session.call_tool("interrupt", {})
432+
except BaseException as exc:
433+
raised.append(exc)
434+
finally:
435+
done.set()
436+
437+
tg.start_soon(call)
438+
await reconnect_attempted.wait()
439+
await done.wait()
440+
allow_exit.set()
441+
tg.cancel_scope.cancel()
442+
443+
assert len(raised) == 1
444+
assert isinstance(raised[0], Exception)
445+
assert "disconnected" in str(raised[0]).lower()
446+
447+
376448
@requirement("client-transport:http:resume-stream-api")
377449
async def test_a_captured_resumption_token_replays_missed_messages_on_a_new_connection() -> None:
378450
"""A resumption token captured via on_resumption_token_update on one connection lets a fresh

0 commit comments

Comments
 (0)