diff --git a/crates/sui-json-rpc/src/error.rs b/crates/sui-json-rpc/src/error.rs index ff9691959208a..18f053aee0ee1 100644 --- a/crates/sui-json-rpc/src/error.rs +++ b/crates/sui-json-rpc/src/error.rs @@ -4,16 +4,18 @@ use fastcrypto::error::FastCryptoError; use hyper::header::InvalidHeaderValue; use jsonrpsee::core::Error as RpcError; -use jsonrpsee::types::error::CallError; +use jsonrpsee::types::error::{CallError, INTERNAL_ERROR_CODE}; use jsonrpsee::types::ErrorObject; +use std::collections::BTreeMap; use sui_types::error::{SuiError, SuiObjectResponseError, UserInputError}; -use sui_types::quorum_driver_types::{QuorumDriverError, NON_RECOVERABLE_ERROR_MSG}; +use sui_types::quorum_driver_types::QuorumDriverError; use thiserror::Error; use tokio::task::JoinError; use crate::authority_state::StateReadError; pub const TRANSIENT_ERROR_CODE: i32 = -32001; +pub const TRANSACTION_EXECUTION_CLIENT_ERROR_CODE: i32 = -32002; pub type RpcInterimResult = Result; @@ -62,12 +64,6 @@ pub enum Error { StateReadError(#[from] StateReadError), } -impl From for RpcError { - fn from(e: Error) -> Self { - e.to_rpc_error() - } -} - impl From for Error { fn from(e: SuiError) -> Self { match e { @@ -78,11 +74,10 @@ impl From for Error { } } -impl Error { - /// `InvalidParams`/`INVALID_PARAMS_CODE` for client errors. - pub fn to_rpc_error(self) -> RpcError { - match self { - Error::UserInputError(_) => RpcError::Call(CallError::InvalidParams(self.into())), +impl From for RpcError { + fn from(e: Error) -> RpcError { + match e { + Error::UserInputError(_) => RpcError::Call(CallError::InvalidParams(e.into())), Error::SuiObjectResponseError(err) => match err { SuiObjectResponseError::NotExists { .. } | SuiObjectResponseError::DynamicFieldNotFound { .. } @@ -101,31 +96,111 @@ impl Error { } _ => RpcError::Call(CallError::Failed(sui_error.into())), }, + Error::StateReadError(err) => match err { + StateReadError::Client(_) => RpcError::Call(CallError::InvalidParams(err.into())), + _ => { + let error_object = ErrorObject::owned( + jsonrpsee::types::error::INTERNAL_ERROR_CODE, + err.to_string(), + None::<()>, + ); + RpcError::Call(CallError::Custom(error_object)) + } + }, Error::QuorumDriverError(err) => match err { + QuorumDriverError::InvalidUserSignature(err) => { + let inner_error_str = match err { + // TODO(wlmyng): update SuiError display trait to render UserInputError with display + SuiError::UserInputError { error } => error.to_string(), + _ => err.to_string(), + }; + + let error_message = format!("Invalid user signature: {inner_error_str}"); + + let error_object = ErrorObject::owned( + TRANSACTION_EXECUTION_CLIENT_ERROR_CODE, + error_message, + None::<()>, + ); + RpcError::Call(CallError::Custom(error_object)) + } + QuorumDriverError::TimeoutBeforeFinality + | QuorumDriverError::FailedWithTransientErrorAfterMaximumAttempts { .. } => { + let error_object = + ErrorObject::owned(TRANSIENT_ERROR_CODE, err.to_string(), None::<()>); + RpcError::Call(CallError::Custom(error_object)) + } + QuorumDriverError::ObjectsDoubleUsed { + conflicting_txes, + retried_tx, + retried_tx_success, + } => { + let error_message = format!( + "Failed to sign transaction by a quorum of validators because of locked objects. Retried a conflicting transaction {:?}, success: {:?}", + retried_tx, + retried_tx_success + ); + + let new_map = conflicting_txes + .into_iter() + .map(|(digest, (pairs, _))| { + ( + digest, + pairs.into_iter().map(|(_, obj_ref)| obj_ref).collect(), + ) + }) + .collect::>>(); + + let error_object = ErrorObject::owned( + TRANSACTION_EXECUTION_CLIENT_ERROR_CODE, + error_message, + Some(new_map), + ); + RpcError::Call(CallError::Custom(error_object)) + } QuorumDriverError::NonRecoverableTransactionError { errors } => { - // Note: we probably want a more precise error than `INVALID_PARAMS_CODE` - // but to keep the error code consistent we still use `INVALID_PARAMS_CODE` + let new_errors: Vec = errors + .into_iter() + .filter(|(err, _, _)| !err.is_retryable().0) // consider retryable errors as transient errors + .map(|(err, _, _)| { + match err { + // TODO(wlmyng): update SuiError display trait to render UserInputError with display + SuiError::UserInputError { error } => error.to_string(), + _ => err.to_string(), + } + }) + .collect(); + + assert!( + !new_errors.is_empty(), + "NonRecoverableTransactionError should have at least one non-retryable error" + ); + + let error_list = new_errors.join(", "); + let error_msg = format!("Transaction execution failed due to issues with transaction inputs, please review the errors and try again: {}.", error_list); + let error_object = ErrorObject::owned( - jsonrpsee::types::error::INVALID_PARAMS_CODE, - NON_RECOVERABLE_ERROR_MSG, - Some(errors), + TRANSACTION_EXECUTION_CLIENT_ERROR_CODE, + error_msg, + None::<()>, ); RpcError::Call(CallError::Custom(error_object)) } - _ => RpcError::Call(CallError::Failed(err.into())), - }, - Error::StateReadError(err) => match err { - StateReadError::Client(_) => RpcError::Call(CallError::InvalidParams(err.into())), - _ => { + QuorumDriverError::QuorumDriverInternalError(_) => { let error_object = ErrorObject::owned( - jsonrpsee::types::error::INTERNAL_ERROR_CODE, - err.to_string(), + INTERNAL_ERROR_CODE, + "Internal error occurred while executing transaction.", None::<()>, ); RpcError::Call(CallError::Custom(error_object)) } + QuorumDriverError::SystemOverload { .. } => { + let error_object = + ErrorObject::owned(TRANSIENT_ERROR_CODE, err.to_string(), None::<()>); + RpcError::Call(CallError::Custom(error_object)) + } }, - _ => RpcError::Call(CallError::Failed(self.into())), + _ => RpcError::Call(CallError::Failed(e.into())), } } } @@ -168,3 +243,221 @@ pub enum SuiRpcInputError { #[error(transparent)] UserInputError(#[from] UserInputError), } + +impl From for RpcError { + fn from(e: SuiRpcInputError) -> Self { + RpcError::Call(CallError::InvalidParams(e.into())) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use expect_test::expect; + use jsonrpsee::types::ErrorObjectOwned; + use sui_types::base_types::AuthorityName; + use sui_types::base_types::ObjectID; + use sui_types::base_types::ObjectRef; + use sui_types::base_types::SequenceNumber; + use sui_types::committee::StakeUnit; + use sui_types::crypto::AuthorityPublicKey; + use sui_types::crypto::AuthorityPublicKeyBytes; + use sui_types::digests::ObjectDigest; + use sui_types::digests::TransactionDigest; + + fn test_object_ref() -> ObjectRef { + ( + ObjectID::ZERO, + SequenceNumber::from_u64(0), + ObjectDigest::new([0; 32]), + ) + } + + mod match_quorum_driver_error_tests { + use super::*; + + #[test] + fn test_invalid_user_signature() { + let quorum_driver_error = + QuorumDriverError::InvalidUserSignature(SuiError::InvalidSignature { + error: "Test inner invalid signature".to_string(), + }); + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32002"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = expect![ + "Invalid user signature: Signature is not valid: Test inner invalid signature" + ]; + expected_message.assert_eq(error_object.message()); + } + + #[test] + fn test_timeout_before_finality() { + let quorum_driver_error = QuorumDriverError::TimeoutBeforeFinality; + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32001"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = expect!["Transaction timed out before reaching finality"]; + expected_message.assert_eq(error_object.message()); + } + + #[test] + fn test_failed_with_transient_error_after_maximum_attempts() { + let quorum_driver_error = + QuorumDriverError::FailedWithTransientErrorAfterMaximumAttempts { + total_attempts: 10, + }; + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32001"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = expect![ + "Transaction failed to reach finality with transient error after 10 attempts." + ]; + expected_message.assert_eq(error_object.message()); + } + + #[test] + fn test_objects_double_used() { + use sui_types::crypto::VerifyingKey; + let mut conflicting_txes: BTreeMap< + TransactionDigest, + (Vec<(AuthorityName, ObjectRef)>, StakeUnit), + > = BTreeMap::new(); + let tx_digest = TransactionDigest::default(); + let object_ref = test_object_ref(); + let stake_unit: StakeUnit = 10; + let authority_name = AuthorityPublicKeyBytes([0; AuthorityPublicKey::LENGTH]); + conflicting_txes.insert(tx_digest, (vec![(authority_name, object_ref)], stake_unit)); + + let quorum_driver_error = QuorumDriverError::ObjectsDoubleUsed { + conflicting_txes, + retried_tx: Some(TransactionDigest::default()), + retried_tx_success: Some(true), + }; + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32002"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = expect!["Failed to sign transaction by a quorum of validators because of locked objects. Retried a conflicting transaction Some(TransactionDigest(11111111111111111111111111111111)), success: Some(true)"]; + expected_message.assert_eq(error_object.message()); + let expected_data = expect![[ + r#"{"11111111111111111111111111111111":[["0x0000000000000000000000000000000000000000000000000000000000000000",0,"11111111111111111111111111111111"]]}"# + ]]; + let actual_data = error_object.data().unwrap().to_string(); + expected_data.assert_eq(&actual_data); + } + + #[test] + fn test_non_recoverable_transaction_error() { + let quorum_driver_error = QuorumDriverError::NonRecoverableTransactionError { + errors: vec![ + ( + SuiError::UserInputError { + error: UserInputError::GasBalanceTooLow { + gas_balance: 10, + needed_gas_amount: 100, + }, + }, + 0, + vec![], + ), + ( + SuiError::UserInputError { + error: UserInputError::ObjectVersionUnavailableForConsumption { + provided_obj_ref: test_object_ref(), + current_version: 10.into(), + }, + }, + 0, + vec![], + ), + ], + }; + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32002"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = + expect!["Transaction execution failed due to issues with transaction inputs, please review the errors and try again: Balance of gas object 10 is lower than the needed amount: 100., Object (0x0000000000000000000000000000000000000000000000000000000000000000, SequenceNumber(0), o#11111111111111111111111111111111) is not available for consumption, its current version: SequenceNumber(10).."]; + expected_message.assert_eq(error_object.message()); + } + + #[test] + #[should_panic( + expected = "NonRecoverableTransactionError should have at least one non-retryable error" + )] + fn test_non_recoverable_transaction_error_with_transient_errors() { + let quorum_driver_error = QuorumDriverError::NonRecoverableTransactionError { + errors: vec![ + ( + SuiError::UserInputError { + error: UserInputError::ObjectNotFound { + object_id: test_object_ref().0, + version: None, + }, + }, + 0, + vec![], + ), + ( + SuiError::RpcError("Hello".to_string(), "Testing".to_string()), + 0, + vec![], + ), + ], + }; + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32001"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = + expect!["Transaction execution failed due to transient errors, please try again."]; + expected_message.assert_eq(error_object.message()); + } + + #[test] + fn test_quorum_driver_internal_error() { + let quorum_driver_error = + QuorumDriverError::QuorumDriverInternalError(SuiError::UnexpectedMessage); + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32603"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = expect!["Internal error occurred while executing transaction."]; + expected_message.assert_eq(error_object.message()); + } + + #[test] + fn test_system_overload() { + let quorum_driver_error = QuorumDriverError::SystemOverload { + overloaded_stake: 10, + errors: vec![(SuiError::UnexpectedMessage, 0, vec![])], + }; + + let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into(); + + let error_object: ErrorObjectOwned = rpc_error.into(); + let expected_code = expect!["-32001"]; + expected_code.assert_eq(&error_object.code().to_string()); + let expected_message = expect!["Transaction is not processed because 10 of validators by stake are overloaded with certificates pending execution."]; + expected_message.assert_eq(error_object.message()); + } + } +} diff --git a/crates/sui-json-rpc/src/logger.rs b/crates/sui-json-rpc/src/logger.rs index 435afb3ef90ae..47aff63055141 100644 --- a/crates/sui-json-rpc/src/logger.rs +++ b/crates/sui-json-rpc/src/logger.rs @@ -17,7 +17,7 @@ macro_rules! with_tracing { let result: RpcResult<_> = interim_result.map_err(|e: Error| { let anyhow_error = anyhow!("{:?}", e); - let rpc_error = e.to_rpc_error(); + let rpc_error: RpcError = e.into(); if !matches!(rpc_error, RpcError::Call(CallError::InvalidParams(_))) { error!(error=?anyhow_error); } diff --git a/crates/sui-json-rpc/src/metrics.rs b/crates/sui-json-rpc/src/metrics.rs index dd625849aa456..26fa0f584ed44 100644 --- a/crates/sui-json-rpc/src/metrics.rs +++ b/crates/sui-json-rpc/src/metrics.rs @@ -5,6 +5,7 @@ use hyper::body::HttpBody; use std::collections::HashSet; use std::net::SocketAddr; +use crate::error::TRANSIENT_ERROR_CODE; use crate::{CLIENT_SDK_TYPE_HEADER, CLIENT_TARGET_API_VERSION_HEADER}; use jsonrpsee::server::logger::{HttpRequest, Logger, MethodKind, TransportProtocol}; use jsonrpsee::types::Params; @@ -31,6 +32,7 @@ pub struct Metrics { errors_by_route: IntCounterVec, server_errors_by_route: IntCounterVec, client_errors_by_route: IntCounterVec, + transient_errors_by_route: IntCounterVec, /// Client info client: IntCounterVec, /// Connection count @@ -94,6 +96,13 @@ impl MetricsLogger { registry, ) .unwrap(), + transient_errors_by_route: register_int_counter_vec_with_registry!( + "transient_errors_by_route", + "Number of transient errors by route", + &["route"], + registry, + ) + .unwrap(), errors_by_route: register_int_counter_vec_with_registry!( "errors_by_route", "Number of client and server errors by route", @@ -228,6 +237,11 @@ impl Logger for MetricsLogger { .server_errors_by_route .with_label_values(&[method_name]) .inc(); + } else if code == TRANSIENT_ERROR_CODE { + self.metrics + .transient_errors_by_route + .with_label_values(&[method_name]) + .inc(); } else { self.metrics .client_errors_by_route