Skip to content

Commit 3f93b73

Browse files
committed
fix: reapply #2983 onto current main
1 parent a4f4ccd commit 3f93b73

2 files changed

Lines changed: 81 additions & 45 deletions

File tree

src/mcp/server/streamable_http.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,14 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
681681
INTERNAL_ERROR,
682682
)
683683
await response(scope, receive, send)
684-
await writer.send(Exception(err))
684+
# The session's read stream may already be closed (e.g. the session task
685+
# crashed and tore down its streams before this handler ran). Sending into a
686+
# closed/broken stream here would raise a secondary error that masks the
687+
# original one and surfaces as "Exception in ASGI application". Guard it.
688+
try:
689+
await writer.send(Exception(err))
690+
except (anyio.ClosedResourceError, anyio.BrokenResourceError): # pragma: lax no cover
691+
pass
685692
return
686693

687694
async def _handle_get_request(self, request: Request, send: Send) -> None:

tests/server/test_streamable_http_router.py

Lines changed: 73 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,6 @@ async def replay_events_after(self, last_event_id: EventId, send_callback: Event
2525
raise NotImplementedError
2626

2727

28-
class _AsgiPost:
29-
"""A one-shot POST driven straight at `handle_request`, capturing what the transport sends."""
30-
31-
def __init__(self, body: bytes, headers: list[tuple[bytes, bytes]]) -> None:
32-
self.scope: Scope = {"type": "http", "method": "POST", "path": "/", "query_string": b"", "headers": headers}
33-
self.sent: list[Message] = []
34-
self._body = body
35-
self._body_sent = False
36-
37-
async def receive(self) -> Message:
38-
if not self._body_sent:
39-
self._body_sent = True
40-
return {"type": "http.request", "body": self._body, "more_body": False}
41-
raise NotImplementedError
42-
43-
async def send(self, message: Message) -> None:
44-
self.sent.append(message)
45-
46-
4728
@pytest.mark.anyio
4829
async def test_router_unconsumed_request_stream_does_not_block_siblings() -> None:
4930
"""A response whose `sse_writer` is not yet receiving must not park the router (#1764).
@@ -92,18 +73,35 @@ async def test_priming_store_failure_leaves_no_per_request_state() -> None:
9273
event_store=_PrimingFailingStore(),
9374
)
9475

95-
post = _AsgiPost(
96-
b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}',
97-
[
76+
body = b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}'
77+
scope: Scope = {
78+
"type": "http",
79+
"method": "POST",
80+
"path": "/",
81+
"query_string": b"",
82+
"headers": [
9883
(b"accept", b"application/json, text/event-stream"),
9984
(b"content-type", b"application/json"),
10085
(b"mcp-protocol-version", b"2025-11-25"),
10186
],
102-
)
87+
}
88+
body_sent = False
89+
90+
async def receive() -> Message:
91+
nonlocal body_sent
92+
if not body_sent:
93+
body_sent = True
94+
return {"type": "http.request", "body": body, "more_body": False}
95+
raise NotImplementedError
96+
97+
sent: list[Message] = []
98+
99+
async def asgi_send(message: Message) -> None:
100+
sent.append(message)
103101

104102
async with transport.connect() as (read_stream, _write_stream):
105103
async with anyio.create_task_group() as tg:
106-
tg.start_soon(transport.handle_request, post.scope, post.receive, post.send)
104+
tg.start_soon(transport.handle_request, scope, receive, asgi_send)
107105
with anyio.fail_after(5):
108106
forwarded = await read_stream.receive()
109107
assert isinstance(forwarded, Exception)
@@ -112,32 +110,63 @@ async def test_priming_store_failure_leaves_no_per_request_state() -> None:
112110
assert transport._request_streams == {}
113111
assert transport._sse_stream_writers == {}
114112

115-
assert post.sent[0]["type"] == "http.response.start"
116-
assert post.sent[0]["status"] == 500
117-
body = b"".join(m.get("body", b"") for m in post.sent if m["type"] == "http.response.body")
113+
assert sent[0]["type"] == "http.response.start"
114+
assert sent[0]["status"] == 500
115+
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
118116
assert b"backend unavailable" not in body
119117

120118

121119
@pytest.mark.anyio
122-
async def test_json_post_answers_500_when_session_terminates_mid_request() -> None:
123-
"""A JSON-mode POST whose session is torn down before the handler answers gets a 500, not a stall."""
124-
transport = StreamableHTTPServerTransport(mcp_session_id="sid", is_json_response_enabled=True)
125-
post = _AsgiPost(
126-
b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}',
127-
[
128-
(b"accept", b"application/json"),
120+
async def test_post_error_path_tolerates_closed_session_stream() -> None:
121+
"""The error path must not raise a secondary error when the read stream is gone (#2741).
122+
123+
When a session task crashes it tears down its read stream before the concurrent
124+
POST handler reaches its `except` block. The trailing ``writer.send(Exception(err))``
125+
then targets a closed/broken stream. Without a guard that raises a secondary
126+
``ClosedResourceError``/``BrokenResourceError`` out of the ASGI app, masking the
127+
original error and surfacing as "Exception in ASGI application". The 500 response
128+
must already be delivered and ``handle_request`` must return cleanly.
129+
"""
130+
transport = StreamableHTTPServerTransport(
131+
mcp_session_id=None,
132+
is_json_response_enabled=False,
133+
event_store=_PrimingFailingStore(),
134+
)
135+
136+
body = b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}'
137+
scope: Scope = {
138+
"type": "http",
139+
"method": "POST",
140+
"path": "/",
141+
"query_string": b"",
142+
"headers": [
143+
(b"accept", b"application/json, text/event-stream"),
129144
(b"content-type", b"application/json"),
130-
(b"mcp-session-id", b"sid"),
131145
(b"mcp-protocol-version", b"2025-11-25"),
132146
],
133-
)
147+
}
148+
body_sent = False
149+
150+
async def receive() -> Message:
151+
nonlocal body_sent
152+
if not body_sent:
153+
body_sent = True
154+
return {"type": "http.request", "body": body, "more_body": False}
155+
raise NotImplementedError
134156

135-
async with transport.connect() as (read_stream, _write_stream):
136-
async with anyio.create_task_group() as tg:
137-
tg.start_soon(transport.handle_request, post.scope, post.receive, post.send)
138-
with anyio.fail_after(5):
139-
await read_stream.receive() # the request reached the session; the POST is parked
140-
await transport.terminate()
157+
sent: list[Message] = []
141158

142-
assert post.sent[0]["type"] == "http.response.start"
143-
assert post.sent[0]["status"] == 500
159+
async def asgi_send(message: Message) -> None:
160+
sent.append(message)
161+
162+
async with transport.connect() as (read_stream, _write_stream):
163+
# Model the crashed-session teardown: the read stream the POST handler would
164+
# send the wrapped error into is already closed before the handler runs.
165+
await read_stream.aclose()
166+
# Must not raise out of the ASGI app despite the closed stream.
167+
await transport.handle_request(scope, receive, asgi_send)
168+
169+
assert sent[0]["type"] == "http.response.start"
170+
assert sent[0]["status"] == 500
171+
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
172+
assert b"backend unavailable" not in body

0 commit comments

Comments
 (0)