Merge #82: Error overhaul

0fe87c1 Rework the crate error type (Tobin C. Harding)
d743f38 Rename error::Error variants (Tobin C. Harding)
ece727c Use lowercase strings for errors (Tobin C. Harding)
8cb4d1c Make try_from more terse (Tobin C. Harding)

Pull request description:

  Overhaul the crate error type including re-naming variants and embedding errors instead of converting to strings.

  There is still some improvements that can be done, it looks like the `Error::Hwi` variant is being abused for multiple things. I can't work out right now how to resolve it, if this gets a concept ack I'll either think harder or write up an issue.

  Please note this is an API breaking change.

ACKs for top commit:
  danielabrozzoni:
    ACK 0fe87c1 - thanks for doing this :)

Tree-SHA512: 210015ae7e16aca3640abbc8f0fcf6ad6187c94bf01689122a4e2ea60dc08b3e537a4f561cddad5a1b56f31175756afc402f7bf55e4c0f0fa48559e93f5cbe34

src/error.rs

@@ -1,5 +1,5 @@
 use std::convert::TryFrom;
-use std::fmt;
+use std::{fmt, io, str};
 
 #[derive(Clone, Copy, PartialEq, Eq)]
 #[repr(u8)]
@@ -36,28 +36,32 @@ impl ErrorCode {
 
 impl TryFrom<i8> for ErrorCode {
     type Error = Error;
+
     fn try_from(code: i8) -> Result<Self, Error> {
-        match code {
-            -1 => Ok(Self::NoDeviceType),
-            -2 => Ok(Self::MissingArguments),
-            -3 => Ok(Self::DeviceConnError),
-            -4 => Ok(Self::UnknownDeviceType),
-            -5 => Ok(Self::InvalidTx),
-            -6 => Ok(Self::NoPassword),
-            -7 => Ok(Self::BadArgument),
-            -8 => Ok(Self::NotImplemented),
-            -9 => Ok(Self::UnavailableAction),
-            -10 => Ok(Self::DeviceAlreadyInit),
-            -11 => Ok(Self::DeviceAlreadyUnlocked),
-            -12 => Ok(Self::DeviceNotReady),
-            -13 => Ok(Self::UnknownError),
-            -14 => Ok(Self::ActionCanceled),
-            -15 => Ok(Self::DeviceBusy),
-            -16 => Ok(Self::NeedToBeRoot),
-            -17 => Ok(Self::HelpText),
-            -18 => Ok(Self::DeviceNotInitialized),
-            _ => Err(Error::HWIError("Invalid error code".to_string(), None)),
-        }
+        use ErrorCode::*;
+
+        let code = match code {
+            -1 => NoDeviceType,
+            -2 => MissingArguments,
+            -3 => DeviceConnError,
+            -4 => UnknownDeviceType,
+            -5 => InvalidTx,
+            -6 => NoPassword,
+            -7 => BadArgument,
+            -8 => NotImplemented,
+            -9 => UnavailableAction,
+            -10 => DeviceAlreadyInit,
+            -11 => DeviceAlreadyUnlocked,
+            -12 => DeviceNotReady,
+            -13 => UnknownError,
+            -14 => ActionCanceled,
+            -15 => DeviceBusy,
+            -16 => NeedToBeRoot,
+            -17 => HelpText,
+            -18 => DeviceNotInitialized,
+            _ => return Err(Error::Hwi("invalid error code".to_string(), None)),
+        };
+        Ok(code)
     }
 }
 
@@ -67,35 +71,63 @@ impl fmt::Debug for ErrorCode {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug)]
 pub enum Error {
-    JSON(String),
-    Utf8(String),
-    IOError(String),
-    InvalidOption(String),
-    HWIError(String, Option<ErrorCode>),
-    PyErr(String),
+    Json(serde_json::Error),
+    Utf8(std::str::Utf8Error),
+    Io(std::io::Error),
+    Hwi(String, Option<ErrorCode>),
+    Python(pyo3::PyErr),
 }
 
