diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs index e55f5e6d7e..e984eb1e90 100644 --- a/dfx/src/commands/canister/call.rs +++ b/dfx/src/commands/canister/call.rs @@ -94,11 +94,13 @@ where let canister_name = args.value_of("canister_name").unwrap(); let canister_info = CanisterInfo::load(config, canister_name)?; + // Read the config. let canister_id = canister_info.get_canister_id().ok_or_else(|| { DfxError::CannotFindBuildOutputForCanister(canister_info.get_name().to_owned()) })?; - - let method_name = args.value_of("method_name").unwrap(); + let method_name = args + .value_of("method_name") + .ok_or_else(|| DfxError::InvalidArgument("method_name".to_string()))?; let arguments: Option<&str> = args.value_of("argument"); let arg_type: Option<&str> = args.value_of("type"); diff --git a/dfx/src/commands/canister/request_status.rs b/dfx/src/commands/canister/request_status.rs index 0b7e86bf78..2184fe4f28 100644 --- a/dfx/src/commands/canister/request_status.rs +++ b/dfx/src/commands/canister/request_status.rs @@ -23,7 +23,11 @@ pub fn exec(env: &T, args: &ArgMatches<'_>) -> DfxResult where T: ClientEnv, { - let request_id = RequestId::from_str(&args.value_of("request_id").unwrap()[2..]) - .map_err(|e| DfxError::InvalidArgument(format!("Invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError + let request_id = RequestId::from_str( + &args + .value_of("request_id") + .ok_or_else(|| DfxError::InvalidArgument("request_id".to_string()))?[2..], + ) + .map_err(|_| DfxError::InvalidArgument("request_id".to_owned()))?; wait_on_request_status(&env.get_client(), request_id) } diff --git a/dfx/src/commands/config.rs b/dfx/src/commands/config.rs index 9be920ee76..75e719b9a3 100644 --- a/dfx/src/commands/config.rs +++ b/dfx/src/commands/config.rs @@ -22,7 +22,9 @@ pub fn exec(env: &T, args: &ArgMatches<'_>) -> DfxResult { .ok_or(DfxError::CommandMustBeRunInAProject)? .clone(); - let config_path = args.value_of("config_path").unwrap(); + let config_path = args + .value_of("config_path") + .ok_or_else(|| DfxError::InvalidArgument("config_path".to_string()))?; // We replace `.` with `/` so the user can use `path.value.field` instead of forcing him // to use `path/value/field`. Since none of our keys have slashes or tildes in them it diff --git a/dfx/src/commands/new.rs b/dfx/src/commands/new.rs index 5b9602375e..6e244bda07 100644 --- a/dfx/src/commands/new.rs +++ b/dfx/src/commands/new.rs @@ -113,7 +113,10 @@ where T: BinaryCacheEnv + PlatformEnv + VersionEnv, { let dry_run = args.is_present(DRY_RUN); - let project_name = Path::new(args.value_of(PROJECT_NAME).unwrap()); + let project_name_path = args + .value_of(PROJECT_NAME) + .ok_or_else(|| DfxError::InvalidArgument("project_path".to_string()))?; + let project_name = Path::new(project_name_path); if project_name.exists() { return Err(DfxError::ProjectExists); @@ -127,6 +130,10 @@ where } let mut new_project_files = assets::new_project_files()?; + let project_name_str = project_name + .to_str() + .ok_or_else(|| DfxError::InvalidArgument("project_name".to_string()))?; + for file in new_project_files.entries()? { let mut file = file?; @@ -135,10 +142,11 @@ where } let mut s = String::new(); - file.read_to_string(&mut s).unwrap(); + file.read_to_string(&mut s) + .or_else(|e| Err(DfxError::Io(e)))?; // Perform replacements. - let s = s.replace("{project_name}", project_name.to_str().unwrap()); + let s = s.replace("{project_name}", project_name_str); let s = s.replace("{dfx_version}", dfx_version); // Perform path replacements. @@ -146,7 +154,7 @@ where project_name .join(file.header().path()?) .to_str() - .unwrap() + .expect("Non unicode project name path.") .replace("__dot__", ".") .as_str(), ); @@ -192,7 +200,7 @@ where include_str!("../../assets/welcome.txt"), dfx_version, assets::dfinity_logo(), - project_name.to_str().unwrap() + project_name_str ); Ok(()) diff --git a/dfx/src/config/dfinity.rs b/dfx/src/config/dfinity.rs index 6a91732111..f215a87214 100644 --- a/dfx/src/config/dfinity.rs +++ b/dfx/src/config/dfinity.rs @@ -218,11 +218,23 @@ impl Config { &self.config } + pub fn get_project_root(&self) -> &Path { + // a configuration path contains a file name specifically. As + // such we should be returning at least root as parent. If + // this is invariance is broken, we must fail. + self.path.parent().expect( + "An incorrect configuration path was set with no parent, i.e. did not include root", + ) + } + pub fn save(&self) -> DfxResult { - std::fs::write( - &self.path, - serde_json::to_string_pretty(&self.json).unwrap(), - )?; + let json_pretty = serde_json::to_string_pretty(&self.json).or_else(|e| { + Err(DfxError::InvalidData(format!( + "Failed to serialize dfx.json: {}", + e + ))) + })?; + std::fs::write(&self.path, json_pretty)?; Ok(()) } } diff --git a/dfx/src/lib/error/mod.rs b/dfx/src/lib/error/mod.rs index ca78abb61e..75de530bb8 100644 --- a/dfx/src/lib/error/mod.rs +++ b/dfx/src/lib/error/mod.rs @@ -6,6 +6,7 @@ pub use cache::CacheErrorKind; // TODO: refactor this enum into a *Kind enum and a struct DfxError. #[derive(Debug)] +/// Provides dfx user facing errors. pub enum DfxError { /// An error happened during build. BuildError(BuildErrorKind), @@ -30,28 +31,45 @@ pub enum DfxError { /// An unknown command was used. The argument is the command itself. UnknownCommand(String), - // Cannot create a new project because the directory already exists. + /// Cannot create a new project because the directory already exists. ProjectExists, - // Not in a project. + #[allow(dead_code)] + /// An error originating from the Client. The enclosed type should be + /// a descriptive error. + // TODO(eftychis): Consider to how to better represent this without a massive change. + ClientContainerError(String), + + /// Not in a project. CommandMustBeRunInAProject, - // The client returned an error. It normally specifies the error as an - // HTTP status (so 400-599), and has a string as the error message. - // Once the client support errors from the public spec or as an enum, - // we should update this type. - // We don't use StatusCode here because the client might return some other - // number if they support public spec's errors (< 100). + /// The client returned an error. It normally specifies the error as an + /// HTTP status (so 400-599), and has a string as the error message. + /// Once the client support errors from the public spec or as an enum, + /// we should update this type. + /// We don't use StatusCode here because the client might return some other + /// number if they support public spec's errors (< 100). ClientError(u16, String), + + /// This option is used when the source/cause of the error is + /// ambiguous. If the cause is known use or add a new option. Unknown(String), - // Configuration path does not exist in the config file. + /// Configuration path does not exist in the config file. ConfigPathDoesNotExist(String), + /// Argument provided is invalid. InvalidArgument(String), + + #[allow(dead_code)] + /// Configuration provided is invalid. + InvalidConfiguration(String), + /// Method called invalid. InvalidMethodCall(String), + + /// Data provided is invalid. InvalidData(String), - // The ide server shouldn't be started from a terminal + /// The ide server shouldn't be started from a terminal. LanguageServerFromATerminal, } diff --git a/lib/serde_idl/src/de.rs b/lib/serde_idl/src/de.rs index b68e33a986..dffa321d8a 100644 --- a/lib/serde_idl/src/de.rs +++ b/lib/serde_idl/src/de.rs @@ -487,11 +487,14 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { let value = visitor.visit_enum(Compound::new(&mut self, Style::Enum { len, fs }))?; Ok(value) } - + /// Deserialize identifier. + /// # Panics + /// *Will Panic* when identifier name is None. fn deserialize_identifier(self, visitor: V) -> Result where V: Visitor<'de>, { + // N.B. Here we want to panic as it indicates a logical error. let label = self.field_name.as_ref().unwrap(); let v = match label { FieldLabel::Named(name) => visitor.visit_str(name),