Skip to content

fix(client): surface HTTP errors on resumption GET and SSE message POST#3278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
claude wants to merge 8 commits into
base:main
Choose a base branch
from
Open

fix(client): surface HTTP errors on resumption GET and SSE message POST #3278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
15b394c
fix(client): surface HTTP errors on resumption GET and SSE message POST
claude Aug 11, 2026
e5fe739
test: use one parenthesized async-with to dodge py3.14 coverage phant…
claude Aug 11, 2026
b4edbd4
fix(client): harden the resumption GET and SSE POST error paths per r…
claude Aug 11, 2026
51d99af
fix(client): cover OAuth failures and 404 session expiry on the SSE P…
claude Aug 11, 2026
6472241
fix(client): contain arbitrary POST failures and dedupe the status-er…
claude Aug 11, 2026
af77621
docs: scope the connect-failure escape note to request POSTs
claude Aug 11, 2026
d25e6f8
fix(client): export status_error_data in _transport's __all__
claude Aug 11, 2026
66a9285
docs: align the SSE notification-POST sentence with its streamable si…
claude Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion docs/migration.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2188,6 +2188,25 @@ In v1, a non-2xx response to a message POST (other than 404) raised `httpx.HTTPS
| 404, no session yet | `McpError` with positive code `32600` | `MCPError(-32601, 'Not Found')` |
| Any other 4xx/5xx | `httpx.HTTPStatusError` escapes as `ExceptionGroup` | `MCPError(-32603, 'Server returned an error response')` |

The same contract covers the resumption GET — a request re-attached with a resumption token (`Last-Event-ID`). In v1 a failure there escaped the context as an `ExceptionGroup` that failed every pending request, or hung the resumed call forever:

| Resumption GET outcome | v1 | v2 |
| --- | --- | --- |
| 404, session established | `httpx.HTTPStatusError` escapes as `ExceptionGroup` | `MCPError(-32600, 'Session terminated')` |
| Any other non-2xx | `httpx.HTTPStatusError` escapes as `ExceptionGroup` | `MCPError(-32603, 'Server returned an error response')` |
| Stream drops mid-read | error escapes as `ExceptionGroup` | `MCPError(-32000, 'resumption stream ended without a response')` |
| Stream ends cleanly with no response | resumed call hangs forever | `MCPError(-32000, 'resumption stream ended without a response')` |

The SSE transport (`sse_client`) applies the same rule to its message POST. In v1 *any* POST failure — a non-2xx status, a network error, or an OAuth re-auth failure raised by the configured `auth` — was caught and logged inside the transport's writer task: the waiting caller hung forever and the write loop died, so every later send was silently dropped. In v2 the failing request resolves promptly and the session stays usable:

| Message POST outcome | v1 | v2 |
| --- | --- | --- |
| 404, endpoint URL carries a session id | caller hangs forever; write loop dies | `MCPError(-32600, 'Session terminated')` |
| Any other non-2xx | caller hangs forever; write loop dies | `MCPError(-32603, 'Server returned an error response')` |
| Network-level failure (`httpx2.ConnectError`, timeouts) or OAuth flow failure | caller hangs forever; write loop dies | `MCPError(-32000, 'Failed to send message: ...')` |

A failed POST of a *notification or response* has no caller to resolve; v2 logs and drops it, keeping the write loop (and every later send) alive.

Both common v1 patterns silently stop working: an `except* httpx.HTTPStatusError` around the transport context becomes dead code because status errors no longer escape the context, and a session-expiry check on `error.code == 32600` never matches again because the code is now the standard negative `-32600`.

**Before (v1):**
Expand DownExpand Up@@ -2230,7 +2249,7 @@ async with streamable_http_client(url) as (read, write):
raise
```

