Add impl From<Nope> for IronError and use it across the codebase

This cleans things up quite a bit. It also makes the eventual transition
to hyper easier, since we can modify the `From` impl instead of changing
each line one at a time.

Author: jyn514

src/web/crate_details.rs

@@ -2,7 +2,7 @@ use super::{match_version, redirect_base, render_markdown, MatchSemver, MetaData
 use crate::{db::Pool, impl_webpage, web::page::WebPage};
 use chrono::{DateTime, NaiveDateTime, Utc};
 use iron::prelude::*;
-use iron::{status, Url};
+use iron::Url;
 use postgres::Client;
 use router::Router;
 use serde::{ser::Serializer, Serialize};
@@ -295,14 +295,14 @@ pub fn crate_details_handler(req: &mut Request) -> IronResult<Response> {
 
     let mut conn = extension!(req, Pool).get()?;
 
-    match match_version(&mut conn, &name, req_version).and_then(|m| m.assume_exact()) {
-        Ok(MatchSemver::Exact((version, _))) => {
+    match match_version(&mut conn, &name, req_version).and_then(|m| m.assume_exact())? {
+        MatchSemver::Exact((version, _)) => {
             let details = cexpect!(req, CrateDetails::new(&mut conn, &name, &version));
 
             CrateDetailsPage { details }.into_response(req)
         }
 
-        Ok(MatchSemver::Semver((version, _))) => {
+        MatchSemver::Semver((version, _)) => {
             let url = ctry!(
                 req,
                 Url::parse(&format!(
@@ -315,8 +315,6 @@ pub fn crate_details_handler(req: &mut Request) -> IronResult<Response> {
 
             Ok(super::redirect(url))
         }
-
-        Err(err) => Err(IronError::new(err, status::NotFound)),
     }
 }
 

src/web/error.rs

@@ -29,6 +29,22 @@ impl fmt::Display for Nope {
 
 impl Error for Nope {}
 
+impl From<Nope> for IronError {
+    fn from(err: Nope) -> IronError {
+        use iron::status;
+
+        let status = match err {
+            Nope::ResourceNotFound
+            | Nope::CrateNotFound
+            | Nope::VersionNotFound
+            | Nope::NoResults => status::NotFound,
+            Nope::InternalServerError => status::InternalServerError,
+        };
+
+        IronError::new(err, status)
+    }
+}
+
 impl Handler for Nope {
     fn handle(&self, req: &mut Request) -> IronResult<Response> {
         match *self {

src/web/file.rs

@@ -2,7 +2,7 @@
 
 use crate::storage::{Blob, Storage};
 use crate::{error::Result, Config};
-use iron::{status, Handler, IronError, IronResult, Request, Response};
+use iron::{status, Handler, IronResult, Request, Response};
 
 #[derive(Debug)]
 pub(crate) struct File(pub(crate) Blob);
@@ -62,10 +62,7 @@ impl Handler for DatabaseFileHandler {
         if let Ok(file) = File::from_path(&storage, &path, &config) {
             Ok(file.serve())
         } else {
-            Err(IronError::new(
-                super::error::Nope::CrateNotFound,
-                status::NotFound,
-            ))
+            Err(super::error::Nope::CrateNotFound.into())
         }
     }
 }

src/web/releases.rs

@@ -12,7 +12,7 @@ use iron::{
     headers::{ContentType, Expires, HttpDate},
     mime::{Mime, SubLevel, TopLevel},
     modifiers::Redirect,
-    status, IronError, IronResult, Request, Response, Url,
+    status, IronResult, Request, Response, Url,
 };
 use postgres::Client;
 use router::Router;
@@ -420,7 +420,7 @@ pub fn author_handler(req: &mut Request) -> IronResult<Response> {
     let author = router
         .find("author")
         // TODO: Accurate error here, the author wasn't provided
-        .ok_or_else(|| IronError::new(Nope::CrateNotFound, status::NotFound))?;
+        .ok_or(Nope::CrateNotFound)?;
 
     let (author_name, releases) = {
         let mut conn = extension!(req, Pool).get()?;
@@ -442,7 +442,7 @@ pub fn author_handler(req: &mut Request) -> IronResult<Response> {
 
     if releases.is_empty() {
         // TODO: Accurate error here, the author wasn't found
-        return Err(IronError::new(Nope::CrateNotFound, status::NotFound));
+        return Err(Nope::CrateNotFound.into());
     }
 
     // Show next and previous page buttons
@@ -621,7 +621,7 @@ pub fn search_handler(req: &mut Request) -> IronResult<Response> {
         }
         .into_response(req)
     } else {
-        Err(IronError::new(Nope::NoResults, status::NotFound))
+        Err(Nope::NoResults.into())
     }
 }
 

src/web/routes.rs

@@ -284,10 +284,7 @@ impl Handler for BlockBlacklistedPrefixes {
     fn handle(&self, req: &mut iron::Request) -> iron::IronResult<iron::Response> {
         if let Some(prefix) = req.url.path().get(0) {
             if self.blacklist.contains(*prefix) {
-                return Err(iron::IronError::new(
-                    super::error::Nope::CrateNotFound,
-                    iron::status::NotFound,
-                ));
+                return Err(super::error::Nope::CrateNotFound.into());
             }
         }
         self.handler.handle(req)

src/web/rustdoc.rs

@@ -12,7 +12,7 @@ use crate::{
 use iron::{
     headers::{CacheControl, CacheDirective, Expires, HttpDate},
     modifiers::Redirect,
-    status, Handler, IronError, IronResult, Request, Response, Url,
+    status, Handler, IronResult, Request, Response, Url,
 };
 use lol_html::errors::RewritingError;
 use router::Router;
@@ -110,10 +110,10 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult<Response> {
 
             let path = req.url.path();
             let path = path.join("/");
-            match File::from_path(&storage, &path, &config) {
-                Ok(f) => return Ok(f.serve()),
-                Err(..) => return Err(IronError::new(Nope::ResourceNotFound, status::NotFound)),
-            }
+            return match File::from_path(&storage, &path, &config) {
+                Ok(f) => Ok(f.serve()),
+                Err(..) => Err(Nope::ResourceNotFound.into()),
+            };
         }
     } else if req
         .url
@@ -142,19 +142,13 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult<Response> {
 
     // it doesn't matter if the version that was given was exact or not, since we're redirecting
     // anyway
-    let (version, id) = match match_version(&mut conn, &crate_name, req_version) {
-        Ok(v) => {
-            if let Some(new_name) = v.corrected_name {
-                // `match_version` checked against -/_ typos, so if we have a name here we should
-                // use that instead
-                crate_name = new_name;
-            }
-            v.version.into_parts()
-        }
-        Err(err) => {
-            return Err(IronError::new(err, status::NotFound));
-        }
-    };
+    let v = match_version(&mut conn, &crate_name, req_version)?;
+    if let Some(new_name) = v.corrected_name {
+        // `match_version` checked against -/_ typos, so if we have a name here we should
+        // use that instead
+        crate_name = new_name;
+    }
+    let (version, id) = v.version.into_parts();
 
     // get target name and whether it has docs
     // FIXME: This is a bit inefficient but allowing us to use less code in general
@@ -288,8 +282,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
     // * If both the name and the version are an exact match, return the version of the crate.
     // * If there is an exact match, but the requested crate name was corrected (dashes vs. underscores), redirect to the corrected name.
     // * If there is a semver (but not exact) match, redirect to the exact version.
-    let release_found = match_version(&mut conn, &name, url_version)
-        .map_err(|err| IronError::new(err, status::NotFound))?;
+    let release_found = match_version(&mut conn, &name, url_version)?;
 
     let version = match release_found.version {
         MatchSemver::Exact((version, _)) => {
@@ -349,7 +342,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
             return if ctry!(req, storage.exists(&path)) {
                 redirect(&name, &version, &req_path[3..])
             } else {
-                Err(IronError::new(Nope::ResourceNotFound, status::NotFound))
+                Err(Nope::ResourceNotFound.into())
             };
         }
     };
@@ -489,7 +482,7 @@ pub fn target_redirect_handler(req: &mut Request) -> IronResult<Response> {
 
     let crate_details = match CrateDetails::new(&mut conn, &name, &version) {
         Some(krate) => krate,
-        None => return Err(IronError::new(Nope::ResourceNotFound, status::NotFound)),
+        None => return Err(Nope::ResourceNotFound.into()),
     };
 
     //   [crate, :name, :version, target-redirect, :target, *path]
@@ -623,7 +616,7 @@ impl Handler for SharedResourceHandler {
         }
 
         // Just always return a 404 here - the main handler will then try the other handlers
-        Err(IronError::new(Nope::ResourceNotFound, status::NotFound))
+        Err(Nope::ResourceNotFound.into())
     }
 }
 

src/web/source.rs

@@ -6,7 +6,7 @@ use crate::{
     web::{error::Nope, file::File as DbFile, page::WebPage, MetaData},
     Config, Storage,
 };
-use iron::{status::Status, IronError, IronResult, Request, Response};
+use iron::{IronResult, Request, Response};
 use postgres::Client;
 use router::Router;
 use serde::Serialize;
@@ -213,8 +213,8 @@ pub fn source_browser_handler(req: &mut Request) -> IronResult<Response> {
         (None, false)
     };
 
-    let file_list = FileList::from_path(&mut conn, &name, &version, &req_path)
-        .ok_or_else(|| IronError::new(Nope::NoResults, Status::NotFound))?;
+    let file_list =
+        FileList::from_path(&mut conn, &name, &version, &req_path).ok_or(Nope::NoResults)?;
 
     SourcePage {
         file_list,

src/web/statics.rs

@@ -4,7 +4,7 @@ use iron::{
     headers::CacheDirective,
     headers::{CacheControl, ContentLength, ContentType, LastModified},
     status::Status,
-    IronError, IronResult, Request, Response, Url,
+    IronResult, Request, Response, Url,
 };
 use mime_guess::MimeGuess;
 use router::Router;
@@ -39,15 +39,15 @@ pub(crate) fn static_handler(req: &mut Request) -> IronResult<Response> {
 fn serve_file(req: &Request, file: &str) -> IronResult<Response> {
     // Filter out files that attempt to traverse directories
     if file.contains("..") || file.contains('/') || file.contains('\\') {
-        return Err(IronError::new(Nope::ResourceNotFound, Status::NotFound));
+        return Err(Nope::ResourceNotFound.into());
     }
 
     // Find the first path that actually exists
     let path = STATIC_SEARCH_PATHS
         .iter()
         .map(|p| Path::new(p).join(file))
         .find(|p| p.exists())
-        .ok_or_else(|| IronError::new(Nope::ResourceNotFound, Status::NotFound))?;
+        .ok_or(Nope::ResourceNotFound)?;
     let contents = ctry!(req, fs::read(&path));
 
     // If we can detect the file's mime type, set it
@@ -106,7 +106,7 @@ pub(super) fn ico_handler(req: &mut Request) -> IronResult<Response> {
     if let Some(&"favicon.ico") = req.url.path().last() {
         // if we're looking for exactly "favicon.ico", we need to defer to the handler that loads
         // from `public_html`, so return a 404 here to make the main handler carry on
-        Err(IronError::new(Nope::ResourceNotFound, Status::NotFound))
+        Err(Nope::ResourceNotFound.into())
     } else {
         // if we're looking for something like "favicon-20190317-1.35.0-nightly-c82834e2b.ico",
         // redirect to the plain one so that the above branch can trigger with the correct filename