From 80583926aae448afd84a089904e192f35c027524 Mon Sep 17 00:00:00 2001 From: oliver Date: Sat, 18 Jul 2026 16:30:41 +0200 Subject: [PATCH 1/3] 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). --- src/mcp/server/session.py | 24 +++++++++++++++++++++++- tests/server/test_session.py | 13 +++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/mcp/server/session.py b/src/mcp/server/session.py index f80971b01..ebca92440 100644 --- a/src/mcp/server/session.py +++ b/src/mcp/server/session.py @@ -202,7 +202,29 @@ async def _received_request(self, responder: RequestResponder[types.ClientReques pass case _: if self._initialization_state != InitializationState.Initialized: - raise RuntimeError("Received request before initialization was complete") + # Answer the request directly with a self-describing error + # instead of raising. A bare exception here would propagate + # to BaseSession._receive_loop's blanket except-Exception + # handler, which discards the exception's message entirely + # and always responds with the generic, misleading + # ErrorData(code=INVALID_PARAMS, message="Invalid request + # parameters") -- indistinguishable from an actually + # malformed request. INVALID_REQUEST (not INVALID_PARAMS) + # is used because the request's parameters aren't the + # problem; the session's state is. + with responder: + await responder.respond( + types.ErrorData( + code=types.INVALID_REQUEST, + message=( + "MCP session not initialized: this session_id's " + "stream was reconnected or lost without a fresh " + "'initialize' handshake. Reconnect and initialize " + "a new session." + ), + data="session_not_initialized", + ) + ) async def _received_notification(self, notification: types.ClientNotification) -> None: # Need this to avoid ASYNC910 diff --git a/tests/server/test_session.py b/tests/server/test_session.py index ba1b44126..2136da47e 100644 --- a/tests/server/test_session.py +++ b/tests/server/test_session.py @@ -472,6 +472,7 @@ async def test_other_requests_blocked_before_initialization(): error_response_received = False error_code = None + error_message_text = None async def run_server(): async with ServerSession( @@ -488,7 +489,7 @@ async def run_server(): await anyio.sleep(0.1) # Give time for the request to be processed async def mock_client(): - nonlocal error_response_received, error_code + nonlocal error_response_received, error_code, error_message_text # Try to send a non-ping request before initialization await client_to_server_send.send( @@ -508,6 +509,7 @@ async def mock_client(): if isinstance(error_message.message.root, types.JSONRPCError): # pragma: no branch error_response_received = True error_code = error_message.message.root.error.code + error_message_text = error_message.message.root.error.message async with ( client_to_server_send, @@ -520,4 +522,11 @@ async def mock_client(): tg.start_soon(mock_client) assert error_response_received - assert error_code == types.INVALID_PARAMS + # INVALID_REQUEST (not INVALID_PARAMS): the request's parameters aren't + # the problem, the session's initialization state is. Previously this + # fell through to BaseSession._receive_loop's generic exception handler, + # which always reported INVALID_PARAMS with a generic "Invalid request + # parameters" message regardless of the actual cause -- indistinguishable + # from a genuinely malformed request. + assert error_code == types.INVALID_REQUEST + assert error_message_text is not None and "session not initialized" in error_message_text.lower() From f7c83538b13e75bc2df78d71e553641dd917899d Mon Sep 17 00:00:00 2001 From: oliver Date: Sat, 18 Jul 2026 19:04:06 +0200 Subject: [PATCH 2/3] fix: don't overclaim cause in error message, test the _in_flight fix, cite precedent Addresses review feedback on the session-not-initialized error fix: - The error message asserted a specific cause ("this session_id's stream was reconnected or lost") that _initialization_state has no way to actually verify -- a session that was never initialized at all (e.g. a client bug) hits the same branch and would get told a story that's simply false in that case. Reworded to state the fact (an initialize handshake is required) without asserting why this particular session never completed one. - Cited the existing INVALID_REQUEST precedent in streamable_http_manager.py's "Session not found" responses (same class of session-validity condition) in the inline comment, rather than relying on JSON-RPC semantics alone. - This fix has an untested, unmentioned side effect: answering via responder.respond() means RequestResponder.__exit__ now sees _completed=True and fires on_complete, popping the request out of _in_flight. The old bare RuntimeError bypassed the responder's context-manager cleanup entirely, so every rejected pre-init request leaked an _in_flight entry for the life of the session -- a client hammering an uninitialized session could grow it unboundedly. Added an assertion pinning this (len(session._in_flight) == 0 after the rejected request), so a future refactor can't silently reintroduce the leak. - Also assert on error.data ("session_not_initialized"), the machine-readable discriminator this fix introduces -- message text isn't meant to be programmatically parsed, this is. Verified: tests/server/test_session.py (7/7) and the broader tests/server/ + tests/shared/test_session.py suite (521 passed, same pre-existing unrelated websockets-extra collection error as before) both pass. ruff check / ruff format --check / pyright clean on both changed files. --- src/mcp/server/session.py | 21 ++++++++++++++++----- tests/server/test_session.py | 19 +++++++++++++++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/mcp/server/session.py b/src/mcp/server/session.py index ebca92440..5acba0b70 100644 --- a/src/mcp/server/session.py +++ b/src/mcp/server/session.py @@ -211,16 +211,27 @@ async def _received_request(self, responder: RequestResponder[types.ClientReques # parameters") -- indistinguishable from an actually # malformed request. INVALID_REQUEST (not INVALID_PARAMS) # is used because the request's parameters aren't the - # problem; the session's state is. + # problem; the session's state is -- matching the existing + # "Session not found" usage in streamable_http_manager.py + # for the same class of session-validity condition. + # + # The message states the fact (an initialize handshake is + # required) without asserting *why* this particular + # session never completed one -- this branch can't tell a + # genuinely uninitialized session (e.g. a client bug) from + # a stream that reconnected without reinitializing, so it + # doesn't claim either. with responder: await responder.respond( types.ErrorData( code=types.INVALID_REQUEST, message=( - "MCP session not initialized: this session_id's " - "stream was reconnected or lost without a fresh " - "'initialize' handshake. Reconnect and initialize " - "a new session." + "MCP session not initialized: an 'initialize' " + "request must complete successfully before " + "other requests are handled. If this session " + "was previously initialized, its stream may " + "have been reconnected without a fresh " + "handshake." ), data="session_not_initialized", ) diff --git a/tests/server/test_session.py b/tests/server/test_session.py index 2136da47e..fcf760504 100644 --- a/tests/server/test_session.py +++ b/tests/server/test_session.py @@ -473,6 +473,8 @@ async def test_other_requests_blocked_before_initialization(): error_response_received = False error_code = None error_message_text = None + error_data = None + session_ref: list[ServerSession] = [] async def run_server(): async with ServerSession( @@ -483,13 +485,14 @@ async def run_server(): server_version="0.1.0", capabilities=ServerCapabilities(), ), - ): + ) as session: + session_ref.append(session) # Server should handle the request and send an error response # No need to process incoming_messages since the error is handled automatically await anyio.sleep(0.1) # Give time for the request to be processed async def mock_client(): - nonlocal error_response_received, error_code, error_message_text + nonlocal error_response_received, error_code, error_message_text, error_data # Try to send a non-ping request before initialization await client_to_server_send.send( @@ -510,6 +513,7 @@ async def mock_client(): error_response_received = True error_code = error_message.message.root.error.code error_message_text = error_message.message.root.error.message + error_data = error_message.message.root.error.data async with ( client_to_server_send, @@ -530,3 +534,14 @@ async def mock_client(): # from a genuinely malformed request. assert error_code == types.INVALID_REQUEST assert error_message_text is not None and "session not initialized" in error_message_text.lower() + # data is the machine-readable discriminator -- message text isn't meant + # to be programmatically parsed, this is. + assert error_data == "session_not_initialized" + # Answering via responder.respond() (rather than raising, as before this + # fix) means RequestResponder.__exit__ sees _completed=True and fires + # on_complete, which pops the request out of _in_flight. Before this fix, + # the bare RuntimeError bypassed the responder's context-manager cleanup + # entirely, so every rejected pre-init request leaked an _in_flight + # entry for the life of the session. + assert session_ref and len(session_ref[0]._in_flight) == 0 + assert error_message_text is not None and "session not initialized" in error_message_text.lower() From cd61762ab931757ab1cd7c2a1ba931ee2b0b88a2 Mon Sep 17 00:00:00 2001 From: oliver Date: Mon, 20 Jul 2026 16:49:25 +0200 Subject: [PATCH 3/3] fix: return a self-describing 404 for an unknown SSE session handle_post_message's session-lookup failure returned a bare "Could not find session" 404, indistinguishable from any other cause and naming neither the reason nor the remedy. This is the more common sibling of the case fixed in 80583926 (self-describing error for a request before initialization): that fix answers through the ServerSession's responder once a session object exists, but a restarted process has no session object at all for an unknown session_id -- the rejection happens in handle_post_message itself, one layer below where a JSON-RPC error could be shaped. Every redeploy invalidates every previously-connected client's session_id at once, so this 404 is what most stranded clients actually hit, and it reads as a service outage rather than "restart your MCP client." Both 404 branches (unknown session_id, and the credential-mismatch branch that deliberately mimics "session doesn't exist" so it can't be used to probe for valid session IDs under another user) now share one message via a single Response built once, so they can't drift apart and leak which case occurred. Updated test_sse_security_post_valid_content_type, which asserted the old exact bare string -- it's testing content-type handling reaches the 404 branch at all, not the message's wording, so relaxed to a substring check. --- src/mcp/server/sse.py | 19 +++++++++++++++---- tests/server/test_sse_security.py | 2 +- tests/shared/test_sse.py | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/mcp/server/sse.py b/src/mcp/server/sse.py index 489785c4c..bdbe331e4 100644 --- a/src/mcp/server/sse.py +++ b/src/mcp/server/sse.py @@ -237,11 +237,23 @@ async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) response = Response("Invalid session ID", status_code=400) return await response(scope, receive, send) + # Both branches below use this identical, self-describing message: + # a session can be missing either because it was never valid or + # because the credential doesn't match its owner, and the second + # case must respond exactly as if the session did not exist (see + # below) -- so the two messages can never diverge without leaking + # which case occurred. + unknown_session_response = Response( + f"Could not find session {session_id.hex}: the server may have restarted since this " + "session was created, or the session_id was never valid. Reconnect (open a new SSE " + "stream) and send a fresh 'initialize' request.", + status_code=404, + ) + writer = self._read_stream_writers.get(session_id) if not writer: logger.warning(f"Could not find session for ID: {session_id}") - response = Response("Could not find session", status_code=404) - return await response(scope, receive, send) + return await unknown_session_response(scope, receive, send) user = scope.get("user") requestor = authorization_context(user) if isinstance(user, AuthenticatedUser) else None @@ -249,8 +261,7 @@ async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) # A session can only be used with the credential that created it. # Respond exactly as if the session did not exist. logger.warning("Rejecting message for session %s: credential does not match", session_id) - response = Response("Could not find session", status_code=404) - return await response(scope, receive, send) + return await unknown_session_response(scope, receive, send) body = await request.body() logger.debug(f"Received JSON: {body}") diff --git a/tests/server/test_sse_security.py b/tests/server/test_sse_security.py index 0978b8a15..43cc98f07 100644 --- a/tests/server/test_sse_security.py +++ b/tests/server/test_sse_security.py @@ -310,7 +310,7 @@ async def test_sse_security_post_valid_content_type(server_port: int): # Will get 404 because session doesn't exist, but that's OK # We're testing that it passes the content-type check assert response.status_code == 404 - assert response.text == "Could not find session" + assert "Could not find session" in response.text finally: process.terminate() diff --git a/tests/shared/test_sse.py b/tests/shared/test_sse.py index 7604450f8..a2e37b007 100644 --- a/tests/shared/test_sse.py +++ b/tests/shared/test_sse.py @@ -5,6 +5,7 @@ from collections.abc import AsyncGenerator, Generator from typing import Any from unittest.mock import AsyncMock, MagicMock, Mock, patch +from uuid import uuid4 import anyio import httpx @@ -176,6 +177,24 @@ async def connection_test() -> None: await connection_test() +@pytest.mark.anyio +async def test_post_message_unknown_session_returns_self_describing_error(http_client: httpx.AsyncClient) -> None: + """A POST for a session_id the server has no record of (e.g. because the + process restarted since the client's SSE stream was opened) should + explain the cause and remedy, not a bare "Could not find session" -- the + same misleading-error problem #19 fixed for the uninitialized-session + case, one layer down where no ServerSession exists to answer through.""" + unknown_session_id = uuid4().hex + response = await http_client.post( + f"/messages/?session_id={unknown_session_id}", + json={"jsonrpc": "2.0", "id": 1, "method": "ping"}, + ) + assert response.status_code == 404 + body = response.text + assert "restart" in body.lower() + assert "reconnect" in body.lower() and "initialize" in body.lower() + + @pytest.mark.anyio async def test_sse_client_basic_connection(server: None, server_url: str) -> None: async with sse_client(server_url + "/sse") as streams: