Statically typed UnexpectedErrorResponse payload + per-passthrough-type contract tests#1410
Open
zancas wants to merge 2 commits into
Open
Statically typed UnexpectedErrorResponse payload + per-passthrough-type contract tests#1410zancas wants to merge 2 commits into
zancas wants to merge 2 commits into
Conversation
…rror `RpcRequestError::UnexpectedErrorResponse` carried its payload as `Box<dyn std::error::Error + Send + Sync + 'static>`, but the box guarded flexibility nothing used: every `TryFrom<RpcError>` impl in the tree — the `impl_rpc_error_passthrough!` macro and all four hand-written impls — fixes `Error = RpcError`, so the boxed value was always concretely an `RpcError`. This commit pins that fact in the type system. The `ResponseToError` trait now bounds its associated type with `TryFrom<RpcError, Error = RpcError>`, and the variant becomes `UnexpectedErrorResponse(#[source] RpcError)`. The payload needs no allocation and no downcast: consumers can match it directly, while the existing `source()`-chain recovery walks keep working unchanged. Unboxing pushed `RpcRequestError` past clippy's `result_large_err` threshold. The weight was `RpcError.data: Option<JsonRpcError>`, a field nothing reads (only the two `RpcError` constructors touch it), so it is now `Option<Box<JsonRpcError>>` — a concrete, statically typed box. That shrinks every `RpcError` from roughly 136 to 40 bytes, which silences the new warning, retires the pre-existing `#[allow(clippy::result_large_err)]` on `rpc_error_code_map`, and clears the three long-standing large-`Err` clippy warnings in the e2e tests that traced back to `RpcError`'s size. Part of the #1404 follow-up series (#1406, #1408). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ugh type The fix for #1404 restored the `source()` link on `RpcRequestError::UnexpectedErrorResponse`, healing rejection attribution for every `impl_rpc_error_passthrough!` type at once — but only `SendTransactionError` had a regression test pinning the invariant. Issue #1406 asks for the contract to be enforced per passthrough type so a future reshaping of the variant cannot silently reintroduce the masking for the other methods. The single-type test in zaino-fetch is generalized into a plain generic helper, `assert_unexpected_error_response_exposes_rpc_error`, whose panic messages name the failing type. The new contract test invokes it for all ten passthrough types: the eight listed in `response.rs` plus `ZValidateAddressError` and `GetAddressDeltasError` from the submodule invocations (the issue's count of eight missed those two). The `impl_rpc_error_passthrough!` doc comment now directs contributors to add new passthrough types to the test. Deliberately severing the `#[source]` link was confirmed to turn the test red with a message naming the first affected type, so the test demonstrably catches the regression it guards against. Closes #1406. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 16, 2026
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
1 participant
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.
Closes #1406. Stacked on #1409 (which stacks on #1405) — the base branch is
fix/1408-severed-source-chains; retarget as the stack merges. Only the top two commits belong to this PR.Two commits, independently revertable:
Commit 1 —
refactor: statically type the UnexpectedErrorResponse payload as RpcErrorThe
Box<dyn Error>payload guarded flexibility nothing used: everyTryFrom<RpcError>impl in the tree (theimpl_rpc_error_passthrough!macro and all four hand-written impls) fixesError = RpcError, so the boxed value was always concretely anRpcError. TheResponseToErrortrait now pins that withTryFrom<RpcError, Error = RpcError>, and the variant becomesUnexpectedErrorResponse(#[source] RpcError)— no allocation, no downcast to match the payload, and the existingsource()-chain recovery walks are unchanged.Unboxing pushed
RpcRequestErrorpast clippy'sresult_large_errthreshold. The weight wasRpcError.data: Option<JsonRpcError>, a field nothing in the tree reads, nowOption<Box<JsonRpcError>>(a concrete, statically typed box). EveryRpcErrorshrinks from roughly 136 to 40 bytes, which silences the new warning, retires the pre-existing#[allow(clippy::result_large_err)]onrpc_error_code_map, and clears the three long-standing large-Errclippy warnings in the e2e tests that traced back toRpcError's size.Commit 2 —
test: enforce the UnexpectedErrorResponse contract for every passthrough typeThe #1404 fix healed rejection attribution for every passthrough type at once, but only
SendTransactionErrorhad a test pinning the invariant. The single-type test is generalized into a plain generic helper (fn-first, no macro) whose panic messages name the failing type, and the contract test invokes it for all ten passthrough types — the eight listed inresponse.rsplusZValidateAddressErrorandGetAddressDeltasErrorfrom the submodule invocations, which the issue's count of eight missed. Theimpl_rpc_error_passthrough!doc comment directs contributors to add new passthrough types to the test.Deliberately severing the
#[source]link turns the test red with a message naming the first affected type, so it demonstrably catches the regression it guards against.Verification
Each commit compiles and passes on its own. At the branch tip:
cargo check --workspace --all-targetsclean,cargo clippy --workspace --all-targetswith zeroresult_large_errwarnings anywhere,cargo fmt --all --checkclean, andcargo nextest runon the production set green (370/370).🤖 Generated with Claude Code