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

[claude]

Requested by Felix Weinberger · Slack thread

Fixes#2110.

Summary

The message-POST half of #2110 is already fixed on main (_handle_post_request answers a non-2xx with a JSONRPCError correlated to the request id). Two client-side paths still swallowed HTTP failures; this PR closes both:

1. Streamable HTTP — resumption GET._handle_resumption_request called a bare raise_for_status(). A 401 (or any non-2xx) on a resumption GET (Last-Event-ID) escaped into the transport's task group as an HTTPStatusError, tearing down every stream on the transport:

ExceptionGroup: unhandled errors in a TaskGroup (1 sub-exception)
HTTPStatusError: Client error '401 Unauthorized' for url 'http://test/mcp'

2. SSE transport — message POST.post_writer's _send_message also called a bare raise_for_status(); the exception landed in post_writer's catch-all except Exception, which logged it and dropped it. The waiting caller hung forever (a 401 was indistinguishable from a slow server), and the write loop died with it, so every later send was silently dropped too.

The fix

Both paths now resolve the waiting request with a JSONRPCError stamped with the original request's id — the same shape _handle_post_request already uses for its non-2xx fallback. The status → error mapping lives once, in mcp.client._transport.status_error_data, and the error reaches the caller promptly through the normal response-correlation path; the transport/session stays usable, so a failed resumption or POST no longer kills the next call.

Per review (thanks claude[bot] — thirteen findings across three rounds addressed), the same hardening covers the adjacent failure legs on these exact paths:

