fix: correlate invalid JSON-RPC envelope errors with the original request id - #2852
Conversation
Sorry, something went wrong.
|
For reviewers — summary to ease review: Bug: when an incoming message is valid JSON but fails JSON-RPC envelope validation (e.g. "jsonrpc":"1.0", missing jsonrpc, non-string method), the error response couldn't be correlated to the request — stdio never answered at all; Streamable HTTP replied with a null id. (Issue #2848 has 3 repro cases.) Fix: a shared extract_raw_request_id() helper pulls the id best-effort from the raw payload (only spec-valid id types — str / non-bool int), preserved in the error response on both transports, with null-id fallback per JSON-RPC 2.0 when no valid id exists. Two deliberate calls worth a look:
Both repro cases covered on both transports + null-id fallbacks. Full suite (1785), ruff, pyright all pass. If you'd rather land just the id-correlation core and split the stdio parse-error / error-code changes, I'm glad to narrow it. |
Sorry, something went wrong.
…uest id When an incoming message is valid JSON but fails JSON-RPC envelope validation, the error response previously could not be correlated with the originating request: the stdio server transport dropped the message with no response at all, and the Streamable HTTP transport replied with a null id. Extract the request id best-effort from the raw parsed payload and preserve it in the error response on both transports, falling back to a null id (per the JSON-RPC 2.0 spec) when no valid id can be extracted. The stdio transport now also answers unparseable lines with a Parse error (-32700, null id), and both transports report envelope-invalid messages as Invalid Request (-32600) instead of Invalid params (-32602), matching the JSON-RPC 2.0 error code semantics.
|
Rebased onto 57394b0 — the branch had drifted into conflicts. Same six files, no substantive change to the diff. On the needs confirmation label: all three envelope-invalid inputs from #2848 still behave as reported on current main. Running this PR's tests against unpatched 57394b0:
Same numbers @Parker-Fawcett got on the issue. One thing worth deciding before this lands. This PR covers stdio and the 2025-era streamable HTTP entry; the 2026-07-28 entry still answers an envelope-invalid body with _INVALID_BODY, i.e. id: null. I left it alone deliberately: that shape is currently asserted as SDK-defined in test_handle_modern_request_rejects_a_body_that_is_neither_request_nor_notification, and correlating there flips two of its params — posted-response and posted-error both carry a valid id, and echoing an id back on a body that isn't a request looks like the wrong answer. Restricting correlation to request-shaped bodies is a semantics call, not a bug fix, so I'd rather you make it: happy to extend this PR, or leave it as a follow-up. Local on f52691d: 5748 passed / 9 skipped / 1 xfailed, coverage report 100.00%, ruff check and ruff format --check clean, pyright 0 errors. |
Sorry, something went wrong.
Fixes #2848
When an incoming message is valid JSON but fails JSON-RPC envelope validation (e.g. "jsonrpc": "1.0", a missing jsonrpc field, or a non-string method), the error response could not be correlated with the originating request:
This PR extracts the request id best-effort from the raw parsed payload (shared helper extract_raw_request_id; only spec-valid id types are accepted — strings and non-bool integers) and preserves it in the error response on both transports, falling back to a null id per the JSON-RPC 2.0 spec when no valid id can be extracted. The stdio transport now also answers unparseable lines with -32700 Parse error (null id), matching the HTTP transport's existing parse-error behavior. Envelope-invalid messages are now reported as -32600 Invalid Request instead of -32602 Invalid params, matching JSON-RPC 2.0 error code semantics (the issue's three repro cases are envelope violations, not parameter errors).
Tests cover all three repro cases from the issue on both transports, plus the null-id fallback (unextractable id and unparseable body). Full suite, ruff, and pyright pass.