Merge pull request #223 from portier/feat-thiserror

Replace err-derive with thiserror

Cargo.toml

@@ -27,7 +27,6 @@ base64 = "0.13.0"
 bytes = "0.6.0"
 docopt = "1.1.0"
 envy = "0.4.1"
-err-derive = "0.3.0"
 futures-util = "0.3.5"
 gettext = "0.4.0"
 headers = "0.3.2"
@@ -45,6 +44,7 @@ native-tls = "0.2.4"
 percent-encoding = "2.1.0"
 ring = "0.16.15"
 serde_json = "1.0.57"
+thiserror = "1.0.22"
 toml = "0.5.6"
 
 [dependencies.lettre]

src/agents/fetch.rs

@@ -1,25 +1,25 @@
 use crate::utils::agent::{Agent, Context, Handler, Message};
 use crate::utils::BoxError;
 use crate::web::read_body;
-use err_derive::Error;
 use headers::{CacheControl, HeaderMapExt};
 use http::{HeaderValue, Request, StatusCode};
 use hyper::client::{Client, HttpConnector};
 use hyper::Body;
 use hyper_tls::HttpsConnector;
 use std::time::Duration;
+use thiserror::Error;
 use url::Url;
 
 #[derive(Debug, Error)]
 pub enum FetchError {
-    #[error(display = "HTTP request failed: {}", _0)]
-    Hyper(#[error(source)] hyper::Error),
-    #[error(display = "unexpected HTTP status code: {}", _0)]
+    #[error("HTTP request failed: {0}")]
+    Hyper(#[from] hyper::Error),
+    #[error("unexpected HTTP status code: {0}")]
     BadStatus(StatusCode),
-    #[error(display = "could not read HTTP response body: {}", _0)]
+    #[error("could not read HTTP response body: {0}")]
     Read(BoxError),
-    #[error(display = "invalid UTF-8 in HTTP response body: {}", _0)]
-    Utf8(#[error(source)] std::string::FromUtf8Error),
+    #[error("invalid UTF-8 in HTTP response body: {0}")]
+    Utf8(#[from] std::string::FromUtf8Error),
 }
 
 /// The result of fetching a URL.

src/agents/key_manager/manual.rs

@@ -6,20 +6,20 @@ use crate::utils::{
     pem::{self, ParsedKeyPair},
     SecureRandom,
 };
-use err_derive::Error;
 use log::{info, warn};
 use ring::signature::{Ed25519KeyPair, RsaKeyPair};
 use std::fs::File;
 use std::io::BufReader;
 use std::path::PathBuf;
+use thiserror::Error;
 
 #[derive(Debug, Error)]
 pub enum ManualKeysError {
-    #[error(display = "could not parse keytext: {}", _0)]
-    InvalidKeytext(#[error(source)] pem::ParseError),
-    #[error(display = "no PEM data found in keytext")]
+    #[error("could not parse keytext: {0}")]
+    InvalidKeytext(#[from] pem::ParseError),
+    #[error("no PEM data found in keytext")]
     EmptyKeytext,
-    #[error(display = "no {} keys found in keyfiles or keytext", signing_alg)]
+    #[error("no {} keys found in keyfiles or keytext", signing_alg)]
     MissingKeys { signing_alg: SigningAlgorithm },
 }
 
@@ -147,9 +147,7 @@ impl Handler<SignJws> for ManualKeys {
                 .last()
                 .map(|key| key.sign_jws(&message.payload, &self.rng)),
         };
-        cx.reply(
-            maybe_jws.unwrap_or_else(|| Err(SignError::UnsupportedAlgorithm(message.signing_alg))),
-        );
+        cx.reply(maybe_jws.unwrap_or(Err(SignError::UnsupportedAlgorithm(message.signing_alg))));
     }
 }
 

src/agents/key_manager/rotating.rs

@@ -345,9 +345,7 @@ impl Handler<SignJws> for RotatingKeys {
                 .as_ref()
                 .map(|set| set.current.sign_jws(&message.payload, &self.rng)),
         };
-        cx.reply(
-            maybe_jws.unwrap_or_else(|| Err(SignError::UnsupportedAlgorithm(message.signing_alg))),
-        )
+        cx.reply(maybe_jws.unwrap_or(Err(SignError::UnsupportedAlgorithm(message.signing_alg))))
     }
 }
 

src/agents/mailer/mailgun.rs

@@ -58,14 +58,12 @@ impl Handler<SendMail> for MailgunMailer {
         let future = self.fetcher.send(FetchUrl { request });
         cx.reply_later(async move {
             match future.await {
-                Ok(_) => {
-                    return true;
-                }
+                Ok(_) => true,
                 Err(err) => {
                     log::error!("Mailgun request failed: {}", err);
-                    return false;
+                    false
                 }
-            };
+            }
         });
     }
 }

