Skip to content

Commit 8058392

Browse files
author
oliver
committed
fix: return a self-describing error for requests before initialization
ServerSession._received_request raised a bare RuntimeError for any request received before the session completed its initialize handshake. That exception propagated to BaseSession._receive_loop's blanket except-Exception handler, which discards the exception's message entirely and always responds with the same hardcoded ErrorData(code=INVALID_PARAMS, message="Invalid request parameters") -- indistinguishable from an actually malformed request. This surfaces most concretely with the SSE transport: connect_sse() issues a brand-new session_id and a fresh, uninitialized ServerSession on every GET /sse. A client that reconnects (after an idle-timeout abort, a network blip, or a proxy restart) without resending 'initialize' on the new stream gets -32602 on every subsequent call, indefinitely -- which reads as "your parameters are wrong" when the real condition is "this session was never initialized." We hit this in production; the -32602 text led to real diagnostic time being spent looking at the wrong tool's parameter schema before server-side logs ("Received request before initialization was complete") revealed the actual cause. Fix: answer the request directly via responder.respond(ErrorData(...)) -- a public API already used by the InitializeRequest branch just above it -- instead of raising, so the generic catch-all never sees it. Uses INVALID_REQUEST rather than INVALID_PARAMS: the request's parameters aren't the problem, the session's initialization state is. Updated the existing test_other_requests_blocked_before_initialization, which had asserted error_code == INVALID_PARAMS -- i.e. it encoded the bug as expected behavior. Now asserts INVALID_REQUEST and checks the message text. Based on v1.x per CONTRIBUTING.md's guidance for bug fixes to a released version (this reproduces against mcp==1.28.1, the latest 1.x release; main is the in-progress v2.0.0 rewrite and has moved past this code shape entirely).
1 parent e828374 commit 8058392

2 files changed

Lines changed: 34 additions & 3 deletions

File tree

src/mcp/server/session.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,29 @@ async def _received_request(self, responder: RequestResponder[types.ClientReques
202202
pass
203203
case _:
204204
if self._initialization_state != InitializationState.Initialized:
205-
raise RuntimeError("Received request before initialization was complete")
205+
# Answer the request directly with a self-describing error
206+
# instead of raising. A bare exception here would propagate
207+
# to BaseSession._receive_loop's blanket except-Exception
208+
# handler, which discards the exception's message entirely
209+
# and always responds with the generic, misleading
210+
# ErrorData(code=INVALID_PARAMS, message="Invalid request
211+
# parameters") -- indistinguishable from an actually
212+
# malformed request. INVALID_REQUEST (not INVALID_PARAMS)
213+
# is used because the request's parameters aren't the
214+
# problem; the session's state is.
215+
with responder:
216+
await responder.respond(
217+
types.ErrorData(
218+
code=types.INVALID_REQUEST,
219+
message=(
220+
"MCP session not initialized: this session_id's "
221+
"stream was reconnected or lost without a fresh "
222+
"'initialize' handshake. Reconnect and initialize "
223+
"a new session."
224+
),
225+
data="session_not_initialized",
226+
)
227+
)
206228

207229
async def _received_notification(self, notification: types.ClientNotification) -> None:
208230
# Need this to avoid ASYNC910

tests/server/test_session.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ async def test_other_requests_blocked_before_initialization():
472472

473473
error_response_received = False
474474
error_code = None
475+
error_message_text = None
475476

476477
async def run_server():
477478
async with ServerSession(
@@ -488,7 +489,7 @@ async def run_server():
488489
await anyio.sleep(0.1) # Give time for the request to be processed
489490

490491
async def mock_client():
491-
nonlocal error_response_received, error_code
492+
nonlocal error_response_received, error_code, error_message_text
492493

493494
# Try to send a non-ping request before initialization
494495
await client_to_server_send.send(
@@ -508,6 +509,7 @@ async def mock_client():
508509
if isinstance(error_message.message.root, types.JSONRPCError): # pragma: no branch
509510
error_response_received = True
510511
error_code = error_message.message.root.error.code
512+
error_message_text = error_message.message.root.error.message
511513

512514
async with (
513515
client_to_server_send,
@@ -520,4 +522,11 @@ async def mock_client():
520522
tg.start_soon(mock_client)
521523

522524
assert error_response_received
523-
assert error_code == types.INVALID_PARAMS
525+
# INVALID_REQUEST (not INVALID_PARAMS): the request's parameters aren't
526+
# the problem, the session's initialization state is. Previously this
527+
# fell through to BaseSession._receive_loop's generic exception handler,
528+
# which always reported INVALID_PARAMS with a generic "Invalid request
529+
# parameters" message regardless of the actual cause -- indistinguishable
530+
# from a genuinely malformed request.
531+
assert error_code == types.INVALID_REQUEST
532+
assert error_message_text is not None and "session not initialized" in error_message_text.lower()

0 commit comments

Comments
 (0)