From f14c0510dbcdc93f20e34002aef9890ef99dae4c Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 16 Jul 2026 09:23:33 -0700 Subject: [PATCH] fix: expose wrapped errors via source() in Method and StatusError variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue zingolabs/zaino#1404 was caused in part by an error-enum variant that carried its cause in a plain field without `#[source]`, silently severing the `source()` chain that the serving surfaces downcast-walk to attribute node rejections. The fix for that issue healed the variant that bit (`RpcRequestError::UnexpectedErrorResponse`); this commit closes the remaining structural gaps the follow-up audit found (zingolabs/zaino#1408). `RpcRequestError::Method` now marks its payload `#[source]`, keeping the typed method-specific rejection reachable from `source()` walks. thiserror infers the required trait bounds per generated impl, so the generic parameter needs no explicit `std::error::Error` bound and no call site changes. Because a `#[source]` payload is a real error with a real message, the variant's display switches from `Debug` to `Display` formatting; nothing in the tree matched on the old format. The three `StatusError` wrapper variants — in `MempoolError`, `NonFinalisedStateError`, and `FinalisedStateError` — likewise gain `#[source]`; their display strings are unchanged. Following the test-first order, four regression tests were written and confirmed red before the fix: one in zaino-fetch pinning that `Method` exposes its payload via `source()`, and one per wrapper enum in a new `status_error_wrappers` test module in zaino-state. Closes zingolabs/zaino#1408. Co-Authored-By: Claude Fable 5 --- .../zaino-fetch/src/jsonrpsee/connector.rs | 28 +++++++++++- packages/zaino-state/src/error.rs | 45 +++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/packages/zaino-fetch/src/jsonrpsee/connector.rs b/packages/zaino-fetch/src/jsonrpsee/connector.rs index 98fa8ac8b..5c2c4e8d5 100644 --- a/packages/zaino-fetch/src/jsonrpsee/connector.rs +++ b/packages/zaino-fetch/src/jsonrpsee/connector.rs @@ -165,8 +165,13 @@ pub enum RpcRequestError { Transport(#[from] TransportError), /// Error variant for errors related to the JSON-RPC method being called. - #[error("Method error: {0:?}")] - Method(MethodError), + /// + /// The payload is `#[source]` for the same reason as + /// [`RpcRequestError::UnexpectedErrorResponse`]'s: the typed rejection + /// must stay reachable through `source()` chains for the serving + /// surfaces' downcast walks (zingolabs/zaino#1408). + #[error("Method error: {0}")] + Method(#[source] MethodError), /// The provided input failed to serialize. #[error("request input failed to serialize: {0:?}")] @@ -1107,6 +1112,25 @@ mod tests { ); } + /// `Method` carries the typed method-specific rejection, so — like + /// `UnexpectedErrorResponse` — its payload must stay reachable through + /// `source()` for the serving surfaces' downcast walks to attribute the + /// rejection (zingolabs/zaino#1408). + #[test] + fn method_exposes_method_error_via_source() { + use crate::jsonrpsee::response::SendTransactionError; + + let error: RpcRequestError = + RpcRequestError::Method(SendTransactionError::MissingInputs); + + let source = std::error::Error::source(&error) + .expect("the method error must be exposed via source()"); + assert!( + source.downcast_ref::().is_some(), + "the source must downcast to the typed method error" + ); + } + #[test] fn test_resolve_address_wraps_common_function() { // Verify the wrapper correctly converts io::Error to TransportError diff --git a/packages/zaino-state/src/error.rs b/packages/zaino-state/src/error.rs index ca4b06490..ce14008a6 100644 --- a/packages/zaino-state/src/error.rs +++ b/packages/zaino-state/src/error.rs @@ -269,7 +269,7 @@ pub enum MempoolError { /// Unexpected status-related error. #[error("Status error: {0:?}")] - StatusError(StatusError), + StatusError(#[source] StatusError), } /// Errors related to the `BlockCache`. @@ -337,7 +337,7 @@ pub enum NonFinalisedStateError { /// Unexpected status-related error. #[error("Status error: {0:?}")] - StatusError(StatusError), + StatusError(#[source] StatusError), } /// These aren't the best conversions, but the NonFinalizedStateError should go away @@ -427,7 +427,7 @@ pub enum FinalisedStateError { /// Unexpected status-related error. #[error("Status error: {0:?}")] - StatusError(StatusError), + StatusError(#[source] StatusError), /// Error from JsonRpcConnector. // TODO: Remove when FinalisedState replaces legacy finalised state. @@ -663,6 +663,45 @@ impl From for ChainIndexError { } } +#[cfg(test)] +mod status_error_wrappers { + use super::*; + + fn status_error() -> StatusError { + StatusError { + server_status: crate::status::StatusType::CriticalError, + } + } + + /// Asserts `error` exposes a [`StatusError`] via `source()`, the link the + /// serving surfaces' downcast walks need for attribution + /// (zingolabs/zaino#1408). + fn assert_status_error_reachable(error: &(dyn std::error::Error + 'static)) { + let source = error + .source() + .expect("the wrapped StatusError must be exposed via source()"); + assert!( + source.downcast_ref::().is_some(), + "the source must downcast to the typed StatusError" + ); + } + + #[test] + fn mempool_error_exposes_status_error_via_source() { + assert_status_error_reachable(&MempoolError::StatusError(status_error())); + } + + #[test] + fn non_finalised_state_error_exposes_status_error_via_source() { + assert_status_error_reachable(&NonFinalisedStateError::StatusError(status_error())); + } + + #[test] + fn finalised_state_error_exposes_status_error_via_source() { + assert_status_error_reachable(&FinalisedStateError::StatusError(status_error())); + } +} + #[cfg(test)] mod tonic_status_from_service_error { use super::*;