From e707db68b9d78932cbf98f24a9de3c043eded045 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Thu, 7 Nov 2019 15:24:30 -0800
Subject: [PATCH 1/8] Remove unwraps (commands); minor IDL

---
 dfx/src/commands/canister/call.rs           | 12 ++++++---
 dfx/src/commands/canister/query.rs          | 14 ++++++----
 dfx/src/commands/canister/request_status.rs |  8 ++++--
 dfx/src/commands/config.rs                  |  4 ++-
 dfx/src/commands/new.rs                     | 30 ++++++++++++++++-----
 dfx/src/config/dfinity.rs                   | 11 +++++---
 dfx/src/lib/error/mod.rs                    |  5 ++--
 lib/serde_idl/src/de.rs                     |  5 +++-
 8 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs
index ee66d5d4e5..5a627f060d 100644
--- a/dfx/src/commands/canister/call.rs
+++ b/dfx/src/commands/canister/call.rs
@@ -54,11 +54,15 @@ where
 
     let canister_name = args.value_of("canister_name").unwrap();
     let canister_info = CanisterInfo::load(config, canister_name)?;
-    let canister_id = canister_info.get_canister_id().ok_or_else(|| {
-        DfxError::CannotFindBuildOutputForCanister(canister_info.get_name().to_owned())
+    // Read the config.
+    let canister_id = args
+        .value_of("deployment_id")
+        .ok_or_else(|| DfxError::InvalidArgument("deployment id".to_string()))?
+        .parse::<CanisterId>()
+        .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?;
+    let method_name = args.value_of("method name").ok_or_else(|| {
+        DfxError::InvalidArgument("method name argument provided invalid".to_string())
     })?;
-
-    let method_name = args.value_of("method_name").unwrap();
     let arguments: Option<&str> = args.value_of("argument");
     let arg_type: Option<&str> = args.value_of("type");
 
diff --git a/dfx/src/commands/canister/query.rs b/dfx/src/commands/canister/query.rs
index 859ff1b3f4..28af91d5ca 100644
--- a/dfx/src/commands/canister/query.rs
+++ b/dfx/src/commands/canister/query.rs
@@ -48,11 +48,15 @@ where
 
     let canister_name = args.value_of("canister_name").unwrap();
     let canister_info = CanisterInfo::load(config, canister_name)?;
-    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();
+    // Read the config.
+    let canister_id = args
+        .value_of("deployment_id")
+        .ok_or_else(|| DfxError::InvalidArgument("deployment id".to_string()))?
+        .parse::<CanisterId>()
+        .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?;
+    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 7bdfe352db..6ae5110708 100644
--- a/dfx/src/commands/canister/request_status.rs
+++ b/dfx/src/commands/canister/request_status.rs
@@ -25,8 +25,12 @@ pub fn exec<T>(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(|e| DfxError::InvalidArgument(format!("invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
     let request_status = request_status(env.get_client(), request_id);
     let mut runtime = Runtime::new().expect("Unable to create a runtime");
     match runtime.block_on(request_status) {
diff --git a/dfx/src/commands/config.rs b/dfx/src/commands/config.rs
index 9be920ee76..6106279f7f 100644
--- a/dfx/src/commands/config.rs
+++ b/dfx/src/commands/config.rs
@@ -22,7 +22,9 @@ pub fn exec<T: ProjectConfigEnv>(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 a7c14097ba..8726233deb 100644
--- a/dfx/src/commands/new.rs
+++ b/dfx/src/commands/new.rs
@@ -86,7 +86,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);
@@ -100,6 +103,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?;
 
@@ -108,10 +115,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.
@@ -119,7 +127,9 @@ where
             project_name
                 .join(file.header().path()?)
                 .to_str()
-                .unwrap()
+                .ok_or_else(|| {
+                    DfxError::InvalidArgument("project name path or file header".to_string())
+                })?
                 .replace("__dot__", ".")
                 .as_str(),
         );
@@ -133,7 +143,15 @@ where
             .arg("init")
             .current_dir(&project_name)
             .status();
-        if init_status.is_ok() && init_status.unwrap().success() {
+        let is_success = init_status
+            .or_else(|e| {
+                Err(DfxError::IO(std::io::Error::new(
+                    e.kind(),
+                    format!("Unable to execute git {}", e),
+                )))
+            })?
+            .success();
+        if is_success {
             eprintln!("Creating git repository...");
             std::process::Command::new("git")
                 .arg("add")
@@ -156,7 +174,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..1201e72ce8 100644
--- a/dfx/src/config/dfinity.rs
+++ b/dfx/src/config/dfinity.rs
@@ -219,10 +219,13 @@ impl Config {
     }
 
     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 -- {}",
+                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 beb7992c18..d53d9df3be 100644
--- a/dfx/src/lib/error/mod.rs
+++ b/dfx/src/lib/error/mod.rs
@@ -16,7 +16,7 @@ pub enum DfxError {
     IdeError(String),
 
     Clap(clap::Error),
-    Io(std::io::Error),
+    IO(std::io::Error),
     Reqwest(reqwest::Error),
     Url(reqwest::UrlError),
 
@@ -48,6 +48,7 @@ pub enum DfxError {
     // Configuration path does not exist in the config file.
     ConfigPathDoesNotExist(String),
     InvalidArgument(String),
+    InvalidConfiguration(String),
     InvalidData(String),
 }
 
@@ -68,6 +69,6 @@ impl From<reqwest::Error> for DfxError {
 
 impl From<std::io::Error> for DfxError {
     fn from(err: std::io::Error) -> DfxError {
-        DfxError::Io(err)
+        DfxError::IO(err)
     }
 }
diff --git a/lib/serde_idl/src/de.rs b/lib/serde_idl/src/de.rs
index 3624cd316c..64bb1eff17 100644
--- a/lib/serde_idl/src/de.rs
+++ b/lib/serde_idl/src/de.rs
@@ -482,11 +482,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<V>(self, visitor: V) -> Result<V::Value>
     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),

From 8692f466217c2f49bcb54bd3472a97dd44b0dd75 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Mon, 11 Nov 2019 10:31:24 -0800
Subject: [PATCH 2/8] Address comments; document dfxerror

---
 dfx/src/commands/canister/call.rs           |  4 +--
 dfx/src/commands/canister/request_status.rs |  8 +++--
 dfx/src/commands/new.rs                     | 39 +++++++++------------
 dfx/src/config/dfinity.rs                   |  9 +++++
 dfx/src/lib/error/mod.rs                    | 30 ++++++++++------
 5 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs
index 5a627f060d..54d126f1a8 100644
--- a/dfx/src/commands/canister/call.rs
+++ b/dfx/src/commands/canister/call.rs
@@ -57,9 +57,9 @@ where
     // Read the config.
     let canister_id = args
         .value_of("deployment_id")
-        .ok_or_else(|| DfxError::InvalidArgument("deployment id".to_string()))?
+        .ok_or_else(|| DfxError::InvalidArgument("deployment_id".to_string()))?
         .parse::<CanisterId>()
-        .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?;
+        .map_err(|e| DfxError::InvalidArgument(format!("invalid deployment_id: {}", e)))?;
     let method_name = args.value_of("method name").ok_or_else(|| {
         DfxError::InvalidArgument("method name argument provided invalid".to_string())
     })?;
diff --git a/dfx/src/commands/canister/request_status.rs b/dfx/src/commands/canister/request_status.rs
index 6ae5110708..e27e8d4262 100644
--- a/dfx/src/commands/canister/request_status.rs
+++ b/dfx/src/commands/canister/request_status.rs
@@ -26,9 +26,11 @@ where
     T: ClientEnv,
 {
     let request_id = RequestId::from_str(
-        &args
-            .value_of("request_id")
-            .ok_or_else(|| DfxError::InvalidArgument("request id".to_string()))?[2..],
+        &args.value_of("request_id").ok_or_else(|| {
+            DfxError::InvalidArgument(
+                "failed to retrieve request_id -- required argument".to_string(),
+            )
+        })?[2..],
     )
     .map_err(|e| DfxError::InvalidArgument(format!("invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
     let request_status = request_status(env.get_client(), request_id);
diff --git a/dfx/src/commands/new.rs b/dfx/src/commands/new.rs
index 8726233deb..6de6b5a124 100644
--- a/dfx/src/commands/new.rs
+++ b/dfx/src/commands/new.rs
@@ -116,7 +116,7 @@ where
 
         let mut s = String::new();
         file.read_to_string(&mut s)
-            .or_else(|e| Err(DfxError::IO(e)))?;
+            .or_else(|e| Err(DfxError::Io(e)))?;
 
         // Perform replacements.
         let s = s.replace("{project_name}", project_name_str);
@@ -143,27 +143,22 @@ where
             .arg("init")
             .current_dir(&project_name)
             .status();
-        let is_success = init_status
-            .or_else(|e| {
-                Err(DfxError::IO(std::io::Error::new(
-                    e.kind(),
-                    format!("Unable to execute git {}", e),
-                )))
-            })?
-            .success();
-        if is_success {
-            eprintln!("Creating git repository...");
-            std::process::Command::new("git")
-                .arg("add")
-                .current_dir(&project_name)
-                .arg(".")
-                .output()?;
-            std::process::Command::new("git")
-                .arg("commit")
-                .current_dir(&project_name)
-                .arg("-a")
-                .arg("--message=Initial commit.")
-                .output()?;
+
+        if let Ok(s) = init_status {
+            if s.success() {
+                eprintln!("Creating git repository...");
+                std::process::Command::new("git")
+                    .arg("add")
+                    .current_dir(&project_name)
+                    .arg(".")
+                    .output()?;
+                std::process::Command::new("git")
+                    .arg("commit")
+                    .current_dir(&project_name)
+                    .arg("-a")
+                    .arg("--message=Initial commit.")
+                    .output()?;
+            }
         }
     }
 
diff --git a/dfx/src/config/dfinity.rs b/dfx/src/config/dfinity.rs
index 1201e72ce8..ded8dccc3c 100644
--- a/dfx/src/config/dfinity.rs
+++ b/dfx/src/config/dfinity.rs
@@ -218,6 +218,15 @@ 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 {
         let json_pretty = serde_json::to_string_pretty(&self.json).or_else(|e| {
             Err(DfxError::InvalidData(format!(
diff --git a/dfx/src/lib/error/mod.rs b/dfx/src/lib/error/mod.rs
index d53d9df3be..99aed5e7d7 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),
@@ -16,7 +17,7 @@ pub enum DfxError {
     IdeError(String),
 
     Clap(clap::Error),
-    IO(std::io::Error),
+    Io(std::io::Error),
     Reqwest(reqwest::Error),
     Url(reqwest::UrlError),
 
@@ -30,25 +31,32 @@ 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.
+    /// 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),
+    /// Data provided is invalid.
     InvalidData(String),
 }
 
@@ -69,6 +77,6 @@ impl From<reqwest::Error> for DfxError {
 
 impl From<std::io::Error> for DfxError {
     fn from(err: std::io::Error) -> DfxError {
-        DfxError::IO(err)
+        DfxError::Io(err)
     }
 }

From 25278f9b29df957f2fde6340863046b82d7491f5 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Mon, 18 Nov 2019 16:07:24 -0800
Subject: [PATCH 3/8] fix

---
 dfx/src/commands/canister/call.rs  | 8 +++-----
 dfx/src/commands/canister/query.rs | 8 +++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs
index 54d126f1a8..e9e7364b4b 100644
--- a/dfx/src/commands/canister/call.rs
+++ b/dfx/src/commands/canister/call.rs
@@ -55,11 +55,9 @@ where
     let canister_name = args.value_of("canister_name").unwrap();
     let canister_info = CanisterInfo::load(config, canister_name)?;
     // Read the config.
-    let canister_id = args
-        .value_of("deployment_id")
-        .ok_or_else(|| DfxError::InvalidArgument("deployment_id".to_string()))?
-        .parse::<CanisterId>()
-        .map_err(|e| DfxError::InvalidArgument(format!("invalid deployment_id: {}", e)))?;
+    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").ok_or_else(|| {
         DfxError::InvalidArgument("method name argument provided invalid".to_string())
     })?;
diff --git a/dfx/src/commands/canister/query.rs b/dfx/src/commands/canister/query.rs
index 28af91d5ca..e28b6e036e 100644
--- a/dfx/src/commands/canister/query.rs
+++ b/dfx/src/commands/canister/query.rs
@@ -49,11 +49,9 @@ where
     let canister_name = args.value_of("canister_name").unwrap();
     let canister_info = CanisterInfo::load(config, canister_name)?;
     // Read the config.
-    let canister_id = args
-        .value_of("deployment_id")
-        .ok_or_else(|| DfxError::InvalidArgument("deployment id".to_string()))?
-        .parse::<CanisterId>()
-        .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?;
+    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")
         .ok_or_else(|| DfxError::InvalidArgument("method name".to_string()))?;

From 5c7fae777768ae84be5586777d6fcd474b19ab28 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Mon, 2 Dec 2019 16:29:38 -0700
Subject: [PATCH 4/8] fixup! oops merge fix

---
 dfx/src/commands/canister/call.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs
index e9e7364b4b..53d5c763ea 100644
--- a/dfx/src/commands/canister/call.rs
+++ b/dfx/src/commands/canister/call.rs
@@ -58,7 +58,7 @@ where
     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").ok_or_else(|| {
+    let method_name = args.value_of("method_name").ok_or_else(|| {
         DfxError::InvalidArgument("method name argument provided invalid".to_string())
     })?;
     let arguments: Option<&str> = args.value_of("argument");

From 8266a11c1ab92c3de2bfe8d5d01a58b602906f11 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Tue, 3 Dec 2019 14:49:29 -0700
Subject: [PATCH 5/8] address

---
 dfx/src/commands/canister/query.rs          |  2 +-
 dfx/src/commands/canister/request_status.rs | 10 ++++------
 dfx/src/commands/config.rs                  |  2 +-
 dfx/src/commands/new.rs                     |  6 +++---
 dfx/src/config/dfinity.rs                   |  2 +-
 dfx/src/lib/error/mod.rs                    | 10 +++++++++-
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/dfx/src/commands/canister/query.rs b/dfx/src/commands/canister/query.rs
index e28b6e036e..8b3f8b0173 100644
--- a/dfx/src/commands/canister/query.rs
+++ b/dfx/src/commands/canister/query.rs
@@ -54,7 +54,7 @@ where
     })?;
     let method_name = args
         .value_of("method_name")
-        .ok_or_else(|| DfxError::InvalidArgument("method name".to_string()))?;
+        .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 e27e8d4262..a8a1d02385 100644
--- a/dfx/src/commands/canister/request_status.rs
+++ b/dfx/src/commands/canister/request_status.rs
@@ -26,13 +26,11 @@ where
     T: ClientEnv,
 {
     let request_id = RequestId::from_str(
-        &args.value_of("request_id").ok_or_else(|| {
-            DfxError::InvalidArgument(
-                "failed to retrieve request_id -- required argument".to_string(),
-            )
-        })?[2..],
+        &args
+            .value_of("request_id")
+            .ok_or_else(|| DfxError::InvalidArgument("request_id".to_string()))?[2..],
     )
-    .map_err(|e| DfxError::InvalidArgument(format!("invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
+    .map_err(|e| DfxError::ICContainerError(format!("invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
     let request_status = request_status(env.get_client(), request_id);
     let mut runtime = Runtime::new().expect("Unable to create a runtime");
     match runtime.block_on(request_status) {
diff --git a/dfx/src/commands/config.rs b/dfx/src/commands/config.rs
index 6106279f7f..75e719b9a3 100644
--- a/dfx/src/commands/config.rs
+++ b/dfx/src/commands/config.rs
@@ -24,7 +24,7 @@ pub fn exec<T: ProjectConfigEnv>(env: &T, args: &ArgMatches<'_>) -> DfxResult {
 
     let config_path = args
         .value_of("config_path")
-        .ok_or_else(|| DfxError::InvalidArgument("config path".to_string()))?;
+        .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 8e8966be9e..70144f0648 100644
--- a/dfx/src/commands/new.rs
+++ b/dfx/src/commands/new.rs
@@ -115,7 +115,7 @@ where
     let dry_run = args.is_present(DRY_RUN);
     let project_name_path = args
         .value_of(PROJECT_NAME)
-        .ok_or_else(|| DfxError::InvalidArgument("project path".to_string()))?;
+        .ok_or_else(|| DfxError::InvalidArgument("project_path".to_string()))?;
     let project_name = Path::new(project_name_path);
 
     if project_name.exists() {
@@ -132,7 +132,7 @@ 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()))?;
+        .ok_or_else(|| DfxError::InvalidArgument("project_name".to_string()))?;
 
     for file in new_project_files.entries()? {
         let mut file = file?;
@@ -155,7 +155,7 @@ where
                 .join(file.header().path()?)
                 .to_str()
                 .ok_or_else(|| {
-                    DfxError::InvalidArgument("project name path or file header".to_string())
+                    DfxError::Impossible("non unicode project name path or file header".to_string())
                 })?
                 .replace("__dot__", ".")
                 .as_str(),
diff --git a/dfx/src/config/dfinity.rs b/dfx/src/config/dfinity.rs
index ded8dccc3c..f215a87214 100644
--- a/dfx/src/config/dfinity.rs
+++ b/dfx/src/config/dfinity.rs
@@ -230,7 +230,7 @@ impl Config {
     pub fn save(&self) -> DfxResult {
         let json_pretty = serde_json::to_string_pretty(&self.json).or_else(|e| {
             Err(DfxError::InvalidData(format!(
-                "Failed to serialize -- {}",
+                "Failed to serialize dfx.json: {}",
                 e
             )))
         })?;
diff --git a/dfx/src/lib/error/mod.rs b/dfx/src/lib/error/mod.rs
index 218dfc3dcc..100f8424f8 100644
--- a/dfx/src/lib/error/mod.rs
+++ b/dfx/src/lib/error/mod.rs
@@ -34,6 +34,11 @@ pub enum DfxError {
     /// Cannot create a new project because the directory already exists.
     ProjectExists,
 
+    /// An error originating from the IC. The enclosed type should be
+    /// a descriptive error.
+    // TODO(eftychis): Consider to how to better represent this without a massive change.
+    ICContainerError(String),
+
     /// Not in a project.
     CommandMustBeRunInAProject,
 
@@ -59,7 +64,10 @@ pub enum DfxError {
     /// Data provided is invalid.
     InvalidData(String),
 
-    // The ide server shouldn't be started from a terminal
+    /// Impossible error has occurred.
+    Impossible(String),
+
+    /// The ide server shouldn't be started from a terminal.
     LanguageServerFromATerminal,
 }
 

From 2ea9ff671f6cb0b0b46bd2ceb6f07d5fa96ba7f7 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Thu, 5 Dec 2019 17:06:01 -0700
Subject: [PATCH 6/8] Apply discussion comments

Co-Authored-By: Hans <hans.larsen@dfinity.org>
---
 dfx/src/commands/canister/call.rs           | 2 +-
 dfx/src/commands/canister/request_status.rs | 2 +-
 dfx/src/commands/new.rs                     | 4 +---
 dfx/src/lib/error/mod.rs                    | 3 ---
 4 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs
index f125c23d46..30e3115e7a 100644
--- a/dfx/src/commands/canister/call.rs
+++ b/dfx/src/commands/canister/call.rs
@@ -98,7 +98,7 @@ where
         DfxError::CannotFindBuildOutputForCanister(canister_info.get_name().to_owned())
     })?;
     let method_name = args.value_of("method_name").ok_or_else(|| {
-        DfxError::InvalidArgument("method name argument provided invalid".to_string())
+        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 2cae1156fb..7ffefa63e3 100644
--- a/dfx/src/commands/canister/request_status.rs
+++ b/dfx/src/commands/canister/request_status.rs
@@ -29,7 +29,7 @@ where
             .value_of("request_id")
             .ok_or_else(|| DfxError::InvalidArgument("request_id".to_string()))?[2..],
     )
-    .map_err(|e| DfxError::ICContainerError(format!("invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
+    .map_err(|e| DfxError::InvalidArgument("request_id".to_owned()))?;
     let request_status = request_status(env.get_client(), request_id);
     let mut runtime = Runtime::new().expect("Unable to create a runtime");
     let response = runtime.block_on(request_status);
diff --git a/dfx/src/commands/new.rs b/dfx/src/commands/new.rs
index 70144f0648..18297d7458 100644
--- a/dfx/src/commands/new.rs
+++ b/dfx/src/commands/new.rs
@@ -154,9 +154,7 @@ where
             project_name
                 .join(file.header().path()?)
                 .to_str()
-                .ok_or_else(|| {
-                    DfxError::Impossible("non unicode project name path or file header".to_string())
-                })?
+                .expect("Non unicode project name path.");
                 .replace("__dot__", ".")
                 .as_str(),
         );
diff --git a/dfx/src/lib/error/mod.rs b/dfx/src/lib/error/mod.rs
index 30629685f8..87190d6b11 100644
--- a/dfx/src/lib/error/mod.rs
+++ b/dfx/src/lib/error/mod.rs
@@ -68,9 +68,6 @@ pub enum DfxError {
     /// Data provided is invalid.
     InvalidData(String),
 
-    /// Impossible error has occurred.
-    Impossible(String),
-
     /// The ide server shouldn't be started from a terminal.
     LanguageServerFromATerminal,
 }

From cfae9e432b023119dce4b3c88c6d916d9f51de77 Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Thu, 5 Dec 2019 20:25:09 -0700
Subject: [PATCH 7/8] Fix merge; fix

---
 dfx/src/commands/canister/request_status.rs | 3 +--
 dfx/src/commands/new.rs                     | 2 +-
 dfx/src/lib/error/mod.rs                    | 5 +++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/dfx/src/commands/canister/request_status.rs b/dfx/src/commands/canister/request_status.rs
index f3719fb0a0..2184fe4f28 100644
--- a/dfx/src/commands/canister/request_status.rs
+++ b/dfx/src/commands/canister/request_status.rs
@@ -28,7 +28,6 @@ where
             .value_of("request_id")
             .ok_or_else(|| DfxError::InvalidArgument("request_id".to_string()))?[2..],
     )
-    .map_err(|e| DfxError::InvalidArgument("request_id".to_owned()))?;
+    .map_err(|_| DfxError::InvalidArgument("request_id".to_owned()))?;
     wait_on_request_status(&env.get_client(), request_id)
-
 }
diff --git a/dfx/src/commands/new.rs b/dfx/src/commands/new.rs
index 18297d7458..6e244bda07 100644
--- a/dfx/src/commands/new.rs
+++ b/dfx/src/commands/new.rs
@@ -154,7 +154,7 @@ where
             project_name
                 .join(file.header().path()?)
                 .to_str()
-                .expect("Non unicode project name path.");
+                .expect("Non unicode project name path.")
                 .replace("__dot__", ".")
                 .as_str(),
         );
diff --git a/dfx/src/lib/error/mod.rs b/dfx/src/lib/error/mod.rs
index 87190d6b11..75de530bb8 100644
--- a/dfx/src/lib/error/mod.rs
+++ b/dfx/src/lib/error/mod.rs
@@ -34,10 +34,11 @@ pub enum DfxError {
     /// Cannot create a new project because the directory already exists.
     ProjectExists,
 
-    /// An error originating from the IC. The enclosed type should be
+    #[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.
-    ICContainerError(String),
+    ClientContainerError(String),
 
     /// Not in a project.
     CommandMustBeRunInAProject,

From 9371097fb210c15864ee4c49f4e6214faea8652c Mon Sep 17 00:00:00 2001
From: eftychis <eftychis@users.noreply.github.com>
Date: Fri, 6 Dec 2019 12:35:53 -0700
Subject: [PATCH 8/8] Format

---
 dfx/src/commands/canister/call.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/dfx/src/commands/canister/call.rs b/dfx/src/commands/canister/call.rs
index 57588de389..e984eb1e90 100644
--- a/dfx/src/commands/canister/call.rs
+++ b/dfx/src/commands/canister/call.rs
@@ -98,9 +98,9 @@ where
     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").ok_or_else(|| {
-        DfxError::InvalidArgument("method_name".to_string())
-    })?;
+    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");