Skip to content

fix: surface node rejections instead of masking them as internal errors#1405

Open
zancas wants to merge 1 commit into
devfrom
fix/1404-send-rejection-masking
Open

fix: surface node rejections instead of masking them as internal errors#1405
zancas wants to merge 1 commit into
devfrom
fix/1404-send-rejection-masking

Conversation

@zancas

@zancas zancas commented Jul 16, 2026

Copy link
Copy Markdown
Member

Closes #1404.

The regression

zainod 0.6.0 reports every zebrad sendrawtransaction rejection to wallets as the fixed string Status { code: Internal, message: "InternalServerError: error receiving data from backing node" }. The 0.6.0-rc.1 baseline surfaced the node's actual rejection, for example RPC Error (code: -25): failed to validate tx: transparent input not found. Wallets therefore cannot distinguish "the node rejected this transaction, and here is why" from "the indexer's connection to the node broke".

Root cause

Two cooperating faults produced the masking, both introduced by the move_functionality_into_blockchainsource refactor between rc.1 and 0.6.0:

  1. A severed source() chain. RpcRequestError::UnexpectedErrorResponse carried the node's typed RpcError in a plain boxed field without #[source], so thiserror severed the error chain at that link. The downcast-walk recovery that zaino-serve's JSON-RPC surface performs (sendrawtransaction_error_object_from_indexer_error) could never find the rejection, for any of the eight impl_rpc_error_passthrough! RPC types.
  2. A wrapper-message-only gRPC conversion. From<NodeBackedIndexerServiceError> for tonic::Status used only ChainIndexError's own message, which ChainIndexError::backing_validator sets to the generic string "error receiving data from backing node" while preserving the real cause as its source.

The issue also asks about the four roughly one-second retries visible in the wallet trace: those are the connector's work-queue retry loop (max_attempts = 5, 500 ms sleep), which retries on "Work queue depth exceeded" in the response body. They are expected behavior, not part of the regression.

The fix

  • UnexpectedErrorResponse's payload is now #[source], healing the source() chain for all eight passthrough RPC types on every serving surface.
  • The tonic::Status conversion now walks the source chain for a typed RpcError and, when one is present, reports it instead of the wrapper message. When none is present (a genuine transport failure), the wrapper message stands.

Tests (written first, confirmed red, now green)

  1. unexpected_error_response_exposes_rpc_error_via_source (zaino-fetch) pins that the variant exposes its payload via source().
  2. send_rejection_survives_conversion_to_tonic_status (zaino-state) drives the exact production error chain — RpcErrorRpcRequestErrorBlockchainSourceError::unrecoverableChainIndexError::backing_validatorNodeBackedIndexerServiceErrortonic::Status — and asserts the rejection reason and legacy code -25 survive.
  3. send_raw_transaction_node_rejection_stays_attributable (mockchain) exercises the path end to end through IndexReader::send_raw_transaction. To support it, MockchainSource::send_raw_transaction now returns a validator-shaped rejection instead of unimplemented!, standing in for a rejecting zebrad.

The existing invalid-hex regression test's source-chain walk is extracted into a shared helper both mockchain tests use.

Verified with cargo fmt --all --check, cargo clippy -p zaino-fetch -p zaino-state --all-targets, and cargo nextest run on the touched crates (all green).

🤖 Generated with Claude Code

zainod 0.6.0 reported every zebrad `sendrawtransaction` rejection to
wallets as the fixed string "InternalServerError: error receiving data
from backing node", where the 0.6.0-rc.1 baseline surfaced the node's
actual rejection (for example "RPC Error (code: -25): failed to
validate tx: ..."). Issue #1404 reports the regression.

Two cooperating faults produced the masking. First,
`RpcRequestError::UnexpectedErrorResponse` carried the node's typed
`RpcError` in a plain boxed field without `#[source]`, so thiserror
severed the `source()` chain at that link and the downcast-walk
recovery that zaino-serve's JSON-RPC surface performs could never find
the rejection. Second, the gRPC surface's `From<NodeBackedIndexerServiceError>
for tonic::Status` used only `ChainIndexError`'s own message, which
`ChainIndexError::backing_validator` sets to a generic wrapper string.

This commit restores attribution at both severing points. The
`UnexpectedErrorResponse` payload is now `#[source]`, which heals the
chain for all eight `impl_rpc_error_passthrough!` RPC types on every
serving surface. The tonic::Status conversion now walks the source
chain for a typed `RpcError` and, when one is present, reports it
instead of the wrapper message.

Following the test-first order for a tracked bug, three regression
tests were written and confirmed red before the fix: a unit test
pinning that `UnexpectedErrorResponse` exposes its payload via
`source()`, a unit test driving the exact production error chain
(RpcError through BlockchainSourceError and ChainIndexError into
tonic::Status) and asserting the rejection reason and legacy code -25
survive, and an end-to-end mockchain test through
`IndexReader::send_raw_transaction`. To support the last,
`MockchainSource::send_raw_transaction` now returns a validator-shaped
rejection instead of `unimplemented!`, standing in for a rejecting
zebrad. The existing invalid-hex regression test's source-chain walk is
extracted into a shared helper both tests use.

Closes #1404.

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.

zainod 0.6.0 masks zebrad sendrawtransaction rejections as Internal "error receiving data from backing node"

1 participant