Skip to content

Commit fc85da9

Browse files
committed
fix: surface stateful HTTP session crash cause
1 parent 6d0c160 commit fc85da9

3 files changed

Lines changed: 74 additions & 5 deletions

File tree

src/mcp/server/streamable_http.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import re
1111
from abc import ABC, abstractmethod
1212
from collections.abc import AsyncGenerator, Awaitable, Callable
13-
from contextlib import asynccontextmanager
13+
from contextlib import asynccontextmanager, suppress
1414
from dataclasses import dataclass
1515
from http import HTTPStatus
1616
from typing import Any
@@ -171,6 +171,7 @@ def __init__(
171171
] = {}
172172
self._sse_stream_writers: dict[RequestId, MemoryObjectSendStream[dict[str, str]]] = {}
173173
self._terminated = False
174+
self._session_run_error: BaseException | None = None
174175
# Idle timeout cancel scope; managed by the session manager.
175176
self.idle_scope: anyio.CancelScope | None = None
176177

@@ -179,6 +180,16 @@ def is_terminated(self) -> bool:
179180
"""Check if this transport has been explicitly terminated."""
180181
return self._terminated
181182

183+
def note_session_run_error(self, exc: BaseException) -> None:
184+
self._session_run_error = exc
185+
186+
def _post_error_message(self, err: BaseException) -> str:
187+
display_error = self._session_run_error or err
188+
display_error_text = str(display_error) or type(display_error).__name__
189+
if display_error is not err and str(display_error):
190+
display_error_text = f"{type(display_error).__name__}: {display_error_text}"
191+
return f"Error handling POST request: {display_error_text}"
192+
182193
def close_sse_stream(self, request_id: RequestId) -> None:
183194
"""Close SSE connection for a specific request without terminating the stream.
184195
@@ -361,6 +372,10 @@ async def _clean_up_memory_streams(self, request_id: RequestId) -> None:
361372
# Remove the request stream from the mapping
362373
self._request_streams.pop(request_id, None)
363374

375+
async def _clean_up_post_request_stream(self, request_id: RequestId | None) -> None:
376+
if request_id is not None:
377+
await self._clean_up_memory_streams(request_id)
378+
364379
async def handle_request(self, scope: Scope, receive: Receive, send: Send) -> None:
365380
"""Application entry point that handles all HTTP requests."""
366381
request = Request(scope, receive)
@@ -441,6 +456,7 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
441456
writer = self._read_stream_writer
442457
if writer is None: # pragma: no cover
443458
raise ValueError("No read stream writer available. Ensure connect() is called first.")
459+
request_id: RequestId | None = None
444460
try:
445461
# Validate Accept header
446462
if not await self._validate_accept_header(request, scope, send):
@@ -638,15 +654,16 @@ async def sse_writer():
638654
# the per-SSE-stream guard above; whether tests reach this depends
639655
# on client teardown timing.
640656
logger.exception("Error handling POST request")
657+
await self._clean_up_post_request_stream(request_id)
641658
response = self._create_error_response(
642-
f"Error handling POST request: {err}",
659+
self._post_error_message(err),
643660
HTTPStatus.INTERNAL_SERVER_ERROR,
644661
INTERNAL_ERROR,
645662
)
646663
await response(scope, receive, send)
647-
if writer:
664+
with suppress(anyio.BrokenResourceError, anyio.ClosedResourceError):
648665
await writer.send(Exception(err))
649-
return # pragma: no cover
666+
return
650667

651668
async def _handle_get_request(self, request: Request, send: Send) -> None:
652669
"""Handle GET request to establish SSE.

src/mcp/server/streamable_http_manager.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
276276
self._server_instances.pop(http_transport.mcp_session_id, None)
277277
self._session_owners.pop(http_transport.mcp_session_id, None)
278278
await http_transport.terminate()
279-
except Exception:
279+
except Exception as exc:
280+
http_transport.note_session_run_error(exc)
280281
logger.exception(f"Session {http_transport.mcp_session_id} crashed")
281282
finally:
282283
if ( # pragma: no branch

tests/server/test_streamable_http_manager.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,57 @@ async def mock_receive(): # pragma: no cover
209209
assert not manager._server_instances, "No sessions should be tracked after the only session crashes"
210210

211211

212+
def test_post_error_message_prefers_session_run_error():
213+
transport = StreamableHTTPServerTransport(mcp_session_id="session-id", is_json_response_enabled=True)
214+
215+
assert transport._post_error_message(anyio.ClosedResourceError()) == (
216+
"Error handling POST request: ClosedResourceError"
217+
)
218+
219+
transport.note_session_run_error(RuntimeError("BOOM-distinctive-root-cause"))
220+
assert transport._post_error_message(anyio.ClosedResourceError()) == (
221+
"Error handling POST request: RuntimeError: BOOM-distinctive-root-cause"
222+
)
223+
224+
225+
@pytest.mark.anyio
226+
async def test_stateful_json_response_includes_session_crash_cause():
227+
app = Server("test-crash-cause")
228+
app.run = AsyncMock(side_effect=RuntimeError("BOOM-distinctive-root-cause"))
229+
manager = StreamableHTTPSessionManager(app=app, json_response=True)
230+
231+
sent_messages: list[Message] = []
232+
response_body = b""
233+
234+
async def mock_send(message: Message) -> None:
235+
nonlocal response_body
236+
sent_messages.append(message)
237+
if message["type"] == "http.response.body":
238+
response_body += message.get("body", b"")
239+
240+
async def mock_receive() -> Message:
241+
body = {
242+
"jsonrpc": "2.0",
243+
"id": 1,
244+
"method": "initialize",
245+
"params": {
246+
"protocolVersion": "2025-06-18",
247+
"capabilities": {},
248+
"clientInfo": {"name": "test-client", "version": "1.0"},
249+
},
250+
}
251+
return {"type": "http.request", "body": json.dumps(body).encode(), "more_body": False}
252+
253+
async with manager.run():
254+
await manager.handle_request(_request_scope(), mock_receive, mock_send)
255+
response_start = next(msg for msg in sent_messages if msg["type"] == "http.response.start")
256+
assert response_start["status"] == 500
257+
258+
error_data = json.loads(response_body)
259+
assert error_data["error"]["code"] == -32603
260+
assert "RuntimeError: BOOM-distinctive-root-cause" in error_data["error"]["message"]
261+
262+
212263
@pytest.mark.anyio
213264
async def test_stateless_requests_memory_cleanup():
214265
"""Test that stateless requests actually clean up resources using real transports."""

0 commit comments

Comments
 (0)