diff --git a/src/mcp/server/session.py b/src/mcp/server/session.py index f80971b01..5acba0b70 100644 --- a/src/mcp/server/session.py +++ b/src/mcp/server/session.py @@ -202,7 +202,40 @@ 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 -- 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: 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", + ) + ) 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..fcf760504 100644 --- a/tests/server/test_session.py +++ b/tests/server/test_session.py @@ -472,6 +472,9 @@ 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( @@ -482,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 + 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( @@ -508,6 +512,8 @@ 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 + error_data = error_message.message.root.error.data async with ( client_to_server_send, @@ -520,4 +526,22 @@ 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() + # 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()