Skip to content

Commit 7c0124c

Browse files
fix(streamable-http): count bare-priming-then-EOF reconnects against the request budget
A reconnect that reaches EOF without delivering any real data (only a bare id-bearing priming event) was resetting the attempt counter to 0 instead of incrementing it. This let a server that repeatedly opened the resumable stream, emitted only a priming event, and closed again reconnect forever rather than giving up after MAX_RECONNECTION_ATTEMPTS and resolving the waiter with CONNECTION_CLOSED. Track whether any event with non-empty data was received during the reconnect. A reconnect that made real progress (delivered a notification) still earns a fresh budget for the next reconnect; a reconnect that saw only bare priming events counts against the budget the same way a transport exception does. Adds a regression test that drives _handle_reconnection with a mock transport returning priming-then-EOF on every reconnect and asserts the waiter resolves with CONNECTION_CLOSED after exactly MAX_RECONNECTION_ATTEMPTS attempts. Fixes #3307
1 parent 31b76cb commit 7c0124c

2 files changed

Lines changed: 61 additions & 2 deletions

File tree

src/mcp/client/streamable_http.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,7 @@ async def _handle_reconnection(
508508
# Track for potential further reconnection
509509
reconnect_last_event_id: str = last_event_id
510510
reconnect_retry_ms = retry_interval_ms
511+
made_progress = False
511512

512513
async for sse in event_source:
513514
if sse.id: # pragma: no branch
@@ -525,9 +526,17 @@ async def _handle_reconnection(
525526
await event_source.response.aclose()
526527
return
527528

528-
# Stream ended again without response - reconnect again (reset attempt counter)
529+
# A real event (notification) earns a fresh budget for the next
530+
# reconnect. A bare priming event (empty data) does not.
531+
if sse.data:
532+
made_progress = True
533+
534+
# Stream ended again without response. Reset the budget only when this
535+
# reconnect actually delivered a real event; bare-priming-then-EOF counts
536+
# against the budget the same way a transport exception does.
529537
logger.info("SSE stream disconnected, reconnecting...")
530-
await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, 0)
538+
next_attempt = 0 if made_progress else attempt + 1
539+
await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, next_attempt)
531540
except Exception as e: # pragma: no cover
532541
logger.debug(f"Reconnection failed: {e}")
533542
# Try to reconnect again if we still have an event ID

tests/client/test_streamable_http.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,56 @@ async def test_exhausted_reconnection_attempts_resolve_the_request_with_an_error
736736
receive.close()
737737

738738

739+
class _PrimingOnlySSEStream(httpx2.AsyncByteStream):
740+
"""Emits a single bare priming event (id only, empty data) then closes."""
741+
742+
def __init__(self, event_id: str) -> None:
743+
self._bytes = f"id: {event_id}\n\n".encode()
744+
745+
async def __aiter__(self) -> AsyncIterator[bytes]:
746+
yield self._bytes
747+
748+
async def aclose(self) -> None:
749+
pass
750+
751+
752+
@pytest.mark.anyio
753+
async def test_empty_resumable_sse_reconnects_count_toward_the_request_budget() -> None:
754+
"""A reconnect that delivers only a bare priming event and reaches EOF must consume
755+
the reconnect budget, the same as the exception path.
756+
757+
Before the fix, the clean-EOF branch always reset attempt to 0, so a server that
758+
kept sending only priming events could reconnect forever. The fix increments the
759+
counter when no real data arrived during the reconnect, giving up after exactly
760+
MAX_RECONNECTION_ATTEMPTS attempts and resolving the waiter with CONNECTION_CLOSED."""
761+
call_count = 0
762+
763+
def handler(request: httpx2.Request) -> httpx2.Response:
764+
nonlocal call_count
765+
call_count += 1
766+
return httpx2.Response(
767+
200,
768+
headers={"content-type": "text/event-stream"},
769+
stream=_PrimingOnlySSEStream(f"evt-{call_count}"),
770+
)
771+
772+
transport = StreamableHTTPTransport("http://test/mcp")
773+
send, receive = create_context_streams[SessionMessage | Exception](1)
774+
async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http:
775+
with anyio.fail_after(5):
776+
await transport._handle_reconnection( # pyright: ignore[reportPrivateUsage]
777+
_abandoned_request_context(http, send), "evt-0", 0
778+
)
779+
reply = await receive.receive()
780+
assert isinstance(reply, SessionMessage)
781+
assert isinstance(reply.message, JSONRPCError)
782+
assert reply.message.id == "listen-1"
783+
assert reply.message.error.code == CONNECTION_CLOSED
784+
assert call_count == MAX_RECONNECTION_ATTEMPTS
785+
send.close()
786+
receive.close()
787+
788+
739789
@pytest.mark.anyio
740790
async def test_resolving_an_abandoned_request_after_the_reader_closed_is_contained() -> None:
741791
"""Teardown race: a stream dying after the reader closed resolves best-effort and must not crash."""

0 commit comments

Comments
 (0)