Use anyhow for error handling

Author: Turbo87

Cargo.lock

@@ -28,6 +28,7 @@ rustdoc-args = [
 ]
 
 [dependencies]
+anyhow = "1.0"
 cargo-registry-s3 = { path = "src/s3", version = "0.2.0" }
 rand = "0.7"
 git2 = "0.13.0"

Cargo.toml

@@ -1,11 +1,12 @@
 #![deny(clippy::all)]
 
-use cargo_registry::{db, env, tasks, util::Error};
+use anyhow::{anyhow, Result};
+use cargo_registry::{db, env, tasks};
 use diesel::prelude::*;
 use swirl::schema::background_jobs::dsl::*;
 use swirl::Job;
 
-fn main() -> Result<(), Error> {
+fn main() -> Result<()> {
     let conn = db::connect_now()?;
     let mut args = std::env::args().skip(1);
 
@@ -34,6 +35,6 @@ fn main() -> Result<(), Error> {
                 .unwrap_or_else(|| String::from("db-dump.tar.gz"));
             Ok(tasks::dump_db(database_url, target_name).enqueue(&conn)?)
         }
-        other => Err(Error::from(format!("Unrecognized job type `{}`", other))),
+        other => Err(anyhow!("Unrecognized job type `{}`", other)),
     }
 }

src/bin/enqueue-job.rs

@@ -8,10 +8,11 @@
 
 mod on_call;
 
-use cargo_registry::{db, schema::*, util::Error};
+use anyhow::Result;
+use cargo_registry::{db, schema::*};
 use diesel::prelude::*;
 
