Skip to content

Commit 559f2df

Browse files
feat(client): carry the originating HTTP status in ErrorData.data
A non-2xx POST is synthesized into a JSON-RPC error whose code cannot express the retry decision: a terminal 401 and a transient 503 both arrive as INTERNAL_ERROR with the same "Server returned an error response" message. A caller cannot tell "my token is bad, stop" from "the server hiccuped, retry". The status now rides along on `error.data` as `{"httpStatus": 503}`. `data` is free-form per the JSON-RPC spec, so this is additive: nothing that reads `code` or `message` changes. An error the server supplied in its body keeps its own `data` untouched — its account of the failure outranks ours. A non-2xx to a notification has no waiter to resolve, so it is logged rather than dropped silently. Requested on #3091 by the reporter, who needs the distinction for retry classification in a downstream client. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1216c53 commit 559f2df

5 files changed

Lines changed: 125 additions & 5 deletions

File tree

docs/migration.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,6 +1688,8 @@ In v1, a non-2xx response to a message POST (other than 404) raised `httpx.HTTPS
16881688
| 404, no session yet | `McpError` with positive code `32600` | `MCPError(-32601, 'Not Found')` |
16891689
| Any other 4xx/5xx | `httpx.HTTPStatusError` escapes as `ExceptionGroup` | `MCPError(-32603, 'Server returned an error response')` |
16901690

1691+
Every error the transport synthesizes from a status (the last three rows) carries that status on `error.data` as `{"httpStatus": 503}`. The JSON-RPC code cannot express the retry decision — a terminal `401` and a transient `503` are both `-32603` — so read the status when deciding whether to retry. An error the *server* supplied in the body (the first row) keeps its own `data` untouched.
1692+
16911693
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`.
16921694

16931695
**Before (v1):**

docs/troubleshooting.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,29 @@ async with Client("https://mcp.example.com/mcp") as client:
162162
mcp.shared.exceptions.MCPError: Server returned an error response
163163
```
164164

165-
The words the server actually sent, `421` and `Invalid Host header`, never reach you: the 421 body has no `Content-Type: application/json`, so the client cannot parse it. They are in the **server's log**, which is where to look next:
165+
The reason phrase the server sent, `Invalid Host header`, never reaches you: the 421 body has no `Content-Type: application/json`, so the client cannot parse it. It is in the **server's log**, which is where to look next:
166166

167167
```text
168168
WARNING mcp.server.transport_security: Invalid Host header: mcp.example.com
169169
```
170170

171+
The *status* does reach you, on the error's `data`, which is how you tell one refusal from another without parsing the message:
172+
173+
```python
174+
from mcp.shared.exceptions import MCPError
175+
176+
177+
async def list_tools() -> None:
178+
try:
179+
async with Client("https://mcp.example.com/mcp") as client:
180+
await client.list_tools()
181+
except MCPError as exc:
182+
status = (exc.error.data or {}).get("httpStatus") # 421 here; 401, 403, 503 … elsewhere
183+
print(status)
184+
```
185+
186+
That is what makes a retry policy possible: a `401` or `403` is terminal and retrying it only burns attempts, while a `502`/`503` is worth another go.
187+
171188
The fix is `transport_security=`. Allowlist the hostname you actually serve:
172189

173190
```python title="server.py" hl_lines="14-17"

src/mcp/client/streamable_http.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@
4949
MCP_SESSION_ID = "mcp-session-id"
5050
LAST_EVENT_ID = "last-event-id"
5151

52+
# Key under which the originating HTTP status is carried in `ErrorData.data` when a
53+
# non-2xx response is synthesized into a JSON-RPC error. Lets callers tell a terminal
54+
# 401/403 from a retryable 5xx, which the JSON-RPC error code alone cannot express.
55+
HTTP_STATUS_KEY = "httpStatus"
56+
5257
# Reconnection defaults
5358
DEFAULT_RECONNECTION_DELAY_MS = 1000 # 1 second fallback when server doesn't provide retry
5459
MAX_RECONNECTION_ATTEMPTS = 2 # Max retry attempts before giving up
@@ -363,13 +368,23 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
363368
# No session yet → 404 is the HTTP-level spelling of
364369
# METHOD_NOT_FOUND (gateway / legacy server doesn't know
365370
# this method); "Session terminated" would be a lie here.
366-
error_data = ErrorData(code=METHOD_NOT_FOUND, message="Not Found")
371+
code, error_message = METHOD_NOT_FOUND, "Not Found"
367372
else:
368-
error_data = ErrorData(code=INVALID_REQUEST, message="Session terminated")
373+
code, error_message = INVALID_REQUEST, "Session terminated"
369374
else:
370-
error_data = ErrorData(code=INTERNAL_ERROR, message="Server returned an error response")
375+
code, error_message = INTERNAL_ERROR, "Server returned an error response"
376+
# The status is what tells a caller whether to retry: 401/403 are
377+
# terminal, 5xx are worth another attempt. The JSON-RPC code above
378+
# cannot express that distinction, so carry the status itself.
379+
error_data = ErrorData(
380+
code=code, message=error_message, data={HTTP_STATUS_KEY: response.status_code}
381+
)
371382
session_message = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data))
372383
await ctx.read_stream_writer.send(session_message)
384+
else:
385+
# Nothing to resolve — a notification/response POST has no waiter. Log
386+
# it, or a server rejecting every write fails completely silently.
387+
logger.warning("Server returned HTTP %d to a %s", response.status_code, type(message).__name__)
373388
return
374389

375390
if self._is_initialization_request(message):

