Error response refactoring step 2

src/controllers.rs

@@ -1,3 +1,13 @@
+mod cargo_prelude {
+    pub use super::prelude::*;
+    pub use crate::util::cargo_err;
+}
+
+mod frontend_prelude {
+    pub use super::prelude::*;
+    pub use crate::util::errors::{bad_request, server_error};
+}
+
 mod prelude {
     pub use super::helpers::ok_true;
     pub use diesel::prelude::*;
@@ -6,7 +16,7 @@ mod prelude {
     pub use conduit_router::RequestParams;
 
     pub use crate::db::RequestTransaction;
-    pub use crate::util::{cargo_err, AppResult};
+    pub use crate::util::{cargo_err, AppResult}; // TODO: Remove cargo_err from here
 
     pub use crate::middleware::app::RequestApp;
     pub use crate::middleware::current_user::RequestUser;

src/controllers/crate_owner_invitation.rs

@@ -1,4 +1,4 @@
-use super::prelude::*;
+use super::frontend_prelude::*;
 
 use crate::models::{CrateOwner, CrateOwnerInvitation, OwnerKind};
 use crate::schema::{crate_owner_invitations, crate_owners};
@@ -38,7 +38,7 @@ pub fn handle_invite(req: &mut dyn Request) -> AppResult<Response> {
     let conn = &*req.db_conn()?;
 
     let crate_invite: OwnerInvitation =
-        serde_json::from_str(&body).map_err(|_| cargo_err("invalid json request"))?;
+        serde_json::from_str(&body).map_err(|_| bad_request("invalid json request"))?;
 
     let crate_invite = crate_invite.crate_owner_invite;
 

src/controllers/krate/downloads.rs

@@ -5,7 +5,7 @@
 
 use std::cmp;
 
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::models::{Crate, CrateVersions, Version, VersionDownload};
 use crate::schema::version_downloads;

src/controllers/krate/follow.rs

@@ -2,7 +2,7 @@
 
 use diesel::associations::Identifiable;
 
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 use crate::db::DieselPooledConn;
 use crate::models::{Crate, Follow};
 use crate::schema::*;

src/controllers/krate/metadata.rs

@@ -4,7 +4,7 @@
 //! index or cached metadata which was extracted (client side) from the
 //! `Cargo.toml` file.
 
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 use crate::models::{
     Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads,
     User, Version,

src/controllers/krate/publish.rs

@@ -4,7 +4,7 @@ use hex::ToHex;
 use std::sync::Arc;
 use swirl::Job;
 
-use crate::controllers::prelude::*;
+use crate::controllers::cargo_prelude::*;
 use crate::git;
 use crate::models::dependency;
 use crate::models::{

src/controllers/krate/search.rs

@@ -3,8 +3,8 @@
 use diesel::dsl::*;
 use diesel_full_text_search::*;
 
+use crate::controllers::cargo_prelude::*;
 use crate::controllers::helpers::Paginate;
-use crate::controllers::prelude::*;
 use crate::models::{Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, Version};
 use crate::schema::*;
 use crate::views::EncodableCrate;

src/controllers/team.rs

@@ -1,4 +1,4 @@
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::models::Team;
 use crate::schema::teams;

src/controllers/user/me.rs

@@ -1,10 +1,9 @@
 use std::collections::HashMap;
 
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::controllers::helpers::*;
 use crate::email;
-use crate::util::bad_request;
 use crate::util::errors::AppError;
 
 use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version};
@@ -118,7 +117,7 @@ pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
 
     // need to check if current user matches user to be updated
     if &user.id.to_string() != name {
-        return Err(cargo_err("current user does not match requested user"));
+        return Err(bad_request("current user does not match requested user"));
     }
 
     #[derive(Deserialize)]
@@ -132,17 +131,17 @@ pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
     }
 
     let user_update: UserUpdate =
-        serde_json::from_str(&body).map_err(|_| cargo_err("invalid json request"))?;
+        serde_json::from_str(&body).map_err(|_| bad_request("invalid json request"))?;
 
     if user_update.user.email.is_none() {
-        return Err(cargo_err("empty email rejected"));
+        return Err(bad_request("empty email rejected"));
     }
 
     let user_email = user_update.user.email.unwrap();
     let user_email = user_email.trim();
 
     if user_email == "" {
-        return Err(cargo_err("empty email rejected"));
+        return Err(bad_request("empty email rejected"));
     }
 
     conn.transaction::<_, Box<dyn AppError>, _>(|| {
@@ -158,7 +157,7 @@ pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
             .set(&new_email)
             .returning(emails::token)
             .get_result::<String>(&*conn)
-            .map_err(|_| cargo_err("Error in creating token"))?;
+            .map_err(|_| server_error("Error in creating token"))?;
 
         crate::email::send_user_confirm_email(user_email, &user.gh_login, &token);
 
@@ -197,7 +196,7 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> AppResult<Response> {
 
     // need to check if current user matches user to be updated
     if &user.id != name {
-        return Err(cargo_err("current user does not match requested user"));
+        return Err(bad_request("current user does not match requested user"));
     }
 
     conn.transaction(|| {
@@ -207,7 +206,7 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> AppResult<Response> {
             .map_err(|_| bad_request("Email could not be found"))?;
 
         email::try_send_user_confirm_email(&email.email, &user.gh_login, &email.token)
-            .map_err(|_| bad_request("Error in sending email"))
+            .map_err(|_| server_error("Error in sending email"))
     })?;
 
     ok_true()

src/controllers/user/other.rs

@@ -1,4 +1,4 @@
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::models::{OwnerKind, User};
 use crate::schema::{crate_owners, crates, users};

src/controllers/user/session.rs

@@ -1,12 +1,13 @@
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::github;
 use conduit_cookie::RequestSession;
+use failure::Fail;
 use oauth2::{prelude::*, AuthorizationCode, TokenResponse};
 
 use crate::models::{NewUser, User};
 use crate::schema::users;
-use crate::util::errors::{AppError, ReadOnlyMode};
+use crate::util::errors::{AppError, ChainError, ReadOnlyMode};
 
 /// Handles the `GET /api/private/session/begin` route.
 ///
@@ -83,7 +84,7 @@ pub fn authorize(req: &mut dyn Request) -> AppResult<Response> {
         let session_state = req.session().remove(&"github_oauth_state".to_string());
         let session_state = session_state.as_ref().map(|a| &a[..]);
         if Some(&state[..]) != session_state {
-            return Err(cargo_err("invalid state parameter"));
+            return Err(bad_request("invalid state parameter"));
         }
     }
 
@@ -94,7 +95,8 @@ pub fn authorize(req: &mut dyn Request) -> AppResult<Response> {
         .app()
         .github
         .exchange_code(code)
-        .map_err(|s| cargo_err(&s))?;
+        .map_err(|e| e.compat())
+        .chain_error(|| server_error("Error obtaining token"))?;
     let token = token.access_token();
     let ghuser = github::github_api::<GithubUser>(req.app(), "/user", token)?;
     let user = ghuser.save_to_database(&token.secret(), &*req.db_conn()?)?;

src/controllers/version/deprecated.rs

@@ -5,7 +5,7 @@
 //! period of time to ensure there are no external users of an endpoint before
 //! it is removed.
 
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::models::{Crate, User, Version};
 use crate::schema::*;

src/controllers/version/metadata.rs

@@ -4,7 +4,7 @@
 //! index or cached metadata which was extracted (client side) from the
 //! `Cargo.toml` file.
 
-use crate::controllers::prelude::*;
+use crate::controllers::frontend_prelude::*;
 
 use crate::schema::*;
 use crate::views::{EncodableDependency, EncodablePublicUser, EncodableVersion};

src/controllers/version/yank.rs

@@ -3,7 +3,7 @@
 use swirl::Job;
 
 use super::version_and_crate;
-use crate::controllers::prelude::*;
+use crate::controllers::cargo_prelude::*;
 use crate::git;
 use crate::models::{insert_version_owner_action, Rights, VersionAction};
 use crate::util::AppError;

src/middleware/log_request.rs

@@ -53,7 +53,7 @@ impl Handler for LogRequests {
         );
 
         if let Err(ref e) = res {
-            print!(" error=\"{}\"", e.description());
+            print!(" error=\"{}\"", e.to_string());
         }
 
         if response_time > 1000 {

src/tests/user.rs

@@ -115,7 +115,7 @@ fn access_token_needs_data() {
     let (_, anon) = TestApp::init().empty();
     let json = anon
         .get::<()>("/api/private/session/authorize")
-        .bad_with_status(200); // Change endpoint to 400?
+        .bad_with_status(400);
     assert!(json.errors[0].detail.contains("invalid state"));
 }
 
@@ -475,7 +475,7 @@ fn test_empty_email_not_added() {
 
     let json = user
         .update_email_more_control(model.id, Some(""))
-        .bad_with_status(200);
+        .bad_with_status(400);
     assert!(
         json.errors[0].detail.contains("empty email rejected"),
         "{:?}",
@@ -484,7 +484,7 @@ fn test_empty_email_not_added() {
 
     let json = user
         .update_email_more_control(model.id, None)
-        .bad_with_status(200);
+        .bad_with_status(400);
 
     assert!(
         json.errors[0].detail.contains("empty email rejected"),
@@ -510,7 +510,7 @@ fn test_other_users_cannot_change_my_email() {
             another_user_model.id,
             Some("[email protected]"),
         )
-        .bad_with_status(200);
+        .bad_with_status(400);
     assert!(
         json.errors[0]
             .detail