-fn main() -> Result<(), Error> {
+fn main() -> Result<()> {
     let conn = db::connect_now()?;
 
     check_failing_background_jobs(&conn)?;
@@ -28,7 +29,7 @@ fn main() -> Result<(), Error> {
 ///
 /// Within the default 15 minute time, a job should have already had several
 /// failed retry attempts.
-fn check_failing_background_jobs(conn: &PgConnection) -> Result<(), Error> {
+fn check_failing_background_jobs(conn: &PgConnection) -> Result<()> {
     use cargo_registry::schema::background_jobs::dsl::*;
     use diesel::dsl::*;
     use diesel::sql_types::Integer;
@@ -70,7 +71,7 @@ fn check_failing_background_jobs(conn: &PgConnection) -> Result<(), Error> {
 }
 
 /// Check for an `update_downloads` job that has run longer than expected
-fn check_stalled_update_downloads(conn: &PgConnection) -> Result<(), Error> {
+fn check_stalled_update_downloads(conn: &PgConnection) -> Result<()> {
     use cargo_registry::schema::background_jobs::dsl::*;
     use chrono::{DateTime, NaiveDateTime, Utc};
 
@@ -107,7 +108,7 @@ fn check_stalled_update_downloads(conn: &PgConnection) -> Result<(), Error> {
 }
 
 /// Check for known spam patterns
-fn check_spam_attack(conn: &PgConnection) -> Result<(), Error> {
+fn check_spam_attack(conn: &PgConnection) -> Result<()> {
     use cargo_registry::models::krate::canon_crate_name;
     use diesel::dsl::*;
     use diesel::sql_types::Bool;
@@ -168,7 +169,7 @@ fn check_spam_attack(conn: &PgConnection) -> Result<(), Error> {
     Ok(())
 }
 
-fn log_and_trigger_event(event: on_call::Event) -> Result<(), Error> {
+fn log_and_trigger_event(event: on_call::Event) -> Result<()> {
     match event {
         on_call::Event::Trigger {
             ref description, ..

src/bin/monitor.rs

@@ -1,5 +1,4 @@
-use cargo_registry::util::Error;
-
+use anyhow::{anyhow, Result};
 use reqwest::{blocking::Client, header, StatusCode as Status};
 
 #[derive(serde::Serialize, Debug)]
@@ -25,7 +24,7 @@ impl Event {
     ///
     /// If the variant is `Trigger`, this will page whoever is on call
     /// (potentially waking them up at 3 AM).
-    pub fn send(self) -> Result<(), Error> {
+    pub fn send(self) -> Result<()> {
         let api_token = dotenv::var("PAGERDUTY_API_TOKEN")?;
         let service_key = dotenv::var("PAGERDUTY_INTEGRATION_KEY")?;
 
@@ -43,15 +42,15 @@ impl Event {
             s if s.is_success() => Ok(()),
             Status::BAD_REQUEST => {
                 let error = response.json::<InvalidEvent>()?;
-                Err(format!("pagerduty error: {:?}", error))
+                Err(anyhow!("pagerduty error: {:?}", error))
             }
-            Status::FORBIDDEN => Err("rate limited by pagerduty".to_string()),
-            n => Err(format!(
+            Status::FORBIDDEN => Err(anyhow!("rate limited by pagerduty")),
+            n => Err(anyhow!(
                 "Got a non 200 response code from pagerduty: {} with {:?}",
-                n, response
+                n,
+                response
             )),
         }
-        .map_err(Into::into)
     }
 }
 

src/bin/on_call/mod.rs

@@ -11,9 +11,9 @@ mod on_call;
 
 use std::env::args;
 
-use cargo_registry::util::Error;
+use anyhow::Result;
 
-fn main() -> Result<(), Error> {
+fn main() -> Result<()> {
     let args = args().collect::<Vec<_>>();
 
     let event_type = &*args[1];

src/bin/test-pagerduty.rs

@@ -1,8 +1,9 @@
 // Sync available crate categories from `src/categories.toml`.
 // Runs when the server is started.
 
-use crate::{db, util::Error};
+use crate::db;
 
+use anyhow::{Context, Result};
 use diesel::prelude::*;
 
 #[derive(Debug)]
@@ -34,16 +35,10 @@ impl Category {
     }
 }
 
-fn required_string_from_toml<'a>(
-    toml: &'a toml::value::Table,
-    key: &str,
-) -> Result<&'a str, Error> {
-    toml.get(key).and_then(toml::Value::as_str).ok_or_else(|| {
-        Error::from(format!(
-            "Expected category TOML attribute '{}' to be a String",
-            key
-        ))
-    })
+fn required_string_from_toml<'a>(toml: &'a toml::value::Table, key: &str) -> Result<&'a str> {
+    toml.get(key)
+        .and_then(toml::Value::as_str)
+        .with_context(|| format!("Expected category TOML attribute '{}' to be a String", key))
 }
 
 fn optional_string_from_toml<'a>(toml: &'a toml::value::Table, key: &str) -> &'a str {
@@ -53,13 +48,13 @@ fn optional_string_from_toml<'a>(toml: &'a toml::value::Table, key: &str) -> &'a
 fn categories_from_toml(
     categories: &toml::value::Table,
     parent: Option<&Category>,
-) -> Result<Vec<Category>, Error> {
+) -> Result<Vec<Category>> {
     let mut result = vec![];
 
     for (slug, details) in categories {
         let details = details
             .as_table()
-            .ok_or_else(|| Error::from(format!("category {} was not a TOML table", slug)))?;
+            .with_context(|| format!("category {} was not a TOML table", slug))?;
 
         let category = Category::from_parent(
             slug,
@@ -71,7 +66,7 @@ fn categories_from_toml(
         if let Some(categories) = details.get("categories") {
             let categories = categories
                 .as_table()
-                .ok_or_else(|| format!("child categories of {} were not a table", slug))?;
+                .with_context(|| format!("child categories of {} were not a table", slug))?;
 
             result.extend(categories_from_toml(categories, Some(&category))?);
         }
@@ -82,18 +77,18 @@ fn categories_from_toml(
     Ok(result)
 }
 
-pub fn sync(toml_str: &str) -> Result<(), Error> {
-    let conn = db::connect_now().unwrap();
+pub fn sync(toml_str: &str) -> Result<()> {
+    let conn = db::connect_now()?;
     sync_with_connection(toml_str, &conn)
 }
 
-pub fn sync_with_connection(toml_str: &str, conn: &PgConnection) -> Result<(), Error> {
+pub fn sync_with_connection(toml_str: &str, conn: &PgConnection) -> Result<()> {
     use crate::schema::categories::dsl::*;
     use diesel::dsl::all;
     use diesel::pg::upsert::excluded;
 
     let toml: toml::value::Table =
-        toml::from_str(toml_str).expect("Could not parse categories toml");
+        toml::from_str(toml_str).context("Could not parse categories toml")?;
 
     let to_insert = categories_from_toml(&toml, None)
         .expect("Could not convert categories from TOML")

src/boot/categories.rs

@@ -11,8 +11,9 @@
 use super::prelude::*;
 use std::fmt::Write;
 
-use crate::util::{errors::NotFound, AppResponse, Error};
+use crate::util::{errors::NotFound, AppResponse};
 
+use anyhow::{ensure, Result};
 use conduit::{Body, HandlerResult};
 use conduit_static::Static;
 use reqwest::blocking::Client;
@@ -56,7 +57,7 @@ impl Handler for EmberHtml {
                 // During local fastboot development, forward requests to the local fastboot server.
                 // In prodution, including when running with fastboot, nginx proxies the requests
                 // to the correct endpoint and requests should never make it here.
-                return proxy_to_fastboot(client, req).map_err(box_error);
+                return proxy_to_fastboot(client, req).map_err(From::from);
             }
 
             if req
@@ -86,14 +87,16 @@ impl Handler for EmberHtml {
 /// # Panics
 ///
 /// This function can panic and should only be used in development mode.
-fn proxy_to_fastboot(client: &Client, req: &mut dyn RequestExt) -> Result<AppResponse, Error> {
-    if req.method() != conduit::Method::GET {
-        return Err(format!("Only support GET but request method was {}", req.method()).into());
-    }
+fn proxy_to_fastboot(client: &Client, req: &mut dyn RequestExt) -> Result<AppResponse> {
+    ensure!(
+        req.method() == conduit::Method::GET,
+        "Only support GET but request method was {}",
+        req.method()
+    );
 
     let mut url = format!("http://127.0.0.1:9000{}", req.path());
     if let Some(query) = req.query_string() {
-        write!(url, "?{}", query).map_err(|e| e.to_string())?;
+        write!(url, "?{}", query)?;
     }
     let mut fastboot_response = client
         .request(req.method().into(), &*url)
@@ -107,5 +110,5 @@ fn proxy_to_fastboot(client: &Client, req: &mut dyn RequestExt) -> Result<AppRes
         .headers_mut()
         .unwrap()
         .extend(fastboot_response.headers().clone());
-    builder.body(Body::from_vec(body)).map_err(Into::into)
+    Ok(builder.body(Body::from_vec(body))?)
 }

src/middleware/ember_html.rs

@@ -1,10 +1,11 @@
+use anyhow::Result;
 use conduit::RequestExt;
 use flate2::read::GzDecoder;
 use reqwest::{blocking::Client, header};
 use sha2::{Digest, Sha256};
 
 use crate::util::errors::{cargo_err, internal, AppResult, ChainError};
-use crate::util::{Error, LimitErrorReader, Maximums};
+use crate::util::{LimitErrorReader, Maximums};
 
 use std::env;
 use std::fs::{self, File};
@@ -101,7 +102,7 @@ impl Uploader {
         content_length: u64,
         content_type: &str,
         extra_headers: header::HeaderMap,
-    ) -> Result<Option<String>, Error> {
+    ) -> Result<Option<String>> {
         match *self {
             Uploader::S3 { ref bucket, .. } => {
                 bucket.put(
@@ -164,7 +165,7 @@ impl Uploader {
         crate_name: &str,
         vers: &str,
         readme: String,
-    ) -> Result<(), Error> {
+    ) -> Result<()> {
         let path = Uploader::readme_path(crate_name, vers);
         let content_length = readme.len() as u64;
         let content = Cursor::new(readme);

src/uploaders.rs

@@ -4,7 +4,6 @@ use conduit::{header, Body, Response};
 use rand::{distributions::Uniform, rngs::OsRng, Rng};
 use serde::Serialize;
 
-pub use self::errors::concrete::Error;
 pub use self::io_util::{read_fill, read_le_u32, LimitErrorReader};
 pub use self::request_helpers::*;
 pub use self::request_proxy::RequestProxy;

src/util.rs

@@ -21,7 +21,6 @@ use diesel::result::Error as DieselError;
 
 use crate::util::AppResponse;
 
-pub(super) mod concrete;
 mod json;
 
 pub(crate) use json::{InsecurelyGeneratedTokenRevoked, NotFound, ReadOnlyMode, TooManyRequests};