-macro_rules! impl_error {
-    ( $from:ty, $to:ident ) => {
-        impl std::convert::From<$from> for Error {
-            fn from(err: $from) -> Self {
-                Error::$to(err.to_string())
-            }
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use Error::*;
+
+        match *self {
+            Json(_) => f.write_str("serde_json error"),
+            Utf8(_) => f.write_str("utf8 error"),
+            Io(_) => f.write_str("I/O error"),
+            Hwi(ref s, ref code) => write!(f, "HWI error: {}, ({:?})", s, code),
+            Python(_) => f.write_str("python error"),
         }
-    };
+    }
 }
 
-impl_error!(serde_json::Error, JSON);
-impl_error!(std::str::Utf8Error, Utf8);
-impl_error!(std::io::Error, IOError);
-impl_error!(pyo3::prelude::PyErr, PyErr);
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
 
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
+        match *self {
+            Json(ref e) => Some(e),
+            Utf8(ref e) => Some(e),
+            Io(ref e) => Some(e),
+            Hwi(_, _) => None,
+            Python(ref e) => Some(e),
+        }
     }
 }
 
-impl std::error::Error for Error {}
+impl From<serde_json::Error> for Error {
+    fn from(e: serde_json::Error) -> Self {
+        Error::Json(e)
+    }
+}
+
+impl From<str::Utf8Error> for Error {
+    fn from(e: str::Utf8Error) -> Self {
+        Error::Utf8(e)
+    }
+}
+
+impl From<io::Error> for Error {
+    fn from(e: io::Error) -> Self {
+        Error::Io(e)
+    }
+}
+
+impl From<pyo3::PyErr> for Error {
+    fn from(e: pyo3::PyErr) -> Self {
+        Error::Python(e)
+    }
+}

src/interface.rs

@@ -25,7 +25,7 @@ macro_rules! deserialize_obj {
         let value: Value = serde_json::from_str($e)?;
         let obj = value.clone();
         serde_json::from_value(value)
-            .map_err(|e| Error::HWIError(format!("Error {} while deserializing {}", e, obj), None))
+            .map_err(|e| Error::Hwi(format!("error {} while deserializing {}", e, obj), None))
     }};
 }
 
@@ -179,7 +179,7 @@ impl HWIClient {
                 .call1(py, client_args)?;
 
             if client.is_none(py) {
-                return Err(Error::HWIError("Device not found".to_string(), None));
+                return Err(Error::Hwi("device not found".to_string(), None));
             }
 
             Ok(HWIClient {
@@ -524,7 +524,7 @@ impl HWIClient {
         if output.status.success() {
             Ok(())
         } else {
-            Err(Error::HWIError(
+            Err(Error::Hwi(
                 std::str::from_utf8(&output.stderr)
                     .expect("Non UTF-8 error while installing")
                     .to_string(),

src/types.rs

@@ -36,7 +36,7 @@ pub struct HWISignature {
 fn from_b64<'de, D: Deserializer<'de>>(d: D) -> Result<Vec<u8>, D::Error> {
     let b64_string = String::deserialize(d)?;
     base64::decode(b64_string)
-        .map_err(|_| serde::de::Error::custom("Error while Deserializing Signature"))
+        .map_err(|_| serde::de::Error::custom("error while deserializing signature"))
 }
 
 impl Deref for HWISignature {
@@ -182,7 +182,7 @@ impl TryFrom<HWIDeviceInternal> for HWIDevice {
         match h.error {
             Some(e) => {
                 let code = h.code.and_then(|c| ErrorCode::try_from(c).ok());
-                Err(Error::HWIError(e, code))
+                Err(Error::Hwi(e, code))
             }
             // When HWIDeviceInternal contains errors, some fields might be missing
             // (depending on the error, hwi might not be able to know all of them).
@@ -214,8 +214,8 @@ impl From<HWIStatus> for Result<(), Error> {
         if s.success {
             Ok(())
         } else {
-            Err(Error::HWIError(
-                "Request returned with failure".to_string(),
+            Err(Error::Hwi(
+                "request returned with failure".to_string(),
                 None,
             ))
         }