From c3d9cc87f34fbe4b83a5898eaa1a72b5e9f24f14 Mon Sep 17 00:00:00 2001 From: wlmyng <127570466+wlmyng@users.noreply.github.com> Date: Tue, 16 May 2023 12:56:52 -0700 Subject: [PATCH] 3/n improve sui-json-rpc error codes and handling (#11928) ## Description Replace anyhow errors with Error enum on sui-json-rpc where possible ## Test Plan How did you test the new or updated feature? --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [x] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes [API behavioral changes] - rpc methods that result in errors of variant `UserInputError`, `SuiRpcInputError`, `SuiError::TransactionNotFound` or `SuiError::TransactionsNotFound` now return error code `-32602` instead of `-32000`. --- .changeset/early-dancers-walk.md | 5 + crates/sui-json-rpc/src/coin_api.rs | 17 +- crates/sui-json-rpc/src/error.rs | 30 +++ crates/sui-json-rpc/src/governance_api.rs | 11 +- crates/sui-json-rpc/src/indexer_api.rs | 108 ++++++--- crates/sui-json-rpc/src/move_utils.rs | 47 ++-- crates/sui-json-rpc/src/read_api.rs | 220 ++++++++++-------- .../src/transaction_execution_api.rs | 15 +- .../test/e2e/object-display-standard.test.ts | 16 +- 9 files changed, 283 insertions(+), 186 deletions(-) create mode 100644 .changeset/early-dancers-walk.md diff --git a/.changeset/early-dancers-walk.md b/.changeset/early-dancers-walk.md new file mode 100644 index 0000000000000..855e8a0868b59 --- /dev/null +++ b/.changeset/early-dancers-walk.md @@ -0,0 +1,5 @@ +--- +"@mysten/sui.js": patch +--- + +Update ts-sdk e2e test to reflect new rpc error language diff --git a/crates/sui-json-rpc/src/coin_api.rs b/crates/sui-json-rpc/src/coin_api.rs index a933c12b909d2..78ca0dec5a617 100644 --- a/crates/sui-json-rpc/src/coin_api.rs +++ b/crates/sui-json-rpc/src/coin_api.rs @@ -3,8 +3,6 @@ use std::sync::Arc; -use anyhow::anyhow; - use async_trait::async_trait; use cached::proc_macro::cached; use cached::SizedCache; @@ -28,7 +26,7 @@ use sui_types::object::Object; use sui_types::parse_sui_struct_tag; use crate::api::{cap_page_limit, CoinReadApiServer, JsonRpcMetrics}; -use crate::error::Error; +use crate::error::{Error, SuiRpcInputError}; use crate::{with_tracing, SuiRpcModule}; pub struct CoinReadApi { @@ -47,7 +45,7 @@ impl CoinReadApi { cursor: (String, ObjectID), limit: Option, one_coin_type_only: bool, - ) -> anyhow::Result { + ) -> Result { let limit = cap_page_limit(limit); self.metrics.get_coins_limit.report(limit as u64); let state = self.state.clone(); @@ -198,15 +196,16 @@ impl CoinReadApiServer for CoinReadApi { Some(obj) => { let coin_type = obj.coin_type_maybe(); if coin_type.is_none() { - Err(anyhow!( - "Invalid Cursor {:?}, Object is not a coin", - object_id - )) + Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid( + format!("Invalid Cursor {:?}, Object is not a coin", object_id), + ))) } else { Ok((coin_type.unwrap().to_string(), object_id)) } } - None => Err(anyhow!("Invalid Cursor {:?}, Object not found", object_id)), + None => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid( + format!("Invalid Cursor {:?}, Object not found", object_id), + ))), } } None => { diff --git a/crates/sui-json-rpc/src/error.rs b/crates/sui-json-rpc/src/error.rs index 54543c33e5a24..a73e2dfcfa10c 100644 --- a/crates/sui-json-rpc/src/error.rs +++ b/crates/sui-json-rpc/src/error.rs @@ -48,6 +48,9 @@ pub enum Error { #[error(transparent)] SuiObjectResponseError(#[from] SuiObjectResponseError), + + #[error(transparent)] + SuiRpcInputError(#[from] SuiRpcInputError), } impl From for RpcError { @@ -62,6 +65,9 @@ impl Error { Error::UserInputError(user_input_error) => { RpcError::Call(CallError::InvalidParams(user_input_error.into())) } + Error::SuiRpcInputError(sui_json_rpc_input_error) => { + RpcError::Call(CallError::InvalidParams(sui_json_rpc_input_error.into())) + } Error::SuiError(sui_error) => match sui_error { SuiError::TransactionNotFound { .. } | SuiError::TransactionsNotFound { .. } => { RpcError::Call(CallError::InvalidParams(sui_error.into())) @@ -83,3 +89,27 @@ impl Error { } } } + +#[derive(Debug, Error)] +pub enum SuiRpcInputError { + #[error("Input contains duplicates")] + ContainsDuplicates, + + #[error("Input exceeds limit of {0}")] + SizeLimitExceeded(String), + + #[error("{0}")] + GenericNotFound(String), + + #[error("{0}")] + GenericInvalid(String), + + #[error("request_type` must set to `None` or `WaitForLocalExecution` if effects is required in the response")] + InvalidExecuteTransactionRequestType, + + #[error("Unsupported protocol version requested. Min supported: {0}, max supported: {1}")] + ProtocolVersionUnsupported(u64, u64), + + #[error("Unable to serialize: {0}")] + CannotSerialize(#[from] bcs::Error), +} diff --git a/crates/sui-json-rpc/src/governance_api.rs b/crates/sui-json-rpc/src/governance_api.rs index 6521dd174d203..ffad81e51d25b 100644 --- a/crates/sui-json-rpc/src/governance_api.rs +++ b/crates/sui-json-rpc/src/governance_api.rs @@ -5,7 +5,6 @@ use std::cmp::max; use std::collections::BTreeMap; use std::sync::Arc; -use anyhow::anyhow; use async_trait::async_trait; use cached::proc_macro::cached; use cached::SizedCache; @@ -32,7 +31,7 @@ use sui_types::sui_system_state::SuiSystemStateTrait; use sui_types::sui_system_state::{get_validator_from_table, SuiSystemState}; use crate::api::{GovernanceReadApiServer, JsonRpcMetrics}; -use crate::error::Error; +use crate::error::{Error, SuiRpcInputError}; use crate::{with_tracing, ObjectProvider, SuiRpcModule}; #[derive(Clone)] @@ -157,9 +156,11 @@ impl GovernanceReadApi { let mut delegated_stakes = vec![]; for (pool_id, stakes) in pools { // Rate table and rate can be null when the pool is not active - let rate_table = rates - .get(&pool_id) - .ok_or_else(|| anyhow!("Cannot find rates for staking pool {pool_id}"))?; + let rate_table = rates.get(&pool_id).ok_or_else(|| { + SuiRpcInputError::GenericNotFound( + "Cannot find rates for staking pool {pool_id}".to_string(), + ) + })?; let current_rate = rate_table.rates.first().map(|(_, rate)| rate); let mut delegations = vec![]; diff --git a/crates/sui-json-rpc/src/indexer_api.rs b/crates/sui-json-rpc/src/indexer_api.rs index 1bab1faa19af3..045b7bb50d65f 100644 --- a/crates/sui-json-rpc/src/indexer_api.rs +++ b/crates/sui-json-rpc/src/indexer_api.rs @@ -3,7 +3,6 @@ use std::sync::Arc; -use anyhow::{anyhow, Context}; use async_trait::async_trait; use futures::Stream; use jsonrpsee::core::error::SubscriptionClosed; @@ -36,7 +35,7 @@ use crate::api::{ cap_page_limit, validate_limit, IndexerApiServer, JsonRpcMetrics, ReadApiServer, QUERY_MAX_RESULT_LIMIT, }; -use crate::error::Error; +use crate::error::{Error, SuiRpcInputError}; use crate::with_tracing; use crate::SuiRpcModule; @@ -105,7 +104,7 @@ impl IndexerApiServer for IndexerApi { let mut objects = self .state .get_owner_objects(address, cursor, limit + 1, filter) - .map_err(|e| anyhow!("{e}"))?; + .map_err(Error::from)?; // objects here are of size (limit + 1), where the last one is the cursor for the next page let has_next_page = objects.len() > limit; @@ -264,7 +263,7 @@ impl IndexerApiServer for IndexerApi { let mut data = self .state .get_dynamic_fields(parent_object_id, cursor, limit + 1) - .map_err(|e| anyhow!("{e}"))?; + .map_err(Error::from)?; let has_next_page = data.len() > limit; data.truncate(limit); let next_cursor = data.last().cloned().map_or(cursor, |c| Some(c.0)); @@ -299,7 +298,7 @@ impl IndexerApiServer for IndexerApi { let id = self .state .get_dynamic_field_object_id(parent_object_id, name_type, &name_bcs_value) - .map_err(|e| anyhow!("{e}"))?; + .map_err(Error::from)?; // TODO(chris): add options to `get_dynamic_field_object` API as well if let Some(id) = id { self.read_api @@ -326,7 +325,12 @@ impl IndexerApiServer for IndexerApi { name: STD_UTF8_STRUCT_NAME.to_owned(), type_params: vec![], })); - let name_bcs_value = bcs::to_bytes(&name).context("Unable to serialize name")?; + let name_bcs_value = bcs::to_bytes(&name).map_err(|e| { + Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!( + "Unable to serialize name: {:?} with error: {:?}", + name, e + ))) + })?; // record of the input `name` let record_object_id_option = self .state @@ -336,10 +340,10 @@ impl IndexerApiServer for IndexerApi { &name_bcs_value, ) .map_err(|e| { - anyhow!( + Error::UnexpectedError(format!( "Read name service dynamic field table failed with error: {:?}", e - ) + )) })?; if let Some(record_object_id) = record_object_id_option { let record_object_read = @@ -348,33 +352,38 @@ impl IndexerApiServer for IndexerApi { "Failed to get object read of name: {:?} with error: {:?}", record_object_id, e ); - anyhow!("{e}") + Error::from(e) })?; let record_parsed_move_object = SuiParsedMoveObject::try_from_object_read(record_object_read)?; // NOTE: "value" is the field name to get the address info let address_info_move_value = record_parsed_move_object .read_dynamic_field_value(NAME_SERVICE_VALUE) - .ok_or_else(|| anyhow!("Cannot find value field in record Move struct"))?; + .ok_or_else(|| { + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "Cannot find value field in record Move struct".to_string(), + )) + })?; let address_info_move_struct = match address_info_move_value { SuiMoveValue::Struct(a) => Ok(a), - _ => Err(anyhow!("value field is not found.")), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "value field is not found.".to_string(), + ))), }?; // NOTE: "marker" is the field name to get the address let address_str_move_value = address_info_move_struct .read_dynamic_field_value(NAME_SERVICE_MARKER) .ok_or_else(|| { - anyhow!( + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!( "Cannot find marker field in address info Move struct: {:?}", address_info_move_struct - ) + ))) })?; let addr = match address_str_move_value { SuiMoveValue::Address(addr) => Ok(addr), - _ => Err(anyhow!( - "No SuiAddress found in: {:?}", - address_str_move_value - )), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!("No SuiAddress found in: {:?}", address_str_move_value), + ))), }?; return Ok(Some(addr)); } @@ -394,7 +403,12 @@ impl IndexerApiServer for IndexerApi { .get_name_service_dynamic_field_table_object_id(/* reverse_lookup */ true) .await?; let name_type_tag = TypeTag::Address; - let name_bcs_value = bcs::to_bytes(&address).context("Unable to serialize address")?; + let name_bcs_value = bcs::to_bytes(&address).map_err(|e| { + Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!( + "Unable to serialize address: {:?} with error: {:?}", + address, e + ))) + })?; let addr_object_id = self .state .get_dynamic_field_object_id( @@ -403,30 +417,41 @@ impl IndexerApiServer for IndexerApi { &name_bcs_value, ) .map_err(|e| { - anyhow!( + Error::UnexpectedError(format!( "Read name service reverse dynamic field table failed with error: {:?}", e - ) + )) })? - .ok_or_else(|| anyhow!("Record not found for address: {:?}", address))?; + .ok_or_else(|| { + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!( + "Record not found for address: {:?}", + address + ))) + })?; let addr_object_read = self.state.get_object_read(&addr_object_id).map_err(|e| { warn!( "Failed to get object read of address {:?} with error: {:?}", addr_object_id, e ); - anyhow!("{e}") + Error::from(e) })?; let addr_parsed_move_object = SuiParsedMoveObject::try_from_object_read(addr_object_read)?; let address_info_move_value = addr_parsed_move_object .read_dynamic_field_value(NAME_SERVICE_VALUE) - .ok_or_else(|| anyhow!("Cannot find value field in record Move struct"))?; + .ok_or_else(|| { + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "Cannot find value field in record Move struct".to_string(), + )) + })?; let primary_name = match address_info_move_value { SuiMoveValue::String(s) => Ok(s), - _ => Err(anyhow!( - "No string field for primary name is found in {:?}", - address_info_move_value - )), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!( + "No string field for primary name is found in {:?}", + address_info_move_value + ), + ))), }?; Ok(Page { data: vec![primary_name], @@ -451,14 +476,14 @@ impl IndexerApi { async fn get_name_service_dynamic_field_table_object_id( &self, reverse_lookup: bool, - ) -> RpcResult { + ) -> Result { if let Some(resolver_id) = self.ns_resolver_id { let resolver_object_read = self.state.get_object_read(&resolver_id).map_err(|e| { warn!( "Failed to get object read of resolver {:?} with error: {:?}", resolver_id, e ); - anyhow!("{e}") + Error::from(e) })?; let resolved_parsed_move_object = @@ -472,30 +497,37 @@ impl IndexerApi { }; let records_value = resolved_parsed_move_object .read_dynamic_field_value(dynamic_field_table_key) - .ok_or_else(|| anyhow!("Cannot find records field in resolved object"))?; + .ok_or_else(|| { + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "Cannot find records field in resolved object".to_string(), + )) + })?; let records_move_struct = match records_value { SuiMoveValue::Struct(s) => Ok(s), - _ => Err(anyhow!( - "{} field is not a Move struct", - dynamic_field_table_key - )), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid( + format!("{} field is not a Move struct", dynamic_field_table_key), + ))), }?; let dynamic_field_table_object_id_struct = records_move_struct .read_dynamic_field_value(NAME_SERVICE_ID) .ok_or_else(|| { - anyhow!( + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!( "Cannot find id field in {} Move struct", dynamic_field_table_key - ) + ))) })?; let dynamic_field_table_object_id = match dynamic_field_table_object_id_struct { SuiMoveValue::UID { id } => Ok(id), - _ => Err(anyhow!("id field is not a UID")), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid( + "id field is not a UID".to_string(), + ))), }?; Ok(dynamic_field_table_object_id) } else { - Err(anyhow!("Name service resolver is not set"))? + Err(Error::UnexpectedError( + "Name service resolver is not set".to_string(), + ))? } } } diff --git a/crates/sui-json-rpc/src/move_utils.rs b/crates/sui-json-rpc/src/move_utils.rs index 22d0c6c877191..0ec8276fd1be2 100644 --- a/crates/sui-json-rpc/src/move_utils.rs +++ b/crates/sui-json-rpc/src/move_utils.rs @@ -2,9 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use crate::api::MoveUtilsServer; +use crate::error::{Error, SuiRpcInputError}; use crate::read_api::{get_move_module, get_move_modules_by_package}; use crate::{with_tracing, SuiRpcModule}; -use anyhow::anyhow; use async_trait::async_trait; use jsonrpsee::core::RpcResult; use jsonrpsee::RpcModule; @@ -82,13 +82,14 @@ impl MoveUtilsServer for MoveUtils { with_tracing!(async move { let module = get_move_module(&self.state, package, module_name).await?; let structs = module.structs; - let identifier = Identifier::new(struct_name.as_str()).map_err(|e| anyhow!("{e}"))?; + let identifier = Identifier::new(struct_name.as_str()).map_err(|e| { + Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!("{e}"))) + })?; Ok(match structs.get(&identifier) { Some(struct_) => Ok(struct_.clone().into()), - None => Err(anyhow!( - "No struct was found with struct name {}", - struct_name - )), + None => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!("No struct was found with struct name {}", struct_name), + ))), }?) }) } @@ -103,13 +104,14 @@ impl MoveUtilsServer for MoveUtils { with_tracing!(async move { let module = get_move_module(&self.state, package, module_name).await?; let functions = module.functions; - let identifier = Identifier::new(function_name.as_str()).map_err(|e| anyhow!("{e}"))?; + let identifier = Identifier::new(function_name.as_str()).map_err(|e| { + Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!("{e}"))) + })?; Ok(match functions.get(&identifier) { Some(function) => Ok(function.clone().into()), - None => Err(anyhow!( - "No function was found with function name {}", - function_name - )), + None => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!("No function was found with function name {}", function_name), + ))), }?) }) } @@ -122,10 +124,7 @@ impl MoveUtilsServer for MoveUtils { function: String, ) -> RpcResult> { with_tracing!(async move { - let object_read = self - .state - .get_object_read(&package) - .map_err(|e| anyhow!("{e}"))?; + let object_read = self.state.get_object_read(&package).map_err(Error::from)?; let normalized = match object_read { ObjectRead::Exists(_obj_ref, object, _layout) => match object.data { @@ -137,14 +136,20 @@ impl MoveUtilsServer for MoveUtils { /* max_binary_format_version */ VERSION_MAX, /* no_extraneous_module_bytes */ false, ) - .map_err(|e| anyhow!("{e}")) + .map_err(Error::from) } - _ => Err(anyhow!("Object is not a package with ID {}", package)), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid( + format!("Object is not a package with ID {}", package), + ))), }, - _ => Err(anyhow!("Package object does not exist with ID {}", package)), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!("Package object does not exist with ID {}", package), + ))), }?; - let identifier = Identifier::new(function.as_str()).map_err(|e| anyhow!("{e}"))?; + let identifier = Identifier::new(function.as_str()).map_err(|e| { + Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!("{e}"))) + })?; let parameters = normalized .get(&module) .and_then(|m| m.functions.get(&identifier).map(|f| f.parameters.clone())); @@ -168,7 +173,9 @@ impl MoveUtilsServer for MoveUtils { _ => MoveFunctionArgType::Pure, }) .collect::>()), - None => Err(anyhow!("No parameters found for function {}", function)), + None => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!("No parameters found for function {}", function), + ))), }?) }) } diff --git a/crates/sui-json-rpc/src/read_api.rs b/crates/sui-json-rpc/src/read_api.rs index 01ba2691ffe1b..6af38870e09dc 100644 --- a/crates/sui-json-rpc/src/read_api.rs +++ b/crates/sui-json-rpc/src/read_api.rs @@ -38,7 +38,7 @@ use sui_types::crypto::default_hash; use sui_types::digests::TransactionEventsDigest; use sui_types::display::DisplayVersionUpdatedEvent; use sui_types::effects::{TransactionEffects, TransactionEffectsAPI, TransactionEvents}; -use sui_types::error::{SuiObjectResponseError, UserInputError}; +use sui_types::error::SuiObjectResponseError; use sui_types::messages_checkpoint::{CheckpointSequenceNumber, CheckpointTimestamp}; use sui_types::move_package::normalize_modules; use sui_types::object::{Data, Object, ObjectRead, PastObjectRead}; @@ -49,7 +49,7 @@ use sui_types::transaction::{TransactionData, VerifiedTransaction}; use crate::api::JsonRpcMetrics; use crate::api::{validate_limit, ReadApiServer}; use crate::api::{QUERY_MAX_RESULT_LIMIT, QUERY_MAX_RESULT_LIMIT_CHECKPOINTS}; -use crate::error::Error; +use crate::error::{Error, SuiRpcInputError}; use crate::with_tracing; use crate::{ get_balance_changes_from_effect, get_object_changes, ObjectProviderCache, SuiRpcModule, @@ -140,10 +140,9 @@ impl ReadApi { ) -> Result, Error> { let num_digests = digests.len(); if num_digests > *QUERY_MAX_RESULT_LIMIT { - return Err(Error::UserInputError(UserInputError::SizeLimitExceeded { - limit: "multi get transaction input limit".to_string(), - value: QUERY_MAX_RESULT_LIMIT.to_string(), - })); + return Err(Error::SuiRpcInputError( + SuiRpcInputError::SizeLimitExceeded(QUERY_MAX_RESULT_LIMIT.to_string()), + )); } self.metrics .get_tx_blocks_limit @@ -159,15 +158,17 @@ impl ReadApi { .map(|k| (k, IntermediateTransactionResponse::new(*k))), ); if temp_response.len() < num_digests { - return Err(anyhow!("The list of digests in the input contain duplicates").into()); + return Err(Error::SuiRpcInputError( + SuiRpcInputError::ContainsDuplicates, + )); } if opts.require_input() { let state = self.state.clone(); - let digest_clone = digests.clone(); + let digests_clone = digests.clone(); let transactions = - state.multi_get_executed_transactions(&digest_clone).tap_err( - |err| debug!(digests=?digest_clone, "Failed to multi get transaction: {:?}", err), + state.multi_get_executed_transactions(&digests_clone).tap_err( + |err| debug!(digests=?digests_clone, "Failed to multi get transactions: {:?}", err), )?; for ((_digest, cache_entry), txn) in @@ -182,7 +183,7 @@ impl ReadApi { let state = self.state.clone(); let digests_clone = digests.clone(); let effects_list = state.multi_get_executed_effects(&digests_clone).tap_err( - |err| debug!(digests=?digests_clone, "Failed to multi get effects: {:?}", err), + |err| debug!(digests=?digests_clone, "Failed to multi get effects for transactions: {:?}", err), )?; for ((_digest, cache_entry), e) in temp_response.iter_mut().zip(effects_list.into_iter()) @@ -218,8 +219,7 @@ impl ReadApi { .state .multi_get_checkpoint_by_sequence_number(&unique_checkpoint_numbers) .map_err(|e| { - error!("Failed to fetch checkpoint summarys by these checkpoint ids: {unique_checkpoint_numbers:?} with error: {e:?}"); - anyhow!("{e}") + Error::UnexpectedError(format!("Failed to fetch checkpoint summaries by these checkpoint ids: {unique_checkpoint_numbers:?} with error: {e:?}")) })? .into_iter() .map(|c| c.map(|checkpoint| checkpoint.timestamp_ms)); @@ -261,8 +261,7 @@ impl ReadApi { .state .multi_get_events(&event_digests_list) .map_err(|e| { - error!("Failed to call multi_get_events for transactions {digests:?} with event digests {event_digests_list:?}"); - anyhow!("{e}") + Error::UnexpectedError(format!("Failed to call multi_get_events for transactions {digests:?} with event digests {event_digests_list:?}: {e:?}")) })? .into_iter(); @@ -283,7 +282,7 @@ impl ReadApi { if event_digest.is_some() { // safe to unwrap because `is_some` is checked let event_digest = event_digest.as_ref().unwrap(); - let events: Option> = event_digest_to_events + let events= event_digest_to_events .get(event_digest) .cloned() .unwrap_or_else(|| panic!("Expect event digest {event_digest:?} to be found in cache for transaction {transaction_digest}")) @@ -324,7 +323,9 @@ impl ReadApi { results.push(get_balance_changes_from_effect( &object_cache, resp.effects.as_ref().ok_or_else(|| { - anyhow!("unable to derive balance changes because effect is empty") + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "unable to derive balance changes because effect is empty".to_string(), + )) })?, input_objects, None, @@ -346,7 +347,9 @@ impl ReadApi { let mut results = vec![]; for resp in temp_response.values() { let effects = resp.effects.as_ref().ok_or_else(|| { - anyhow!("unable to derive object changes because effect is empty") + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "unable to derive object changes because effect is empty".to_string(), + )) })?; results.push(get_object_changes( @@ -354,7 +357,10 @@ impl ReadApi { resp.transaction .as_ref() .ok_or_else(|| { - anyhow!("unable to derive object changes because effect is empty") + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "unable to derive object changes because transaction is empty" + .to_string(), + )) })? .data() .intent_message() @@ -406,11 +412,11 @@ impl ReadApiServer for ReadApi { let object_read = spawn_monitored_task!(async move { state.get_object_read(&object_id).map_err(|e| { warn!(?object_id, "Failed to get object: {:?}", e); - anyhow!("{e}") + Error::from(e) }) }) .await - .map_err(|e| anyhow!(e))??; + .map_err(Error::from)??; let options = options.unwrap_or_default(); match object_read { @@ -487,10 +493,9 @@ impl ReadApiServer for ReadApi { .inc_by(objects.len() as u64); Ok(objects) } else { - Err(Error::UserInputError(UserInputError::SizeLimitExceeded { - limit: "input limit".to_string(), - value: QUERY_MAX_RESULT_LIMIT.to_string(), - }) + Err(Error::SuiRpcInputError(SuiRpcInputError::SizeLimitExceeded( + QUERY_MAX_RESULT_LIMIT.to_string(), + )) .into()) } }) @@ -509,8 +514,8 @@ impl ReadApiServer for ReadApi { state.get_past_object_read(&object_id, version) .map_err(|e| { error!("Failed to call try_get_past_object for object: {object_id:?} version: {version:?} with error: {e:?}"); - anyhow!("{e}") - })}).await.map_err(|e| anyhow!(e))??; + Error::from(e) + })}).await.map_err(Error::from)??; let options = options.unwrap_or_default(); match past_read { PastObjectRead::ObjectNotExists(id) => { @@ -573,15 +578,14 @@ impl ReadApiServer for ReadApi { .map(|e| e.to_string()) .collect::>() .join("; "); - Err(anyhow!("{error_string}").into()) + Err(anyhow!("{error_string}").into()) // Collects errors not related to SuiPastObjectResponse variants } else { Ok(success) } } else { - Err(Error::UserInputError(UserInputError::SizeLimitExceeded { - limit: "input limit".to_string(), - value: QUERY_MAX_RESULT_LIMIT.to_string(), - }) + Err(Error::SuiRpcInputError(SuiRpcInputError::SizeLimitExceeded( + QUERY_MAX_RESULT_LIMIT.to_string(), + )) .into()) } }) @@ -611,13 +615,13 @@ impl ReadApiServer for ReadApi { // Fetch transaction to determine existence let state = self.state.clone(); let transaction = spawn_monitored_task!(async move { - state.get_transaction_block(digest).await.tap_err( - |err| debug!(tx_digest=?digest, "Failed to get transaction: {:?}", err), - ) + state.get_transaction_block(digest).await.map_err(|err| { + debug!(tx_digest=?digest, "Failed to get transaction: {:?}", err); + Error::from(err) + }) }) .await - .map_err(Error::from)? - .map_err(Error::from)?; + .map_err(Error::from)??; let input_objects = transaction .data() .inner() @@ -636,13 +640,13 @@ impl ReadApiServer for ReadApi { let state = self.state.clone(); temp_response.effects = Some( spawn_monitored_task!(async move { - state.get_executed_effects(digest).tap_err( - |err| debug!(tx_digest=?digest, "Failed to get effects: {:?}", err), - ) + state.get_executed_effects(digest).map_err(|err| { + debug!(tx_digest=?digest, "Failed to get effects: {:?}", err); + Error::from(err) + }) }) .await - .map_err(Error::from)? - .map_err(Error::from)?, + .map_err(Error::from)??, ); } @@ -651,8 +655,8 @@ impl ReadApiServer for ReadApi { state.get_transaction_checkpoint_sequence(&digest) .map_err(|e| { error!("Failed to retrieve checkpoint sequence for transaction {digest:?} with error: {e:?}"); - anyhow!("{e}") - })}).await.map_err(|e|anyhow!(e))?? + Error::from(e) + })}).await.map_err(Error::from)?? { temp_response.checkpoint_seq = Some(seq); } @@ -666,9 +670,8 @@ impl ReadApiServer for ReadApi { .get_checkpoint_by_sequence_number(checkpoint_seq) .map_err(|e| { error!("Failed to get checkpoint by sequence number: {checkpoint_seq:?} with error: {e:?}"); - anyhow!("{e}" - ) - })}).await.map_err(|e|anyhow!(e))??; + Error::from(e) + })}).await.map_err(Error::from)??; // TODO(chris): we don't need to fetch the whole checkpoint summary temp_response.timestamp = checkpoint.as_ref().map(|c| c.timestamp_ms); } @@ -686,7 +689,7 @@ impl ReadApiServer for ReadApi { { error!("Failed to call get transaction events for events digest: {event_digest:?} with error {e:?}"); Error::from(e) - })}).await.map_err(|e|anyhow!(e))??; + })}).await.map_err(Error::from)??; match to_sui_transaction_events(self, digest, events) { Ok(e) => temp_response.events = Some(e), Err(e) => temp_response.errors.push(e.to_string()), @@ -800,7 +803,7 @@ impl ReadApiServer for ReadApi { vec![] }; Ok(events) - }).await.map_err(|e| anyhow!(e))? + }).await.map_err(Error::from)? }) } @@ -811,7 +814,9 @@ impl ReadApiServer for ReadApi { .state .get_latest_checkpoint_sequence_number() .map_err(|e| { - anyhow!("Latest checkpoint sequence number was not found with error :{e}") + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!( + "Latest checkpoint sequence number was not found with error :{e}" + ))) })? .into()) }) @@ -915,11 +920,12 @@ impl ReadApiServer for ReadApi { with_tracing!(async move { Ok(version .map(|v| { - ProtocolConfig::get_for_version_if_supported((*v).into()).ok_or(anyhow!( - "Unsupported protocol version requested. Min supported: {}, max supported: {}", - ProtocolVersion::MIN.as_u64(), - ProtocolVersion::MAX.as_u64() - )) + ProtocolConfig::get_for_version_if_supported((*v).into()).ok_or( + Error::SuiRpcInputError(SuiRpcInputError::ProtocolVersionUnsupported( + ProtocolVersion::MIN.as_u64(), + ProtocolVersion::MAX.as_u64(), + )), + ) }) .unwrap_or(Ok(self .state @@ -945,7 +951,7 @@ fn to_sui_transaction_events( fullnode_api: &ReadApi, tx_digest: TransactionDigest, events: TransactionEvents, -) -> RpcResult { +) -> Result { Ok(SuiTransactionBlockEvents::try_from( events, tx_digest, @@ -960,15 +966,14 @@ fn to_sui_transaction_events( .load_epoch_store_one_call_per_task() .module_cache() .as_ref(), - ) - .map_err(Error::SuiError)?) + )?) } fn get_display_fields( fullnode_api: &ReadApi, original_object: &Object, original_layout: &Option, -) -> RpcResult { +) -> Result { let Some((object_type, layout)) = get_object_type_and_struct(original_object, original_layout)? else { return Ok(DisplayFieldsResponse { data: None, error: None }); }; @@ -985,22 +990,19 @@ fn get_display_object_by_type( fullnode_api: &ReadApi, object_type: &StructTag, // TODO: add query version support -) -> RpcResult> { - let mut events = fullnode_api - .state - .query_events( - EventFilter::MoveEventType(DisplayVersionUpdatedEvent::type_(object_type)), - None, - 1, - true, - ) - .map_err(Error::from)?; +) -> Result, Error> { + let mut events = fullnode_api.state.query_events( + EventFilter::MoveEventType(DisplayVersionUpdatedEvent::type_(object_type)), + None, + 1, + true, + )?; // If there's any recent version of Display, give it to the client. // TODO: add support for version query. if let Some(event) = events.pop() { let display: DisplayVersionUpdatedEvent = bcs::from_bytes(&event.bcs[..]) - .map_err(|e| anyhow!("Failed to deserialize 'VersionUpdatedEvent': {e}"))?; + .map_err(|e| anyhow!("Failed to deserialize 'VersionUpdatedEvent': {e}"))?; // TODO: currently, this is called on a value returned from state. Is this a client or server error? Ok(Some(display)) } else { @@ -1011,7 +1013,7 @@ fn get_display_object_by_type( fn get_object_type_and_struct( o: &Object, layout: &Option, -) -> RpcResult> { +) -> Result, Error> { if let Some(object_type) = o.type_() { let move_struct = get_move_struct(o, layout)?; Ok(Some((object_type.clone().into(), move_struct))) @@ -1020,39 +1022,46 @@ fn get_object_type_and_struct( } } -fn get_move_struct(o: &Object, layout: &Option) -> RpcResult { - let layout = layout - .as_ref() - .ok_or_else(|| anyhow!("Failed to extract layout"))?; +fn get_move_struct(o: &Object, layout: &Option) -> Result { + let layout = layout.as_ref().ok_or_else(|| { + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "Failed to extract layout".to_string(), + )) + })?; Ok(o.data .try_as_move() - .ok_or_else(|| anyhow!("Failed to extract Move object"))? - .to_move_struct(layout) - .map_err(|err| anyhow!("{err}"))?) + .ok_or_else(|| { + Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + "Failed to extract Move object".to_string(), + )) + })? + .to_move_struct(layout)?) } pub async fn get_move_module( state: &AuthorityState, package: ObjectID, module_name: String, -) -> RpcResult { +) -> Result { let normalized = get_move_modules_by_package(state, package).await?; Ok(match normalized.get(&module_name) { Some(module) => Ok(module.clone()), - None => Err(anyhow!("No module found with module name {}", module_name)), + None => Err(SuiRpcInputError::GenericNotFound(format!( + "No module found with module name {}", + module_name + ))), }?) } pub async fn get_move_modules_by_package( state: &AuthorityState, package: ObjectID, -) -> RpcResult> { - let object_read = state.get_object_read(&package).map_err(|e| { +) -> Result, Error> { + let object_read = state.get_object_read(&package).tap_err(|_| { warn!("Failed to call get_move_modules_by_package for package: {package:?}"); - anyhow!("{e}") })?; - Ok(match object_read { + match object_read { ObjectRead::Exists(_obj_ref, object, _layout) => match object.data { Data::Package(p) => { // we are on the read path - it's OK to use VERSION_MAX of the supported Move @@ -1064,20 +1073,24 @@ pub async fn get_move_modules_by_package( ) .map_err(|e| { error!("Failed to call get_move_modules_by_package for package: {package:?}"); - anyhow!("{e}") + Error::from(e) }) } - _ => Err(anyhow!("Object is not a package with ID {}", package)), + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid( + format!("Object is not a package with ID {}", package), + ))), }, - _ => Err(anyhow!("Package object does not exist with ID {}", package)), - }?) + _ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound( + format!("Package object does not exist with ID {}", package), + ))), + } } pub fn get_transaction_data_and_digest( tx_bytes: Base64, -) -> RpcResult<(TransactionData, TransactionDigest)> { +) -> Result<(TransactionData, TransactionDigest), Error> { let tx_data = - bcs::from_bytes(&tx_bytes.to_vec().map_err(|e| anyhow!(e))?).map_err(|e| anyhow!(e))?; + bcs::from_bytes(&tx_bytes.to_vec().map_err(|e| anyhow!(e))?).map_err(Error::from)?; // TODO: is this a client or server error? Only used by dry_run_transaction_block let intent_msg = IntentMessage::new( Intent { version: IntentVersion::V0, @@ -1093,7 +1106,7 @@ pub fn get_transaction_data_and_digest( pub fn get_rendered_fields( fields: VecMap, move_struct: &MoveStruct, -) -> RpcResult { +) -> Result { let sui_move_value: SuiMoveValue = MoveValue::Struct(move_struct.clone()).into(); if let SuiMoveValue::Struct(move_struct) = sui_move_value { let fields = @@ -1125,10 +1138,12 @@ pub fn get_rendered_fields( error, }); } - Err(anyhow!("Failed to parse move struct"))? + Err(SuiRpcInputError::GenericInvalid( + "Failed to parse move struct".to_string(), + ))? } -fn parse_template(template: &str, move_struct: &SuiMoveStruct) -> RpcResult { +fn parse_template(template: &str, move_struct: &SuiMoveStruct) -> Result { let mut output = template.to_string(); let mut var_name = String::new(); let mut in_braces = false; @@ -1162,7 +1177,10 @@ fn parse_template(template: &str, move_struct: &SuiMoveStruct) -> RpcResult RpcResult { +fn get_value_from_move_struct( + move_struct: &SuiMoveStruct, + var_name: &str, +) -> Result { let parts: Vec<&str> = var_name.split('.').collect(); if parts.is_empty() { return Err(anyhow!("Display template value cannot be empty"))?; @@ -1190,13 +1208,18 @@ fn get_value_from_move_struct(move_struct: &SuiMoveStruct, var_name: &str) -> Rp ))?; } } else { - return Err(anyhow!( + return Err(Error::UnexpectedError(format!( "Unexpected move struct type for field {}", var_name - ))?; + )))?; } } - _ => return Err(anyhow!("Unexpected move value type for field {}", var_name))?, + _ => { + return Err(Error::UnexpectedError(format!( + "Unexpected move value type for field {}", + var_name + )))? + } } } @@ -1225,7 +1248,7 @@ fn convert_to_response( if opts.show_raw_input && cache.transaction.is_some() { let sender_signed_data = cache.transaction.as_ref().unwrap().data(); let raw_tx = bcs::to_bytes(sender_signed_data) - .map_err(|e| anyhow!("Failed to serialize raw transaction with error: {}", e))?; + .map_err(|e| anyhow!("Failed to serialize raw transaction with error: {}", e))?; // TODO: is this a client or server error? response.raw_transaction = raw_tx; } @@ -1238,6 +1261,7 @@ fn convert_to_response( if opts.show_effects && cache.effects.is_some() { let effects = cache.effects.unwrap().try_into().map_err(|e| { anyhow!( + // TODO: is this a client or server error? "Failed to convert transaction block effects with error: {}", e ) diff --git a/crates/sui-json-rpc/src/transaction_execution_api.rs b/crates/sui-json-rpc/src/transaction_execution_api.rs index 7885835f73358..2cf6cab7d9ab1 100644 --- a/crates/sui-json-rpc/src/transaction_execution_api.rs +++ b/crates/sui-json-rpc/src/transaction_execution_api.rs @@ -3,7 +3,6 @@ use std::sync::Arc; -use anyhow::anyhow; use async_trait::async_trait; use fastcrypto::encoding::Base64; use fastcrypto::traits::ToFromBytes; @@ -32,7 +31,7 @@ use tracing::instrument; use crate::api::JsonRpcMetrics; use crate::api::WriteApiServer; -use crate::error::Error; +use crate::error::{Error, SuiRpcInputError}; use crate::read_api::get_transaction_data_and_digest; use crate::{ get_balance_changes_from_effect, get_object_changes, with_tracing, ObjectProviderCache, @@ -69,11 +68,9 @@ impl TransactionExecutionApi { let request_type = match (request_type, opts.require_local_execution()) { (Some(ExecuteTransactionRequestType::WaitForEffectsCert), true) => { - return Err(anyhow!( - "`request_type` must set to `None` or `WaitForLocalExecution`\ - if effects is required in the response" - ) - .into()); + return Err(Error::SuiRpcInputError( + SuiRpcInputError::InvalidExecuteTransactionRequestType, + )); } (t, _) => t.unwrap_or_else(|| opts.default_execution_request_type()), }; @@ -234,9 +231,9 @@ impl WriteApiServer for TransactionExecutionApi { gas_price: Option>, _epoch: Option>, ) -> RpcResult { - let tx_kind: TransactionKind = - bcs::from_bytes(&tx_bytes.to_vec().map_err(|e| anyhow!(e))?).map_err(|e| anyhow!(e))?; with_tracing!(async move { + let tx_kind: TransactionKind = + bcs::from_bytes(&tx_bytes.to_vec().map_err(Error::from)?).map_err(Error::from)?; Ok(self .state .dev_inspect_transaction_block(sender_address, tx_kind, gas_price.map(|i| *i)) diff --git a/sdk/typescript/test/e2e/object-display-standard.test.ts b/sdk/typescript/test/e2e/object-display-standard.test.ts index e6256111ad3d1..d31e2f681d16a 100644 --- a/sdk/typescript/test/e2e/object-display-standard.test.ts +++ b/sdk/typescript/test/e2e/object-display-standard.test.ts @@ -31,7 +31,7 @@ describe('Test Object Display Standard', () => { options: { showDisplay: true }, }), ); - expect(display).toEqual({ + const expectedData = { data: { age: '10', buyer: toolbox.address(), @@ -44,12 +44,14 @@ describe('Test Object Display Standard', () => { full_url: 'https://get-a-boar.fullurl.com/', escape_syntax: '{name}', }, - error: { - code: 'displayError', - error: - 'RPC call failed: Field value idd cannot be found in struct; RPC call failed: Field value namee cannot be found in struct', - }, - }); + }; + expect(display).toEqual(expect.objectContaining(expectedData)); + const errorMessage1 = + 'RPC call failed: Field value idd cannot be found in struct; RPC call failed: Field value namee cannot be found in struct'; + const errorMessage2 = + 'Field value idd cannot be found in struct; Field value namee cannot be found in struct'; + + expect([errorMessage1, errorMessage2]).toContain(display.error?.error); }); it('Test getting Display fields for object that has no display object', async () => {