diff --git a/src/mcp/client/sse.py b/src/mcp/client/sse.py index 8f8e4dadc..7c309ecb5 100644 --- a/src/mcp/client/sse.py +++ b/src/mcp/client/sse.py @@ -138,7 +138,7 @@ async def post_writer(endpoint_url: str): json=session_message.message.model_dump( by_alias=True, mode="json", - exclude_none=True, + exclude_unset=True, ), ) response.raise_for_status() diff --git a/src/mcp/client/stdio.py b/src/mcp/client/stdio.py index 605c5ea24..5b8209eeb 100644 --- a/src/mcp/client/stdio.py +++ b/src/mcp/client/stdio.py @@ -167,7 +167,7 @@ async def stdin_writer(): try: async with write_stream_reader: async for session_message in write_stream_reader: - json = session_message.message.model_dump_json(by_alias=True, exclude_none=True) + json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True) await process.stdin.send( (json + "\n").encode( encoding=server.encoding, diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index 9d45bec6e..bd77b3e2f 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -259,7 +259,7 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: async with ctx.client.stream( "POST", self.url, - json=message.model_dump(by_alias=True, mode="json", exclude_none=True), + json=message.model_dump(by_alias=True, mode="json", exclude_unset=True), headers=headers, ) as response: if response.status_code == 202: diff --git a/src/mcp/client/websocket.py b/src/mcp/client/websocket.py index cf4b86e99..bda199f36 100644 --- a/src/mcp/client/websocket.py +++ b/src/mcp/client/websocket.py @@ -65,7 +65,7 @@ async def ws_writer(): async with write_stream_reader: async for session_message in write_stream_reader: # Convert to a dict, then to JSON - msg_dict = session_message.message.model_dump(by_alias=True, mode="json", exclude_none=True) + msg_dict = session_message.message.model_dump(by_alias=True, mode="json", exclude_unset=True) await ws.send(json.dumps(msg_dict)) async with anyio.create_task_group() as tg: diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 04404a3fc..9ca5ac4fc 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -490,7 +490,7 @@ async def _handle_request( except Exception as err: if raise_exceptions: # pragma: no cover raise err - response = types.ErrorData(code=0, message=str(err), data=None) + response = types.ErrorData(code=0, message=str(err)) await message.respond(response) else: # pragma: no cover diff --git a/src/mcp/server/sse.py b/src/mcp/server/sse.py index 5be6b78ca..674294c5c 100644 --- a/src/mcp/server/sse.py +++ b/src/mcp/server/sse.py @@ -170,7 +170,7 @@ async def sse_writer(): await sse_stream_writer.send( { "event": "message", - "data": session_message.message.model_dump_json(by_alias=True, exclude_none=True), + "data": session_message.message.model_dump_json(by_alias=True, exclude_unset=True), } ) diff --git a/src/mcp/server/stdio.py b/src/mcp/server/stdio.py index 7f3aa2ac2..864d387bd 100644 --- a/src/mcp/server/stdio.py +++ b/src/mcp/server/stdio.py @@ -71,7 +71,7 @@ async def stdout_writer(): try: async with write_stream_reader: async for session_message in write_stream_reader: - json = session_message.message.model_dump_json(by_alias=True, exclude_none=True) + json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True) await stdout.write(json + "\n") await stdout.flush() except anyio.ClosedResourceError: # pragma: no cover diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py index 54ac7374a..a8202e385 100644 --- a/src/mcp/server/streamable_http.py +++ b/src/mcp/server/streamable_http.py @@ -298,12 +298,12 @@ 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), ) return Response( - error_response.model_dump_json(by_alias=True, exclude_none=True), + error_response.model_dump_json(by_alias=True, exclude_unset=True), status_code=status_code, headers=response_headers, ) @@ -323,7 +323,7 @@ def _create_json_response( response_headers[MCP_SESSION_ID_HEADER] = self.mcp_session_id return Response( - response_message.model_dump_json(by_alias=True, exclude_none=True) if response_message else None, + response_message.model_dump_json(by_alias=True, exclude_unset=True) if response_message else None, status_code=status_code, headers=response_headers, ) @@ -336,7 +336,7 @@ def _create_event_data(self, event_message: EventMessage) -> dict[str, str]: """Create event data dictionary from an EventMessage.""" event_data = { "event": "message", - "data": event_message.message.model_dump_json(by_alias=True, exclude_none=True), + "data": event_message.message.model_dump_json(by_alias=True, exclude_unset=True), } # If an event ID was provided, include it @@ -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..9ffabf109 100644 --- a/src/mcp/server/streamable_http_manager.py +++ b/src/mcp/server/streamable_http_manager.py @@ -244,11 +244,11 @@ 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( - content=error_response.model_dump_json(by_alias=True, exclude_none=True), + content=error_response.model_dump_json(by_alias=True, exclude_unset=True), status_code=HTTPStatus.NOT_FOUND, media_type="application/json", ) diff --git a/src/mcp/server/websocket.py b/src/mcp/server/websocket.py index a4c844811..7b00f7905 100644 --- a/src/mcp/server/websocket.py +++ b/src/mcp/server/websocket.py @@ -47,7 +47,7 @@ async def ws_writer(): try: async with write_stream_reader: async for session_message in write_stream_reader: - obj = session_message.message.model_dump_json(by_alias=True, exclude_none=True) + obj = session_message.message.model_dump_json(by_alias=True, exclude_unset=True) await websocket.send_text(obj) except anyio.ClosedResourceError: await websocket.close() diff --git a/src/mcp/shared/exceptions.py b/src/mcp/shared/exceptions.py index 7a2b2ded4..6c3a7745c 100644 --- a/src/mcp/shared/exceptions.py +++ b/src/mcp/shared/exceptions.py @@ -12,7 +12,10 @@ class MCPError(Exception): def __init__(self, code: int, message: str, data: Any = None): super().__init__(code, message, data) - self.error = ErrorData(code=code, message=message, data=data) + if data is not None: + self.error = ErrorData(code=code, message=message, data=data) + else: + self.error = ErrorData(code=code, message=message) @property def code(self) -> int: diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 453e36274..5ee8f3baa 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -142,7 +142,7 @@ async def cancel(self) -> None: # Send an error response to indicate cancellation await self._session._send_response( # type: ignore[reportPrivateUsage] request_id=self.request_id, - response=ErrorData(code=0, message="Request cancelled", data=None), + response=ErrorData(code=0, message="Request cancelled"), ) @property @@ -458,6 +458,12 @@ async def _handle_response(self, message: SessionMessage) -> None: if not isinstance(message.message, JSONRPCResponse | JSONRPCError): return # pragma: no cover + if message.message.id is None: + # Narrows to JSONRPCError since JSONRPCResponse.id is always RequestId + 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 # 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..84304a37c 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -75,7 +75,7 @@ class JSONRPCError(BaseModel): """A response to a request that indicates an error occurred.""" jsonrpc: Literal["2.0"] - id: RequestId + id: RequestId | None error: ErrorData 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)