feat: improve errors in the CLI

CHANGELOG.md

@@ -15,6 +15,7 @@
 * [BREAKING] Removed the `testing` feature from the CLI (#670).
 * Added per transaction prover support to the web client (#674).
 * [BREAKING] Added `BlockNumber` structure (#677).
+* Improved CLI error messages (#682).
 
 ### Fixes
 

Cargo.toml

@@ -23,6 +23,7 @@ miden-lib = { git = "https://github.com/0xPolygonMiden/miden-base", branch = "ne
 miden-objects = { git = "https://github.com/0xPolygonMiden/miden-base", branch = "next", default-features = false }
 miden-tx = { git = "https://github.com/0xPolygonMiden/miden-base", branch = "next", default-features = false, features = ["async"] }
 miden-proving-service-client = { git = "https://github.com/0xPolygonMiden/miden-base", branch = "next", features = ["tx-prover"] }
+miette = { version = "7.2", features = ["fancy"] }
 rand = { version = "0.8" }
 serde = { version = "1.0", features = ["derive"] }
 thiserror = { version = "2.0", default-features = false }

bin/miden-cli/Cargo.toml

@@ -38,8 +38,11 @@ figment = { version = "0.10", features = ["toml", "env"] }
 miden-client = { version = "0.6", path = "../../crates/rust-client", features = ["sqlite", "tonic"] }
 miden-lib = { workspace = true }
 miden-proving-service-client = { workspace = true , features = ["std", "tx-prover"]}
+miden-objects = { workspace = true }
+miette ={ workspace = true }
 rand = { workspace = true }
 serde = { version = "1.0", features = ["derive"] }
+thiserror ={ workspace = true }
 tokio = { workspace = true }
 toml = { version = "0.8" }
 tracing = { workspace = true }

bin/miden-cli/src/commands/account.rs

@@ -9,6 +9,7 @@ use miden_client::{
 use crate::{
     config::CliConfig,
     create_dynamic_table,
+    errors::CliError,
     utils::{load_config_file, load_faucet_details_map, parse_account_id, update_config},
     CLIENT_BINARY_NAME,
 };
@@ -35,7 +36,7 @@ pub struct AccountCmd {
 }
 
 impl AccountCmd {
-    pub async fn execute<R: FeltRng>(&self, client: Client<R>) -> Result<(), String> {
+    pub async fn execute<R: FeltRng>(&self, client: Client<R>) -> Result<(), CliError> {
         match self {
             AccountCmd {
                 list: false,
@@ -58,8 +59,12 @@ impl AccountCmd {
                         let default_account = if id == "none" {
                             None
                         } else {
-                            let account_id: AccountId = AccountId::from_hex(id)
-                                .map_err(|_| "Input number was not a valid Account Id")?;
+                            let account_id: AccountId = AccountId::from_hex(id).map_err(|err| {
+                                CliError::AccountId(
+                                    err,
+                                    "Input number was not a valid Account ID".to_string(),
+                                )
+                            })?;
 
                             // Check whether we're tracking that account
                             let (account, _) =
@@ -90,7 +95,7 @@ impl AccountCmd {
 // LIST ACCOUNTS
 // ================================================================================================
 
-async fn list_accounts<R: FeltRng>(client: Client<R>) -> Result<(), String> {
+async fn list_accounts<R: FeltRng>(client: Client<R>) -> Result<(), CliError> {
     let accounts = client.get_account_headers().await?;
 
     let mut table =
@@ -119,11 +124,11 @@ async fn list_accounts<R: FeltRng>(client: Client<R>) -> Result<(), String> {
 pub async fn show_account<R: FeltRng>(
     client: Client<R>,
     account_id: AccountId,
-) -> Result<(), String> {
+) -> Result<(), CliError> {
     let account: Account = client
         .get_account(account_id)
         .await?
-        .ok_or(format!("Account with ID {account_id} not found"))?
+        .ok_or(CliError::Custom(format!("Account with ID {account_id} not found")))?
         .into();
 
     let mut table = create_dynamic_table(&[
@@ -191,7 +196,9 @@ pub async fn show_account<R: FeltRng>(
         ]);
 
         for (idx, entry) in account_storage.slots().iter().enumerate() {
-            let item = account_storage.get_item(idx as u8).map_err(|e| e.to_string())?;
+            let item = account_storage
+                .get_item(idx as u8)
+                .map_err(|err| CliError::Account(err, "Index out of bounds".to_string()))?;
 
             // Last entry is reserved so I don't think the user cares about it. Also, to keep the
             // output smaller, if the [StorageSlot] is a value and it's 0 we assume it's not
@@ -217,7 +224,7 @@ pub async fn show_account<R: FeltRng>(
 // HELPERS
 // ================================================================================================
 
-fn account_type_display_name(account_id: &AccountId) -> Result<String, String> {
+fn account_type_display_name(account_id: &AccountId) -> Result<String, CliError> {
     Ok(match account_id.account_type() {
         AccountType::FungibleFaucet => {
             let faucet_details_map = load_faucet_details_map()?;
@@ -232,19 +239,20 @@ fn account_type_display_name(account_id: &AccountId) -> Result<String, String> {
 }
 
 /// Loads config file and displays current default account ID.
-fn display_default_account_id() -> Result<(), String> {
+fn display_default_account_id() -> Result<(), CliError> {
     let (cli_config, _) = load_config_file()?;
 
-    let default_account = cli_config.default_account_id.ok_or(
-        "No default account found in the CLI options from the client config file.".to_string(),
-    )?;
+    let default_account = cli_config.default_account_id.ok_or(CliError::Config(
+        "Default account".to_string(),
+        "No default account found in the configuration file".to_string(),
+    ))?;
     println!("Current default account ID: {default_account}");
     Ok(())
 }
 
 /// Sets the provided account ID as the default account ID if provided. Unsets the current default
 /// account ID if `None` is provided.
-pub(crate) fn set_default_account(account_id: Option<AccountId>) -> Result<(), String> {
+pub(crate) fn set_default_account(account_id: Option<AccountId>) -> Result<(), CliError> {
     // load config
     let (mut current_config, config_path) = load_config_file()?;
 
@@ -259,7 +267,7 @@ pub(crate) fn set_default_account(account_id: Option<AccountId>) -> Result<(), S
 pub(crate) fn maybe_set_default_account(
     current_config: &mut CliConfig,
     account_id: AccountId,
-) -> Result<(), String> {
+) -> Result<(), CliError> {
     if current_config.default_account_id.is_some() {
         return Ok(());
     }

bin/miden-cli/src/commands/export.rs

@@ -5,7 +5,7 @@ use miden_client::{
 };
 use tracing::info;
 
-use crate::{get_output_note_with_id_prefix, utils::parse_account_id, Parser};
+use crate::{errors::CliError, get_output_note_with_id_prefix, utils::parse_account_id, Parser};
 
 #[derive(Debug, Parser, Clone)]
 #[clap(about = "Export client output notes")]
@@ -49,14 +49,16 @@ impl From<ExportType> for NoteExportType {
 }
 
 impl ExportCmd {
-    pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), String> {
+    pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
         if self.account {
             export_account(&client, self.id.as_str(), self.filename.clone()).await?;
         } else if let Some(export_type) = &self.export_type {
             export_note(&mut client, self.id.as_str(), self.filename.clone(), export_type.clone())
                 .await?;
         } else {
-            return Err("Export type is required when exporting a note".to_string());
+            return Err(CliError::Export(
+                "Export type is required when exporting a note".to_string(),
+            ));
         }
         Ok(())
     }
@@ -69,31 +71,31 @@ async fn export_account<R: FeltRng>(
     client: &Client<R>,
     account_id: &str,
     filename: Option<PathBuf>,
-) -> Result<File, String> {
+) -> Result<File, CliError> {
     let account_id = parse_account_id(client, account_id).await?;
 
     let account = client
         .get_account(account_id)
         .await?
-        .ok_or(format!("Account with ID {account_id} not found"))?;
+        .ok_or(CliError::Export(format!("Account with ID {account_id} not found")))?;
     let account_seed = account.seed().cloned();
 
     let auth = client
         .get_account_auth(account_id)
         .await?
-        .ok_or(format!("Account with ID {account_id} not found"))?;
+        .ok_or(CliError::Export(format!("Account with ID {account_id} not found")))?;
 
     let account_data = AccountData::new(account.into(), account_seed, auth);
 
     let file_path = if let Some(filename) = filename {
         filename
     } else {
-        let current_dir = std::env::current_dir().map_err(|err| err.to_string())?;
+        let current_dir = std::env::current_dir()?;
         current_dir.join(format!("{}.mac", account_id))
     };
 
     info!("Writing file to {}", file_path.to_string_lossy());
-    let mut file = File::create(file_path).map_err(|err| err.to_string())?;
+    let mut file = File::create(file_path)?;
     account_data.write_into(&mut file);
 
     println!("Succesfully exported account {}", account_id);
@@ -108,10 +110,10 @@ async fn export_note(
     note_id: &str,
     filename: Option<PathBuf>,
     export_type: ExportType,
-) -> Result<File, String> {
+) -> Result<File, CliError> {
     let note_id = get_output_note_with_id_prefix(client, note_id)
         .await
-        .map_err(|err| err.to_string())?
+        .map_err(|err| CliError::Export(err.to_string()))?
         .id();
 
     let output_note = client
@@ -120,18 +122,20 @@ async fn export_note(
         .pop()
         .expect("should have an output note");
 
-    let note_file = output_note.into_note_file(export_type.into())?;
+    let note_file = output_note
+        .into_note_file(export_type.into())
+        .map_err(|err| CliError::Export(err.to_string()))?;
 
     let file_path = if let Some(filename) = filename {
         filename
     } else {
-        let current_dir = std::env::current_dir().map_err(|err| err.to_string())?;
+        let current_dir = std::env::current_dir()?;
         current_dir.join(format!("{}.mno", note_id.inner()))
     };
 
     info!("Writing file to {}", file_path.to_string_lossy());
-    let mut file = File::create(file_path).map_err(|err| err.to_string())?;
-    file.write_all(&note_file.to_bytes()).map_err(|err| err.to_string())?;
+    let mut file = File::create(file_path)?;
+    file.write_all(&note_file.to_bytes()).map_err(CliError::IO)?;
 
     println!("Succesfully exported note {}", note_id);
     Ok(file)

bin/miden-cli/src/commands/import.rs

@@ -13,7 +13,9 @@ use miden_client::{
 };
 use tracing::info;
 
-use crate::{commands::account::maybe_set_default_account, utils::load_config_file, Parser};
+use crate::{
+    commands::account::maybe_set_default_account, errors::CliError, utils::load_config_file, Parser,
+};
 
 #[derive(Debug, Parser, Clone)]
 #[clap(about = "Import client objects. It is capable of importing notes and accounts.")]
@@ -27,27 +29,25 @@ pub struct ImportCmd {
 }
 
 impl ImportCmd {
-    pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), String> {
+    pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
         validate_paths(&self.filenames)?;
         let (mut current_config, _) = load_config_file()?;
         for filename in &self.filenames {
             let note_file = read_note_file(filename.clone());
 
             if let Ok(note_file) = note_file {
-                let note_id = client.import_note(note_file).await.map_err(|err| err.to_string())?;
+                let note_id = client.import_note(note_file).await?;
                 println!("Succesfully imported note {}", note_id.inner());
             } else {
                 info!(
                     "Attempting to import account data from {}...",
-                    fs::canonicalize(filename).map_err(|err| err.to_string())?.as_path().display()
+                    fs::canonicalize(filename)?.as_path().display()
                 );
-                let account_data_file_contents =
-                    fs::read(filename).map_err(|err| err.to_string())?;
+                let account_data_file_contents = fs::read(filename)?;
 
                 let account_id =
                     import_account(&mut client, &account_data_file_contents, self.overwrite)
-                        .await
-                        .map_err(|err| err.to_string())?;
+                        .await?;
 
                 println!("Successfully imported account {}", account_id);
 
@@ -67,7 +67,7 @@ async fn import_account(
     client: &mut Client<impl FeltRng>,
     account_data_file_contents: &[u8],
     overwrite: bool,
-) -> Result<AccountId, ClientError> {
+) -> Result<AccountId, CliError> {
     let account_data = AccountData::read_from_bytes(account_data_file_contents)
         .map_err(ClientError::DataDeserializationError)?;
     let account_id = account_data.account.id();
@@ -87,25 +87,27 @@ async fn import_account(
 // IMPORT NOTE
 // ================================================================================================
 
-fn read_note_file(filename: PathBuf) -> Result<NoteFile, String> {
+fn read_note_file(filename: PathBuf) -> Result<NoteFile, CliError> {
     let mut contents = vec![];
-    let mut _file = File::open(filename)
-        .and_then(|mut f| f.read_to_end(&mut contents))
-        .map_err(|err| err.to_string());
+    let mut _file = File::open(filename).and_then(|mut f| f.read_to_end(&mut contents))?;
 
-    NoteFile::read_from_bytes(&contents).map_err(|err| err.to_string())
+    NoteFile::read_from_bytes(&contents)
+        .map_err(|err| CliError::Client(ClientError::DataDeserializationError(err)))
 }
 
 // HELPERS
 // ================================================================================================
 
 /// Checks that all files exist, otherwise returns an error. It also ensures that all files have a
 /// specific extension.
-fn validate_paths(paths: &[PathBuf]) -> Result<(), String> {
+fn validate_paths(paths: &[PathBuf]) -> Result<(), CliError> {
     let invalid_path = paths.iter().find(|path| !path.exists());
 
     if let Some(path) = invalid_path {
-        Err(format!("The path `{}` does not exist", path.to_string_lossy()).to_string())
+        Err(CliError::Custom(format!(
+            "The path `{}` does not exist",
+            path.to_string_lossy()
+        )))
     } else {
         Ok(())
     }