Skip to content

fix: recover typed node rejections on every JSON-RPC method#1411

Open
zancas wants to merge 1 commit into
test/1406-passthrough-contract-testsfrom
fix/1407-jsonrpc-rejection-recovery
Open

fix: recover typed node rejections on every JSON-RPC method#1411
zancas wants to merge 1 commit into
test/1406-passthrough-contract-testsfrom
fix/1407-jsonrpc-rejection-recovery

Conversation

@zancas

@zancas zancas commented Jul 16, 2026

Copy link
Copy Markdown
Member

Closes #1407. Top of the #1404 follow-up stack — the base branch is test/1406-passthrough-contract-tests (PR #1410, which stacks on #1409#1405); retarget as the stack merges. Only the top commit belongs to this PR.

Problem

Only two of the 28 JSON-RPC methods recovered the node's typed rejection from the indexer error's source() chain: sendrawtransaction walked for the typed RpcError, and getblock recognized a single hard-coded transport case (HTTP 500 → "block not found"). The other 26 mapped every error through invalid_params_error_object, so a node rejection of, say, getrawtransaction or z_gettreestate reached the client as a generic invalid-params object — wrong code, no legacy error code, reason buried in a data string. This is the JSON-RPC face of the masking family from #1404, and an in-code comment acknowledged it: "Currently all errors are hidden from downstream client, a full fix should be implemented."

Fix

  • error_object_from_source_chain (the shared walk skeleton) now takes its fallback as a parameter; internal_error_object is extracted alongside invalid_params_error_object.
  • A new general translator, error_object_from_indexer_error, recovers the typed RpcError when the chain carries one and forwards its code and message. Otherwise it falls back to the exact invalid-params object the plain-delegation methods returned before — non-rejection errors keep their wire shape, so behavior changes only when a node rejection is present.
  • All 26 plain-delegation sites now use the general translator. getblock keeps its zcashd-compatible "block not found" arm ahead of the generic recovery rather than instead of it. sendrawtransaction is behaviorally unchanged.
  • The stale "temporary fix" comment is replaced with a description of the implemented design.

Tests (translator tests written first; the meaningful case confirmed red)

Before the fix, getblock_error_object_from_indexer_error masked a legacy-code -25 rejection as a -32603 internal error — that test was confirmed red, then green. Six tests now cover the three translators (recovery plus fallback pinning for each), built on a small nestable Wrap fixture: the translators consume only source() chains, so a minimal typed wrapper reproduces exactly what they see from production error types.

Verification

cargo fmt --all --check clean, cargo clippy --workspace --all-targets clean, and cargo nextest run on the production set green — 376/376, the prior 370 plus the six new translator tests.

🤖 Generated with Claude Code

Only two of the 28 JSON-RPC methods recovered the node's typed
rejection from the indexer error's `source()` chain: `sendrawtransaction`
walked for the typed `RpcError`, and `getblock` recognized a single
hard-coded transport case. The other 26 mapped every error through
`invalid_params_error_object`, so a node rejection reached the client
as a generic invalid-params object with the wrong code and no legacy
error code — the JSON-RPC face of the masking family in
#1404. An in-code comment acknowledged the gap:
"Currently all errors are hidden from downstream client, a full fix
should be implemented."

This commit implements that full fix (#1407). The shared
walk skeleton, `error_object_from_source_chain`, now takes its fallback
as a parameter instead of hard-coding an internal-server-error object.
A new general translator, `error_object_from_indexer_error`, recovers
the typed `RpcError` when the chain carries one and forwards its code
and message; otherwise it falls back to the exact invalid-params object
the plain-delegation methods returned before, so non-rejection errors
keep their wire shape and behavior changes only when a node rejection
is present. All 26 plain-delegation sites now use it. `getblock` keeps
its zcashd-compatible "block not found" arm ahead of the generic
recovery rather than instead of it, and `sendrawtransaction` retains
its internal-error fallback unchanged.

Following the test-first order, the translator tests were written
before the fix and the meaningful case confirmed red: `getblock`
masked a legacy-code -25 rejection as a -32603 internal error. Six
tests now cover the three translators — recovery plus fallback pinning
for each — built on a small nestable wrapper fixture, since the
translators consume only `source()` chains. The stale "temporary fix"
comment is replaced with a description of the implemented design.

Closes #1407.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant