From adb096709bc93a188964203a4c247f204248a4ae Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Thu, 13 Jun 2024 15:37:40 -0700 Subject: [PATCH] fixup! fixup! [CLI] Add clever error support to Sui CLI --- crates/sui/src/clever_error_rendering.rs | 18 +++- crates/sui/src/client_commands.rs | 83 +++++++++++++++---- ...ing__tests__parse_abort_status_string.snap | 16 ++++ .../tests/snapshots/cli_tests__body_fn.snap | 8 +- 4 files changed, 101 insertions(+), 24 deletions(-) diff --git a/crates/sui/src/clever_error_rendering.rs b/crates/sui/src/clever_error_rendering.rs index 833df2b815039..e32be6e1123ce 100644 --- a/crates/sui/src/clever_error_rendering.rs +++ b/crates/sui/src/clever_error_rendering.rs @@ -111,13 +111,21 @@ fn parse_abort_status_string( s: &str, ) -> Result<(AccountAddress, Identifier, Identifier, u16, u64, u16), anyhow::Error> { use regex::Regex; - let re = Regex::new(r"MoveAbort.*address:\s*(.*?),.*Identifier...(.*?)\\.*instruction:\s+(\d+),.*function_name:\s*Some...(\w+?)\\.*},\s*(\d+).*in command\s*(\d+)").unwrap(); - let captures = re.captures(s).unwrap(); + let re = Regex::new(r#"MoveAbort.*address:\s*(.*?),.* name:.*Identifier\((.*?)\).*instruction:\s+(\d+),.*function_name:.*Some\((.*?)\).*},\s*(\d+).*in command\s*(\d+)"#).unwrap(); + let Some(captures) = re.captures(s) else { + anyhow::bail!( + "Cannot parse abort status string: {} as a move abort string", + s + ); + }; + + // Remove any escape characters from the string if present. + let clean_string = |s: &str| s.replace(['\\', '\"'], ""); let address = AccountAddress::from_hex(&captures[1])?; - let module_name = Identifier::new(&captures[2])?; + let module_name = Identifier::new(clean_string(&captures[2]))?; let instruction = captures[3].parse::()?; - let function_name = Identifier::new(&captures[4])?; + let function_name = Identifier::new(clean_string(&captures[4]))?; let abort_code = captures[5].parse::()?; let command_index = captures[6].parse::()?; Ok(( @@ -141,6 +149,8 @@ mod tests { r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 1, instruction: 1, function_name: Some(\"aborter_line_no\") }, 9223372105574252543) in command 0" }"#, r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 2, instruction: 1, function_name: Some(\"clever_aborter\") }, 9223372118459154433) in command 0" }"#, r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 3, instruction: 1, function_name: Some(\"clever_aborter_not_a_string\") }, 9223372135639154691) in command 0" }"#, + r#"MoveAbort(MoveLocation { module: ModuleId { address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b, name: Identifier(\"tester\") }, function: 0, instruction: 1, function_name: Some(\"test\") }, 9223372071214514177) in command 0"#, + r#"MoveAbort(MoveLocation { module: ModuleId { address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b, name: Identifier("tester") }, function: 0, instruction: 1, function_name: Some("test") }, 9223372071214514177) in command 0"#, ]; let parsed: Vec<_> = corpus.into_iter().map(|c| { let (address, module_name, function_name, instruction, abort_code, command_index) = diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index a0882785dcb6a..349c5ce2df5bc 100644 --- a/crates/sui/src/client_commands.rs +++ b/crates/sui/src/client_commands.rs @@ -43,7 +43,8 @@ use sui_json_rpc_types::{ Coin, DryRunTransactionBlockResponse, DynamicFieldPage, SuiCoinMetadata, SuiData, SuiExecutionStatus, SuiObjectData, SuiObjectDataOptions, SuiObjectResponse, SuiObjectResponseQuery, SuiParsedData, SuiProtocolConfigValue, SuiRawData, - SuiTransactionBlockEffectsAPI, SuiTransactionBlockResponse, SuiTransactionBlockResponseOptions, + SuiTransactionBlockEffects, SuiTransactionBlockEffectsAPI, SuiTransactionBlockResponse, + SuiTransactionBlockResponseOptions, }; use sui_keys::keystore::AccountKeystore; use sui_move_build::{ @@ -663,7 +664,7 @@ impl SuiClientCommands { self, context: &mut WalletContext, ) -> Result { - let ret = Ok(match self { + let ret = match self { SuiClientCommands::ProfileTransaction { tx_digest, profile_output, @@ -1583,8 +1584,9 @@ impl SuiClientCommands { ptb.execute(context).await?; SuiClientCommandResult::NoOutput } - }); - ret + }; + let client = context.get_client().await?; + Ok(ret.prerender_clever_errors(client.read_api()).await) } pub fn switch_env(config: &mut SuiClientConfig, env: &str) -> Result<(), anyhow::Error> { @@ -2204,6 +2206,42 @@ impl SuiClientCommandResult { _ => None, } } + + pub async fn prerender_clever_errors(mut self, read_api: &ReadApi) -> Self { + match &mut self { + SuiClientCommandResult::DryRun(DryRunTransactionBlockResponse { effects, .. }) + | SuiClientCommandResult::TransactionBlock(SuiTransactionBlockResponse { + effects: Some(effects), + .. + }) => prerender_clever_errors(effects, read_api).await, + + SuiClientCommandResult::TransactionBlock(SuiTransactionBlockResponse { + effects: None, + .. + }) => (), + SuiClientCommandResult::ActiveAddress(_) + | SuiClientCommandResult::ActiveEnv(_) + | SuiClientCommandResult::Addresses(_) + | SuiClientCommandResult::Balance(_, _) + | SuiClientCommandResult::ChainIdentifier(_) + | SuiClientCommandResult::DynamicFieldQuery(_) + | SuiClientCommandResult::Envs(_, _) + | SuiClientCommandResult::Gas(_) + | SuiClientCommandResult::NewAddress(_) + | SuiClientCommandResult::NewEnv(_) + | SuiClientCommandResult::NoOutput + | SuiClientCommandResult::Object(_) + | SuiClientCommandResult::Objects(_) + | SuiClientCommandResult::RawObject(_) + | SuiClientCommandResult::SerializedSignedTransaction(_) + | SuiClientCommandResult::SerializedUnsignedTransaction(_) + | SuiClientCommandResult::Switch(_) + | SuiClientCommandResult::SyncClientState + | SuiClientCommandResult::VerifyBytecodeMeter { .. } + | SuiClientCommandResult::VerifySource => (), + } + self + } } #[derive(Serialize)] @@ -2568,7 +2606,10 @@ pub async fn execute_dry_run( .dry_run_transaction_block(dry_run_tx_data) .await .map_err(|e| anyhow!("Dry run failed: {e}"))?; - Ok(SuiClientCommandResult::DryRun(response)) + let resp = SuiClientCommandResult::DryRun(response) + .prerender_clever_errors(client.read_api()) + .await; + Ok(resp) } /// Call a dry run with the transaction data to estimate the gas budget. @@ -2707,22 +2748,32 @@ pub(crate) async fn dry_run_or_execute_or_serialize( )) } else { let transaction = Transaction::new(sender_signed_data); - let response = context.execute_transaction_may_fail(transaction).await?; + let mut response = context.execute_transaction_may_fail(transaction).await?; + if let Some(effects) = response.effects.as_mut() { + prerender_clever_errors(effects, client.read_api()).await; + } let effects = response.effects.as_ref().ok_or_else(|| { anyhow!("Effects from SuiTransactionBlockResult should not be empty") })?; - if matches!(effects.status(), SuiExecutionStatus::Failure { .. }) { - let error_string = if let Some(parsed_abort) = - render_clever_error_opt(&format!("{:?}", effects.status()), client.read_api()) - .await - { - parsed_abort - } else { - format!("{:#?}", effects.status()) - }; - return Err(anyhow!("Error executing transaction: {error_string}",)); + if let SuiExecutionStatus::Failure { error } = effects.status() { + return Err(anyhow!( + "Error executing transaction '{}': {error}", + response.digest + )); } Ok(SuiClientCommandResult::TransactionBlock(response)) } } } + +pub(crate) async fn prerender_clever_errors( + effects: &mut SuiTransactionBlockEffects, + read_api: &ReadApi, +) { + let SuiTransactionBlockEffects::V1(effects) = effects; + if let SuiExecutionStatus::Failure { error } = &mut effects.status { + if let Some(rendered) = render_clever_error_opt(error, read_api).await { + *error = rendered; + } + } +} diff --git a/crates/sui/src/snapshots/sui__clever_error_rendering__tests__parse_abort_status_string.snap b/crates/sui/src/snapshots/sui__clever_error_rendering__tests__parse_abort_status_string.snap index 0485a115bb4ef..d6590d51596a3 100644 --- a/crates/sui/src/snapshots/sui__clever_error_rendering__tests__parse_abort_status_string.snap +++ b/crates/sui/src/snapshots/sui__clever_error_rendering__tests__parse_abort_status_string.snap @@ -33,3 +33,19 @@ original abort message: Failure { error: "MoveAbort(MoveLocation { module: Modul instruction: 1 abort_code: 9223372135639154691 command_index: 0 +------ +original abort message: MoveAbort(MoveLocation { module: ModuleId { address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b, name: Identifier(\"tester\") }, function: 0, instruction: 1, function_name: Some(\"test\") }, 9223372071214514177) in command 0 + address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b + module_name: tester + function_name: test + instruction: 1 + abort_code: 9223372071214514177 + command_index: 0 +------ +original abort message: MoveAbort(MoveLocation { module: ModuleId { address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b, name: Identifier("tester") }, function: 0, instruction: 1, function_name: Some("test") }, 9223372071214514177) in command 0 + address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b + module_name: tester + function_name: test + instruction: 1 + abort_code: 9223372071214514177 + command_index: 0 diff --git a/crates/sui/tests/snapshots/cli_tests__body_fn.snap b/crates/sui/tests/snapshots/cli_tests__body_fn.snap index 3eba4b17b5551..494366a1fc6cb 100644 --- a/crates/sui/tests/snapshots/cli_tests__body_fn.snap +++ b/crates/sui/tests/snapshots/cli_tests__body_fn.snap @@ -4,18 +4,18 @@ expression: "format!(\"Non-clever-abort\\n---\\n{}\\n---\\nLine-only-abort\\n--- --- Non-clever-abort --- -Error executing transaction: 1st command aborted within function '0x60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20::clever_errors::aborter' at instruction 1 with code 0 +Error executing transaction 'BQaYuRVbz7BjpjmDXUmFYyxADDK4WySrg1AfTqTao1jo': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::aborter' at instruction 1 with code 0 --- Line-only-abort --- -Error executing transaction: 1st command aborted within function '0x60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20::clever_errors::aborter_line_no' at line 15 +Error executing transaction '3w3diTUzG3ryjpMA3WPrfx9eLNArYqRn1dMDWyTggKGA': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::aborter_line_no' at line 15 --- Clever-error-utf8 --- -Error executing transaction: 1st command aborted within function '0x60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20::clever_errors::clever_aborter' at line 19. Aborted with 'ENotFound' -- 'Element not found in vector 💥 🚀 🌠' +Error executing transaction '7YmFWxA3WKzKF7Bp3sJisD83EZTGXFosD4TKmq8ughLY': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::clever_aborter' at line 19. Aborted with 'ENotFound' -- 'Element not found in vector 💥 🚀 🌠' --- Clever-error-non-utf8 --- -Error executing transaction: 1st command aborted within function '0x60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20::clever_errors::clever_aborter_not_a_string' at line 23. Aborted with 'ENotAString' -- 'BAEAAAAAAAAAAgAAAAAAAAADAAAAAAAAAAQAAAAAAAAA' +Error executing transaction 'FK7UrheD4TWakAnK5j5tC3LySXd9obtixumbE9ZS5UYp': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::clever_aborter_not_a_string' at line 23. Aborted with 'ENotAString' -- 'BAEAAAAAAAAAAgAAAAAAAAADAAAAAAAAAAQAAAAAAAAA' ---