refactor: concrete error type for Identity::new

src/dfx-core/src/error/identity/create_new_identity.rs

@@ -1,6 +1,7 @@
 use crate::error::fs::FsError;
 use crate::error::identity::convert_mnemonic_to_key::ConvertMnemonicToKeyError;
 use crate::error::identity::generate_key::GenerateKeyError;
+use crate::error::identity::load_pem_from_file::LoadPemFromFileError;
 use crate::error::identity::remove_identity::RemoveIdentityError;
 use crate::error::identity::save_pem::SavePemError;
 use crate::error::identity::IdentityError;
@@ -36,7 +37,7 @@ pub enum CreateNewIdentityError {
     IdentityAlreadyExists(),
 
     #[error("Failed to load pem file: {0}")]
-    LoadPemFromFileFailed(IdentityError),
+    LoadPemFromFileFailed(LoadPemFromFileError),
 
     #[error("Failed to remove identity: {0}")]
     RemoveIdentityFailed(RemoveIdentityError),

src/dfx-core/src/error/identity/export_identity.rs

@@ -1,17 +1,19 @@
+use crate::error::identity::get_identity_config_or_default::GetIdentityConfigOrDefaultError;
+use crate::error::identity::load_pem::LoadPemError;
 use crate::error::identity::IdentityError;
 use std::string::FromUtf8Error;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum ExportIdentityError {
     #[error("Failed to get identity config: {0}")]
-    GetIdentityConfigFailed(IdentityError),
+    GetIdentityConfigFailed(GetIdentityConfigOrDefaultError),
 
     #[error("The specified identity does not exist: {0}")]
     IdentityDoesNotExist(IdentityError),
 
     #[error("Failed to load pem file: {0}")]
-    LoadPemFailed(IdentityError),
+    LoadPemFailed(LoadPemError),
 
     #[error("Could not translate pem file to text: {0}")]
     TranslatePemContentToTextFailed(FromUtf8Error),

src/dfx-core/src/error/identity/get_identity_config_or_default.rs

@@ -0,0 +1,8 @@
+use crate::error::structured_file::StructuredFileError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum GetIdentityConfigOrDefaultError {
+    #[error("Failed to load configuration for identity '{0}': {1}")]
+    LoadIdentityConfigurationFailed(String, StructuredFileError),
+}

src/dfx-core/src/error/identity/instantiate_identity_from_name.rs

@@ -0,0 +1,15 @@
+use crate::error::identity::load_identity::LoadIdentityError;
+use crate::error::identity::IdentityError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum InstantiateIdentityFromNameError {
+    #[error("Failed to get principal of identity: {0}")]
+    GetIdentityPrincipalFailed(String),
+
+    #[error("Failed to load identity: {0}")]
+    LoadIdentityFailed(LoadIdentityError),
+
+    #[error("Identity must exist: {0}")]
+    RequireIdentityExistsFailed(IdentityError),
+}

src/dfx-core/src/error/identity/load_identity.rs

@@ -0,0 +1,12 @@
+use crate::error::identity::get_identity_config_or_default::GetIdentityConfigOrDefaultError;
+use crate::error::identity::new_identity::NewIdentityError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum LoadIdentityError {
+    #[error("Failed to get identity config: {0}")]
+    GetIdentityConfigOrDefaultFailed(GetIdentityConfigOrDefaultError),
+
+    #[error("Failed to instantiate identity: {0}")]
+    NewIdentityFailed(NewIdentityError),
+}

src/dfx-core/src/error/identity/load_pem.rs

@@ -0,0 +1,12 @@
+use crate::error::identity::load_pem_from_file::LoadPemFromFileError;
+use crate::error::keyring::KeyringError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum LoadPemError {
+    #[error("Failed to load PEM file from file : {0}")]
+    LoadFromFileFailed(LoadPemFromFileError),
+
+    #[error("Failed to load PEM file from keyring for identity '{0}': {1}")]
+    LoadFromKeyringFailed(Box<String>, KeyringError),
+}

src/dfx-core/src/error/identity/load_pem_from_file.rs

@@ -0,0 +1,13 @@
+use crate::error::encryption::EncryptionError;
+use crate::error::fs::FsError;
+use std::path::PathBuf;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum LoadPemFromFileError {
+    #[error("Failed to decrypt PEM file: {0}")]
+    DecryptPemFileFailed(PathBuf, EncryptionError),
+
+    #[error("Failed to read pem file: {0}")]
+    ReadPemFileFailed(FsError),
+}

src/dfx-core/src/error/identity/load_pem_identity.rs

@@ -0,0 +1,8 @@
+use ic_agent::identity::PemError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum LoadPemIdentityError {
+    #[error("Cannot read identity file '{0}': {1:#}")]
+    ReadIdentityFileFailed(String, Box<PemError>),
+}

src/dfx-core/src/error/identity/mod.rs

@@ -2,8 +2,16 @@ pub mod convert_mnemonic_to_key;
 pub mod create_new_identity;
 pub mod export_identity;
 pub mod generate_key;
+pub mod get_identity_config_or_default;
 pub mod get_legacy_credentials_pem_path;
 pub mod initialize_identity_manager;
+pub mod instantiate_identity_from_name;
+pub mod load_identity;
+pub mod load_pem;
+pub mod load_pem_from_file;
+pub mod load_pem_identity;
+pub mod new_hardware_identity;
+pub mod new_identity;
 pub mod new_identity_manager;
 pub mod remove_identity;
 pub mod rename_identity;
@@ -13,20 +21,15 @@ pub mod write_pem_to_file;
 use crate::error::config::ConfigError;
 use crate::error::encryption::EncryptionError;
 use crate::error::fs::FsError;
-use crate::error::keyring::KeyringError;
 use crate::error::structured_file::StructuredFileError;
 use crate::error::wallet_config::WalletConfigError;
 use ic_agent::export::PrincipalError;
 use ic_agent::identity::PemError;
-use ic_identity_hsm::HardwareIdentityError;
 use std::path::PathBuf;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum IdentityError {
-    #[error("Failed to decrypt PEM file: {0}")]
-    DecryptPemFileFailed(PathBuf, EncryptionError),
-
     #[error("Failed to ensure identity configuration directory exists: {0}")]
     EnsureIdentityConfigurationDirExistsFailed(FsError),
 
@@ -39,30 +42,12 @@ pub enum IdentityError {
     #[error("Failed to get shared network data directory: {0}")]
     GetSharedNetworkDataDirectoryFailed(ConfigError),
 
-    #[error("Failed to get principal of identity: {0}")]
-    GetIdentityPrincipalFailed(String),
-
     #[error("Identity {0} does not exist at '{1}'.")]
     IdentityDoesNotExist(String, PathBuf),
 
-    #[error("Failed to instantiate hardware identity for identity '{0}': {1}.")]
-    InstantiateHardwareIdentityFailed(String, Box<HardwareIdentityError>),
-
-    #[error("Failed to load configuration for identity '{0}': {1}")]
-    LoadIdentityConfigurationFailed(String, StructuredFileError),
-
-    #[error("Failed to load PEM file from keyring for identity '{0}': {1}")]
-    LoadPemFromKeyringFailed(Box<String>, KeyringError),
-
     #[error("Failed to read principal from id '{0}': {1}")]
     ParsePrincipalFromIdFailed(String, PrincipalError),
 
-    #[error("Cannot read identity file '{0}': {1:#}")]
-    ReadIdentityFileFailed(String, Box<PemError>),
-
-    #[error("Failed to read pem file: {0}")]
-    ReadPemFileFailed(FsError),
-
     #[error("Failed to rename '{0}' to '{1}' in the global wallet config: {2}")]
     RenameWalletFailed(Box<String>, Box<String>, WalletConfigError),
 

src/dfx-core/src/error/identity/new_hardware_identity.rs

@@ -0,0 +1,8 @@
+use ic_identity_hsm::HardwareIdentityError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum NewHardwareIdentityError {
+    #[error("Failed to instantiate hardware identity for identity '{0}': {1}.")]
+    InstantiateHardwareIdentityFailed(String, Box<HardwareIdentityError>),
+}

src/dfx-core/src/error/identity/new_identity.rs

@@ -0,0 +1,16 @@
+use crate::error::identity::load_pem::LoadPemError;
+use crate::error::identity::load_pem_identity::LoadPemIdentityError;
+use crate::error::identity::new_hardware_identity::NewHardwareIdentityError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum NewIdentityError {
+    #[error("Failed to load PEM: {0}")]
+    LoadPemFailed(LoadPemError),
+
+    #[error("Failed to load PEM identity: {0}")]
+    LoadPemIdentityFailed(LoadPemIdentityError),
+
+    #[error("Failed to instantiate hardware identity: {0}")]
+    NewHardwareIdentityFailed(NewHardwareIdentityError),
+}

src/dfx-core/src/error/identity/rename_identity.rs

@@ -1,4 +1,6 @@
 use crate::error::fs::FsError;
+use crate::error::identity::get_identity_config_or_default::GetIdentityConfigOrDefaultError;
+use crate::error::identity::load_pem::LoadPemError;
 use crate::error::identity::save_pem::SavePemError;
 use crate::error::identity::IdentityError;
 use crate::error::keyring::KeyringError;
@@ -10,7 +12,7 @@ pub enum RenameIdentityError {
     CannotCreateAnonymousIdentity(),
 
     #[error("Failed to get identity config: {0}")]
-    GetIdentityConfigFailed(IdentityError),
+    GetIdentityConfigFailed(GetIdentityConfigOrDefaultError),
 
     #[error("Identity already exists.")]
     IdentityAlreadyExists(),
@@ -19,7 +21,7 @@ pub enum RenameIdentityError {
     IdentityDoesNotExist(IdentityError),
 
     #[error("Failed to load pem: {0}")]
-    LoadPemFailed(IdentityError),
+    LoadPemFailed(LoadPemError),
 
     #[error("Failed to map wallets to renamed identity: {0}")]
     MapWalletsToRenamedIdentityFailed(IdentityError /*MapWalletsToRenamedIdentityError*/),

src/dfx-core/src/identity/identity_manager.rs

@@ -17,13 +17,20 @@ use crate::error::identity::export_identity::ExportIdentityError;
 use crate::error::identity::export_identity::ExportIdentityError::TranslatePemContentToTextFailed;
 use crate::error::identity::generate_key::GenerateKeyError;
 use crate::error::identity::generate_key::GenerateKeyError::GenerateFreshSecp256k1KeyFailed;
+use crate::error::identity::get_identity_config_or_default::GetIdentityConfigOrDefaultError;
+use crate::error::identity::get_identity_config_or_default::GetIdentityConfigOrDefaultError::LoadIdentityConfigurationFailed;
 use crate::error::identity::get_legacy_credentials_pem_path::GetLegacyCredentialsPemPathError;
 use crate::error::identity::get_legacy_credentials_pem_path::GetLegacyCredentialsPemPathError::GetLegacyPemPathFailed;
 use crate::error::identity::initialize_identity_manager::InitializeIdentityManagerError;
 use crate::error::identity::initialize_identity_manager::InitializeIdentityManagerError::{
     CreateIdentityDirectoryFailed, GenerateKeyFailed, MigrateLegacyIdentityFailed,
     WritePemToFileFailed,
 };
+use crate::error::identity::instantiate_identity_from_name::InstantiateIdentityFromNameError;
+use crate::error::identity::instantiate_identity_from_name::InstantiateIdentityFromNameError::{
+    GetIdentityPrincipalFailed, LoadIdentityFailed,
+};
+use crate::error::identity::load_identity::LoadIdentityError;
 use crate::error::identity::new_identity_manager::NewIdentityManagerError;
 use crate::error::identity::new_identity_manager::NewIdentityManagerError::LoadIdentityManagerConfigurationFailed;
 use crate::error::identity::remove_identity::RemoveIdentityError;
@@ -39,7 +46,6 @@ use crate::error::identity::rename_identity::RenameIdentityError::{
 use crate::error::identity::IdentityError;
 use crate::error::identity::IdentityError::{
     EnsureIdentityConfigurationDirExistsFailed, GenerateFreshEncryptionConfigurationFailed,
-    GetIdentityPrincipalFailed, LoadIdentityConfigurationFailed, SaveIdentityConfigurationFailed,
 };
 use crate::error::structured_file::StructuredFileError;
 use crate::foundation::get_user_home;
@@ -238,7 +244,7 @@ impl IdentityManager {
     pub fn instantiate_selected_identity(
         &mut self,
         log: &Logger,
-    ) -> Result<Box<DfxIdentity>, IdentityError> {
+    ) -> Result<Box<DfxIdentity>, InstantiateIdentityFromNameError> {
         let name = self.selected_identity.clone();
         self.instantiate_identity_from_name(name.as_str(), log)
     }
@@ -248,12 +254,16 @@ impl IdentityManager {
         &mut self,
         identity_name: &str,
         log: &Logger,
-    ) -> Result<Box<DfxIdentity>, IdentityError> {
+    ) -> Result<Box<DfxIdentity>, InstantiateIdentityFromNameError> {
         let identity = match identity_name {
             ANONYMOUS_IDENTITY_NAME => Box::new(DfxIdentity::anonymous()),
             identity_name => {
-                self.require_identity_exists(log, identity_name)?;
-                Box::new(self.load_identity(identity_name, log)?)
+                self.require_identity_exists(log, identity_name)
+                    .map_err(InstantiateIdentityFromNameError::RequireIdentityExistsFailed)?;
+                Box::new(
+                    self.load_identity(identity_name, log)
+                        .map_err(LoadIdentityFailed)?,
+                )
             }
         };
         use ic_agent::identity::Identity;
@@ -262,9 +272,12 @@ impl IdentityManager {
         Ok(identity)
     }
 
-    fn load_identity(&self, name: &str, log: &Logger) -> Result<DfxIdentity, IdentityError> {
-        let config = self.get_identity_config_or_default(name)?;
+    fn load_identity(&self, name: &str, log: &Logger) -> Result<DfxIdentity, LoadIdentityError> {
+        let config = self
+            .get_identity_config_or_default(name)
+            .map_err(LoadIdentityError::GetIdentityConfigOrDefaultFailed)?;
         DfxIdentity::new(name, config, self.file_locations(), log)
+            .map_err(LoadIdentityError::NewIdentityFailed)
     }
 
     /// Create a new identity (name -> generated key)
@@ -641,7 +654,7 @@ impl IdentityManager {
     pub fn get_identity_config_or_default(
         &self,
         identity: &str,
-    ) -> Result<IdentityConfiguration, IdentityError> {
+    ) -> Result<IdentityConfiguration, GetIdentityConfigOrDefaultError> {
         let json_path = self.get_identity_json_path(identity);
         if json_path.exists() {
             load_json_file(&json_path)
@@ -758,7 +771,7 @@ pub(super) fn save_identity_configuration(
     trace!(log, "Writing identity configuration to {}", path.display());
     ensure_parent_dir_exists(path).map_err(EnsureIdentityConfigurationDirExistsFailed)?;
 
-    save_json_file(path, &config).map_err(SaveIdentityConfigurationFailed)
+    save_json_file(path, &config).map_err(IdentityError::SaveIdentityConfigurationFailed)
 }
 
 /// Removes the file if it exists.

src/dfx-core/src/identity/mod.rs

@@ -3,10 +3,14 @@
 //! Wallets are a map of network-identity, but don't have their own types or manager
 //! type.
 use crate::config::directories::{get_config_dfx_dir_path, get_shared_network_data_directory};
+use crate::error::identity::load_pem_identity::LoadPemIdentityError;
+use crate::error::identity::load_pem_identity::LoadPemIdentityError::ReadIdentityFileFailed;
+use crate::error::identity::new_hardware_identity::NewHardwareIdentityError;
+use crate::error::identity::new_hardware_identity::NewHardwareIdentityError::InstantiateHardwareIdentityFailed;
+use crate::error::identity::new_identity::NewIdentityError;
 use crate::error::identity::IdentityError;
 use crate::error::identity::IdentityError::{
-    GetConfigDirectoryFailed, GetSharedNetworkDataDirectoryFailed,
-    InstantiateHardwareIdentityFailed, ReadIdentityFileFailed, RenameWalletFailed,
+    GetConfigDirectoryFailed, GetSharedNetworkDataDirectoryFailed, RenameWalletFailed,
 };
 use crate::error::wallet_config::WalletConfigError;
 use crate::error::wallet_config::WalletConfigError::{
@@ -74,7 +78,11 @@ impl Identity {
         }
     }
 
-    fn basic(name: &str, pem_content: &[u8], was_encrypted: bool) -> Result<Self, IdentityError> {
+    fn basic(
+        name: &str,
+        pem_content: &[u8],
+        was_encrypted: bool,
+    ) -> Result<Self, LoadPemIdentityError> {
         let inner = Box::new(
             BasicIdentity::from_pem(pem_content)
                 .map_err(|e| ReadIdentityFileFailed(name.into(), Box::new(e)))?,
@@ -91,7 +99,7 @@ impl Identity {
         name: &str,
         pem_content: &[u8],
         was_encrypted: bool,
-    ) -> Result<Self, IdentityError> {
+    ) -> Result<Self, LoadPemIdentityError> {
         let inner = Box::new(
             Secp256k1Identity::from_pem(pem_content)
                 .map_err(|e| ReadIdentityFileFailed(name.into(), Box::new(e)))?,
@@ -104,7 +112,10 @@ impl Identity {
         })
     }
 
-    fn hardware(name: &str, hsm: HardwareIdentityConfiguration) -> Result<Self, IdentityError> {
+    fn hardware(
+        name: &str,
+        hsm: HardwareIdentityConfiguration,
+    ) -> Result<Self, NewHardwareIdentityError> {
         let inner = Box::new(
             HardwareIdentity::new(
                 hsm.pkcs11_lib_path,
@@ -126,14 +137,16 @@ impl Identity {
         config: IdentityConfiguration,
         locations: &IdentityFileLocations,
         log: &Logger,
-    ) -> Result<Self, IdentityError> {
+    ) -> Result<Self, NewIdentityError> {
         if let Some(hsm) = config.hsm {
-            Identity::hardware(name, hsm)
+            Identity::hardware(name, hsm).map_err(NewIdentityError::NewHardwareIdentityFailed)
         } else {
             let (pem_content, was_encrypted) =
-                pem_safekeeping::load_pem(log, locations, name, &config)?;
+                pem_safekeeping::load_pem(log, locations, name, &config)
+                    .map_err(NewIdentityError::LoadPemFailed)?;
             Identity::secp256k1(name, &pem_content, was_encrypted)
                 .or_else(|e| Identity::basic(name, &pem_content, was_encrypted).map_err(|_| e))
+                .map_err(NewIdentityError::LoadPemIdentityFailed)
         }
     }