src/agents/store/redis.rs

@@ -40,7 +40,7 @@ impl Message for UpdateKeysLocked {
 /// Store implementation using Redis.
 pub struct RedisStore {
     /// A random unique ID for ourselves.
-    id: Arc<Vec<u8>>,
+    id: Arc<[u8]>,
     /// The connection.
     conn: RedisConn,
     /// Pubsub client.
@@ -77,7 +77,7 @@ impl RedisStore {
         } else if !url.starts_with("redis://") {
             url = format!("redis://{}", &url);
         }
-        let id = Arc::new(rng.generate_async(16).await);
+        let id = rng.generate_async(16).await.into();
         let info = url.as_str().into_connection_info()?;
         let pubsub = pubsub::connect(&info).await?;
         let conn = RedisClient::open(info)?
@@ -293,7 +293,7 @@ impl Handler<EnableRotatingKeys> for RedisStore {
                 tokio::spawn(async move {
                     loop {
                         let from_id = sub.recv().await.expect("Redis keys subscription failed");
-                        if from_id != *my_id2 {
+                        if from_id.as_slice() != my_id2.as_ref() {
                             me2.send(UpdateKeysLocked(signing_alg));
                         }
                     }

src/bridges/oidc.rs

@@ -282,8 +282,8 @@ pub async fn callback(ctx: &mut Context) -> HandlerResult {
         }
         Relation::Google => {
             // Check `email` after additional Google-specific normalization.
-            let email_addr: EmailAddress = email.parse().map_err(|_| {
-                BrokerError::ProviderInput(format!("failed to parse email in {}", descr))
+            let email_addr: EmailAddress = email.parse().map_err(|err| {
+                BrokerError::ProviderInput(format!("failed to parse email in {}: {}", descr, err))
             })?;
             let google_email_addr = email_addr.normalize_google();
             let expected = data.email_addr.normalize_google();

src/config/limits.rs

@@ -1,19 +1,19 @@
 use crate::email_address::EmailAddress;
-use err_derive::Error;
 use serde::de::{Deserialize, Deserializer, Error as DeError};
 use std::{net::IpAddr, num::ParseIntError, str::FromStr, time::Duration};
+use thiserror::Error;
 
 #[derive(Debug, Error, Eq, PartialEq)]
 pub enum LimitConfigError {
-    #[error(display = "rate limit must contain a '/' fraction separator")]
+    #[error("rate limit must contain a '/' fraction separator")]
     NoSeparator,
-    #[error(display = "rate limit window is missing a time unit")]
+    #[error("rate limit window is missing a time unit")]
     NoWindowUnit,
-    #[error(display = "rate limit window has an invalid unit: {}", _0)]
+    #[error("rate limit window has an invalid unit: {0}")]
     InvalidUnit(String),
-    #[error(display = "could not parse rate limit count as integer: {}", _0)]
+    #[error("could not parse rate limit count as integer: {0}")]
     InvalidCount(ParseIntError),
-    #[error(display = "rate limit contains an invalid keyword: {}", _0)]
+    #[error("rate limit contains an invalid keyword: {0}")]
     InvalidKeyword(String),
 }
 

src/config/mod.rs

@@ -22,7 +22,6 @@ use crate::utils::{
     SecureRandom,
 };
 use crate::webfinger::{Link, ParseLinkError, Relation};
-use err_derive::Error;
 use ipnetwork::IpNetwork;
 use std::{
     borrow::ToOwned,
@@ -33,20 +32,27 @@ use std::{
     sync::Arc,
     time::Duration,
 };
+use thiserror::Error;
 
 /// Union of all possible error types seen while parsing.
 #[derive(Debug, Error)]
 pub enum ConfigError {
-    #[error(display = "configuration error: {}", _0)]
-    Custom(#[error(from)] &'static str),
-    #[error(display = "IO error: {}", _0)]
-    Io(#[error(source)] IoError),
-    #[error(display = "TOML error: {}", _0)]
-    Toml(#[error(source)] ::toml::de::Error),
-    #[error(display = "keys configuration error: {}", _0)]
-    ManualKeys(#[error(source)] ManualKeysError),
-    #[error(display = "domain override configuration error: {}", _0)]
-    DomainOverride(#[error(source)] ParseLinkError),
+    #[error("configuration error: {0}")]
+    Custom(&'static str),
+    #[error("IO error: {0}")]
+    Io(#[from] IoError),
+    #[error("TOML error: {0}")]
+    Toml(#[from] ::toml::de::Error),
+    #[error("keys configuration error: {0}")]
+    ManualKeys(#[from] ManualKeysError),
+    #[error("domain override configuration error: {0}")]
+    DomainOverride(#[from] ParseLinkError),
+}
+
+impl From<&'static str> for ConfigError {
+    fn from(msg: &'static str) -> ConfigError {
+        ConfigError::Custom(msg)
+    }
 }
 
 pub type ConfigRc = Arc<Config>;

src/crypto.rs

@@ -3,7 +3,6 @@ use crate::bridges::oidc::ProviderKey;
 use crate::config::Config;
 use crate::email_address::EmailAddress;
 use crate::utils::{base64url, keys::SignError, unix_duration, SecureRandom};
-use err_derive::Error;
 use ring::{
     digest,
     error::Unspecified,
@@ -13,6 +12,7 @@ use serde_json as json;
 use serde_json::{json, Error as JsonError, Value};
 use std::fmt;
 use std::iter::Iterator;
+use thiserror::Error;
 
 type RsaPublicKey = signature::RsaPublicKeyComponents<Vec<u8>>;
 
@@ -118,36 +118,33 @@ pub async fn random_zbase32(len: usize, rng: &SecureRandom) -> String {
 
 #[derive(Debug, Error)]
 pub enum VerifyError {
-    #[error(display = "the token must consist of three dot-separated parts")]
+    #[error("the token must consist of three dot-separated parts")]
     IncorrectFormat,
-    #[error(display = "token part {} contained invalid base64: {}", index, reason)]
+    #[error("token part {index} contained invalid base64: {reason}")]
     InvalidPartBase64 {
         index: usize,
         reason: base64::DecodeError,
     },
-    #[error(display = "the token header contained invalid JSON: {}", _0)]
+    #[error("the token header contained invalid JSON: {0}")]
     InvalidHeaderJson(JsonError),
-    #[error(display = "did not find a string 'kid' property in the token header")]
+    #[error("did not find a string 'kid' property in the token header")]
     KidMissing,
-    #[error(
-        display = "the token 'kid' could not be found in the JWKs document: {}",
-        kid
-    )]
+    #[error("the token 'kid' could not be found in the JWKs document: {kid}")]
     KidNotMatched { kid: String },
     #[error(
-        display = "the '{}' field of the matching JWK contains invalid base64: {}",
+        "the '{}' field of the matching JWK contains invalid base64: {}",
         property,
         reason
     )]
     InvalidJwkBase64 {
         property: &'static str,
         reason: base64::DecodeError,
     },
-    #[error(display = "the matching JWK is of an unsupported type")]
+    #[error("the matching JWK is of an unsupported type")]
     UnsupportedKeyType,
-    #[error(display = "the token signature did not validate using the matching JWK")]
+    #[error("the token signature did not validate using the matching JWK")]
     BadSignature,
-    #[error(display = "the token payload contained invalid JSON: {}", _0)]
+    #[error("the token payload contained invalid JSON: {0}")]
     InvalidPayloadJson(JsonError),
 }
 
@@ -224,7 +221,7 @@ pub fn verify_jws(
     let message_len = parts[0].len() + parts[1].len() + 1;
     pub_key
         .verify(jws[..message_len].as_bytes(), &decoded[2])
-        .map_err(|_| VerifyError::BadSignature)?;
+        .map_err(|_err| VerifyError::BadSignature)?;
 
     // Return the payload.
     Ok(json::from_slice(&decoded[1]).map_err(VerifyError::InvalidPayloadJson)?)

src/email_address.rs

@@ -1,9 +1,9 @@
 use crate::utils::TLDS;
-use err_derive::Error;
 use matches::matches;
 use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
 use std::hash::{Hash, Hasher};
 use std::str::FromStr;
+use thiserror::Error;
 
 fn is_invalid_domain_char(c: char) -> bool {
     matches!(
@@ -14,17 +14,17 @@ fn is_invalid_domain_char(c: char) -> bool {
 
 #[derive(Debug, Error)]
 pub enum ParseEmailError {
-    #[error(display = "missing '@' separator in email address")]
+    #[error("missing '@' separator in email address")]
     NoSeparator,
-    #[error(display = "local part of an email address cannot be empty")]
+    #[error("local part of an email address cannot be empty")]
     EmptyLocal,
-    #[error(display = "invalid international domain name in email address")]
-    InvalidIdna(#[error(from)] idna::Errors),
-    #[error(display = "domain part of an email address cannot be empty")]
+    #[error("invalid international domain name in email address")]
+    InvalidIdna(idna::Errors),
+    #[error("domain part of an email address cannot be empty")]
     EmptyDomain,
-    #[error(display = "email address contains invalid characters in the domain part")]
+    #[error("email address contains invalid characters in the domain part")]
     InvalidDomainChars,
-    #[error(display = "email address domain part is not in a public top-level domain")]
+    #[error("email address domain part is not in a public top-level domain")]
     InvalidTld,
 }
 
@@ -61,7 +61,8 @@ impl FromStr for EmailAddress {
             return Err(ParseEmailError::EmptyLocal);
         }
         // Verify and normalize the domain
-        let domain = idna::domain_to_ascii(&input[local_end + 1..])?;
+        let domain =
+            idna::domain_to_ascii(&input[local_end + 1..]).map_err(ParseEmailError::InvalidIdna)?;
         if domain == "" {
             return Err(ParseEmailError::EmptyDomain);
         }

src/handlers/auth.rs

@@ -80,14 +80,14 @@ pub async fn auth(ctx: &mut Context) -> HandlerResult {
 
     // Parse response_mode by wrapping it a JSON Value.
     // This has minimal overhead, and saves us a separate implementation.
-    let response_mode = from_value(Value::String(response_mode)).map_err(|_| {
+    let response_mode = from_value(Value::String(response_mode)).map_err(|_err| {
         BrokerError::Input("unsupported response_mode, must be fragment or form_post".to_owned())
     })?;
 
     // NOTE: This query parameter is non-standard.
     let response_errors = response_errors
         .parse::<bool>()
-        .map_err(|_| BrokerError::Input("response_errors must be true or false".to_owned()))?;
+        .map_err(|_err| BrokerError::Input("response_errors must be true or false".to_owned()))?;
 
     // Per the OAuth2 spec, we may redirect to the RP once we have validated client_id and
     // redirect_uri. In our case, this means we make redirect_uri available to error handling.
@@ -169,9 +169,9 @@ pub async fn auth(ctx: &mut Context) -> HandlerResult {
     }
 
     // Verify and normalize the email.
-    let email_addr = login_hint
-        .parse::<EmailAddress>()
-        .map_err(|_| BrokerError::Input("login_hint is not a valid email address".to_owned()))?;
+    let email_addr = login_hint.parse::<EmailAddress>().map_err(|err| {
+        BrokerError::Input(format!("login_hint is not a valid email address: {}", err))
+    })?;
 
     // Enforce rate limits.
     match ctx

src/utils/keys.rs

@@ -4,7 +4,6 @@ use crate::utils::{
     pem::{self, ParsedKeyPair},
     SecureRandom,
 };
-use err_derive::Error;
 use ring::{
     digest,
     io::Positive,
@@ -13,12 +12,13 @@ use ring::{
 use serde_json::{json, Value as JsonValue};
 use std::ffi::OsString;
 use std::process::{Command, Stdio};
+use thiserror::Error;
 
 #[derive(Debug, Error)]
 pub enum SignError {
-    #[error(display = "unsupported signing algorithm {}", _0)]
+    #[error("unsupported signing algorithm {0}")]
     UnsupportedAlgorithm(SigningAlgorithm),
-    #[error(display = "unspecified signing error")]
+    #[error("unspecified signing error")]
     Unspecified,
 }