Move HTTP-status failure handling from around the transport context to around the individual calls, catching `MCPError` (see [`McpError` renamed to `MCPError`](#mcperror-renamed-to-mcperror)). Connect-level failures such as `httpx2.ConnectError` still escape the transport context as before; keep context-level handling for those only.
Move HTTP-status failure handling from around the transport context to around the individual calls, catching `MCPError` (see [`McpError` renamed to `MCPError`](#mcperror-renamed-to-mcperror)). On the streamable HTTP transport, connect-level failures such as `httpx2.ConnectError` on a *request's* message POST still escape the transport context as before — keep context-level handling for those; on the resumption GET and on the SSE transport's message POST they resolve the failing request instead, as above. A connect-level failure POSTing a *notification or response* on streamable HTTP is logged and does not escape the context, but it kills the transport's write loop — pre-existing behavior, unchanged from v1.

### `terminate_windows_process` removed

Expand Down
16 changes: 15 additions & 1 deletion src/mcp/client/_transport.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,28 @@
from contextlib import AbstractAsyncContextManager
from typing import Protocol

from mcp_types import INTERNAL_ERROR, INVALID_REQUEST, ErrorData

from mcp.shared._stream_protocols import ReadStream, WriteStream
from mcp.shared.message import SessionMessage

__all__ = ["ReadStream", "WriteStream", "Transport", "TransportStreams"]
__all__ = ["ReadStream", "WriteStream", "Transport", "TransportStreams", "status_error_data"]

TransportStreams = tuple[ReadStream[SessionMessage | Exception], WriteStream[SessionMessage]]


def status_error_data(status_code: int, *, has_session: bool) -> ErrorData:
"""Map a non-2xx HTTP status on a client transport request to the error its waiting caller receives.

A 404 while a session is held is the session-expiry signal (`INVALID_REQUEST`,
"Session terminated"); anything else gets the generic stand-in. A call site with
an extra status mapping (e.g. the message POST's pre-session 404) branches first.
"""
if status_code == 404 and has_session:
return ErrorData(code=INVALID_REQUEST, message="Session terminated")
return ErrorData(code=INTERNAL_ERROR, message="Server returned an error response")

Comment thread
claude[bot] marked this conversation as resolved.

class Transport(AbstractAsyncContextManager[TransportStreams], Protocol):
"""Protocol for MCP transports.

Expand Down
56 changes: 46 additions & 10 deletions src/mcp/client/sse.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@
fromanyio.abcimportTaskStatus
fromhttpx2importSSEError

frommcp.client._transportimportstatus_error_data
frommcp.shared._compatimportresync_tracer
frommcp.shared._context_streamsimportcreate_context_streams
frommcp.shared._httpx_utilsimportMcpHttpClientFactory, create_mcp_http_client
Expand DownExpand Up@@ -120,17 +121,52 @@ async def post_writer(endpoint_url: str):
asyncwithwrite_stream_reader, write_stream:

asyncdef_send_message(session_message: SessionMessage) ->None:
# A POST failure must not raise: the post_writer handler below
# would swallow it, hanging the waiting caller forever and killing
# the write loop (#2110). Mirror the streamable-HTTP transport
# instead: resolve the waiter with an error correlated to its
# request id, keeping the session usable.
logger.debug(f"Sending client message: {session_message}")
response=awaitclient.post(
endpoint_url,
json=session_message.message.model_dump(
by_alias=True,
mode="json",
exclude_unset=True,
),
)
response.raise_for_status()
logger.debug(f"Client message sent successfully: {response.status_code}")
message=session_message.message
try:
response=awaitclient.post(
endpoint_url,
json=message.model_dump(
by_alias=True,
mode="json",
exclude_unset=True,
),
)
exceptExceptionasexc:
# Terminal containment boundary: beyond httpx's own errors,
Comment on lines +124 to +141

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing issue (not introduced by this PR, but in code it rewrites end-to-end): _send_message POSTs with client.post(endpoint_url, json=...) and never consults session_message.metadata, so the per-request headers documented as a cross-transport contract in CallOptions.headers (per-request auth, tracing) and the session-stamped MCP-Protocol-Version are silently dropped on every SSE message POST — while streamable HTTP honors the same metadata via headers.update(ctx.metadata.headers). Same class as the resumption/reconnection-GET header drop already slated for the grouped follow-up on streamable_http.py; the follow-up should cover this leg too, with a one-line headers= merge here.

Extended reasoning...

What the bug is.CallOptions.headers is documented at src/mcp/shared/dispatcher.py:127-128 as a transport-layer contract: "HTTP transports merge these onto the outgoing request; non-HTTP transports ignore." The SSE transport is an HTTP transport, but its message POST never honors the contract: _send_message in src/mcp/client/sse.py reads only session_message.message and calls client.post(endpoint_url, json=...) with no headers= argument and no look at session_message.metadata. Everything the caller stamped via the public CallOptions["headers"] — per-request auth tokens, tracing headers, tenant routing — is silently discarded, and nothing logs the drop.\n\nThe metadata is populated on essentially every SSE message. The dispatcher honors its side of the contract: _plan_outbound (src/mcp/shared/jsonrpc_dispatcher.py:250-256) attaches the caller's headers to ClientMessageMetadata(headers=headers) on the outbound SessionMessage. And ClientSession stamps headers regardless of transport: after a legacy handshake — the only kind an SSE server produces — self._stamp = _make_handshake_stamp(version) (src/mcp/client/session.py:666) writes opts["headers"][MCP_PROTOCOL_VERSION_HEADER] = version into every subsequent request and notification (session.py:113-117, applied at 530/573). So the drop fires constantly, not just for exotic callers — the SDK's own protocol-version stamp never reaches the wire on this transport.\n\nWhy the sibling transport makes this surprising. The streamable HTTP transport honors the identical metadata in _handle_post_request: headers.update(ctx.metadata.headers). Code that works there — e.g. per-request auth against a proxy — silently loses its headers when pointed at an SSE server, with no client-side signal to diagnose.\n\nStep-by-step proof. (1) A user calls session.call_tool(..., options={"headers": {"authorization": "Bearer per-request-token"}}) against an SSE server behind an auth proxy. (2) _plan_outbound attaches ClientMessageMetadata(headers={"authorization": ...}) to the outbound SessionMessage. (3) The SSE post_writer dequeues it and _send_message POSTs the JSON body with no headers — the token never leaves the client. (4) The proxy answers 401. (5) Under this PR's new mapping the caller receives a definitive-looking MCPError(INTERNAL_ERROR, "Server returned an error response") — or, since SseServerTransport endpoints always carry a session id, a 404-ing proxy yields the actively misleading MCPError(INVALID_REQUEST, "Session terminated") — when the real failure is that the client dropped the credentials it was explicitly given.\n\nWhy this is pre-existing, and why it is still worth flagging on this PR. The pre-PR _send_message had the identical bare client.post(endpoint_url, json=...) (visible in the diff's removed lines); the PR rewrote the function end-to-end around the gap without introducing it, so it should not block merging. It is flagged because (a) the PR touches exactly this function, and (b) its new status mapping gives the downstream failure a misleading shape — a client-side credential drop now reads as a definitive server error or session expiry, which reconnect logic keyed on Session terminated will act on falsely.\n\nNot a duplicate, and how to fix. The already-acknowledged follow-up item on streamable_http.py (dropped ctx.metadata.headers on the resumption/reconnection GETs) is the same conceptual class but a disjoint site with a disjoint fix — merging headers there never touches sse.py. This leg needs its own one-line change in _send_message, e.g.:\n\npython\nfrom mcp.shared.message import ClientMessageMetadata\n\nmetadata = session_message.metadata\nheaders = (\n dict(metadata.headers)\n if isinstance(metadata, ClientMessageMetadata) and metadata.headers\n else None\n)\nresponse = await client.post(endpoint_url, headers=headers, json=...)\n\n\nIt belongs in the grouped follow-up the author already planned for the other header-drop legs, extended to cover this one.

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the SSE message POST dropping session_message.metadata headers is a real, pre-existing gap, the same class as the two streamable GET-leg header drops already slated for the grouped follow-up; that follow-up should cover this leg too so per-message headers flow uniformly on every outbound HTTP call. Orthogonal to the error-surfacing contract this PR fixes, so declining here.

Generated by Claude Code

# user-supplied auth flows and hooks can raise arbitrary types
# from inside `client.post()`, so an enumerated catch cannot
# keep the caller from hanging.
logger.exception("Error POSTing message")
error=types.ErrorData(
code=types.CONNECTION_CLOSED, message=f"Failed to send message: {exc}"
)
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
else:
ifresponse.is_success:
logger.debug(f"Client message sent successfully: {response.status_code}")
return
logger.error(f"Message POST returned HTTP status {response.status_code}")
Comment on lines +150 to +153

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Under the default client factory (create_mcp_http_client sets follow_redirects=True), a Location-bearing 301/302/303 on the SSE message POST is followed by httpx2, which rewrites the POST to a bodyless GET — so the JSON-RPC message is silently dropped, and a 2xx at the redirect target (e.g. an SSO login page) makes response.is_success pass, logging 'sent successfully' while the waiting caller hangs forever: the residual #2110 hang the is_success widening does not close, since the PR's 302 tests use Location-less responses that httpx cannot follow. Consider treating a method-rewriting redirect as a delivery failure (e.g. response.history non-empty and final request method != POST) and resolving the waiter via the same correlated path — method-preserving 307/308 keep working.

Extended reasoning...

What the bug is. The new success check at src/mcp/client/sse.py:150 — if response.is_success: → return — only ever sees the final response of httpx2's redirect-following. The default factory create_mcp_http_client (src/mcp/shared/_httpx_utils.py:79) hardcodes follow_redirects=True (its docstring: "Always enables follow_redirects"), and sse_client uses it by default. httpx2's redirect handling rewrites POST to GET on 301/302/303 (_redirect_method, browser semantics) and drops the request body when the method changes (_redirect_stream returns no stream). So a Location-bearing 3xx on the message POST silently discards the JSON-RPC message, GETs the redirect target with no body, and if that target answers any 2xx, is_success is True: the transport logs "Client message sent successfully" at debug level and returns without resolving the waiter.\n\nWhy this hangs the caller. On this transport, unlike streamable HTTP, real responses only ever arrive on the SSE stream — the POST response body is never inspected. With the message never delivered, nothing will arrive on the SSE stream for that request id, and _send_message reported success, so the correlated-error path this PR built is never taken. The caller of session.call_tool() hangs until its own timeout — the exact #2110 symptom, indistinguishable from a slow server, with no log above debug.\n\nWhy the PR's hardening doesn't cover it. The is_success widening and the [302] parametrization of test_sse_client_request_post_http_error_reaches_caller_and_session_survives cover only unfollowed redirects: make_app_rejecting_posts returns Response(status_code=302) with no Location header, which httpx cannot follow (has_redirect_location is False), so the 3xx reaches the is_success check — the test docstring itself says "An unfollowed redirect counts." With a Location present (the realistic production shape), the 3xx never reaches the check; only the redirect target's 2xx does. The in-process test factory explicitly enables follow_redirects=True "to match create_mcp_http_client," confirming the followed case is simply untested.\n\nWhy only this leg. The sibling paths self-heal in the same scenario: on the streamable message POST, a followed redirect to a 200 HTML page falls into _handle_post_request's unexpected-content-type branch and resolves the waiter; on the resumption GET and SSE initial GET, a redirect landing on non-text/event-stream content makes httpx2's EventSource raise SSEError, which the new containment resolves. The SSE message POST is the one leg that never inspects the response, so a followed-redirect-to-2xx uniquely reads as success.\n\nStep-by-step proof. (1) A client connects through a corporate gateway; the SSE GET stream is established while auth is valid. (2) Auth expires mid-session; the gateway answers the next message POST with 302 Location: https://sso.example/login. (3) httpx2 follows: _redirect_method rewrites POST→GET, _redirect_stream drops the JSON-RPC body, and the login page returns 200 text/html. (4) response.is_success is True; _send_message logs "sent successfully" (debug) and returns. (5) The waiter is never resolved; session.call_tool() hangs into its timeout, and every subsequent request repeats the cycle — no MCPError, no visible log.\n\nSeverity and fix. This is behaviorally pre-existing — v1's raise_for_status() on the followed final 200 passed identically, so merging this PR causes no new failure, and the PR strictly improves the path (the migration table's non-2xx rows aren't literally violated: a followed redirect terminates in a 2xx). That's why this is a nit, not blocking. But since the PR rewrites this exact line, reasons explicitly about redirects on this path, and documents/tests a redirects-resolve-the-caller contract, it's worth closing here or in a follow-up: after the POST, treat a method-rewriting redirect as a delivery failure — e.g. if response.history and response.request.method != "POST": resolve the waiter via the same status_error_data/correlated path (using the first redirect's status, or a generic delivery-failure error). Method-preserving 307/308 redirects keep the POST and body, so they genuinely deliver and continue to work. All three verifiers confirmed this chain end-to-end; none refuted it.

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real residual case — but behaviorally pre-existing per your own analysis: v1 passed identically on the followed 200, so this PR neither introduces nor worsens it. Treating method-rewriting redirects (301/302/303 turning the POST into a GET) as delivery failures is a deliberate behavior change, and it belongs in the grouped follow-up rather than another round here. Adding it to that follow-up's list alongside the reconnection-loop raise_for_status(), the uncontained POST-path reply sends, and the resumption/reconnection GET header drops. Declining for this PR.

Generated by Claude Code

# The endpoint URL carrying a session id is this transport's
# "session established" signal, as `self.session_id` is for
# streamable HTTP.
error=status_error_data(
response.status_code,
has_session=_extract_session_id_from_endpoint(endpoint_url) isnotNone,
)
# A notification has no waiter to resolve, so its failure is only logged.
Comment thread
claude[bot] marked this conversation as resolved.
ifisinstance(message, types.JSONRPCRequest):
reply=types.JSONRPCError(jsonrpc="2.0", id=message.id, error=error)
try:
awaitread_stream_writer.send(SessionMessage(reply))
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
# Teardown race: the reader is gone, so there is nobody
# left to resolve - contain it, keeping the write loop up.
logger.debug("read stream closed before request %r could be resolved", message.id)

asyncforsession_messageinwrite_stream_reader:
Comment thread
claude[bot] marked this conversation as resolved.
sender_ctx=write_stream_reader.last_context
Expand Down
77 changes: 47 additions & 30 deletions src/mcp/client/streamable_http.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,6 @@
from httpx2 import EventSource, ServerSentEvent
from mcp_types import (
CONNECTION_CLOSED,
INTERNAL_ERROR,
INVALID_REQUEST,
METHOD_NOT_FOUND,
PARSE_ERROR,
Expand All@@ -30,7 +29,7 @@
from mcp_types.version import MODERN_PROTOCOL_VERSIONS
from pydantic import ValidationError

from mcp.client._transport import TransportStreams
from mcp.client._transport import TransportStreams, status_error_data
from mcp.shared._compat import resync_tracer
from mcp.shared._context_streams import ContextReceiveStream, ContextSendStream, create_context_streams
from mcp.shared._httpx_utils import create_mcp_http_client
Expand DownExpand Up@@ -248,25 +247,44 @@ async def _handle_resumption_request(self, ctx: RequestContext) -> None:
else:
raise ResumptionError("Resumption request requires a resumption token") # pragma: no cover

# Extract original request ID to map responses
original_request_id = None
if isinstance(ctx.session_message.message, JSONRPCRequest): # pragma: no branch
original_request_id = ctx.session_message.message.id
# Only requests resume: post_writer dispatches here on message type as well as
# metadata, so the original id is always available to map responses.
assert isinstance(ctx.session_message.message, JSONRPCRequest)
original_request_id = ctx.session_message.message.id

async with ctx.client.sse(self.url, headers=headers) as event_source:
event_source.response.raise_for_status()
logger.debug("Resumption GET SSE connection established")
try:
async with ctx.client.sse(self.url, headers=headers) as event_source:
if not event_source.response.is_success:
# Resolve the waiting caller with an error correlated to its request,
# mirroring `_handle_post_request`: an escaping `HTTPStatusError` would
# tear down the transport's task group and every stream with it (#2110).
error_data = status_error_data(
event_source.response.status_code, has_session=self.session_id is not None
Comment on lines +250 to +262

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing issue (not introduced by this PR, but in code this PR rewrites end-to-end): _handle_resumption_request builds its GET headers from _prepare_headers() plus Last-Event-ID only and never merges ctx.metadata.headers, unlike _handle_post_request — so per-request headers documented in CallOptions.headers ("HTTP transports merge these onto the outgoing request") are silently dropped on a resumed request; _handle_reconnection has the same gap. Fix mirrors the POST path: merge ctx.metadata.headers in both GET legs.

Extended reasoning...

What the bug is.CallOptions.headers is documented at src/mcp/shared/dispatcher.py:127-128 as a transport-layer hint: "HTTP transports merge these onto the outgoing request; non-HTTP transports ignore." The dispatcher honors that on resumed requests: _plan_outbound (src/mcp/shared/jsonrpc_dispatcher.py:250-254) attaches the caller's headers to the sameClientMessageMetadata that carries the resumption token — ClientMessageMetadata(resumption_token=token, on_resumption_token_update=on_token, headers=headers) — so a resumed request's per-request headers are expected to reach the wire. The message POST honors this (_handle_post_request: headers.update(ctx.metadata.headers)), but the resumption GET does not: _handle_resumption_request builds its headers from _prepare_headers() plus LAST_EVENT_ID only and never consults ctx.metadata.headers. _handle_reconnection has the identical gap.\n\nThe code path.post_writer dispatches a request stamped with a resumption token to _handle_resumption_request, which issues ctx.client.sse(self.url, headers=headers) with only transport-derived headers. Everything the caller stamped via the public CallOptions["headers"] — per-request auth, tracing headers, tenant routing — is silently discarded. Nothing logs the drop, so the failure is undiagnosable from the client side.\n\nWhy existing code hides it. The transport's _protocol_version_header cache compensates for exactly one header: post_writer caches the stamped MCP-Protocol-Version in _handle_message before dispatch, and _prepare_headers replays it. So the one header the SDK itself stamps happens to survive the resumption GET — which is why no existing test exposes the gap. Every user-supplied header is lost, and no test stamps custom metadata headers on a resumed request.\n\nStep-by-step proof. (1) A caller re-attaches an interrupted request: opts = {"resumption_token": tok, "headers": {"authorization": "Bearer per-request-token"}}. (2) _plan_outbound builds ClientMessageMetadata(resumption_token=tok, ..., headers={"authorization": ...}) and the session sends the SessionMessage. (3) post_writer sees the token on a JSONRPCRequest and routes to _handle_resumption_request. (4) headers = self._prepare_headers() yields only accept/content-type/mcp-session-id/mcp-protocol-version; LAST_EVENT_ID is added; the authorization header never makes it in. (5) The server answers 401 (or 404 after evicting the unauthenticated session). (6) Under this PR's new mapping the caller receives MCPError(INTERNAL_ERROR, "Server returned an error response") — or, for a 404 while self.session_id is held, MCPError(INVALID_REQUEST, "Session terminated"), which is actively misleading here: the session is fine, the client dropped the credentials, and reconnect logic keyed on that error will act on a false signal.\n\nWhy this is pre-existing. The header-building lines sit above the PR's diff hunk and are unchanged — the pre-PR code built the GET headers identically (it just let the resulting HTTPStatusError escape instead). The PR does not introduce the drop; it only rewrites _handle_resumption_request end-to-end around it and gives the downstream failure a new, potentially misleading shape, which is why it's worth recording against this code rather than blocking this PR. All three verifiers independently confirmed both the gap and its pre-existing status.\n\nHow to fix. Mirror _handle_post_request in both GET legs — after building headers in _handle_resumption_request and _handle_reconnection, add:\n\npython\nif ctx.metadata is not None and ctx.metadata.headers is not None:\n headers.update(ctx.metadata.headers)\n\n\nplus a test that stamps a custom header alongside a resumption token and asserts it appears on the GET (the existing test_post_request_merges_per_message_metadata_headers is the template).

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the resumption and reconnection GETs building headers from _prepare_headers() alone and dropping ctx.metadata.headers is a real, pre-existing gap (auth-bearing per-message headers don't reach the resumed stream). It's orthogonal to the error-surfacing contract this PR fixes, so declining here; it rounds out the same grouped follow-up as this pass's other two pre-existing findings (reconnection-loop raise_for_status(), uncontained POST-path reply sends).

Generated by Claude Code

)
await self._resolve_abandoned_request(
ctx.read_stream_writer, original_request_id, error_data.message, code=error_data.code
)
return
logger.debug("Resumption GET SSE connection established")

async for sse in event_source: # pragma: no branch
is_complete = await self._handle_sse_event(
sse,
ctx.read_stream_writer,
original_request_id,
ctx.metadata.on_resumption_token_update if ctx.metadata else None,
)
if is_complete:
await event_source.response.aclose()
break
async for sse in event_source:
is_complete = await self._handle_sse_event(
sse,
ctx.read_stream_writer,
original_request_id,
Comment on lines +261 to +274

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing issue (not introduced by this PR): the automatic Last-Event-ID reconnection GET in _handle_reconnection still does a bare raise_for_status() inside its except Exception retry loop, so a deterministic session-expiry 404 is retried MAX_RECONNECTION_ATTEMPTS times (~2s of futile sleeps) and then surfaced as CONNECTION_CLOSED / "SSE stream ended and reconnection attempts were exhausted" — never the INVALID_REQUEST / "Session terminated" mapping this PR standardized on the resumption GET and both POST paths (handle_get_stream has the same pattern, with lower impact since it has no waiter). Now that status_error_data exists, the fix is a small mirror of this PR's own resumption-GET branch: check event_source.response.is_success before retrying and resolve the waiter via status_error_data(status, has_session=self.session_id is not None), since a non-2xx GET can never deliver the stream.

Extended reasoning...

What the bug is. This PR establishes a cross-path contract — a 404 while a session is held maps to MCPError(INVALID_REQUEST, "Session terminated"), now centralized in status_error_data() (src/mcp/client/_transport.py) and applied on the streamable message POST, the metadata-driven resumption GET, and the SSE message POST. But there is a third Last-Event-ID GET leg the mapping never reaches: _handle_reconnection (src/mcp/client/streamable_http.py:518-549), the automatic reconnect that fires from _handle_sse_response whenever a request's SSE response stream drops after carrying event ids. It still calls a bare event_source.response.raise_for_status() (line ~520) inside a try whose except Exception handler retries with attempt + 1 (lines ~546-549). handle_get_stream (line ~213) has the same bare-raise_for_status-inside-retry pattern for the server-initiated stream.\n\nThe code path. The trigger is realistic — a server restart drops the in-flight SSE stream and evicts the session:\n\n1. session.call_tool() over streamable HTTP; the server answers with an SSE stream that emits an event id, then the connection drops mid-stream.\n2. _handle_sse_response catches the read error and, because last_event_id is set, calls _handle_reconnection(ctx, last_event_id, ...).\n3. Meanwhile the session has expired. The SDK's own server answers any request bearing an unknown/expired Mcp-Session-Id with HTTP 404 (src/mcp/server/streamable_http_manager.py:361-371, "return 404 per MCP spec"), and the reconnection GET carries the stale Mcp-Session-Id via _prepare_headers().\n4. raise_for_status() raises HTTPStatusError; the except Exception handler never inspects the status, sleeps DEFAULT_RECONNECTION_DELAY_MS (1s, or the server-provided retry), and re-sends the doomed GET — MAX_RECONNECTION_ATTEMPTS times.\n5. On give-up it resolves the waiter via _resolve_abandoned_request with the default code=CONNECTION_CLOSED and "SSE stream ended and reconnection attempts were exhausted".\n\nWhy existing code doesn't prevent it. The retry handler treats every failure as transient — it sees only the exception, never the status, and the except branch carries a # pragma: no cover, so no test drives a non-2xx through this leg. The mapping this PR added lives in _handle_resumption_request, which handles only metadata-driven resumption (an explicit resumption_token stamped by the caller); the automatic mid-stream reconnect never routes through it.\n\nImpact. The caller's error shape for the identical server-side event — session expired, observed on a Last-Event-ID GET — depends on which internal leg observed it. On the metadata-driven resumption GET the caller gets MCPError(-32600, "Session terminated") promptly (pinned by test_resumption_get_404_with_session_reports_session_terminated). On the automatic reconnection GET the same expiry costs ~2s of futile retries and then arrives as MCPError(-32000, "SSE stream ended and reconnection attempts were exhausted"). A reconnect wrapper written per the migration guide's v2 pattern (exc.code == INVALID_REQUEST and exc.message == "Session terminated" → rebuild the connection) silently never matches on this leg, so "reconnecting would fix this" is indistinguishable from a dead network — the precise ambiguity the PR's 404 mapping exists to remove. In handle_get_stream the impact is only the futile retry budget plus a silently abandoned server-initiated stream (no waiter to mis-resolve).\n\nWhy pre-existing, not blocking. All three verifiers converged on this: _handle_reconnection and handle_get_stream are untouched by this PR's diff, and their behavior is unchanged from before — the error was already contained and retried pre-PR; nothing hangs and nothing tears down, so merging this PR breaks nothing that worked. The migration table's literal wording ("a request re-attached with a resumption token (Last-Event-ID)") is scoped to the metadata-driven resumption GET, so no documented claim is strictly falsified — only the PR body's "works everywhere" prose and cross-path consistency. This PR merely makes the residual gap salient (and cheap to close).\n\nHow to fix. Mirror this PR's own _handle_resumption_request branch in _handle_reconnection: before raise_for_status(), check event_source.response.is_success; on a non-2xx, resolve the waiter immediately via self._resolve_abandoned_request(ctx.read_stream_writer, original_request_id, error_data.message, code=error_data.code) with error_data = status_error_data(event_source.response.status_code, has_session=self.session_id is not None) and return — a non-2xx GET can never deliver the stream, so retrying is pointless. The same two-line guard fits handle_get_stream (there, just return on non-2xx instead of resolving a waiter). Fine as a follow-up PR.

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this is a real gap — but it's pre-existing, on a path this PR doesn't rewrite (_handle_reconnection's retry loop has its own containment and give-up resolution, so the failure mode is bounded retries rather than a hang or teardown), and as you note it's not blocking. Declining here to keep this PR reviewable; it belongs in a grouped follow-up together with the other two pre-existing findings from this pass (uncontained POST-path reply sends, dropped ctx.metadata.headers on the resumption/reconnection GETs).

Generated by Claude Code

ctx.metadata.on_resumption_token_update if ctx.metadata else None,
)
if is_complete:
await event_source.response.aclose()
return
except Exception:
logger.debug("Resumption stream ended", exc_info=True)

# Stream ended without a response, cleanly or mid-read: resolve the waiter,
# mirroring `_handle_sse_response`, else the caller would hang forever.
await self._resolve_abandoned_request(
ctx.read_stream_writer, original_request_id, "resumption stream ended without a response"
)

def _consume_modern_cancellation(self, session_message: SessionMessage) -> bool:
"""Translate an outbound `notifications/cancelled` at 2026; True means "do not POST".
Expand DownExpand Up@@ -358,16 +376,13 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
except (httpx2.StreamError, ValidationError):
pass
logger.debug("Non-2xx body was not a JSON-RPC error; using fallback")
if response.status_code == 404:
if self.session_id is None:
# No session yet → 404 is the HTTP-level spelling of
# METHOD_NOT_FOUND (gateway / legacy server doesn't know
# this method); "Session terminated" would be a lie here.
error_data = ErrorData(code=METHOD_NOT_FOUND, message="Not Found")
else:
error_data = ErrorData(code=INVALID_REQUEST, message="Session terminated")
if response.status_code == 404 and self.session_id is None:
# No session yet → 404 is the HTTP-level spelling of
# METHOD_NOT_FOUND (gateway / legacy server doesn't know
# this method); "Session terminated" would be a lie here.
error_data = ErrorData(code=METHOD_NOT_FOUND, message="Not Found")
else:
error_data = ErrorData(code=INTERNAL_ERROR, message="Server returned an error response")
error_data = status_error_data(response.status_code, has_session=self.session_id is not None)
session_message = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data))
await ctx.read_stream_writer.send(session_message)
return
Comment on lines 384 to 388

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing issue (not introduced by this PR): the five reply sends on the streamable POST path — _handle_post_request (lines ~374, ~387, ~405) and _handle_json_response (lines ~419, ~424) — deliver to ctx.read_stream_writer bare, without the BrokenResourceError/ClosedResourceError containment that _resolve_abandoned_request, the resumption-GET branch (b4edbd4), and the SSE _send_message tail (6472241) now all have. A late reply landing after the read stream closes (e.g. an orphaned pre-2026 POST completing during teardown) escapes the transport task group as an ExceptionGroup from streamable_http_client — the same #2110 teardown symptom class this PR eliminates on the sibling paths; routing these sends through the same containment closes the last gap.

Extended reasoning...

What the bug is. The reply-delivery sends on the streamable POST path are the last uncontained ones on the client transports. In _handle_post_request, the non-2xx JSON-error-body reply (await ctx.read_stream_writer.send(SessionMessage(reply))), the status-derived fallback (await ctx.read_stream_writer.send(session_message)), and the unexpected-content-type reply are all bare awaits; in _handle_json_response, both the parsed-message send and the PARSE_ERROR fallback are bare, with the adjacent except (httpx2.StreamError, ValidationError) catching only parse-side failures — nothing catches anyio.BrokenResourceError/anyio.ClosedResourceError from the send itself. Every sibling delivery site has that containment: _resolve_abandoned_request (streamable_http.py, "Best-effort: a closed read stream means the session is tearing down", pinned by test_resolving_an_abandoned_request_after_the_reader_closed_is_contained), the resumption-GET branch this PR routed through it in b4edbd4, and the SSE transport's _send_message tail added in 6472241 for exactly this race.\n\nWhy nothing catches the escape. A request's POST does not run inside post_writer's try/except Exception — it is spawned as a task on the transport-level task group (tg.start_soon(self._run_request_post, handle_request_async, post, message.id)). _run_request_post is only with post.scope: plus an identity-guarded finally; it has no exception handler. So a stream-closed error raised by one of these sends propagates into the transport task group, which retains it and re-raises it as ExceptionGroup(ClosedResourceError) from streamable_http_client.__aexit__ — a clean user shutdown crashes with the same teardown-ExceptionGroup symptom class as #2110.\n\nStep-by-step proof. (1) A request is in flight under a pre-2026 negotiated version; the caller times out or cancels. Per _consume_modern_cancellation, the legacy era deliberately leaves the POST running (test_legacy_cancelled_frame_posts_and_leaves_the_stream_open pins this) — the POST task is now orphaned, still awaiting the server. (2) The user exits ClientSession; the dispatcher's async with closes its side of the write stream, post_writer's loop sees EOF, and its async with write_stream_reader, read_stream_writer, write_stream: closes read_stream_writer. (3) streamable_http_client's finally widens the window: it awaits terminate_session(client) — a full DELETE round-trip — beforetg.cancel_scope.cancel(), giving the orphaned POST time to complete. (4) The server answers with a non-2xx (the fallback send), a JSON body (_handle_json_response's send), or an unexpected content type: the send hits the closed stream and raises ClosedResourceError. (5) Nothing on the path catches it; the transport task group re-raises it as an ExceptionGroup from the context exit, and the user's clean shutdown crashes.\n\nWhy this is pre-existing, not introduced here. The diff against the PR base shows none of these sends were added or changed by this PR — it only restructured the ErrorData construction feeding the fallback send (the 404/status_error_data mapping) and left the delivery mechanism untouched. What makes them worth flagging now is that this PR completes the containment convention on every sibling path (the resumption GET via _resolve_abandoned_request in b4edbd4, the SSE POST tail in 6472241), leaving these five sends as the now-inconsistent stragglers inside the very block the PR touches.\n\nImpact. Narrow but real: it requires an orphaned POST (legacy-era abandon or teardown racing an in-flight request) completing after the read stream closes. When it fires, the failure is the worst-shaped one for a transport: a clean async with streamable_http_client(...) exit raises ExceptionGroup(ClosedResourceError) at the user, indistinguishable from the #2110 crashes this PR set out to eliminate.\n\nHow to fix. One root cause, five sites: route the request-reply sends through _resolve_abandoned_request-style containment, or wrap each in the same two-exception catch with a debug log (except (anyio.BrokenResourceError, anyio.ClosedResourceError): logger.debug("read stream closed before request %r could be resolved", ...)), matching stdio.py, sse.py, and the rest of streamable_http.py. Wire-identical when the stream is open; contained when it is not. Fine as a follow-up — it should not block this PR.

@claudeclaudeBotAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed the asymmetry is real: this PR's new resolution paths are contained against the teardown race while the five pre-existing reply sends on the POST path are not. That gap predates this PR and spans code it doesn't otherwise touch, so declining here rather than growing the diff further — it's a natural member of a grouped follow-up PR with this pass's other two pre-existing findings (the reconnection loop's bare raise_for_status(), the dropped ctx.metadata.headers on resumption/reconnection GETs), where the containment can be applied uniformly (probably by routing those sends through _resolve_abandoned_request-style delivery) instead of piecemeal.

Generated by Claude Code

Expand DownExpand Up@@ -556,8 +571,10 @@ async def _handle_message(session_message: SessionMessage) -> None:
else None
)

# Check if this is a resumption request
is_resumption = bool(metadata and metadata.resumption_token)
# Only a request resumes: the token names an interrupted request's
# stream, and `_handle_resumption_request` needs the id to correlate
# its outcome. A notification stamped with one is POSTed as usual.
is_resumption = bool(metadata and metadata.resumption_token) and isinstance(message, JSONRPCRequest)

logger.debug(f"Sending client message: {message}")

Expand Down
Loading
Loading