CLI: Error message cleanup (#8804)

automerge

cli/src/cli.rs

@@ -18,7 +18,10 @@ use solana_clap_utils::{
     input_parsers::*, input_validators::*, keypair::signer_from_path, offline::SIGN_ONLY_ARG,
     ArgConstant,
 };
-use solana_client::{client_error::ClientError, rpc_client::RpcClient};
+use solana_client::{
+    client_error::{ClientErrorKind, Result as ClientResult},
+    rpc_client::RpcClient,
+};
 #[cfg(not(test))]
 use solana_faucet::faucet::request_airdrop_transaction;
 #[cfg(test)]
@@ -47,14 +50,15 @@ use solana_stake_program::{
 use solana_storage_program::storage_instruction::StorageAccountType;
 use solana_vote_program::vote_state::VoteAuthorize;
 use std::{
+    error,
     fs::File,
     io::{Read, Write},
     net::{IpAddr, SocketAddr},
     sync::Arc,
     thread::sleep,
     time::Duration,
-    {error, fmt},
 };
+use thiserror::Error;
 use url::Url;
 
 pub type CliSigners = Vec<Box<dyn Signer>>;
@@ -409,46 +413,34 @@ pub struct CliCommandInfo {
     pub signers: CliSigners,
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, Error, PartialEq)]
 pub enum CliError {
+    #[error("bad parameter: {0}")]
     BadParameter(String),
+    #[error("command not recognized: {0}")]
     CommandNotRecognized(String),
+    #[error("insuficient funds for fee")]
     InsufficientFundsForFee,
+    #[error(transparent)]
     InvalidNonce(CliNonceError),
+    #[error("dynamic program error: {0}")]
     DynamicProgramError(String),
+    #[error("rpc request error: {0}")]
     RpcRequestError(String),
+    #[error("keypair file not found: {0}")]
     KeypairFileNotFound(String),
 }
 
-impl fmt::Display for CliError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "invalid")
-    }
-}
-
-impl error::Error for CliError {
-    fn description(&self) -> &str {
-        "invalid"
-    }
-
-    fn cause(&self) -> Option<&dyn error::Error> {
-        // Generic error, underlying cause isn't tracked.
-        None
-    }
-}
-
 impl From<Box<dyn error::Error>> for CliError {
     fn from(error: Box<dyn error::Error>) -> Self {
-        CliError::DynamicProgramError(format!("{:?}", error))
+        CliError::DynamicProgramError(error.to_string())
     }
 }
 
 impl From<CliNonceError> for CliError {
     fn from(error: CliNonceError) -> Self {
         match error {
-            CliNonceError::Client(client_error) => {
-                Self::RpcRequestError(format!("{:?}", client_error))
-            }
+            CliNonceError::Client(client_error) => Self::RpcRequestError(client_error),
             _ => Self::InvalidNonce(error),
         }
     }
@@ -721,7 +713,7 @@ pub fn parse_command(
                 .parse()
                 .or_else(|err| {
                     Err(CliError::BadParameter(format!(
-                        "Invalid faucet port: {:?}",
+                        "Invalid faucet port: {}",
                         err
                     )))
                 })?;
@@ -729,7 +721,7 @@ pub fn parse_command(
             let faucet_host = if let Some(faucet_host) = matches.value_of("faucet_host") {
                 Some(solana_net_utils::parse_host(faucet_host).or_else(|err| {
                     Err(CliError::BadParameter(format!(
-                        "Invalid faucet host: {:?}",
+                        "Invalid faucet host: {}",
                         err
                     )))
                 })?)
@@ -1141,13 +1133,13 @@ fn process_confirm(rpc_client: &RpcClient, signature: &Signature) -> ProcessResu
             if let Some(result) = status {
                 match result {
                     Ok(_) => Ok("Confirmed".to_string()),
-                    Err(err) => Ok(format!("Transaction failed with error {:?}", err)),
+                    Err(err) => Ok(format!("Transaction failed with error: {}", err)),
                 }
             } else {
                 Ok("Not found".to_string())
             }
         }
-        Err(err) => Err(CliError::RpcRequestError(format!("Unable to confirm: {:?}", err)).into()),
+        Err(err) => Err(CliError::RpcRequestError(format!("Unable to confirm: {}", err)).into()),
     }
 }
 
@@ -2118,18 +2110,18 @@ pub fn request_and_confirm_airdrop(
     log_instruction_custom_error::<SystemError>(result)
 }
 
-pub fn log_instruction_custom_error<E>(result: Result<String, ClientError>) -> ProcessResult
+pub fn log_instruction_custom_error<E>(result: ClientResult<String>) -> ProcessResult
 where
     E: 'static + std::error::Error + DecodeError<E> + FromPrimitive,
 {
     match result {
         Err(err) => {
-            if let ClientError::TransactionError(TransactionError::InstructionError(
+            if let ClientErrorKind::TransactionError(TransactionError::InstructionError(
                 _,
                 InstructionError::CustomError(code),
-            )) = err
+            )) = err.kind()
             {
-                if let Some(specific_error) = E::decode_custom_error_to_enum(code) {
+                if let Some(specific_error) = E::decode_custom_error_to_enum(*code) {
                     error!("{}::{:?}", E::type_of(), specific_error);
                     eprintln!(
                         "Program Error ({}::{:?}): {}",
@@ -3332,7 +3324,7 @@ mod tests {
         assert_eq!(
             process_command(&config).unwrap(),
             format!(
-                "Transaction failed with error {:?}",
+                "Transaction failed with error: {}",
                 TransactionError::AccountInUse
             )
         );

cli/src/cluster_query.rs

@@ -1011,7 +1011,7 @@ pub fn process_live_slots(url: &str) -> ProcessResult {
                 current = Some(new_info);
             }
             Err(err) => {
-                eprintln!("disconnected: {:?}", err);
+                eprintln!("disconnected: {}", err);
                 break;
             }
         }

cli/src/main.rs

@@ -1,7 +1,9 @@
 use clap::{crate_description, crate_name, AppSettings, Arg, ArgGroup, ArgMatches, SubCommand};
 use console::style;
 
-use solana_clap_utils::{input_validators::is_url, keypair::SKIP_SEED_PHRASE_VALIDATION_ARG};
+use solana_clap_utils::{
+    input_validators::is_url, keypair::SKIP_SEED_PHRASE_VALIDATION_ARG, DisplayError,
+};
 use solana_cli::{
     cli::{app, parse_command, process_command, CliCommandInfo, CliConfig, CliSigners},
     display::{println_name_value, println_name_value_or},
@@ -230,6 +232,10 @@ fn main() -> Result<(), Box<dyn error::Error>> {
     )
     .get_matches();
 
+    do_main(&matches).map_err(|err| DisplayError::new_as_boxed(err).into())
+}
+
+fn do_main(matches: &ArgMatches<'_>) -> Result<(), Box<dyn error::Error>> {
     if parse_settings(&matches)? {
         let wallet_manager = maybe_wallet_manager()?;
 
@@ -237,6 +243,6 @@ fn main() -> Result<(), Box<dyn error::Error>> {
         config.signers = signers.iter().map(|s| s.as_ref()).collect();
         let result = process_command(&config)?;
         println!("{}", result);
-    }
+    };
     Ok(())
 }

cli/src/nonce.rs

@@ -238,7 +238,7 @@ pub fn get_account(
 ) -> Result<Account, CliNonceError> {
     rpc_client
         .get_account(nonce_pubkey)
-        .map_err(|e| CliNonceError::Client(format!("{:?}", e)))
+        .map_err(|e| CliNonceError::Client(format!("{}", e)))
         .and_then(|a| match account_identity_ok(&a) {
             Ok(()) => Ok(a),
             Err(e) => Err(e),

cli/src/stake.rs

@@ -1300,7 +1300,7 @@ pub fn process_show_stake_account(
             Ok("".to_string())
         }
         Err(err) => Err(CliError::RpcRequestError(format!(
-            "Account data could not be deserialized to stake state: {:?}",
+            "Account data could not be deserialized to stake state: {}",
             err
         ))
         .into()),
@@ -1396,11 +1396,11 @@ pub fn process_delegate_stake(
             }
         };
 
-        if sanity_check_result.is_err() {
+        if let Err(err) = &sanity_check_result {
             if !force {
                 sanity_check_result?;
             } else {
-                println!("--force supplied, ignoring: {:?}", sanity_check_result);
+                println!("--force supplied, ignoring: {}", err);
             }
         }
     }

cli/src/storage.rs

@@ -266,7 +266,7 @@ pub fn process_show_storage_account(
 
     use solana_storage_program::storage_contract::StorageContract;
     let storage_contract: StorageContract = account.state().map_err(|err| {
-        CliError::RpcRequestError(format!("Unable to deserialize storage account: {:?}", err))
+        CliError::RpcRequestError(format!("Unable to deserialize storage account: {}", err))
     })?;
     println!("{:#?}", storage_contract);
     println!("Account Lamports: {}", account.lamports);

cli/src/validator_info.rs

@@ -274,7 +274,7 @@ pub fn process_set_validator_info(
                 println!("--force supplied, ignoring: {:?}", result);
             } else {
                 result.map_err(|err| {
-                    CliError::BadParameter(format!("Invalid validator keybase username: {:?}", err))
+                    CliError::BadParameter(format!("Invalid validator keybase username: {}", err))
                 })?;
             }
         }