From 592d1a55f0c05c1dcba58261f327066b4832a703 Mon Sep 17 00:00:00 2001
From: Daniel Mueller <deso@posteo.net>
Date: Fri, 28 Aug 2020 18:44:45 -0700
Subject: Use anyhow for error handling

This patch changes our error handling approach from the ground up:
instead of having a globally used Error enum that contains variants for
all possible errors, we now use anyhow's Error type. This approach is
more dynamic (and not statically typed), but it allows for more fine
grained error messages and overall more user-friendly error reporting.
Overall it also is a net simplification. While we have one dynamic cast
now, in order to be able to handle erroneous password/PIN entries
correctly, that is considered a reasonable compromise.
---
 src/pinentry.rs | 62 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 27 deletions(-)

(limited to 'src/pinentry.rs')

diff --git a/src/pinentry.rs b/src/pinentry.rs
index e1dec3e..510d7b0 100644
--- a/src/pinentry.rs
+++ b/src/pinentry.rs
@@ -19,12 +19,12 @@
 
 use std::borrow;
 use std::fmt;
-use std::io;
 use std::process;
 use std::str;
 
+use anyhow::Context as _;
+
 use crate::args;
-use crate::error::Error;
 use crate::ExecCtx;
 
 type CowStr = borrow::Cow<'static, str>;
@@ -49,12 +49,15 @@ pub struct PinEntry {
 }
 
 impl PinEntry {
-  pub fn from<'mgr, D>(pin_type: args::PinType, device: &D) -> crate::Result<Self>
+  pub fn from<'mgr, D>(pin_type: args::PinType, device: &D) -> anyhow::Result<Self>
   where
     D: nitrokey::Device<'mgr>,
   {
     let model = device.get_model();
-    let serial = device.get_serial_number()?;
+    let serial = device
+      .get_serial_number()
+      .context("Failed to retrieve serial number")?;
+
     Ok(Self {
       pin_type,
       model,
@@ -122,12 +125,15 @@ pub struct PwdEntry {
 }
 
 impl PwdEntry {
-  pub fn from<'mgr, D>(device: &D) -> crate::Result<Self>
+  pub fn from<'mgr, D>(device: &D) -> anyhow::Result<Self>
   where
     D: nitrokey::Device<'mgr>,
   {
     let model = device.get_model();
-    let serial = device.get_serial_number()?;
+    let serial = device
+      .get_serial_number()
+      .context("Failed to retrieve serial number")?;
+
     Ok(Self { model, serial })
   }
 }
@@ -186,7 +192,7 @@ impl Mode {
   }
 }
 
-fn parse_pinentry_pin<R>(response: R) -> crate::Result<String>
+fn parse_pinentry_pin<R>(response: R) -> anyhow::Result<String>
 where
   R: AsRef<str>,
 {
@@ -208,9 +214,9 @@ where
   // specially.
   if !lines.is_empty() && lines[0].starts_with("ERR ") {
     let (_, error) = lines[0].split_at(4);
-    return Err(Error::from(error));
+    anyhow::bail!("{}", error);
   }
-  Err(Error::Error(format!("Unexpected response: {}", string)))
+  anyhow::bail!("Unexpected response: {}", string)
 }
 
 /// Inquire a secret from the user.
@@ -226,7 +232,7 @@ pub fn inquire<E>(
   entry: &E,
   mode: Mode,
   error_msg: Option<&str>,
-) -> crate::Result<String>
+) -> anyhow::Result<String>
 where
   E: SecretEntry,
 {
@@ -256,30 +262,28 @@ where
     .arg(command)
     .arg("/bye")
     .output()
-    .map_err(|err| match err.kind() {
-      io::ErrorKind::NotFound => {
-        io::Error::new(io::ErrorKind::NotFound, "gpg-connect-agent not found")
-      }
-      _ => err,
-    })?;
-  parse_pinentry_pin(str::from_utf8(&output.stdout)?)
+    .context("Failed to invoke gpg-connect-agent")?;
+
+  let response =
+    str::from_utf8(&output.stdout).context("Failed to parse gpg-connect-agent output as UTF-8")?;
+  parse_pinentry_pin(response).context("Failed to parse pinentry secret")
 }
 
-fn check<E>(entry: &E, secret: &str) -> crate::Result<()>
+fn check<E>(entry: &E, secret: &str) -> anyhow::Result<()>
 where
   E: SecretEntry,
 {
   if secret.len() < usize::from(entry.min_len()) {
-    Err(Error::Error(format!(
+    anyhow::bail!(
       "The secret must be at least {} characters long",
       entry.min_len()
-    )))
+    )
   } else {
     Ok(())
   }
 }
 
-pub fn choose<E>(ctx: &mut ExecCtx<'_>, entry: &E) -> crate::Result<String>
+pub fn choose<E>(ctx: &mut ExecCtx<'_>, entry: &E) -> anyhow::Result<String>
 where
   E: SecretEntry,
 {
@@ -292,13 +296,13 @@ where
   clear(entry)?;
 
   if chosen != confirmed {
-    Err(Error::from("Entered secrets do not match"))
+    anyhow::bail!("Entered secrets do not match")
   } else {
     Ok(chosen)
   }
 }
 
-fn parse_pinentry_response<R>(response: R) -> crate::Result<()>
+fn parse_pinentry_response<R>(response: R) -> anyhow::Result<()>
 where
   R: AsRef<str>,
 {
@@ -309,11 +313,11 @@ where
     // We got the only valid answer we accept.
     return Ok(());
   }
-  Err(Error::Error(format!("Unexpected response: {}", string)))
+  anyhow::bail!("Unexpected response: {}", string)
 }
 
 /// Clear the cached secret represented by the given entry.
-pub fn clear<E>(entry: &E) -> crate::Result<()>
+pub fn clear<E>(entry: &E) -> anyhow::Result<()>
 where
   E: SecretEntry,
 {
@@ -322,9 +326,13 @@ where
     let output = process::Command::new("gpg-connect-agent")
       .arg(command)
       .arg("/bye")
-      .output()?;
+      .output()
+      .context("Failed to invoke gpg-connect-agent")?;
+
+    let response = str::from_utf8(&output.stdout)
+      .context("Failed to parse gpg-connect-agent output as UTF-8")?;
 
-    parse_pinentry_response(str::from_utf8(&output.stdout)?)
+    parse_pinentry_response(response).context("Failed to parse pinentry response")
   } else {
     Ok(())
   }
-- 
cgit v1.2.1