Skip to content

Commit 31c4268

Browse files
committed
fix(client): propagate SSE POST errors to caller instead of hanging
In `sse_client`, the message-POST coroutine `_send_message` called `response.raise_for_status()` with no handler. When the server returned a non-2xx (401/403/404/5xx) or the POST hit a network error, the exception propagated into the `post_writer` task group and was swallowed by its `except Exception: logger.exception("Error in post_writer")`. The failure was never delivered through the read stream, so a caller blocked on `read_stream.receive()` (e.g. `ClientSession.initialize()`) hung forever. Catch `httpx.HTTPError` inside `_send_message` and forward it to `read_stream_writer`, the same pattern stdio.py and websocket.py already use, and that `streamable_http.py` uses for its >= 400 responses. The caller now receives the error promptly instead of deadlocking. Adds a regression test driving a real SSE handshake whose message POST returns 503; it asserts the caller receives the `HTTPStatusError` via the read stream within a bounded timeout. The test fails (times out) against the unpatched client. Refs #2110
1 parent ed6adee commit 31c4268

2 files changed

Lines changed: 80 additions & 10 deletions

File tree

src/mcp/client/sse.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,24 @@ async def post_writer(endpoint_url: str):
120120

121121
async def _send_message(session_message: SessionMessage) -> None:
122122
logger.debug(f"Sending client message: {session_message}")
123-
response = await client.post(
124-
endpoint_url,
125-
json=session_message.message.model_dump(
126-
by_alias=True,
127-
mode="json",
128-
exclude_unset=True,
129-
),
130-
)
131-
response.raise_for_status()
123+
try:
124+
response = await client.post(
125+
endpoint_url,
126+
json=session_message.message.model_dump(
127+
by_alias=True,
128+
mode="json",
129+
exclude_unset=True,
130+
),
131+
)
132+
response.raise_for_status()
133+
except httpx.HTTPError as exc:
134+
# Forward the failure to the caller via the read stream instead of
135+
# letting it surface as a swallowed task-group error, which would
136+
# leave read_stream.receive() blocked forever (#2110). Mirrors the
137+
# stream-error handling in stdio.py and streamable_http.py.
138+
logger.exception("Error sending client message")
139+
await read_stream_writer.send(exc)
140+
return
132141
logger.debug(f"Client message sent successfully: {response.status_code}")
133142

134143
async for session_message in write_stream_reader:

tests/shared/test_sse.py

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from inline_snapshot import snapshot
1515
from starlette.applications import Starlette
1616
from starlette.requests import Request
17-
from starlette.responses import Response
17+
from starlette.responses import Response, StreamingResponse
1818
from starlette.routing import Mount, Route
1919

2020
import mcp.client.sse
@@ -25,6 +25,7 @@
2525
from mcp.server.sse import SseServerTransport
2626
from mcp.server.transport_security import TransportSecuritySettings
2727
from mcp.shared.exceptions import MCPError
28+
from mcp.shared.message import SessionMessage
2829
from mcp.types import (
2930
CallToolRequestParams,
3031
CallToolResult,
@@ -161,6 +162,66 @@ async def http_client(server: None, server_url: str) -> AsyncGenerator[httpx.Asy
161162
yield client
162163

163164

165+
def make_failing_post_app() -> Starlette: # pragma: no cover
166+
"""SSE app that completes the handshake but fails every message POST.
167+
168+
The `/sse` stream emits a valid `endpoint` event (so the client reaches the
169+
POST path) and then stays open; `/messages/` always returns 503. Used to
170+
drive the client's POST-error propagation path (#2110).
171+
"""
172+
173+
async def handle_sse(request: Request) -> Response:
174+
async def event_stream() -> AsyncGenerator[bytes, None]:
175+
yield b"event: endpoint\r\ndata: /messages/\r\n\r\n"
176+
await anyio.sleep(30) # keep the SSE connection open for the test's duration
177+
178+
return StreamingResponse(event_stream(), media_type="text/event-stream")
179+
180+
async def handle_message(request: Request) -> Response:
181+
return Response("upstream exploded", status_code=503)
182+
183+
return Starlette(
184+
routes=[
185+
Route("/sse", endpoint=handle_sse),
186+
Route("/messages/", endpoint=handle_message, methods=["POST"]),
187+
]
188+
)
189+
190+
191+
def run_failing_post_server(server_port: int) -> None: # pragma: no cover
192+
app = make_failing_post_app()
193+
server = uvicorn.Server(config=uvicorn.Config(app=app, host="127.0.0.1", port=server_port, log_level="error"))
194+
server.run()
195+
196+
197+
@pytest.fixture()
198+
def failing_post_server(server_port: int) -> Generator[None, None, None]:
199+
proc = multiprocessing.Process(target=run_failing_post_server, kwargs={"server_port": server_port}, daemon=True)
200+
proc.start()
201+
wait_for_server(server_port)
202+
yield
203+
proc.kill()
204+
proc.join(timeout=2)
205+
if proc.is_alive(): # pragma: no cover
206+
print("failing-post server process failed to terminate")
207+
208+
209+
@pytest.mark.anyio
210+
async def test_sse_client_post_error_propagates_to_caller(failing_post_server: None, server_url: str) -> None:
211+
"""A non-2xx on the message POST must surface to the caller via the read stream.
212+
213+
Regression test for #2110: previously the error was swallowed by the post_writer
214+
task group and `read_stream.receive()` blocked forever.
215+
"""
216+
async with sse_client(server_url + "/sse") as (read_stream, write_stream):
217+
await write_stream.send(SessionMessage(types.JSONRPCRequest(jsonrpc="2.0", id=1, method="ping")))
218+
with anyio.fail_after(10):
219+
item = await read_stream.receive()
220+
221+
assert isinstance(item, httpx.HTTPStatusError)
222+
assert item.response.status_code == 503
223+
224+
164225
# Tests
165226
@pytest.mark.anyio
166227
async def test_raw_sse_connection(http_client: httpx.AsyncClient) -> None:

0 commit comments

Comments
 (0)