refactor: split up DfxConfigError into method-specific error types (#…

…3384)

Eliminates some boxing and makes it more clear what errors are possible for any given method.

src/dfx-core/src/config/model/dfinity.rs

@@ -3,11 +3,17 @@
 use crate::config::directories::get_config_dfx_dir_path;
 use crate::config::model::bitcoin_adapter::BitcoinAdapterLogLevel;
 use crate::config::model::canister_http_adapter::HttpAdapterLogLevel;
-use crate::error::dfx_config::DfxConfigError;
-use crate::error::dfx_config::DfxConfigError::{
-    CanisterCircularDependency, CanisterNotFound, CanistersFieldDoesNotExist,
-    GetCanistersWithDependenciesFailed, GetComputeAllocationFailed, GetFreezingThresholdFailed,
-    GetMemoryAllocationFailed, GetRemoteCanisterIdFailed, PullCanistersSameId,
+use crate::error::dfx_config::AddDependenciesError::CanisterCircularDependency;
+use crate::error::dfx_config::GetCanisterNamesWithDependenciesError::AddDependenciesFailed;
+use crate::error::dfx_config::GetComputeAllocationError::GetComputeAllocationFailed;
+use crate::error::dfx_config::GetFreezingThresholdError::GetFreezingThresholdFailed;
+use crate::error::dfx_config::GetMemoryAllocationError::GetMemoryAllocationFailed;
+use crate::error::dfx_config::GetPullCanistersError::PullCanistersSameId;
+use crate::error::dfx_config::GetRemoteCanisterIdError::GetRemoteCanisterIdFailed;
+use crate::error::dfx_config::{
+    AddDependenciesError, GetCanisterConfigError, GetCanisterNamesWithDependenciesError,
+    GetComputeAllocationError, GetFreezingThresholdError, GetMemoryAllocationError,
+    GetPullCanistersError, GetRemoteCanisterIdError,
 };
 use crate::error::load_dfx_config::LoadDfxConfigError;
 use crate::error::load_dfx_config::LoadDfxConfigError::{
@@ -749,36 +755,34 @@ impl ConfigInterface {
     pub fn get_canister_names_with_dependencies(
         &self,
         some_canister: Option<&str>,
-    ) -> Result<Vec<String>, DfxConfigError> {
+    ) -> Result<Vec<String>, GetCanisterNamesWithDependenciesError> {
         self.canisters
             .as_ref()
-            .ok_or(CanistersFieldDoesNotExist())
+            .ok_or(GetCanisterNamesWithDependenciesError::CanistersFieldDoesNotExist())
             .and_then(|canister_map| match some_canister {
                 Some(specific_canister) => {
                     let mut names = HashSet::new();
                     let mut path = vec![];
                     add_dependencies(canister_map, &mut names, &mut path, specific_canister)
                         .map(|_| names.into_iter().collect())
+                        .map_err(|err| AddDependenciesFailed(specific_canister.to_string(), err))
                 }
                 None => Ok(canister_map.keys().cloned().collect()),
             })
-            .map_err(|cause| {
-                GetCanistersWithDependenciesFailed(some_canister.map(String::from), Box::new(cause))
-            })
     }
 
     pub fn get_remote_canister_id(
         &self,
         canister: &str,
         network: &str,
-    ) -> Result<Option<Principal>, DfxConfigError> {
+    ) -> Result<Option<Principal>, GetRemoteCanisterIdError> {
         let maybe_principal = self
             .get_canister_config(canister)
             .map_err(|e| {
                 GetRemoteCanisterIdFailed(
                     Box::new(canister.to_string()),
                     Box::new(network.to_string()),
-                    Box::new(e),
+                    e,
                 )
             })?
             .remote
@@ -793,17 +797,17 @@ impl ConfigInterface {
         &self,
         canister: &str,
         network: &str,
-    ) -> Result<bool, DfxConfigError> {
+    ) -> Result<bool, GetRemoteCanisterIdError> {
         Ok(self.get_remote_canister_id(canister, network)?.is_some())
     }
 
     pub fn get_compute_allocation(
         &self,
         canister_name: &str,
-    ) -> Result<Option<u64>, DfxConfigError> {
+    ) -> Result<Option<u64>, GetComputeAllocationError> {
         Ok(self
             .get_canister_config(canister_name)
-            .map_err(|e| GetComputeAllocationFailed(canister_name.to_string(), Box::new(e)))?
+            .map_err(|e| GetComputeAllocationFailed(canister_name.to_string(), e))?
             .initialization_values
             .compute_allocation
             .map(|x| x.0))
@@ -812,37 +816,37 @@ impl ConfigInterface {
     pub fn get_memory_allocation(
         &self,
         canister_name: &str,
-    ) -> Result<Option<Byte>, DfxConfigError> {
+    ) -> Result<Option<Byte>, GetMemoryAllocationError> {
         Ok(self
             .get_canister_config(canister_name)
-            .map_err(|e| GetMemoryAllocationFailed(canister_name.to_string(), Box::new(e)))?
+            .map_err(|e| GetMemoryAllocationFailed(canister_name.to_string(), e))?
             .initialization_values
             .memory_allocation)
     }
 
     pub fn get_freezing_threshold(
         &self,
         canister_name: &str,
-    ) -> Result<Option<Duration>, DfxConfigError> {
+    ) -> Result<Option<Duration>, GetFreezingThresholdError> {
         Ok(self
             .get_canister_config(canister_name)
-            .map_err(|e| GetFreezingThresholdFailed(canister_name.to_string(), Box::new(e)))?
+            .map_err(|e| GetFreezingThresholdFailed(canister_name.to_string(), e))?
             .initialization_values
             .freezing_threshold)
     }
 
     fn get_canister_config(
         &self,
         canister_name: &str,
-    ) -> Result<&ConfigCanistersCanister, DfxConfigError> {
+    ) -> Result<&ConfigCanistersCanister, GetCanisterConfigError> {
         self.canisters
             .as_ref()
-            .ok_or(CanistersFieldDoesNotExist())?
+            .ok_or(GetCanisterConfigError::CanistersFieldDoesNotExist())?
             .get(canister_name)
-            .ok_or_else(|| CanisterNotFound(canister_name.to_string()))
+            .ok_or_else(|| GetCanisterConfigError::CanisterNotFound(canister_name.to_string()))
     }
 
-    pub fn get_pull_canisters(&self) -> Result<BTreeMap<String, Principal>, DfxConfigError> {
+    pub fn get_pull_canisters(&self) -> Result<BTreeMap<String, Principal>, GetPullCanistersError> {
         let mut res = BTreeMap::new();
         let mut id_to_name: BTreeMap<Principal, &String> = BTreeMap::new();
         if let Some(map) = &self.canisters {
@@ -865,7 +869,7 @@ fn add_dependencies(
     names: &mut HashSet<String>,
     path: &mut Vec<String>,
     canister_name: &str,
-) -> Result<(), DfxConfigError> {
+) -> Result<(), AddDependenciesError> {
     let inserted = names.insert(String::from(canister_name));
 
     if !inserted {
@@ -879,7 +883,7 @@ fn add_dependencies(
 
     let canister_config = all_canisters
         .get(canister_name)
-        .ok_or_else(|| CanisterNotFound(canister_name.to_string()))?;
+        .ok_or_else(|| AddDependenciesError::CanisterNotFound(canister_name.to_string()))?;
 
     path.push(String::from(canister_name));
 

src/dfx-core/src/error/canister_id_store.rs

@@ -1,5 +1,5 @@
 use crate::error::{
-    dfx_config::DfxConfigError, fs::FsError, structured_file::StructuredFileError,
+    dfx_config::GetPullCanistersError, fs::FsError, structured_file::StructuredFileError,
     unified_io::UnifiedIoError,
 };
 use thiserror::Error;
@@ -37,7 +37,7 @@ pub enum CanisterIdStoreError {
     },
 
     #[error(transparent)]
-    DfxConfigError(#[from] DfxConfigError),
+    GetPullCanistersFailed(#[from] GetPullCanistersError),
 }
 
 impl From<FsError> for CanisterIdStoreError {

src/dfx-core/src/error/dfx_config.rs

@@ -2,31 +2,58 @@ use candid::Principal;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
-pub enum DfxConfigError {
+pub enum AddDependenciesError {
     #[error("Circular canister dependencies: {}", _0.join(" -> "))]
     CanisterCircularDependency(Vec<String>),
 
     #[error("Canister '{0}' not found in dfx.json.")]
     CanisterNotFound(String),
+}
+
+#[derive(Error, Debug)]
+pub enum GetCanisterConfigError {
+    #[error("No canisters in the configuration file.")]
+    CanistersFieldDoesNotExist(),
+
+    #[error("Canister '{0}' not found in dfx.json.")]
+    CanisterNotFound(String),
+}
 
+#[derive(Error, Debug)]
+pub enum GetCanisterNamesWithDependenciesError {
     #[error("No canisters in the configuration file.")]
     CanistersFieldDoesNotExist(),
 
-    #[error("Failed to get canisters with their dependencies (for {}): {1}", _0.as_deref().unwrap_or("all canisters"))]
-    GetCanistersWithDependenciesFailed(Option<String>, Box<DfxConfigError>),
+    #[error("Failed to add dependencies for canister '{0}': {1}")]
+    AddDependenciesFailed(String, AddDependenciesError),
+}
 
+#[derive(Error, Debug)]
+pub enum GetComputeAllocationError {
     #[error("Failed to get compute allocation for canister '{0}': {1}")]
-    GetComputeAllocationFailed(String, Box<DfxConfigError>),
+    GetComputeAllocationFailed(String, GetCanisterConfigError),
+}
 
+#[derive(Error, Debug)]
+pub enum GetFreezingThresholdError {
     #[error("Failed to get freezing threshold for canister '{0}': {1}")]
-    GetFreezingThresholdFailed(String, Box<DfxConfigError>),
+    GetFreezingThresholdFailed(String, GetCanisterConfigError),
+}
 
+#[derive(Error, Debug)]
+pub enum GetMemoryAllocationError {
     #[error("Failed to get memory allocation for canister '{0}': {1}")]
-    GetMemoryAllocationFailed(String, Box<DfxConfigError>),
-
-    #[error("Failed to figure out if canister '{0}' has a remote id on network '{1}': {2}")]
-    GetRemoteCanisterIdFailed(Box<String>, Box<String>, Box<DfxConfigError>),
+    GetMemoryAllocationFailed(String, GetCanisterConfigError),
+}
 
+#[derive(Error, Debug)]
+pub enum GetPullCanistersError {
     #[error("Pull dependencies '{0}' and '{1}' have the same canister ID: {2}")]
     PullCanistersSameId(String, String, Principal),
 }
+
+#[derive(Error, Debug)]
+pub enum GetRemoteCanisterIdError {
+    #[error("Failed to figure out if canister '{0}' has a remote id on network '{1}': {2}")]
+    GetRemoteCanisterIdFailed(Box<String>, Box<String>, GetCanisterConfigError),
+}