Fix error handling

- Return 500, not 400, for internal errors
- Return an error for internal errors in search, not 'not found'

Author: jyn514

src/web/mod.rs

@@ -29,7 +29,7 @@ macro_rules! ctry {
                     message: ::std::option::Option::Some(::std::borrow::Cow::Owned(
                         ::std::format!("{}", error),
                     )),
-                    status: ::iron::status::BadRequest,
+                    status: ::iron::status::InternalServerError,
                 };
 
                 return $crate::web::page::WebPage::into_response(error, request);
@@ -58,7 +58,7 @@ macro_rules! cexpect {
                 let error = $crate::web::ErrorPage {
                     title: "Internal Server Error",
                     message: None,
-                    status: ::iron::status::BadRequest,
+                    status: ::iron::status::InternalServerError,
                 };
 
                 return $crate::web::page::WebPage::into_response(error, request);

src/web/releases.rs

@@ -227,7 +227,7 @@ fn get_search_results(
     mut query: &str,
     page: i64,
     limit: i64,
-) -> (i64, Vec<Release>) {
+) -> Result<(i64, Vec<Release>), failure::Error> {
     query = query.trim();
     let offset = (page - 1) * limit;
 
@@ -265,11 +265,7 @@ fn get_search_results(
             releases.downloads DESC
         LIMIT $2 OFFSET $3";
 
-    let rows = if let Ok(rows) = conn.query(statement, &[&query, &limit, &offset]) {
-        rows
-    } else {
-        return (0, Vec::new());
-    };
+    let rows = conn.query(statement, &[&query, &limit, &offset])?;
 
     // Each row contains the total number of possible/valid results, just get it once
     let total_results = rows
@@ -289,7 +285,7 @@ fn get_search_results(
         })
         .collect();
 
-    (total_results, packages)
+    Ok((total_results, packages))
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
@@ -605,7 +601,10 @@ pub fn search_handler(req: &mut Request) -> IronResult<Response> {
             }
         }
 
-        let (_, results) = get_search_results(&mut conn, &query, 1, RELEASES_IN_RELEASES);
+        let (_, results) = ctry!(
+            req,
+            get_search_results(&mut conn, &query, 1, RELEASES_IN_RELEASES)
+        );
         let title = if results.is_empty() {
             format!("No results found for '{}'", query)
         } else {
@@ -719,7 +718,7 @@ mod tests {
                 .version("0.0.0")
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "foo", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "foo", 1, 100)?;
             assert_eq!(num_results, 4);
 
             let mut results = results.into_iter();
@@ -748,7 +747,7 @@ mod tests {
 
             for name in near_matches.iter() {
                 let (num_results, mut results) =
-                    dbg!(get_search_results(&mut db.conn(), *name, 1, 100));
+                    dbg!(get_search_results(&mut db.conn(), *name, 1, 100))?;
                 assert_eq!(num_results, 3);
 
                 for name in releases.iter() {
@@ -771,7 +770,7 @@ mod tests {
                 .build_result_successful(false)
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "regex", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "regex", 1, 100)?;
             assert_eq!(num_results, 0);
 
             let results = results.into_iter();
@@ -791,7 +790,7 @@ mod tests {
                 .yanked(true)
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "regex", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "regex", 1, 100)?;
             assert_eq!(num_results, 0);
 
             let results = results.into_iter();
@@ -807,7 +806,7 @@ mod tests {
             let db = env.db();
             env.fake_release().name("regex").version("0.0.0").create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "redex", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "redex", 1, 100)?;
             assert_eq!(num_results, 1);
 
             let mut results = results.into_iter();
@@ -829,7 +828,7 @@ mod tests {
     //             .create()?;
     //
     //         let (num_results, results) =
-    //             get_search_results(&mut db.conn(), "supercalifragilisticexpialidocious", 1, 100);
+    //             get_search_results(&mut db.conn(), "supercalifragilisticexpialidocious", 1, 100)?;
     //         assert_eq!(num_results, 1);
     //
     //         let mut results = results.into_iter();
@@ -855,7 +854,7 @@ mod tests {
                 .name("something_completely_unrelated")
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "something", 1, 2);
+            let (num_results, results) = get_search_results(&mut db.conn(), "something", 1, 2)?;
             assert_eq!(num_results, 4);
 
             let mut results = results.into_iter();
@@ -878,7 +877,7 @@ mod tests {
                 .name("something_completely_unrelated")
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "something", 2, 2);
+            let (num_results, results) = get_search_results(&mut db.conn(), "something", 2, 2)?;
             assert_eq!(num_results, 4);
 
             let mut results = results.into_iter();
@@ -922,7 +921,7 @@ mod tests {
                 .version("0.0.0")
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "somethang", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "somethang", 1, 100)?;
             assert_eq!(num_results, 1);
 
             let mut results = results.into_iter();
@@ -954,7 +953,7 @@ mod tests {
     //             .create()?;
     //
     //         let (num_results, results) =
-    //             get_search_results(&mut db.conn(), "name_better_than_description", 1, 100);
+    //             get_search_results(&mut db.conn(), "name_better_than_description", 1, 100)?;
     //         assert_eq!(num_results, 2);
     //
     //         let mut results = results.into_iter();
@@ -987,7 +986,7 @@ mod tests {
                 .name("i_am_useless_and_mean_nothing")
                 .create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "match", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "match", 1, 100)?;
             assert_eq!(num_results, 3);
 
             let mut results = results.into_iter();
@@ -1008,7 +1007,7 @@ mod tests {
             env.fake_release().name("matcb").downloads(10).create()?;
             env.fake_release().name("matcc").downloads(1).create()?;
 
-            let (num_results, results) = get_search_results(&mut db.conn(), "match", 1, 100);
+            let (num_results, results) = get_search_results(&mut db.conn(), "match", 1, 100)?;
             assert_eq!(num_results, 3);
 
             let mut results = results.into_iter();