diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py index 54ac7374a..5254a5066 100644 --- a/src/mcp/server/streamable_http.py +++ b/src/mcp/server/streamable_http.py @@ -298,7 +298,7 @@ def _create_error_response( # Return a properly formatted JSON error response error_response = JSONRPCError( jsonrpc="2.0", - id="server-error", # We don't have a request ID for general errors + id=None, error=ErrorData(code=error_code, message=error_message), ) @@ -975,12 +975,11 @@ async def message_router(): # Determine which request stream(s) should receive this message message = session_message.message target_request_id = None - # Check if this is a response - if isinstance(message, JSONRPCResponse | JSONRPCError): - response_id = str(message.id) - # If this response is for an existing request stream, - # send it there - target_request_id = response_id + # Check if this is a response with a known request id. + # Null-id errors (e.g., parse errors) fall through to + # the GET stream since they can't be correlated. + if isinstance(message, JSONRPCResponse | JSONRPCError) and message.id is not None: + target_request_id = str(message.id) # Extract related_request_id from meta if it exists elif ( # pragma: no cover session_message.metadata is not None diff --git a/src/mcp/server/streamable_http_manager.py b/src/mcp/server/streamable_http_manager.py index 8eb29c4d4..288bba6d1 100644 --- a/src/mcp/server/streamable_http_manager.py +++ b/src/mcp/server/streamable_http_manager.py @@ -244,7 +244,7 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE # See: https://github.com/modelcontextprotocol/python-sdk/issues/1821 error_response = JSONRPCError( jsonrpc="2.0", - id="server-error", + id=None, error=ErrorData(code=INVALID_REQUEST, message="Session not found"), ) response = Response( diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 453e36274..ab059b530 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -458,6 +458,19 @@ async def _handle_response(self, message: SessionMessage) -> None: if not isinstance(message.message, JSONRPCResponse | JSONRPCError): return # pragma: no cover + # Handle null-id errors (e.g., parse errors, invalid requests) before + # routing. Per JSON-RPC 2.0, id is null when the request id could not + # be determined — these cannot be correlated to any pending request. + if isinstance(message.message, JSONRPCError) and message.message.id is None: + error = message.message.error + logging.warning(f"Received error with null ID: {error.message}") + await self._handle_incoming(MCPError(error.code, error.message, error.data)) + return + + # After the null-id guard above, id is guaranteed to be non-None. + # JSONRPCResponse.id is always RequestId, and JSONRPCError.id is only + # None for parse errors (handled above). + assert message.message.id is not None # Normalize response ID to handle type mismatches (e.g., "0" vs 0) response_id = self._normalize_request_id(message.message.id) diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py index 0cfdc993a..aaea1fd53 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -4,7 +4,7 @@ from typing import Annotated, Any, Literal -from pydantic import BaseModel, Field, TypeAdapter +from pydantic import BaseModel, Field, SerializationInfo, SerializerFunctionWrapHandler, TypeAdapter, model_serializer RequestId = Annotated[int, Field(strict=True)] | str """The ID of a JSON-RPC request.""" @@ -75,9 +75,19 @@ class JSONRPCError(BaseModel): """A response to a request that indicates an error occurred.""" jsonrpc: Literal["2.0"] - id: RequestId + id: RequestId | None error: ErrorData + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler, _: SerializationInfo) -> dict[str, Any]: + result = handler(self) + # JSON-RPC 2.0 requires id to always be present in error responses, + # even when null (e.g. parse errors). Ensure exclude_none=True + # cannot strip it. + if "id" not in result and self.id is None: + result["id"] = None + return result + JSONRPCMessage = JSONRPCRequest | JSONRPCNotification | JSONRPCResponse | JSONRPCError jsonrpc_message_adapter: TypeAdapter[JSONRPCMessage] = TypeAdapter(JSONRPCMessage) diff --git a/tests/server/test_streamable_http_manager.py b/tests/server/test_streamable_http_manager.py index 475eaa167..e9a8720f1 100644 --- a/tests/server/test_streamable_http_manager.py +++ b/tests/server/test_streamable_http_manager.py @@ -312,7 +312,7 @@ async def mock_receive(): # Verify JSON-RPC error format error_data = json.loads(response_body) assert error_data["jsonrpc"] == "2.0" - assert error_data["id"] == "server-error" + assert error_data["id"] is None assert error_data["error"]["code"] == INVALID_REQUEST assert error_data["error"]["message"] == "Session not found" diff --git a/tests/shared/test_session.py b/tests/shared/test_session.py index 2c220f737..d7c6cc3b5 100644 --- a/tests/shared/test_session.py +++ b/tests/shared/test_session.py @@ -7,14 +7,19 @@ from mcp.shared.exceptions import MCPError from mcp.shared.memory import create_client_server_memory_streams from mcp.shared.message import SessionMessage +from mcp.shared.session import RequestResponder from mcp.types import ( + PARSE_ERROR, CancelledNotification, CancelledNotificationParams, + ClientResult, EmptyResult, ErrorData, JSONRPCError, JSONRPCRequest, JSONRPCResponse, + ServerNotification, + ServerRequest, ) @@ -297,3 +302,117 @@ async def mock_server(): await ev_closed.wait() with anyio.fail_after(1): # pragma: no branch await ev_response.wait() + + +@pytest.mark.anyio +async def test_null_id_error_surfaced_via_message_handler(): + """Test that a JSONRPCError with id=None is surfaced to the message handler. + + Per JSON-RPC 2.0, error responses use id=null when the request id could not + be determined (e.g., parse errors). These cannot be correlated to any pending + request, so they are forwarded to the message handler as MCPError. + """ + ev_error_received = anyio.Event() + error_holder: list[MCPError] = [] + + async def capture_errors( + message: RequestResponder[ServerRequest, ClientResult] | ServerNotification | Exception, + ) -> None: + assert isinstance(message, MCPError) + error_holder.append(message) + ev_error_received.set() + + sent_error = ErrorData(code=PARSE_ERROR, message="Parse error") + + async with create_client_server_memory_streams() as (client_streams, server_streams): + client_read, client_write = client_streams + _server_read, server_write = server_streams + + async def mock_server(): + """Send a null-id error (simulating a parse error).""" + error_response = JSONRPCError(jsonrpc="2.0", id=None, error=sent_error) + await server_write.send(SessionMessage(message=error_response)) + + async with ( + anyio.create_task_group() as tg, + ClientSession( + read_stream=client_read, + write_stream=client_write, + message_handler=capture_errors, + ) as _client_session, + ): + tg.start_soon(mock_server) + + with anyio.fail_after(2): # pragma: no branch + await ev_error_received.wait() + + assert len(error_holder) == 1 + assert error_holder[0].error == sent_error + + +@pytest.mark.anyio +async def test_null_id_error_does_not_affect_pending_request(): + """Test that a null-id error doesn't interfere with an in-flight request. + + When a null-id error arrives while a request is pending, the error should + go to the message handler and the pending request should still complete + normally with its own response. + """ + ev_error_received = anyio.Event() + ev_response_received = anyio.Event() + error_holder: list[MCPError] = [] + result_holder: list[EmptyResult] = [] + + async def capture_errors( + message: RequestResponder[ServerRequest, ClientResult] | ServerNotification | Exception, + ) -> None: + assert isinstance(message, MCPError) + error_holder.append(message) + ev_error_received.set() + + sent_error = ErrorData(code=PARSE_ERROR, message="Parse error") + + async with create_client_server_memory_streams() as (client_streams, server_streams): + client_read, client_write = client_streams + server_read, server_write = server_streams + + async def mock_server(): + """Read a request, inject a null-id error, then respond normally.""" + message = await server_read.receive() + assert isinstance(message, SessionMessage) + assert isinstance(message.message, JSONRPCRequest) + request_id = message.message.id + + # First, send a null-id error (should go to message handler) + await server_write.send(SessionMessage(message=JSONRPCError(jsonrpc="2.0", id=None, error=sent_error))) + + # Then, respond normally to the pending request + await server_write.send(SessionMessage(message=JSONRPCResponse(jsonrpc="2.0", id=request_id, result={}))) + + async def make_request(client_session: ClientSession): + result = await client_session.send_ping() + result_holder.append(result) + ev_response_received.set() + + async with ( + anyio.create_task_group() as tg, + ClientSession( + read_stream=client_read, + write_stream=client_write, + message_handler=capture_errors, + ) as client_session, + ): + tg.start_soon(mock_server) + tg.start_soon(make_request, client_session) + + with anyio.fail_after(2): # pragma: no branch + await ev_error_received.wait() + await ev_response_received.wait() + + # Null-id error reached the message handler + assert len(error_holder) == 1 + assert error_holder[0].error == sent_error + + # Pending request completed successfully + assert len(result_holder) == 1 + assert isinstance(result_holder[0], EmptyResult) diff --git a/tests/test_types.py b/tests/test_types.py index f424efdbf..51507a9e5 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -1,16 +1,20 @@ +import json from typing import Any import pytest from mcp.types import ( LATEST_PROTOCOL_VERSION, + PARSE_ERROR, ClientCapabilities, CreateMessageRequestParams, CreateMessageResult, CreateMessageResultWithTools, + ErrorData, Implementation, InitializeRequest, InitializeRequestParams, + JSONRPCError, JSONRPCRequest, ListToolsResult, SamplingCapability, @@ -360,3 +364,20 @@ def test_list_tools_result_preserves_json_schema_2020_12_fields(): assert tool.input_schema["$schema"] == "https://json-schema.org/draft/2020-12/schema" assert "$defs" in tool.input_schema assert tool.input_schema["additionalProperties"] is False + + +def test_jsonrpc_error_null_id_serialization_preserves_id(): + """Test that id: null is preserved in JSON output even with exclude_none=True. + + JSON-RPC 2.0 requires the id field to be present with value null for + parse errors, not absent entirely. + """ + error = JSONRPCError(jsonrpc="2.0", id=None, error=ErrorData(code=PARSE_ERROR, message="Parse error")) + serialized = error.model_dump(by_alias=True, exclude_none=True) + assert "id" in serialized + assert serialized["id"] is None + + json_str = error.model_dump_json(by_alias=True, exclude_none=True) + parsed = json.loads(json_str) + assert "id" in parsed + assert parsed["id"] is None