• Any non-2xx counts, not just >= 400 (response.is_success), matching the raise_for_status() semantics being replaced: an unfollowed redirect on either path can never deliver a response, so it resolves the caller instead of logging "sent successfully".
• A 404 while a session is held maps to INVALID_REQUEST / "Session terminated" on both the resumption GET (self.session_id) and the SSE message POST (session id in the endpoint URL), the same session-expiry signal as the streamable POST path, so reconnect logic keyed on that error works everywhere.
• The resumption read loop is contained like its sibling _handle_sse_response: a stream dying mid-read resolves the waiter (CONNECTION_CLOSED) instead of tearing down the transport, and a stream that ends cleanly without a response resolves the waiter instead of hanging it forever.
• Any failure inside the SSE message POST — httpx network errors, OAuthFlowError from a failing OAuthClientProvider re-auth, or arbitrary exceptions from user-supplied hooks — resolves the waiter through the same correlated path (a terminal containment boundary; on this transport, unlike streamable HTTP, nothing escapes loudly, so the parity argument for scoping them out didn't hold).
• The SSE error-resolution send is contained against the teardown race (read stream already closed), mirroring _resolve_abandoned_request, so an undeliverable error cannot kill the write loop either.
• Resumption is dispatched on message type as well as metadata: a notification stamped with a resumption token is POSTed as usual instead of asserting inside the resumption path.
• On the SSE transport, a POST failure for a notification has no waiter to resolve, so it is logged and contained instead of killing the write loop.
• docs/migration.md documents the new contract — resumption-GET and SSE message-POST outcome tables extending the existing non-2xx section — and corrects its previously overbroad "connect-level failures still escape" note, now scoped to the streamable message POST where it still holds.

Still deliberately out of scope: status-code distinguishability (a 401 arrives as the same INTERNAL_ERROR as a 503) — that's #3091's follow-up territory (#3094, #3276), orthogonal to the hang/teardown fixed here.

How has this been tested?

Nineteen new regression tests, all in-process (httpx MockTransport / ASGI), all bounded by anyio.fail_after(5); each fails against the code it patches (hang into the timeout, transport teardown, or wrong error) and passes with the fix:

• tests/client/test_streamable_http.py
  • test_resumption_get_http_error_resolves_caller_and_transport_survives[302|401|403|500]
  • test_resumption_get_404_with_session_reports_session_terminated
  • test_resumption_stream_dying_mid_read_resolves_caller_and_transport_survives
  • test_resumption_stream_clean_end_without_response_resolves_caller
  • test_notification_with_resumption_token_is_posted_not_resumed
• tests/shared/test_sse.py (driven through ClientSession where the property allows)
  • test_sse_client_request_post_http_error_reaches_caller_and_session_survives[302|401|403|500]
  • test_sse_client_post_404_with_session_endpoint_reports_session_terminated
  • test_sse_client_post_404_without_session_endpoint_keeps_generic_error
  • test_sse_client_request_post_network_error_reaches_caller_and_session_survives
  • test_sse_client_auth_failure_on_post_reaches_caller_and_session_survives[OAuthTokenError|RuntimeError]
  • test_sse_client_post_error_after_reader_closed_is_contained
  • test_sse_client_notification_post_http_error_leaves_session_usable

Full suite green under branch coverage; ruff format, ruff check, pyright clean; no new # pragma (two removed).

Related

• streamable_http: HTTP status errors (401/403/5xx) on tools/list, tools/call never reach the caller — indistinguishable from a timeout #3091 is the v1.x report of the POST half; fix(client): surface non-2xx HTTP responses to the waiting caller #3092 is its v1.x backport. Its PR body explicitly flags the resumption-GET raise_for_status() fixed here as "the same bug class on the resumption GET path, unfixed on 2.0 as well".
• Prior art on the SSE half: fix(client): propagate SSE POST errors to caller instead of hanging (#2110) #2483 (closed as duplicate of fix: propagate SSE stream errors to waiting requests #2122, itself closed unmerged). This PR uses the correlated-JSONRPCError approach the modern transport standardized on rather than forwarding the raw exception.

Disclosure

This PR was authored by Claude (an AI assistant) as part of a maintainer-requested triage workflow, and reviewed gate-by-gate as described above.

---

Generated by Claude Code

[claude]

Beyond the inline findings, this pass also examined and ruled out two candidates: (1) ambiguous SSE POST failures (ReadTimeout/reset after the request may have reached the server) being synthesized into a terminal error — the CONNECTION_CLOSED resolution is correct for this transport since the response would arrive on the SSE stream, not the POST; (2) the resumption GET giving up on the first stream drop instead of reconnecting with Last-Event-ID like its siblings — resolving the waiter there is consistent with the PR's stated scope, and reconnection is follow-up territory.

Extended reasoning...

Bugs were found this run (posted inline), so the review body records only what else was examined. The prior review pass's findings were all addressed by commit b4edbd4 (is_success checks, 404→Session-terminated mapping on the resumption GET, _resolve_abandoned_request reuse, HTTPError containment on the SSE POST, resumption-loop containment, the notification-resumption guard, and the test-helper dedup). No prior ruled-out note exists on this PR; the two items above are new this run and are informational only, not a guarantee of correctness.

[claude]

Beyond the inline findings, this run also examined whether the resumption GET should auto-reconnect on a mid-read drop (it always holds a Last-Event-ID, unlike _handle_sse_response which reconnects) — ruled out: resumption is caller-driven on this path, the caller keeps receiving token updates via on_resumption_token_update and can re-resume from the surfaced error, and pre-PR the drop tore down the whole transport, so no reconnect behavior was lost.

Extended reasoning...

Bugs were found this run (three nit-level inline comments), so no approval verdict is posted. This note records the one candidate issue that was investigated and refuted and is not covered by the inline findings: the lack of auto-reconnect in _handle_resumption_request on a mid-read stream drop. Unlike _handle_sse_response, which reconnects when it has captured an event id, the resumption path resolves the waiter with CONNECTION_CLOSED — but that path is caller-driven (the caller supplied the resumption token and receives updates through on_resumption_token_update), so it can re-issue the resumption itself, and the pre-PR behavior was a full transport teardown, so nothing regressed. The other two candidates raised by finders overlap with findings that are being posted inline, so they are not restated here.

[claude]

Beyond the inline findings (all pre-existing or doc-level), this run also examined whether the resumption GET should reconnect with Last-Event-ID on a mid-read stream drop like its siblings, and ruled it out as a bug: the caller holds the resumption token (kept fresh via on_resumption_token_update) and re-attaching is the designed recovery on that leg, matching the outcome table this PR adds to docs/migration.md.

Extended reasoning...

Bugs were found this run, but all four are flagged as pre-existing gaps in adjacent code or a documentation-scoping nit — none are regressions introduced by this PR, so the inline comments carry the signal. This note only records the one additional candidate examined and refuted (resumption-GET auto-reconnection), so a later review pass does not re-explore it from scratch. It is informational, not a correctness guarantee, and no prior run of mine has posted a ruled-out note on this PR.

[claude]

🟣 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.

[claude]

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

[claude]

🟣 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.

[claude]

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

[claude]

🟣 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).

[claude]

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

[claude]

🟡 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.

[claude]

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

[claude]

🟣 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.

[claude]

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

[claude]

I reviewed this revision (through 66a9285) and found no new bugs — but this PR rewrites error-handling contracts on critical client transport paths, so a human maintainer's look is still worthwhile before merge.

What was reviewed this run:

• The final state of both hardened paths (streamable resumption GET, SSE message POST) after the three earlier review rounds — the fix commits (contained error send, __all__ export, migration-doc scoping) introduced no regressions.
• _handle_post_request's refactor onto status_error_data — checked it preserves the prior 404/no-session → METHOD_NOT_FOUND and 404/session → INVALID_REQUEST mapping exactly.
• The new is_resumption type gate and the resumption path's assert — the gate in _handle_message guarantees the assert; a token-stamped notification takes the plain POST path.
• The known residual gaps (followed-redirect POST, metadata-header drops, reconnection-loop raise_for_status) are all pre-existing and already recorded on the thread for the grouped follow-up; nothing new surfaced.

Extended reasoning...

Overview

This PR closes the remaining #2110 legs: a bare raise_for_status() on the streamable-HTTP resumption GET (which tore down the whole transport task group on any non-2xx) and the SSE transport's message POST (which hung the caller forever and killed the write loop). Both now resolve the waiting request with a correlated JSONRPCError via a new shared status_error_data helper in mcp.client._transport, with additional containment for mid-read stream death, clean stream end without a response, arbitrary auth/hook exceptions, and the reader-already-closed teardown race. It also documents the new error contract in docs/migration.md and adds nineteen in-process regression tests.

Security risks

No direct security surface is added — no new auth, crypto, or input-parsing logic. The main risk class is error-contract fidelity: a 401 now arrives as a generic INTERNAL_ERROR rather than escaping loudly, which is a deliberate, documented tradeoff (status distinguishability is explicitly deferred to #3094/#3276). The SSE path's broad except Exception containment boundaries are justified inline (user-supplied auth flows can raise arbitrary types) but deviate from the repo's stated exception-handling rules — a call a maintainer should consciously bless.

Level of scrutiny

High. This is production-critical client transport code (both HTTP transports' failure paths), the diff is large (~150 lines of src changes plus ~700 of tests and docs), and it changes user-visible behavior: errors that previously escaped as ExceptionGroup or hung forever now resolve through the response-correlation path. The migration-doc tables pin this as a cross-transport contract. These are design decisions with ecosystem impact, well outside the bot-approvable band regardless of implementation correctness.

Other factors

The PR went through three prior bug-hunt rounds on this thread (thirteen findings addressed across five fix commits); this run re-reviewed the resulting final state and found nothing new. Test coverage is strong — each new test is claimed to fail against the unfixed code, all are in-process and time-bounded, and CI enforces 100% branch coverage. Several real but pre-existing gaps (followed-redirect POST swallowing, per-message metadata-header drops on SSE POST and resumption/reconnection GETs, the reconnection loop's bare raise_for_status(), uncontained POST-path reply sends) were examined in earlier rounds, explicitly declined as out of scope, and are recorded on the thread for a grouped follow-up — they should not block this PR, but the scope call is itself something a human can ratify.