fix: accept spec-valid null/string JSON-RPC response ids - #159
Open
sainathr19 wants to merge 2 commits into
Open
fix: accept spec-valid null/string JSON-RPC response ids#159sainathr19 wants to merge 2 commits into
sainathr19 wants to merge 2 commits into
Conversation
JsonRpcResponse typed the response id as u64, but JSON-RPC 2.0 allows an id to be a string, number, or null (servers send null for errors raised before the id is known). An error response with "id": null failed to deserialize the untagged JsonRpcResponse, so the server's error message was discarded and callers only saw "data did not match any variant of untagged enum JsonRpcResponse". Introduce a JsonRpcId enum (Number | String) and type the field as Option<JsonRpcId> (null -> None), with a JsonRpcId::as_u64 helper. The HTTP batch transport and the WebSocket transport both correlate responses to requests by numeric id; each now recovers the number via as_u64 and treats a non-numeric id as unmatched. Adds regression tests. Fixes software-mansion#158
sainathr19
force-pushed
the
fix/jsonrpc-response-nullable-id
branch
from
August 22, 2026 04:26
cb66888 to
7ea3606
Compare
Contributor
|
Hey @sainathr19 👋 , did you have a chance to see my comment? |
franciszekjob
left a comment
franciszekjob
left a comment
Contributor
There was a problem hiding this comment.
@sainathr19 thanks for reply, pls take a look at the comment.
Also, let's update changelog.
…-numeric batch id Replace the JsonRpcId enum with Option<u64> deserialized via a small helper (Value::as_u64), so a null or non-numeric response id maps to None instead of adding a public type. In the batch transport, a non-numeric id now returns a dedicated NonNumericResponseId error rather than being routed through the range check with a fabricated id. Update the changelog.
Contributor
Author
|
Done, pushed. Switched to |
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.
Fixes #158.
Problem
JsonRpcResponsetyped the responseidasu64, but JSON-RPC 2.0 allows an id to be astring, a number, or null (a server sends
nullwhen it can't determine the request id,e.g. parse / server errors). An error response with
"id": nullfailed to deserialize the#[serde(untagged)]enum, so the server'serror.messagewas discarded and callers only saw:Change
JsonRpcIdenum (Number(u64) | String(String)) and type bothJsonRpcResponsevariants'
idasOption<JsonRpcId>(null → None).JsonRpcId::Numberand treats a non-numeric id as out of range (routed through the existingUnexpectedResponseIdcheck). No new error variant.Note: this changes the public
idfield type fromu64toOption<JsonRpcId>.Verification
cargo build -p starknet-rust-providers✓cargo test -p starknet-rust-providers --lib→ 39 passed, 0 failed (incl. 2 new tests) ✓cargo fmt --check✓ ·cargo clippy✓