Commit b2032c3
committed
Preserve the Request ID in Invalid Request Error Responses
## Motivation and Context
Resolves #398.
`JsonRpcHandler` returned `"id": null` for JSON-RPC envelope errors
(wrong or missing `jsonrpc` version, non-string `method`) even when
the incoming payload carried a concrete, valid request id.
From the client's perspective the original request stayed pending forever,
since no response with the matching id ever arrived:
```
{"jsonrpc":"1.0","id":3,"method":"ping","params":{}}
=> {"jsonrpc":"2.0","id":null,"error":{"code":-32600,...}}
```
Per JSON-RPC 2.0 (Response object, `id`), the response id MUST be
the same as the request's id; null is reserved for requests whose id
could not be detected (e.g. Parse error). The envelope validation in
`process_request` runs after JSON parsing with the request hash in
hand, so the id is detectable - it was simply discarded by passing
the `:unknown_id` sentinel unconditionally.
The fix passes the request's id to `error_response` whenever
the request carries one. The existing guards keep every other case intact:
`error_response` already nils out ids that fail validation
(so a type-invalid or pattern-violating id still yields `"id": null`,
preserving the XSS-prevention policy on echoed ids), and the `:unknown_id`
sentinel is kept for requests without an id so an Invalid Request error
still produces a null-id response, matching the spec's own example.
Parse errors and non-object requests are unchanged (null is correct there),
and batch entries pick up the fix automatically since each entry goes through
`process_request`.
For reference, the TypeScript and Python SDKs currently respond with
`"id": null` in these cases as well, because both validate the whole
message against a schema (zod / pydantic) before extracting the id.
This change is driven by the JSON-RPC 2.0 requirement rather than
parity; the Ruby transport layer already echoes the request id where
recoverable (`invalid_request_response(request_id:)` in
`StreamableHTTPTransport`), and this closes the remaining gap in
`JsonRpcHandler`.
## How Has This Been Tested?
- `bundle exec rake` (tests, RuboCop, and conformance baseline all
green; 1064 runs, 0 failures)
- Reproduced the three payloads from #398 before and after the fix;
they now return `"id":3`, `"id":4`, and `"id":8` respectively
- New and updated unit tests covering: id preserved for wrong version,
missing `jsonrpc`, numeric `method`, and `rpc.`-prefixed `method`;
id preserved for an invalid entry inside a batch; null id kept for
requests without an id, with an explicit null id, and with an id
that fails validation
## Breaking Changes
None in the protocol sense - this aligns the error responses with
JSON-RPC 2.0. Clients that matched `"id": null` on these envelope
errors will now see the request's own id, which is what allows them
to correlate the error with the pending request.1 parent 9193745 commit b2032c3
2 files changed
Lines changed: 81 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
83 | 88 | | |
84 | 89 | | |
85 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
46 | 58 | | |
47 | 59 | | |
48 | 60 | | |
| |||
58 | 70 | | |
59 | 71 | | |
60 | 72 | | |
| 73 | + | |
61 | 74 | | |
62 | 75 | | |
63 | 76 | | |
| |||
68 | 81 | | |
69 | 82 | | |
70 | 83 | | |
| 84 | + | |
71 | 85 | | |
72 | 86 | | |
73 | 87 | | |
| |||
315 | 329 | | |
316 | 330 | | |
317 | 331 | | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
318 | 376 | | |
319 | 377 | | |
320 | 378 | | |
| |||
610 | 668 | | |
611 | 669 | | |
612 | 670 | | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
613 | 685 | | |
614 | 686 | | |
615 | 687 | | |
| |||
756 | 828 | | |
757 | 829 | | |
758 | 830 | | |
759 | | - | |
| 831 | + | |
760 | 832 | | |
761 | 833 | | |
762 | | - | |
| 834 | + | |
| 835 | + | |
763 | 836 | | |
764 | 837 | | |
765 | 838 | | |
| |||
0 commit comments