tests/client/test_streamable_http.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import base64
1010
import json
11+
import logging
1112
from collections.abc import AsyncIterator, Callable, Mapping
1213
from typing import Any
1314

@@ -19,6 +20,7 @@
1920
CLIENT_CAPABILITIES_META_KEY,
2021
CLIENT_INFO_META_KEY,
2122
CONNECTION_CLOSED,
23+
INTERNAL_ERROR,
2224
INVALID_REQUEST,
2325
METHOD_NOT_FOUND,
2426
PROTOCOL_VERSION_META_KEY,
@@ -31,6 +33,7 @@
3133
from starlette.types import Receive, Scope, Send
3234

3335
from mcp.client.streamable_http import (
36+
HTTP_STATUS_KEY,
3437
MAX_RECONNECTION_ATTEMPTS,
3538
RequestContext,
3639
StreamableHTTPTransport,
@@ -130,6 +133,87 @@ def handler(request: httpx.Request) -> httpx.Response:
130133
assert isinstance(reply, SessionMessage)
131134
assert isinstance(reply.message, JSONRPCError)
132135
assert reply.message.error.code == METHOD_NOT_FOUND
136+
assert reply.message.error.data == {HTTP_STATUS_KEY: 404}
137+
138+
139+
@pytest.mark.anyio
140+
@pytest.mark.parametrize("status", [401, 403, 500, 503])
141+
async def test_a_non_2xx_status_reaches_the_caller_in_the_errors_data(status: int) -> None:
142+
"""The originating HTTP status rides along in `ErrorData.data`.
143+
144+
The JSON-RPC code is INTERNAL_ERROR for every one of these, so it alone cannot tell a
145+
caller whether to retry. The status can: 401/403 are terminal, 5xx are worth retrying.
146+
"""
147+
148+
def handler(request: httpx.Request) -> httpx.Response:
149+
return httpx.Response(status)
150+
151+
with anyio.fail_after(5):
152+
async with (
153+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http,
154+
streamable_http_client("http://test/mcp", http_client=http) as (read, write),
155+
):
156+
await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})))
157+
reply = await read.receive()
158+
assert isinstance(reply, SessionMessage)
159+
assert isinstance(reply.message, JSONRPCError)
160+
assert reply.message.error.code == INTERNAL_ERROR
161+
assert reply.message.error.data == {HTTP_STATUS_KEY: status}
162+
163+
164+
@pytest.mark.anyio
165+
async def test_a_servers_own_jsonrpc_error_body_survives_the_non_2xx_status() -> None:
166+
"""A JSON-RPC error in the body wins: the server said what went wrong, so don't overwrite its `data`."""
167+
168+
def handler(request: httpx.Request) -> httpx.Response:
169+
body = json.loads(request.content)
170+
error = {"code": INVALID_REQUEST, "message": "no", "data": {"detail": "from the server"}}
171+
return httpx.Response(400, json={"jsonrpc": "2.0", "id": body["id"], "error": error})
172+
173+
with anyio.fail_after(5):
174+
async with (
175+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http,
176+
streamable_http_client("http://test/mcp", http_client=http) as (read, write),
177+
):
178+
await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={})))
179+
reply = await read.receive()
180+
assert isinstance(reply, SessionMessage)
181+
assert isinstance(reply.message, JSONRPCError)
182+
assert reply.message.error.data == {"detail": "from the server"}
183+
184+
185+
@pytest.mark.anyio
186+
async def test_a_non_2xx_to_a_notification_is_logged() -> None:
187+
"""A notification has no waiter to resolve, so a rejected write can only be logged — but it must be.
188+
189+
Waiting on the log record itself (not on the POST) is what makes this deterministic: the
190+
warning is emitted after the response returns, so the handler is too early a signal.
191+
"""
192+
logged = anyio.Event()
193+
records: list[logging.LogRecord] = []
194+
195+
class _Capture(logging.Handler):
196+
def emit(self, record: logging.LogRecord) -> None:
197+
records.append(record)
198+
logged.set()
199+
200+
def handler(request: httpx.Request) -> httpx.Response:
201+
return httpx.Response(500)
202+
203+
transport_logger = logging.getLogger("mcp.client.streamable_http")
204+
capture = _Capture(level=logging.WARNING)
205+
transport_logger.addHandler(capture)
206+
try:
207+
with anyio.fail_after(5):
208+
async with (
209+
httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http,
210+
streamable_http_client("http://test/mcp", http_client=http) as (_, write),
211+
):
212+
await write.send(SessionMessage(JSONRPCNotification(jsonrpc="2.0", method="notifications/progress")))
213+
await logged.wait()
214+
finally:
215+
transport_logger.removeHandler(capture)
216+
assert [r.getMessage() for r in records] == ["Server returned HTTP 500 to a JSONRPCNotification"]
133217

134218

135219
@pytest.mark.anyio

tests/interaction/transports/test_client_transport_http.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,9 @@ async def first_post_then_404(scope: Scope, receive: Receive, send: Send) -> Non
246246
with pytest.raises(MCPError) as exc_info: # pragma: no branch
247247
await client.list_tools()
248248

249-
assert exc_info.value.error == snapshot(ErrorData(code=INVALID_REQUEST, message="Session terminated"))
249+
assert exc_info.value.error == snapshot(
250+
ErrorData(code=INVALID_REQUEST, message="Session terminated", data={"httpStatus": 404})
251+
)
250252

251253

252254
def _blocking_server(started: anyio.Event, cancelled: anyio.Event) -> Server:

0 commit comments

Comments
 (0)