Report command errors properly

So far we have taken all nitrokey::CommandError objects and put them in
formatted form into the Error::Error variant. What we really should do,
though, is to preserve the original error, with the additional context
provided by the caller, and report that up the stack directly. Doing so
has at least the benefit that we are able to check for expected errors
without hard coding the textual representation as maintained by the
nitrokey create.
This change refactors the code accordingly and adds two tests for such
expected error codes.

4 files changed, 63 insertions, 7 deletions

diff --git a/nitrocli/src/commands.rs b/nitrocli/src/commands.rs
index f8765cc..ed3c2c4 100644
--- a/nitrocli/src/commands.rs
+++ b/nitrocli/src/commands.rs
@@ -34,8 +34,8 @@ use crate::pinentry;
 use crate::Result;
 
 /// Create an `error::Error` with an error message of the format `msg: err`.
-fn get_error(msg: &str, err: nitrokey::CommandError) -> Error {
-  Error::Error(format!("{}: {}", msg, err))
+fn get_error(msg: &'static str, err: nitrokey::CommandError) -> Error {
+  Error::CommandError(Some(msg), err)
 }
 
 /// Set `libnitrokey`'s log level based on the execution context's verbosity.
@@ -201,8 +201,7 @@ where
             data = new_data;
             continue;
           }
-          let error = format!("{}: Wrong password", msg);
-          return Err(Error::Error(error));
+          return Err(get_error(msg, err));
         }
         err => return Err(get_error(msg, err)),
       },
diff --git a/nitrocli/src/error.rs b/nitrocli/src/error.rs
index ffcc56b..78b8148 100644
--- a/nitrocli/src/error.rs
+++ b/nitrocli/src/error.rs
@@ -25,7 +25,7 @@ use std::string;
 #[derive(Debug)]
 pub enum Error {
   ArgparseError(i32),
-  CommandError(nitrokey::CommandError),
+  CommandError(Option<&'static str>, nitrokey::CommandError),
   IoError(io::Error),
   Utf8Error(str::Utf8Error),
   Error(String),
@@ -33,7 +33,7 @@ pub enum Error {
 
 impl From<nitrokey::CommandError> for Error {
   fn from(e: nitrokey::CommandError) -> Error {
-    Error::CommandError(e)
+    Error::CommandError(None, e)
   }
 }
 
@@ -59,7 +59,12 @@ impl fmt::Display for Error {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     match *self {
       Error::ArgparseError(_) => write!(f, "Could not parse arguments"),
-      Error::CommandError(ref e) => write!(f, "Command error: {}", e),
+      Error::CommandError(ref ctx, ref e) => {
+        if let Some(ctx) = ctx {
+          write!(f, "{}: ", ctx)?;
+        }
+        write!(f, "{}", e)
+      }
       Error::Utf8Error(_) => write!(f, "Encountered UTF-8 conversion error"),
       Error::IoError(ref e) => write!(f, "IO error: {}", e),
       Error::Error(ref e) => write!(f, "{}", e),
diff --git a/nitrocli/src/tests/mod.rs b/nitrocli/src/tests/mod.rs
index ef8d2bd..4a4ab09 100644
--- a/nitrocli/src/tests/mod.rs
+++ b/nitrocli/src/tests/mod.rs
@@ -36,6 +36,7 @@ const NITROKEY_DEFAULT_USER_PIN: &str = "123456";
 #[test_device]
 fn dummy() {}
 
+mod otp;
 mod pin;
 mod run;
 mod status;
@@ -44,6 +45,8 @@ mod status;
 pub trait UnwrapError {
   /// Unwrap an Error::Error variant.
   fn unwrap_str_err(self) -> String;
+  /// Unwrap a Error::CommandError variant.
+  fn unwrap_cmd_err(self) -> (Option<&'static str>, nitrokey::CommandError);
 }
 
 impl<T> UnwrapError for crate::Result<T>
@@ -56,6 +59,13 @@ where
       err => panic!("Unexpected error variant found: {:?}", err),
     }
   }
+
+  fn unwrap_cmd_err(self) -> (Option<&'static str>, nitrokey::CommandError) {
+    match self.unwrap_err() {
+      crate::Error::CommandError(ctx, err) => (ctx, err),
+      err => panic!("Unexpected error variant found: {:?}", err),
+    }
+  }
 }
 
 struct Nitrocli {
diff --git a/nitrocli/src/tests/otp.rs b/nitrocli/src/tests/otp.rs
new file mode 100644
index 0000000..403cc80
--- /dev/null
+++ b/nitrocli/src/tests/otp.rs
@@ -0,0 +1,42 @@
+// otp.rs
+
+// *************************************************************************
+// * Copyright (C) 2019 Daniel Mueller (deso@posteo.net)                   *
+// *                                                                       *
+// * This program is free software: you can redistribute it and/or modify  *
+// * it under the terms of the GNU General Public License as published by  *
+// * the Free Software Foundation, either version 3 of the License, or     *
+// * (at your option) any later version.                                   *
+// *                                                                       *
+// * This program is distributed in the hope that it will be useful,       *
+// * but WITHOUT ANY WARRANTY; without even the implied warranty of        *
+// * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
+// * GNU General Public License for more details.                          *
+// *                                                                       *
+// * You should have received a copy of the GNU General Public License     *
+// * along with this program.  If not, see <http://www.gnu.org/licenses/>. *
+// *************************************************************************
+
+use super::*;
+
+#[test_device]
+fn set_invalid_slot_raw(device: nitrokey::DeviceWrapper) {
+  let (rc, out, err) = Nitrocli::with_dev(device).run(&["otp", "set", "100", "name", "1234"]);
+
+  assert_ne!(rc, 0);
+  assert_eq!(out, b"");
+  assert_eq!(&err[..24], b"Could not write OTP slot");
+}
+
+#[test_device]
+fn set_invalid_slot(device: nitrokey::DeviceWrapper) {
+  let res = Nitrocli::with_dev(device).handle(&["otp", "set", "100", "name", "1234"]);
+
+  assert_eq!(
+    res.unwrap_cmd_err(),
+    (
+      Some("Could not write OTP slot"),
+      nitrokey::CommandError::InvalidSlot
+    )
+  );
+}