Skip to content

Commit

Permalink
fixup! fixup! fixup! [CLI] Add clever error support to Sui CLI
Browse files Browse the repository at this point in the history
  • Loading branch information
tzakian committed Jun 17, 2024
1 parent 99332d0 commit 0e4914d
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
33 changes: 32 additions & 1 deletion crates/sui/src/clever_error_rendering.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

//! This module provides a function to render a Move abort status string into a more human-readable
//! error message using the clever error rendering logic.
//!
//! The logic in this file is largely a stop-gap to provide Clever Error rendering in the CLI while
//! it still uses the JSON-RPC API. The new GraphQL API already renderd Clever Errors on the server
//! side in a much more robust and efficient way.
//!
//! Once the CLI is updated to use the GraphQL API, this file can be removed, and the GraphQL-based
//! rendering logic for Clever Errors should be used instead.

use fastcrypto::encoding::{Base64, Encoding};
use move_binary_format::{
binary_config::BinaryConfig, file_format::SignatureToken, CompiledModule,
Expand All @@ -14,7 +24,18 @@ use sui_json_rpc_types::{SuiObjectDataOptions, SuiRawData};
use sui_sdk::apis::ReadApi;
use sui_types::{base_types::ObjectID, Identifier};

pub async fn render_clever_error_opt(error_string: &str, read_api: &ReadApi) -> Option<String> {
/// Take a Move abort status string and render it into a more human-readable error message using
/// by parsing the string (as best we can) and seeing if the abort code is a Clever Error abort
/// code. If it is, we attempt to render the error in a more huma-readable manner using the Read
/// API and decoding the Clever Error encoding in the abort code.
///
/// This function is used to render Clever Errors for on-chain errors only within the Sui CLI. This
/// function is _not_ used at all for off-chain errors or Move unit tests. You should only use this
/// function within this crate.
pub(crate) async fn render_clever_error_opt(
error_string: &str,
read_api: &ReadApi,
) -> Option<String> {
let (address, module_name, function_name, instruction, abort_code, command_index) =
parse_abort_status_string(error_string).ok()?;

Expand Down Expand Up @@ -107,6 +128,16 @@ pub async fn render_clever_error_opt(error_string: &str, read_api: &ReadApi) ->
Some(format!("{command}{suffix} command aborted within {error}"))
}

/// Parsing the error with a regex is not great, but it's the best we can do with the current
/// JSON-RPC API since we only get error messages as strings. This function attempts to parse a
/// Move abort status string into its different parts, and then parses it back into the structured
/// format that we can then use to render a Clever Error.
///
/// If we are able to parse the string, we return a tuple with the address, module name, function
/// name, instruction, abort code, and command index. If we are unable to parse the string, we
/// return `Err`.
///
/// You should delete this function with glee once the CLI is updated to use the GraphQL API.
fn parse_abort_status_string(
s: &str,
) -> Result<(AccountAddress, Identifier, Identifier, u16, u64, u16), anyhow::Error> {
Expand Down
19 changes: 14 additions & 5 deletions crates/sui/tests/cli_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3729,6 +3729,12 @@ async fn test_clever_errors() -> Result<(), anyhow::Error> {
.find(|refe| matches!(refe.owner, Owner::Immutable))
.unwrap();

let elide_transaction_digest = |s: String| -> String {
let mut x = s.splitn(3, '\'').collect::<Vec<_>>();
x[1] = "<ELIDED_TRANSACTION_DIGEST>";
x.join("'")
};

// Normal abort
let non_clever_abort = SuiClientCommands::Call {
package: package.reference.object_id,
Expand Down Expand Up @@ -3785,11 +3791,14 @@ async fn test_clever_errors() -> Result<(), anyhow::Error> {
.await
.unwrap_err();

insta::assert_snapshot!(
format!(
"Non-clever-abort\n---\n{}\n---\nLine-only-abort\n---\n{}\n---\nClever-error-utf8\n---\n{}\n---\nClever-error-non-utf8\n---\n{}\n---\n",
non_clever_abort, line_only_abort, clever_error_utf8, clever_error_non_utf8
)
let error_string = format!(
"Non-clever-abort\n---\n{}\n---\nLine-only-abort\n---\n{}\n---\nClever-error-utf8\n---\n{}\n---\nClever-error-non-utf8\n---\n{}\n---\n",
elide_transaction_digest(non_clever_abort.to_string()),
elide_transaction_digest(line_only_abort.to_string()),
elide_transaction_digest(clever_error_utf8.to_string()),
elide_transaction_digest(clever_error_non_utf8.to_string())
);

insta::assert_snapshot!(error_string);
Ok(())
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// Do _not_ edit this file (yes, even whitespace!). Editing this file will
// cause the tests that use this module to fail.
module clever_errors::clever_errors {
Expand Down
10 changes: 5 additions & 5 deletions crates/sui/tests/snapshots/cli_tests__body_fn.snap
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
---
source: crates/sui/tests/cli_tests.rs
expression: "format!(\"Non-clever-abort\\n---\\n{}\\n---\\nLine-only-abort\\n---\\n{}\\n---\\nClever-error-utf8\\n---\\n{}\\n---\\nClever-error-non-utf8\\n---\\n{}\\n---\\n\",\n non_clever_abort, line_only_abort, clever_error_utf8,\n clever_error_non_utf8)"
expression: error_string
---
Non-clever-abort
---
Error executing transaction 'BQaYuRVbz7BjpjmDXUmFYyxADDK4WySrg1AfTqTao1jo': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::aborter' at instruction 1 with code 0
Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::aborter' at instruction 1 with code 0
---
Line-only-abort
---
Error executing transaction '3w3diTUzG3ryjpMA3WPrfx9eLNArYqRn1dMDWyTggKGA': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::aborter_line_no' at line 15
Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::aborter_line_no' at line 18
---
Clever-error-utf8
---
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 💥 🚀 🌠'
Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::clever_aborter' at line 22. Aborted with 'ENotFound' -- 'Element not found in vector 💥 🚀 🌠'
---
Clever-error-non-utf8
---
Error executing transaction 'FK7UrheD4TWakAnK5j5tC3LySXd9obtixumbE9ZS5UYp': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::clever_aborter_not_a_string' at line 23. Aborted with 'ENotAString' -- 'BAEAAAAAAAAAAgAAAAAAAAADAAAAAAAAAAQAAAAAAAAA'
Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::clever_aborter_not_a_string' at line 26. Aborted with 'ENotAString' -- 'BAEAAAAAAAAAAgAAAAAAAAADAAAAAAAAAAQAAAAAAAAA'
---

0 comments on commit 0e4914d

Please sign in to comment.