From 4c2c11a13b2453566857c41244f73afd2142f6c1 Mon Sep 17 00:00:00 2001 From: Himanshu Gupta Date: Tue, 14 Jul 2026 00:41:39 +0200 Subject: [PATCH 1/2] feat(client): carry the originating HTTP status in ErrorData.data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/migration.md | 2 + docs/troubleshooting.md | 19 ++++- src/mcp/client/streamable_http.py | 21 ++++- tests/client/test_streamable_http.py | 84 +++++++++++++++++++ .../transports/test_client_transport_http.py | 4 +- 5 files changed, 125 insertions(+), 5 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index 8822d449d0..c72fcdc95b 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1688,6 +1688,8 @@ 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')` | +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. + 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):** diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 621b32c6cd..bec074c793 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -162,12 +162,29 @@ async with Client("https://mcp.example.com/mcp") as client: mcp.shared.exceptions.MCPError: Server returned an error response ``` -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: +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: ```text WARNING mcp.server.transport_security: Invalid Host header: mcp.example.com ``` +The *status* does reach you, on the error's `data`, which is how you tell one refusal from another without parsing the message: + +```python +from mcp.shared.exceptions import MCPError + + +async def list_tools() -> None: + try: + async with Client("https://mcp.example.com/mcp") as client: + await client.list_tools() + except MCPError as exc: + status = (exc.error.data or {}).get("httpStatus") # 421 here; 401, 403, 503 … elsewhere + print(status) +``` + +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. + The fix is `transport_security=`. Allowlist the hostname you actually serve: ```python title="server.py" hl_lines="14-17" diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index 5c3501d8c4..a79c132a2f 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -49,6 +49,11 @@ MCP_SESSION_ID = "mcp-session-id" LAST_EVENT_ID = "last-event-id" +# Key under which the originating HTTP status is carried in `ErrorData.data` when a +# non-2xx response is synthesized into a JSON-RPC error. Lets callers tell a terminal +# 401/403 from a retryable 5xx, which the JSON-RPC error code alone cannot express. +HTTP_STATUS_KEY = "httpStatus" + # Reconnection defaults DEFAULT_RECONNECTION_DELAY_MS = 1000 # 1 second fallback when server doesn't provide retry MAX_RECONNECTION_ATTEMPTS = 2 # Max retry attempts before giving up @@ -363,13 +368,23 @@ async def _handle_post_request(self, ctx: RequestContext) -> 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") + code, error_message = METHOD_NOT_FOUND, "Not Found" else: - error_data = ErrorData(code=INVALID_REQUEST, message="Session terminated") + code, error_message = INVALID_REQUEST, "Session terminated" else: - error_data = ErrorData(code=INTERNAL_ERROR, message="Server returned an error response") + code, error_message = INTERNAL_ERROR, "Server returned an error response" + # The status is what tells a caller whether to retry: 401/403 are + # terminal, 5xx are worth another attempt. The JSON-RPC code above + # cannot express that distinction, so carry the status itself. + error_data = ErrorData( + code=code, message=error_message, data={HTTP_STATUS_KEY: response.status_code} + ) session_message = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data)) await ctx.read_stream_writer.send(session_message) + else: + # Nothing to resolve — a notification/response POST has no waiter. Log + # it, or a server rejecting every write fails completely silently. + logger.warning("Server returned HTTP %d to a %s", response.status_code, type(message).__name__) return if self._is_initialization_request(message): diff --git a/tests/client/test_streamable_http.py b/tests/client/test_streamable_http.py index da17a71c1e..3db0c76cc5 100644 --- a/tests/client/test_streamable_http.py +++ b/tests/client/test_streamable_http.py @@ -8,6 +8,7 @@ import base64 import json +import logging from collections.abc import AsyncIterator, Callable, Mapping from typing import Any @@ -19,6 +20,7 @@ CLIENT_CAPABILITIES_META_KEY, CLIENT_INFO_META_KEY, CONNECTION_CLOSED, + INTERNAL_ERROR, INVALID_REQUEST, METHOD_NOT_FOUND, PROTOCOL_VERSION_META_KEY, @@ -31,6 +33,7 @@ from starlette.types import Receive, Scope, Send from mcp.client.streamable_http import ( + HTTP_STATUS_KEY, MAX_RECONNECTION_ATTEMPTS, RequestContext, StreamableHTTPTransport, @@ -130,6 +133,87 @@ def handler(request: httpx.Request) -> httpx.Response: assert isinstance(reply, SessionMessage) assert isinstance(reply.message, JSONRPCError) assert reply.message.error.code == METHOD_NOT_FOUND + assert reply.message.error.data == {HTTP_STATUS_KEY: 404} + + +@pytest.mark.anyio +@pytest.mark.parametrize("status", [401, 403, 500, 503]) +async def test_a_non_2xx_status_reaches_the_caller_in_the_errors_data(status: int) -> None: + """The originating HTTP status rides along in `ErrorData.data`. + + The JSON-RPC code is INTERNAL_ERROR for every one of these, so it alone cannot tell a + caller whether to retry. The status can: 401/403 are terminal, 5xx are worth retrying. + """ + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status) + + with anyio.fail_after(5): + async with ( + httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http, + streamable_http_client("http://test/mcp", http_client=http) as (read, write), + ): + await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={}))) + reply = await read.receive() + assert isinstance(reply, SessionMessage) + assert isinstance(reply.message, JSONRPCError) + assert reply.message.error.code == INTERNAL_ERROR + assert reply.message.error.data == {HTTP_STATUS_KEY: status} + + +@pytest.mark.anyio +async def test_a_servers_own_jsonrpc_error_body_survives_the_non_2xx_status() -> None: + """A JSON-RPC error in the body wins: the server said what went wrong, so don't overwrite its `data`.""" + + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) + error = {"code": INVALID_REQUEST, "message": "no", "data": {"detail": "from the server"}} + return httpx.Response(400, json={"jsonrpc": "2.0", "id": body["id"], "error": error}) + + with anyio.fail_after(5): + async with ( + httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http, + streamable_http_client("http://test/mcp", http_client=http) as (read, write), + ): + await write.send(SessionMessage(JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={}))) + reply = await read.receive() + assert isinstance(reply, SessionMessage) + assert isinstance(reply.message, JSONRPCError) + assert reply.message.error.data == {"detail": "from the server"} + + +@pytest.mark.anyio +async def test_a_non_2xx_to_a_notification_is_logged() -> None: + """A notification has no waiter to resolve, so a rejected write can only be logged — but it must be. + + Waiting on the log record itself (not on the POST) is what makes this deterministic: the + warning is emitted after the response returns, so the handler is too early a signal. + """ + logged = anyio.Event() + records: list[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + records.append(record) + logged.set() + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(500) + + transport_logger = logging.getLogger("mcp.client.streamable_http") + capture = _Capture(level=logging.WARNING) + transport_logger.addHandler(capture) + try: + with anyio.fail_after(5): + async with ( + httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http, + streamable_http_client("http://test/mcp", http_client=http) as (_, write), + ): + await write.send(SessionMessage(JSONRPCNotification(jsonrpc="2.0", method="notifications/progress"))) + await logged.wait() + finally: + transport_logger.removeHandler(capture) + assert [r.getMessage() for r in records] == ["Server returned HTTP 500 to a JSONRPCNotification"] @pytest.mark.anyio diff --git a/tests/interaction/transports/test_client_transport_http.py b/tests/interaction/transports/test_client_transport_http.py index 61b0e21c9b..9890c341b5 100644 --- a/tests/interaction/transports/test_client_transport_http.py +++ b/tests/interaction/transports/test_client_transport_http.py @@ -246,7 +246,9 @@ async def first_post_then_404(scope: Scope, receive: Receive, send: Send) -> Non with pytest.raises(MCPError) as exc_info: # pragma: no branch await client.list_tools() - assert exc_info.value.error == snapshot(ErrorData(code=INVALID_REQUEST, message="Session terminated")) + assert exc_info.value.error == snapshot( + ErrorData(code=INVALID_REQUEST, message="Session terminated", data={"httpStatus": 404}) + ) def _blocking_server(started: anyio.Event, cancelled: anyio.Event) -> Server: From eea58dd22d8df91bd7fa2143c0fffbfb4e60a7ad Mon Sep 17 00:00:00 2001 From: Himanshu Gupta Date: Tue, 14 Jul 2026 16:09:43 +0200 Subject: [PATCH 2/2] fix: keep the troubleshooting snippet ASCII-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/test_examples.py` pipes every code block in `docs/troubleshooting.md` through ruff, and on Windows that pipe is cp1252, not UTF-8: a `…` in the snippet reached ruff as invalid UTF-8 and failed every Windows job. The character carried no meaning the prose didn't already. --- docs/troubleshooting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index bec074c793..dd828b0cfa 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -179,7 +179,7 @@ async def list_tools() -> None: async with Client("https://mcp.example.com/mcp") as client: await client.list_tools() except MCPError as exc: - status = (exc.error.data or {}).get("httpStatus") # 421 here; 401, 403, 503 … elsewhere + status = (exc.error.data or {}).get("httpStatus") # 421 here; 401, 403, 503 elsewhere print(status) ```