fix: allow null id in JSONRPCError per JSON-RPC 2.0 spec#2056
Open
fix: allow null id in JSONRPCError per JSON-RPC 2.0 spec#2056
Conversation
The JSON-RPC 2.0 specification requires error responses to use id: null when the request id could not be determined (e.g., parse errors, invalid requests). The SDK rejected null ids, forcing a non-compliant id="server-error" sentinel workaround. Changes: - JSONRPCError.id now accepts None (JSONRPCResponse.id unchanged) - Add model_serializer to preserve id: null under exclude_none=True - Replace id="server-error" sentinel with id=None in server transports - Add null-id guard in session layer to surface errors via message handler - Guard server-side message router against str(None) misrouting Github-Issue: #1821
maxisbey
force-pushed
the
fix/jsonrpc-error-null-id
branch
from
030a6c3 to
88bb121
Compare
Merge the nested if conditions in the server message router into a single condition so the false branch is naturally exercised by non-response messages. Remove isinstance guards in test callbacks since we control the input.
Kludex
requested changes
Feb 14, 2026
Comment on lines
+978
to
+982
| # 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) |
Comment on lines
+470
to
+473
| # 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 |
Member
There was a problem hiding this comment.
You shouldn't need to do this after. This information should be inferred by the type checker, why is it not?
Comment on lines
+470
to
+472
| # 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). |
Member
There was a problem hiding this comment.
Please drop verbose comments as well.
Suggested change
| # 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). |
Comment on lines
+81
to
+89
| @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 |
Member
There was a problem hiding this comment.
Is exclude_none applied recursively? I think the point is that we shouldn't be using it at the JSONRPC level.
I think we can avoid using model_serializer and all that.
Comment on lines
+369
to
+383
| 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 |
Member
There was a problem hiding this comment.
There's no need to test Pydantic. Pydantic already tests Pydantic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The JSON-RPC 2.0 specification states:
The SDK's
JSONRPCError.idwas typed asRequestId(int | str), rejectingnull. This forced a non-compliantid="server-error"sentinel workaround in the server transports. The TypeScript SDK and MCP schema both allow null ids — the Python SDK was the sole outlier.Changes
Type layer (
src/mcp/types/jsonrpc.py):JSONRPCError.idchanged fromRequestIdtoRequestId | None@model_serializerto ensureid: nullis preserved in JSON output even when serializing withexclude_none=True(which is used across 20+ call sites)JSONRPCResponse.idunchanged — successful responses always have a non-null idServer transports (
streamable_http.py,streamable_http_manager.py):id="server-error"sentinel withid=Nonestr(None)producing"None"Session layer (
shared/session.py):_handle_responsebefore response routing — surfaces null-id errors asMCPErrorvia the message handler instead of silently discarding themPartially addresses #1821
This fixes the
idfield problem described in #1821 (Python usedid="server-error"where the spec requiresnull). The error code divergence tracked in that issue is still pending spec clarification.AI Disclaimer