diff --git a/packages/zaino-fetch/src/jsonrpsee/connector.rs b/packages/zaino-fetch/src/jsonrpsee/connector.rs index 5c2c4e8d5..5951be357 100644 --- a/packages/zaino-fetch/src/jsonrpsee/connector.rs +++ b/packages/zaino-fetch/src/jsonrpsee/connector.rs @@ -78,7 +78,12 @@ pub struct RpcError { /// Error Message. pub message: String, /// Error Data. - pub data: Option, + /// + /// Boxed to keep `RpcError` small: it rides in the `Err` variant of every + /// RPC-call `Result` (see [`RpcRequestError::UnexpectedErrorResponse`] and + /// the `TryFrom` impls), where a large payload bloats each such + /// `Result` even on the success path. + pub data: Option>, } impl RpcError { @@ -148,8 +153,13 @@ enum AuthMethod { /// Trait to convert a JSON-RPC response to an error. pub trait ResponseToError: Sized { /// The error type. - type RpcError: std::fmt::Debug - + TryFrom; + /// + /// The conversion error is pinned to [`RpcError`] (rather than any + /// `std::error::Error`) so an unmapped server rejection stays a statically + /// typed [`RpcError`] all the way into + /// [`RpcRequestError::UnexpectedErrorResponse`] — no boxing, and consumers + /// can match the payload directly instead of downcasting. + type RpcError: std::fmt::Debug + TryFrom; /// Converts a JSON-RPC response to an error. fn to_error(self) -> Result { @@ -190,13 +200,15 @@ pub enum RpcRequestError { /// Zaino has not yet accounted for the possibilty of this error, /// or the Node returned an undocumented/malformed error response. /// - /// The payload is `#[source]` so the node's typed rejection (usually an - /// [`RpcError`]) stays reachable through `source()` chains: the serving + /// The payload is the node's rejection as a statically typed + /// [`RpcError`] (guaranteed by [`ResponseToError`]'s + /// `TryFrom` bound), and it is `#[source]` so + /// the rejection stays reachable through `source()` chains: the serving /// surfaces recover rejection codes and messages by downcast-walking those /// chains, and severing the link here masks every unmapped node rejection /// as a generic internal error (zingolabs/zaino#1404). #[error("unexpected error response from server: {0}")] - UnexpectedErrorResponse(#[source] Box), + UnexpectedErrorResponse(#[source] RpcError), } /// JsonRpSee Client config data. @@ -424,9 +436,8 @@ impl JsonRpSeeConnector { match response.error { Some(error) => Err(RpcRequestError::Method( - R::RpcError::try_from(error).map_err(|e| { - RpcRequestError::UnexpectedErrorResponse(Box::new(e)) - })?, + R::RpcError::try_from(error) + .map_err(RpcRequestError::UnexpectedErrorResponse)?, )), None => { let result: R = @@ -1081,37 +1092,72 @@ pub async fn test_node_and_return_url( mod tests { use super::*; - /// zaino-serve attributes node rejections by downcast-walking `source()` - /// chains for the typed [`RpcError`] (see - /// `sendrawtransaction_error_object_from_indexer_error` in - /// `zaino-serve/src/rpc/jsonrpc/service.rs`). `UnexpectedErrorResponse` is - /// the variant that carries a node's structured rejection whenever no - /// method-specific mapping exists (every `impl_rpc_error_passthrough!` - /// type, including `sendrawtransaction`'s), so its boxed payload must stay - /// reachable through `source()`. Without the link, every such rejection is - /// masked as a generic internal error (zingolabs/zaino#1404). - #[test] - fn unexpected_error_response_exposes_rpc_error_via_source() { - use crate::jsonrpsee::response::SendTransactionError; - + /// Asserts the `UnexpectedErrorResponse` source-reachability contract for + /// one `MethodError` instantiation: the node rejection must be exposed via + /// `source()` and downcast to the typed [`RpcError`] with its legacy code + /// intact. + fn assert_unexpected_error_response_exposes_rpc_error() + where + MethodError: std::error::Error + 'static, + { + let type_name = std::any::type_name::(); let rejection = RpcError::new_from_legacycode( zebra_rpc::server::error::LegacyCode::Verify, "failed to validate tx: transparent input not found", ); - let error: RpcRequestError = - RpcRequestError::UnexpectedErrorResponse(Box::new(rejection)); - - let source = std::error::Error::source(&error) - .expect("the boxed node rejection must be exposed via source()"); - let rpc_error = source - .downcast_ref::() - .expect("the source must downcast to the typed RpcError"); + let error: RpcRequestError = + RpcRequestError::UnexpectedErrorResponse(rejection); + + let source = std::error::Error::source(&error).unwrap_or_else(|| { + panic!( + "RpcRequestError::<{type_name}>::UnexpectedErrorResponse \ + must expose the node rejection via source()" + ) + }); + let rpc_error = source.downcast_ref::().unwrap_or_else(|| { + panic!( + "the source for RpcRequestError::<{type_name}> must downcast to the typed RpcError" + ) + }); assert_eq!( rpc_error.code, - zebra_rpc::server::error::LegacyCode::Verify as i64 + zebra_rpc::server::error::LegacyCode::Verify as i64, + "the legacy error code must survive for RpcRequestError::<{type_name}>" ); } + /// zaino-serve attributes node rejections by downcast-walking `source()` + /// chains for the typed [`RpcError`] (see + /// `sendrawtransaction_error_object_from_indexer_error` in + /// `zaino-serve/src/rpc/jsonrpc/service.rs`). `UnexpectedErrorResponse` is + /// the variant that carries a node's structured rejection whenever no + /// method-specific mapping exists — every `impl_rpc_error_passthrough!` + /// type — so its boxed payload must stay reachable through `source()` for + /// each of them. Without the link, every such rejection is masked as a + /// generic internal error (zingolabs/zaino#1404, zingolabs/zaino#1406). + /// + /// When a type is added to an `impl_rpc_error_passthrough!` invocation, it + /// belongs in this list too. + #[test] + fn unexpected_error_response_exposes_rpc_error_for_every_passthrough_type() { + use crate::jsonrpsee::response::{ + address_deltas::GetAddressDeltasError, z_validate_address::ZValidateAddressError, + ChainWorkError, GetBalanceError, GetSubtreesError, GetTreestateError, GetUtxosError, + SendTransactionError, TxidsError, + }; + + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + } + /// `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 diff --git a/packages/zaino-fetch/src/jsonrpsee/response.rs b/packages/zaino-fetch/src/jsonrpsee/response.rs index d88722ec4..4b2d9305d 100644 --- a/packages/zaino-fetch/src/jsonrpsee/response.rs +++ b/packages/zaino-fetch/src/jsonrpsee/response.rs @@ -36,6 +36,11 @@ use super::connector::RpcError; /// Implements the pass-through `TryFrom` for RPC-specific error /// types that map no JSON-RPC error codes to variants of their own: every /// server-reported error surfaces unchanged as the generic [`RpcError`]. +/// +/// Every type listed in an invocation also belongs in the contract test +/// `unexpected_error_response_exposes_rpc_error_for_every_passthrough_type` +/// in `connector.rs`, which enforces that its rejections stay attributable +/// via `source()` chains (zingolabs/zaino#1406). // TODO: attempt to convert RpcError into errors specific to each RPC // response, removing types from these invocations as mappings become known. macro_rules! impl_rpc_error_passthrough { @@ -88,10 +93,6 @@ impl_infallible_response_to_error!( /// Maps a JSON-RPC error carrying `code` to the RPC-specific variant `ctor` /// builds from its message, passing every other error through unchanged. -// The Err type is fixed by the `TryFrom` impls this serves: their -// `Error = RpcError` (unboxed) is the trait contract, so the large-Err lint -// cannot be satisfied here without changing every impl's signature. -#[allow(clippy::result_large_err)] pub(crate) fn rpc_error_code_map( value: RpcError, code: i64, diff --git a/packages/zaino-state/src/chain_index/source/mockchain_source.rs b/packages/zaino-state/src/chain_index/source/mockchain_source.rs index 76045b9ad..c013def2e 100644 --- a/packages/zaino-state/src/chain_index/source/mockchain_source.rs +++ b/packages/zaino-state/src/chain_index/source/mockchain_source.rs @@ -815,12 +815,12 @@ impl BlockchainSource for MockchainSource { Err(BlockchainSourceError::unrecoverable( zaino_fetch::jsonrpsee::connector::RpcRequestError::< zaino_fetch::jsonrpsee::response::SendTransactionError, - >::UnexpectedErrorResponse(Box::new( + >::UnexpectedErrorResponse( zaino_fetch::jsonrpsee::connector::RpcError::new_from_legacycode( zebra_rpc::server::error::LegacyCode::Verify, "MockchainSource rejects all transaction submissions (static chain, no mempool)", ), - )), + ), )) } diff --git a/packages/zaino-state/src/error.rs b/packages/zaino-state/src/error.rs index ce14008a6..a3d877c0c 100644 --- a/packages/zaino-state/src/error.rs +++ b/packages/zaino-state/src/error.rs @@ -724,7 +724,7 @@ mod tonic_status_from_service_error { "failed to validate tx: transparent input not found", ); let request_error: RpcRequestError = - RpcRequestError::UnexpectedErrorResponse(Box::new(rejection)); + RpcRequestError::UnexpectedErrorResponse(rejection); let source_error = BlockchainSourceError::unrecoverable(request_error); let index_error = ChainIndexError::backing_validator(source_error);