From f241aa9bbec36ebc2fba45709931e610c1203d43 Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Sat, 23 Nov 2019 12:29:59 -0500 Subject: [PATCH 1/5] Rename `Cargo{Err,Error,Result}` to `App*` These types represent generic application errors, and not errors specific to cargo requests/endpoints. --- src/background_jobs.rs | 10 +-- src/bin/enqueue-job.rs | 10 +-- src/bin/monitor.rs | 10 +-- src/bin/on_call/mod.rs | 4 +- src/boot/categories.rs | 10 +-- src/controllers.rs | 2 +- src/controllers/category.rs | 6 +- src/controllers/crate_owner_invitation.rs | 8 +- src/controllers/helpers.rs | 4 +- src/controllers/helpers/pagination.rs | 6 +- src/controllers/keyword.rs | 4 +- src/controllers/krate/downloads.rs | 2 +- src/controllers/krate/follow.rs | 8 +- src/controllers/krate/metadata.rs | 12 +-- src/controllers/krate/owners.rs | 14 +-- src/controllers/krate/publish.rs | 10 +-- src/controllers/krate/search.rs | 2 +- src/controllers/site_metadata.rs | 2 +- src/controllers/team.rs | 2 +- src/controllers/token.rs | 6 +- src/controllers/user/me.rs | 16 ++-- src/controllers/user/other.rs | 4 +- src/controllers/user/session.rs | 12 +-- src/controllers/version.rs | 2 +- src/controllers/version/deprecated.rs | 4 +- src/controllers/version/downloads.rs | 6 +- src/controllers/version/metadata.rs | 6 +- src/controllers/version/yank.rs | 10 +-- src/db.rs | 8 +- src/email.rs | 8 +- src/git.rs | 6 +- src/github.rs | 6 +- src/middleware/current_user.rs | 10 +-- src/models/dependency.rs | 4 +- src/models/krate.rs | 20 ++--- src/models/owner.rs | 4 +- src/models/team.rs | 12 +-- src/models/user.rs | 8 +- src/models/version.rs | 8 +- src/publish_rate_limit.rs | 32 +++---- src/router.rs | 6 +- src/tests/all.rs | 4 +- src/tests/builders.rs | 6 +- src/uploaders.rs | 10 +-- src/util.rs | 2 +- src/util/errors.rs | 104 +++++++++++----------- 46 files changed, 223 insertions(+), 227 deletions(-) diff --git a/src/background_jobs.rs b/src/background_jobs.rs index b9309d731e8..45b50d8d05b 100644 --- a/src/background_jobs.rs +++ b/src/background_jobs.rs @@ -5,17 +5,17 @@ use swirl::PerformError; use crate::db::{DieselPool, DieselPooledConn}; use crate::git::Repository; use crate::uploaders::Uploader; -use crate::util::errors::{CargoErrToStdErr, CargoResult}; +use crate::util::errors::{AppErrToStdErr, AppResult}; impl<'a> swirl::db::BorrowedConnection<'a> for DieselPool { type Connection = DieselPooledConn<'a>; } impl swirl::db::DieselPool for DieselPool { - type Error = CargoErrToStdErr; + type Error = AppErrToStdErr; fn get(&self) -> Result, Self::Error> { - self.get().map_err(CargoErrToStdErr) + self.get().map_err(AppErrToStdErr) } } @@ -59,10 +59,10 @@ impl Environment { pub fn connection(&self) -> Result, PerformError> { self.connection_pool .get() - .map_err(|e| CargoErrToStdErr(e).into()) + .map_err(|e| AppErrToStdErr(e).into()) } - pub fn lock_index(&self) -> CargoResult> { + pub fn lock_index(&self) -> AppResult> { let repo = self.index.lock().unwrap_or_else(PoisonError::into_inner); repo.reset_head()?; Ok(repo) diff --git a/src/bin/enqueue-job.rs b/src/bin/enqueue-job.rs index 290219421a8..f91bc270705 100644 --- a/src/bin/enqueue-job.rs +++ b/src/bin/enqueue-job.rs @@ -1,8 +1,8 @@ -use cargo_registry::util::{human, CargoError, CargoResult}; +use cargo_registry::util::{human, AppError, AppResult}; use cargo_registry::{db, env, tasks}; use diesel::PgConnection; -fn main() -> CargoResult<()> { +fn main() -> AppResult<()> { let conn = db::connect_now()?; let mut args = std::env::args().skip(1); match &*args.next().unwrap_or_default() { @@ -19,10 +19,10 @@ fn main() -> CargoResult<()> { } /// Helper to map the `PerformError` returned by `swirl::Job::enqueue()` to a -/// `CargoError`. Can be removed once `map_err()` isn't needed any more. +/// `AppError`. Can be removed once `map_err()` isn't needed any more. trait Enqueue: swirl::Job { - fn enqueue(self, conn: &PgConnection) -> CargoResult<()> { - ::enqueue(self, conn).map_err(|e| CargoError::from_std_error(e)) + fn enqueue(self, conn: &PgConnection) -> AppResult<()> { + ::enqueue(self, conn).map_err(|e| AppError::from_std_error(e)) } } diff --git a/src/bin/monitor.rs b/src/bin/monitor.rs index bd97e7c20f5..182330fb6e4 100644 --- a/src/bin/monitor.rs +++ b/src/bin/monitor.rs @@ -8,10 +8,10 @@ mod on_call; -use cargo_registry::{db, schema::*, util::CargoResult}; +use cargo_registry::{db, schema::*, util::AppResult}; use diesel::prelude::*; -fn main() -> CargoResult<()> { +fn main() -> AppResult<()> { let conn = db::connect_now()?; check_stalled_background_jobs(&conn)?; @@ -19,7 +19,7 @@ fn main() -> CargoResult<()> { Ok(()) } -fn check_stalled_background_jobs(conn: &PgConnection) -> CargoResult<()> { +fn check_stalled_background_jobs(conn: &PgConnection) -> AppResult<()> { use cargo_registry::schema::background_jobs::dsl::*; use diesel::dsl::*; @@ -55,7 +55,7 @@ fn check_stalled_background_jobs(conn: &PgConnection) -> CargoResult<()> { Ok(()) } -fn check_spam_attack(conn: &PgConnection) -> CargoResult<()> { +fn check_spam_attack(conn: &PgConnection) -> AppResult<()> { use cargo_registry::models::krate::canon_crate_name; use diesel::dsl::*; use diesel::sql_types::Bool; @@ -116,7 +116,7 @@ fn check_spam_attack(conn: &PgConnection) -> CargoResult<()> { Ok(()) } -fn log_and_trigger_event(event: on_call::Event) -> CargoResult<()> { +fn log_and_trigger_event(event: on_call::Event) -> AppResult<()> { match event { on_call::Event::Trigger { ref description, .. diff --git a/src/bin/on_call/mod.rs b/src/bin/on_call/mod.rs index 4e99d83b163..991dfe03bba 100644 --- a/src/bin/on_call/mod.rs +++ b/src/bin/on_call/mod.rs @@ -1,4 +1,4 @@ -use cargo_registry::util::{internal, CargoResult}; +use cargo_registry::util::{internal, AppResult}; use reqwest::{header, StatusCode as Status}; @@ -25,7 +25,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) -> CargoResult<()> { + pub fn send(self) -> AppResult<()> { let api_token = dotenv::var("PAGERDUTY_API_TOKEN")?; let service_key = dotenv::var("PAGERDUTY_INTEGRATION_KEY")?; diff --git a/src/boot/categories.rs b/src/boot/categories.rs index b001baa2c44..861707594f0 100644 --- a/src/boot/categories.rs +++ b/src/boot/categories.rs @@ -3,7 +3,7 @@ use crate::{ db, - util::errors::{internal, CargoResult, ChainError}, + util::errors::{internal, AppResult, ChainError}, }; use diesel::prelude::*; @@ -37,7 +37,7 @@ impl Category { } } -fn required_string_from_toml<'a>(toml: &'a toml::value::Table, key: &str) -> CargoResult<&'a str> { +fn required_string_from_toml<'a>(toml: &'a toml::value::Table, key: &str) -> AppResult<&'a str> { toml.get(key).and_then(toml::Value::as_str).chain_error(|| { internal(&format_args!( "Expected category TOML attribute '{}' to be a String", @@ -53,7 +53,7 @@ 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>, -) -> CargoResult> { +) -> AppResult> { let mut result = vec![]; for (slug, details) in categories { @@ -85,12 +85,12 @@ fn categories_from_toml( Ok(result) } -pub fn sync(toml_str: &str) -> CargoResult<()> { +pub fn sync(toml_str: &str) -> AppResult<()> { let conn = db::connect_now().unwrap(); sync_with_connection(toml_str, &conn) } -pub fn sync_with_connection(toml_str: &str, conn: &PgConnection) -> CargoResult<()> { +pub fn sync_with_connection(toml_str: &str, conn: &PgConnection) -> AppResult<()> { use crate::schema::categories::dsl::*; use diesel::dsl::all; use diesel::pg::upsert::excluded; diff --git a/src/controllers.rs b/src/controllers.rs index 0bf1cfca13c..f8f5d672b2f 100644 --- a/src/controllers.rs +++ b/src/controllers.rs @@ -6,7 +6,7 @@ mod prelude { pub use conduit_router::RequestParams; pub use crate::db::RequestTransaction; - pub use crate::util::{human, CargoResult}; + pub use crate::util::{human, AppResult}; pub use crate::middleware::app::RequestApp; pub use crate::middleware::current_user::RequestUser; diff --git a/src/controllers/category.rs b/src/controllers/category.rs index c16aa74f49c..693ab288ea6 100644 --- a/src/controllers/category.rs +++ b/src/controllers/category.rs @@ -6,7 +6,7 @@ use crate::schema::categories; use crate::views::{EncodableCategory, EncodableCategoryWithSubcategories}; /// Handles the `GET /categories` route. -pub fn index(req: &mut dyn Request) -> CargoResult { +pub fn index(req: &mut dyn Request) -> AppResult { let conn = req.db_conn()?; let query = req.query(); // FIXME: There are 69 categories, 47 top level. This isn't going to @@ -40,7 +40,7 @@ pub fn index(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /categories/:category_id` route. -pub fn show(req: &mut dyn Request) -> CargoResult { +pub fn show(req: &mut dyn Request) -> AppResult { let slug = &req.params()["category_id"]; let conn = req.db_conn()?; let cat = Category::by_slug(slug).first::(&*conn)?; @@ -77,7 +77,7 @@ pub fn show(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /category_slugs` route. -pub fn slugs(req: &mut dyn Request) -> CargoResult { +pub fn slugs(req: &mut dyn Request) -> AppResult { let conn = req.db_conn()?; let slugs = categories::table .select((categories::slug, categories::slug, categories::description)) diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index 600659e7d52..28ab7c2c135 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -5,7 +5,7 @@ use crate::schema::{crate_owner_invitations, crate_owners}; use crate::views::{EncodableCrateOwnerInvitation, InvitationResponse}; /// Handles the `GET /me/crate_owner_invitations` route. -pub fn list(req: &mut dyn Request) -> CargoResult { +pub fn list(req: &mut dyn Request) -> AppResult { let conn = &*req.db_conn()?; let user_id = req.user()?.id; @@ -31,7 +31,7 @@ struct OwnerInvitation { } /// Handles the `PUT /me/crate_owner_invitations/:crate_id` route. -pub fn handle_invite(req: &mut dyn Request) -> CargoResult { +pub fn handle_invite(req: &mut dyn Request) -> AppResult { let mut body = String::new(); req.body().read_to_string(&mut body)?; @@ -53,7 +53,7 @@ fn accept_invite( req: &dyn Request, conn: &PgConnection, crate_invite: InvitationResponse, -) -> CargoResult { +) -> AppResult { use diesel::{delete, insert_into}; let user_id = req.user()?.id; @@ -92,7 +92,7 @@ fn decline_invite( req: &dyn Request, conn: &PgConnection, crate_invite: InvitationResponse, -) -> CargoResult { +) -> AppResult { use diesel::delete; let user_id = req.user()?.id; diff --git a/src/controllers/helpers.rs b/src/controllers/helpers.rs index 1f81c956eba..056fc944daf 100644 --- a/src/controllers/helpers.rs +++ b/src/controllers/helpers.rs @@ -1,11 +1,11 @@ -use crate::util::{json_response, CargoResult}; +use crate::util::{json_response, AppResult}; use conduit::Response; pub(crate) mod pagination; pub(crate) use self::pagination::Paginate; -pub fn ok_true() -> CargoResult { +pub fn ok_true() -> AppResult { #[derive(Serialize)] struct R { ok: bool, diff --git a/src/controllers/helpers/pagination.rs b/src/controllers/helpers/pagination.rs index f028833947f..50ddf6cb171 100644 --- a/src/controllers/helpers/pagination.rs +++ b/src/controllers/helpers/pagination.rs @@ -14,7 +14,7 @@ pub(crate) enum Page { } impl Page { - fn new(params: &IndexMap) -> CargoResult { + fn new(params: &IndexMap) -> AppResult { if let Some(s) = params.get("page") { let numeric_page = s.parse()?; if numeric_page < 1 { @@ -38,7 +38,7 @@ pub(crate) struct PaginationOptions { } impl PaginationOptions { - pub(crate) fn new(params: &IndexMap) -> CargoResult { + pub(crate) fn new(params: &IndexMap) -> AppResult { const DEFAULT_PER_PAGE: u32 = 10; const MAX_PER_PAGE: u32 = 100; @@ -70,7 +70,7 @@ impl PaginationOptions { } pub(crate) trait Paginate: Sized { - fn paginate(self, params: &IndexMap) -> CargoResult> { + fn paginate(self, params: &IndexMap) -> AppResult> { Ok(PaginatedQuery { query: self, options: PaginationOptions::new(params)?, diff --git a/src/controllers/keyword.rs b/src/controllers/keyword.rs index 4edbc550b21..1176a82044f 100644 --- a/src/controllers/keyword.rs +++ b/src/controllers/keyword.rs @@ -5,7 +5,7 @@ use crate::models::Keyword; use crate::views::EncodableKeyword; /// Handles the `GET /keywords` route. -pub fn index(req: &mut dyn Request) -> CargoResult { +pub fn index(req: &mut dyn Request) -> AppResult { use crate::schema::keywords; let conn = req.db_conn()?; @@ -41,7 +41,7 @@ pub fn index(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /keywords/:keyword_id` route. -pub fn show(req: &mut dyn Request) -> CargoResult { +pub fn show(req: &mut dyn Request) -> AppResult { let name = &req.params()["keyword_id"]; let conn = req.db_conn()?; diff --git a/src/controllers/krate/downloads.rs b/src/controllers/krate/downloads.rs index c556df57ee7..155d7126746 100644 --- a/src/controllers/krate/downloads.rs +++ b/src/controllers/krate/downloads.rs @@ -14,7 +14,7 @@ use crate::views::EncodableVersionDownload; use crate::models::krate::to_char; /// Handles the `GET /crates/:crate_id/downloads` route. -pub fn downloads(req: &mut dyn Request) -> CargoResult { +pub fn downloads(req: &mut dyn Request) -> AppResult { use diesel::dsl::*; use diesel::sql_types::BigInt; diff --git a/src/controllers/krate/follow.rs b/src/controllers/krate/follow.rs index 3729618b430..d0e86df18a1 100644 --- a/src/controllers/krate/follow.rs +++ b/src/controllers/krate/follow.rs @@ -7,7 +7,7 @@ use crate::db::DieselPooledConn; use crate::models::{Crate, Follow}; use crate::schema::*; -fn follow_target(req: &dyn Request, conn: &DieselPooledConn<'_>) -> CargoResult { +fn follow_target(req: &dyn Request, conn: &DieselPooledConn<'_>) -> AppResult { let user = req.user()?; let crate_name = &req.params()["crate_id"]; let crate_id = Crate::by_name(crate_name) @@ -20,7 +20,7 @@ fn follow_target(req: &dyn Request, conn: &DieselPooledConn<'_>) -> CargoResult< } /// Handles the `PUT /crates/:crate_id/follow` route. -pub fn follow(req: &mut dyn Request) -> CargoResult { +pub fn follow(req: &mut dyn Request) -> AppResult { let conn = req.db_conn()?; let follow = follow_target(req, &conn)?; diesel::insert_into(follows::table) @@ -32,7 +32,7 @@ pub fn follow(req: &mut dyn Request) -> CargoResult { } /// Handles the `DELETE /crates/:crate_id/follow` route. -pub fn unfollow(req: &mut dyn Request) -> CargoResult { +pub fn unfollow(req: &mut dyn Request) -> AppResult { let conn = req.db_conn()?; let follow = follow_target(req, &conn)?; diesel::delete(&follow).execute(&*conn)?; @@ -41,7 +41,7 @@ pub fn unfollow(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id/following` route. -pub fn following(req: &mut dyn Request) -> CargoResult { +pub fn following(req: &mut dyn Request) -> AppResult { use diesel::dsl::exists; let conn = req.db_conn()?; diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 3a9aa90e298..3d063eb1389 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -19,7 +19,7 @@ use crate::views::{ use crate::models::krate::ALL_COLUMNS; /// Handles the `GET /summary` route. -pub fn summary(req: &mut dyn Request) -> CargoResult { +pub fn summary(req: &mut dyn Request) -> AppResult { use crate::schema::crates::dsl::*; let conn = req.db_conn()?; @@ -28,7 +28,7 @@ pub fn summary(req: &mut dyn Request) -> CargoResult { .select(metadata::total_downloads) .get_result(&*conn)?; - let encode_crates = |krates: Vec| -> CargoResult> { + let encode_crates = |krates: Vec| -> AppResult> { let versions = krates.versions().load::(&*conn)?; versions .grouped_by(&krates) @@ -102,7 +102,7 @@ pub fn summary(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id` route. -pub fn show(req: &mut dyn Request) -> CargoResult { +pub fn show(req: &mut dyn Request) -> AppResult { let name = &req.params()["crate_id"]; let conn = req.db_conn()?; let krate = Crate::by_name(name).first::(&*conn)?; @@ -161,7 +161,7 @@ pub fn show(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id/:version/readme` route. -pub fn readme(req: &mut dyn Request) -> CargoResult { +pub fn readme(req: &mut dyn Request) -> AppResult { let crate_name = &req.params()["crate_id"]; let version = &req.params()["version"]; @@ -185,7 +185,7 @@ pub fn readme(req: &mut dyn Request) -> CargoResult { /// Handles the `GET /crates/:crate_id/versions` route. // FIXME: Not sure why this is necessary since /crates/:crate_id returns // this information already, but ember is definitely requesting it -pub fn versions(req: &mut dyn Request) -> CargoResult { +pub fn versions(req: &mut dyn Request) -> AppResult { let crate_name = &req.params()["crate_id"]; let conn = req.db_conn()?; let krate = Crate::by_name(crate_name).first::(&*conn)?; @@ -208,7 +208,7 @@ pub fn versions(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id/reverse_dependencies` route. -pub fn reverse_dependencies(req: &mut dyn Request) -> CargoResult { +pub fn reverse_dependencies(req: &mut dyn Request) -> AppResult { use diesel::dsl::any; let name = &req.params()["crate_id"]; diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 73b294035df..b372c504712 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -7,7 +7,7 @@ use crate::models::{Crate, Owner, Rights, Team, User}; use crate::views::EncodableOwner; /// Handles the `GET /crates/:crate_id/owners` route. -pub fn owners(req: &mut dyn Request) -> CargoResult { +pub fn owners(req: &mut dyn Request) -> AppResult { let crate_name = &req.params()["crate_id"]; let conn = req.db_conn()?; let krate = Crate::by_name(crate_name).first::(&*conn)?; @@ -25,7 +25,7 @@ pub fn owners(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id/owner_team` route. -pub fn owner_team(req: &mut dyn Request) -> CargoResult { +pub fn owner_team(req: &mut dyn Request) -> AppResult { let crate_name = &req.params()["crate_id"]; let conn = req.db_conn()?; let krate = Crate::by_name(crate_name).first::(&*conn)?; @@ -42,7 +42,7 @@ pub fn owner_team(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id/owner_user` route. -pub fn owner_user(req: &mut dyn Request) -> CargoResult { +pub fn owner_user(req: &mut dyn Request) -> AppResult { let crate_name = &req.params()["crate_id"]; let conn = req.db_conn()?; let krate = Crate::by_name(crate_name).first::(&*conn)?; @@ -59,12 +59,12 @@ pub fn owner_user(req: &mut dyn Request) -> CargoResult { } /// Handles the `PUT /crates/:crate_id/owners` route. -pub fn add_owners(req: &mut dyn Request) -> CargoResult { +pub fn add_owners(req: &mut dyn Request) -> AppResult { modify_owners(req, true) } /// Handles the `DELETE /crates/:crate_id/owners` route. -pub fn remove_owners(req: &mut dyn Request) -> CargoResult { +pub fn remove_owners(req: &mut dyn Request) -> AppResult { modify_owners(req, false) } @@ -72,7 +72,7 @@ pub fn remove_owners(req: &mut dyn Request) -> CargoResult { /// The format is /// /// {"owners": ["username", "github:org:team", ...]} -fn parse_owners_request(req: &mut dyn Request) -> CargoResult> { +fn parse_owners_request(req: &mut dyn Request) -> AppResult> { let mut body = String::new(); req.body().read_to_string(&mut body)?; #[derive(Deserialize)] @@ -89,7 +89,7 @@ fn parse_owners_request(req: &mut dyn Request) -> CargoResult> { .ok_or_else(|| human("invalid json request")) } -fn modify_owners(req: &mut dyn Request, add: bool) -> CargoResult { +fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { let logins = parse_owners_request(req)?; let app = req.app(); let user = req.user()?; diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 5e0cd08aa4b..0af7a61aa61 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -10,7 +10,7 @@ use crate::models::dependency; use crate::models::{Badge, Category, Keyword, NewCrate, NewVersion, Rights, User}; use crate::render; use crate::util::{read_fill, read_le_u32}; -use crate::util::{CargoError, ChainError, Maximums}; +use crate::util::{AppError, ChainError, Maximums}; use crate::views::{EncodableCrateUpload, GoodCrate, PublishWarnings}; /// Handles the `PUT /crates/new` route. @@ -20,7 +20,7 @@ use crate::views::{EncodableCrateUpload, GoodCrate, PublishWarnings}; /// Currently blocks the HTTP thread, perhaps some function calls can spawn new /// threads and return completion or error through other methods a `cargo publish /// --status` command, via crates.io's front end, or email. -pub fn publish(req: &mut dyn Request) -> CargoResult { +pub fn publish(req: &mut dyn Request) -> AppResult { let app = Arc::clone(req.app()); // The format of the req.body() of a publish request is as follows: @@ -168,7 +168,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { repo, ) .enqueue(&conn) - .map_err(|e| CargoError::from_std_error(e))?; + .map_err(|e| AppError::from_std_error(e))?; } let cksum = app @@ -191,7 +191,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { }; git::add_crate(git_crate) .enqueue(&conn) - .map_err(|e| CargoError::from_std_error(e))?; + .map_err(|e| AppError::from_std_error(e))?; // The `other` field on `PublishWarnings` was introduced to handle a temporary warning // that is no longer needed. As such, crates.io currently does not return any `other` @@ -214,7 +214,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { /// This function parses the JSON headers to interpret the data and validates /// the data during and after the parsing. Returns crate metadata and user /// information. -fn parse_new_headers(req: &mut dyn Request) -> CargoResult<(EncodableCrateUpload, User)> { +fn parse_new_headers(req: &mut dyn Request) -> AppResult<(EncodableCrateUpload, User)> { // Read the json upload request let metadata_length = u64::from(read_le_u32(req.body())?); req.mut_extensions().insert(metadata_length); diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 21484981727..50aaa6c27ee 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -32,7 +32,7 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS}; /// caused the break. In the future, we should look at splitting this /// function out to cover the different use cases, and create unit tests /// for them. -pub fn search(req: &mut dyn Request) -> CargoResult { +pub fn search(req: &mut dyn Request) -> AppResult { use diesel::sql_types::{Bool, Text}; let conn = req.db_conn()?; diff --git a/src/controllers/site_metadata.rs b/src/controllers/site_metadata.rs index 4011f74c3c7..bc9ea754f72 100644 --- a/src/controllers/site_metadata.rs +++ b/src/controllers/site_metadata.rs @@ -4,7 +4,7 @@ use super::prelude::*; /// /// The sha is contained within the `HEROKU_SLUG_COMMIT` environment variable. /// If `HEROKU_SLUG_COMMIT` is not set, returns `"unknown"`. -pub fn show_deployed_sha(req: &mut dyn Request) -> CargoResult { +pub fn show_deployed_sha(req: &mut dyn Request) -> AppResult { let deployed_sha = dotenv::var("HEROKU_SLUG_COMMIT").unwrap_or_else(|_| String::from("unknown")); diff --git a/src/controllers/team.rs b/src/controllers/team.rs index eb16bdb6ba3..f62b9821e0e 100644 --- a/src/controllers/team.rs +++ b/src/controllers/team.rs @@ -5,7 +5,7 @@ use crate::schema::teams; use crate::views::EncodableTeam; /// Handles the `GET /teams/:team_id` route. -pub fn show_team(req: &mut dyn Request) -> CargoResult { +pub fn show_team(req: &mut dyn Request) -> AppResult { use self::teams::dsl::{login, teams}; let name = &req.params()["team_id"]; diff --git a/src/controllers/token.rs b/src/controllers/token.rs index 24b3c482fde..67fce78c340 100644 --- a/src/controllers/token.rs +++ b/src/controllers/token.rs @@ -9,7 +9,7 @@ use crate::views::EncodableApiTokenWithToken; use serde_json as json; /// Handles the `GET /me/tokens` route. -pub fn list(req: &mut dyn Request) -> CargoResult { +pub fn list(req: &mut dyn Request) -> AppResult { let tokens = ApiToken::belonging_to(req.user()?) .filter(api_tokens::revoked.eq(false)) .order(api_tokens::created_at.desc()) @@ -22,7 +22,7 @@ pub fn list(req: &mut dyn Request) -> CargoResult { } /// Handles the `PUT /me/tokens` route. -pub fn new(req: &mut dyn Request) -> CargoResult { +pub fn new(req: &mut dyn Request) -> AppResult { /// The incoming serialization format for the `ApiToken` model. #[derive(Deserialize, Serialize)] struct NewApiToken { @@ -90,7 +90,7 @@ pub fn new(req: &mut dyn Request) -> CargoResult { } /// Handles the `DELETE /me/tokens/:id` route. -pub fn revoke(req: &mut dyn Request) -> CargoResult { +pub fn revoke(req: &mut dyn Request) -> AppResult { let id = req.params()["id"] .parse::() .map_err(|e| bad_request(&format!("invalid token id: {:?}", e)))?; diff --git a/src/controllers/user/me.rs b/src/controllers/user/me.rs index f8f6241920c..3e9ace1b5e0 100644 --- a/src/controllers/user/me.rs +++ b/src/controllers/user/me.rs @@ -5,7 +5,7 @@ use crate::controllers::prelude::*; use crate::controllers::helpers::*; use crate::email; use crate::util::bad_request; -use crate::util::errors::CargoError; +use crate::util::errors::AppError; use crate::models::user::{UserNoEmailType, ALL_COLUMNS}; use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version}; @@ -13,7 +13,7 @@ use crate::schema::{crate_owners, crates, emails, follows, users, versions}; use crate::views::{EncodableMe, EncodableVersion, OwnedCrate}; /// Handles the `GET /me` route. -pub fn me(req: &mut dyn Request) -> CargoResult { +pub fn me(req: &mut dyn Request) -> AppResult { // Changed to getting User information from database because in // src/tests/user.rs, when testing put and get on updating email, // request seems to be somehow 'cached'. When we try to get a @@ -68,7 +68,7 @@ pub fn me(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /me/updates` route. -pub fn updates(req: &mut dyn Request) -> CargoResult { +pub fn updates(req: &mut dyn Request) -> AppResult { use diesel::dsl::any; let user = req.user()?; @@ -109,7 +109,7 @@ pub fn updates(req: &mut dyn Request) -> CargoResult { } /// Handles the `PUT /user/:user_id` route. -pub fn update_user(req: &mut dyn Request) -> CargoResult { +pub fn update_user(req: &mut dyn Request) -> AppResult { use self::emails::user_id; use self::users::dsl::{email, gh_login, users}; use diesel::{insert_into, update}; @@ -149,7 +149,7 @@ pub fn update_user(req: &mut dyn Request) -> CargoResult { return Err(human("empty email rejected")); } - conn.transaction::<_, Box, _>(|| { + conn.transaction::<_, Box, _>(|| { update(users.filter(gh_login.eq(&user.gh_login))) .set(email.eq(user_email)) .execute(&*conn)?; @@ -181,7 +181,7 @@ pub fn update_user(req: &mut dyn Request) -> CargoResult { } /// Handles the `PUT /confirm/:email_token` route -pub fn confirm_user_email(req: &mut dyn Request) -> CargoResult { +pub fn confirm_user_email(req: &mut dyn Request) -> AppResult { use diesel::update; let conn = req.db_conn()?; @@ -203,7 +203,7 @@ pub fn confirm_user_email(req: &mut dyn Request) -> CargoResult { } /// Handles `PUT /user/:user_id/resend` route -pub fn regenerate_token_and_send(req: &mut dyn Request) -> CargoResult { +pub fn regenerate_token_and_send(req: &mut dyn Request) -> AppResult { use diesel::dsl::sql; use diesel::update; @@ -234,7 +234,7 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> CargoResult } /// Handles `PUT /me/email_notifications` route -pub fn update_email_notifications(req: &mut dyn Request) -> CargoResult { +pub fn update_email_notifications(req: &mut dyn Request) -> AppResult { use self::crate_owners::dsl::*; use diesel::pg::upsert::excluded; diff --git a/src/controllers/user/other.rs b/src/controllers/user/other.rs index 9ec66bb9b92..839e11d353b 100644 --- a/src/controllers/user/other.rs +++ b/src/controllers/user/other.rs @@ -7,7 +7,7 @@ use crate::schema::{crate_owners, crates, users}; use crate::views::EncodablePublicUser; /// Handles the `GET /users/:user_id` route. -pub fn show(req: &mut dyn Request) -> CargoResult { +pub fn show(req: &mut dyn Request) -> AppResult { use self::users::dsl::{gh_login, id, users}; let name = &req.params()["user_id"].to_lowercase(); @@ -28,7 +28,7 @@ pub fn show(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /users/:user_id/stats` route. -pub fn stats(req: &mut dyn Request) -> CargoResult { +pub fn stats(req: &mut dyn Request) -> AppResult { use diesel::dsl::sum; let user_id = &req.params()["user_id"].parse::().ok().unwrap(); diff --git a/src/controllers/user/session.rs b/src/controllers/user/session.rs index 33c03febcbd..43d33b682c7 100644 --- a/src/controllers/user/session.rs +++ b/src/controllers/user/session.rs @@ -8,7 +8,7 @@ use crate::models::user; use crate::models::user::UserNoEmailType; use crate::models::{NewUser, User}; use crate::schema::users; -use crate::util::errors::{CargoError, ReadOnlyMode}; +use crate::util::errors::{AppError, ReadOnlyMode}; /// Handles the `GET /authorize_url` route. /// @@ -25,7 +25,7 @@ use crate::util::errors::{CargoError, ReadOnlyMode}; /// "url": "https://github.com/login/oauth/authorize?client_id=...&state=...&scope=read%3Aorg" /// } /// ``` -pub fn github_authorize(req: &mut dyn Request) -> CargoResult { +pub fn github_authorize(req: &mut dyn Request) -> AppResult { let (url, state) = req .app() .github @@ -73,7 +73,7 @@ pub fn github_authorize(req: &mut dyn Request) -> CargoResult { /// } /// } /// ``` -pub fn github_access_token(req: &mut dyn Request) -> CargoResult { +pub fn github_access_token(req: &mut dyn Request) -> AppResult { // Parse the url query let mut query = req.query(); let code = query.remove("code").unwrap_or_default(); @@ -116,7 +116,7 @@ struct GithubUser { } impl GithubUser { - fn save_to_database(&self, access_token: &str, conn: &PgConnection) -> CargoResult { + fn save_to_database(&self, access_token: &str, conn: &PgConnection) -> AppResult { NewUser::new( self.id, &self.login, @@ -127,7 +127,7 @@ impl GithubUser { ) .create_or_update(conn) .map_err(Into::into) - .or_else(|e: Box| { + .or_else(|e: Box| { // If we're in read only mode, we can't update their details // just look for an existing user if e.is::() { @@ -145,7 +145,7 @@ impl GithubUser { } /// Handles the `GET /logout` route. -pub fn logout(req: &mut dyn Request) -> CargoResult { +pub fn logout(req: &mut dyn Request) -> AppResult { req.session().remove(&"user_id".to_string()); Ok(req.json(&true)) } diff --git a/src/controllers/version.rs b/src/controllers/version.rs index 5c1195882ca..bffdd3ec50d 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -8,7 +8,7 @@ use super::prelude::*; use crate::models::{Crate, CrateVersions, Version}; use crate::schema::versions; -fn version_and_crate(req: &mut dyn Request) -> CargoResult<(Version, Crate)> { +fn version_and_crate(req: &mut dyn Request) -> AppResult<(Version, Crate)> { let crate_name = &req.params()["crate_id"]; let semver = &req.params()["version"]; if semver::Version::parse(semver).is_err() { diff --git a/src/controllers/version/deprecated.rs b/src/controllers/version/deprecated.rs index 71739034dd7..c40709da68a 100644 --- a/src/controllers/version/deprecated.rs +++ b/src/controllers/version/deprecated.rs @@ -14,7 +14,7 @@ use crate::schema::*; use crate::views::EncodableVersion; /// Handles the `GET /versions` route. -pub fn index(req: &mut dyn Request) -> CargoResult { +pub fn index(req: &mut dyn Request) -> AppResult { use diesel::dsl::any; let conn = req.db_conn()?; @@ -50,7 +50,7 @@ pub fn index(req: &mut dyn Request) -> CargoResult { /// Handles the `GET /versions/:version_id` route. /// The frontend doesn't appear to hit this endpoint. Instead, the version information appears to /// be returned by `krate::show`. -pub fn show_by_id(req: &mut dyn Request) -> CargoResult { +pub fn show_by_id(req: &mut dyn Request) -> AppResult { let id = &req.params()["version_id"]; let id = id.parse().unwrap_or(0); let conn = req.db_conn()?; diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index b1843f09271..1cb3e8f0968 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -14,7 +14,7 @@ use super::version_and_crate; /// Handles the `GET /crates/:crate_id/:version/download` route. /// This returns a URL to the location where the crate is stored. -pub fn download(req: &mut dyn Request) -> CargoResult { +pub fn download(req: &mut dyn Request) -> AppResult { let crate_name = &req.params()["crate_id"]; let version = &req.params()["version"]; @@ -50,7 +50,7 @@ fn increment_download_counts( req: &dyn Request, crate_name: &str, version: &str, -) -> CargoResult { +) -> AppResult { use self::versions::dsl::*; let conn = req.db_conn()?; @@ -68,7 +68,7 @@ fn increment_download_counts( } /// Handles the `GET /crates/:crate_id/:version/downloads` route. -pub fn downloads(req: &mut dyn Request) -> CargoResult { +pub fn downloads(req: &mut dyn Request) -> AppResult { let (version, _) = version_and_crate(req)?; let conn = req.db_conn()?; let cutoff_end_date = req diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index bacf03e3be3..9b985944a4f 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -18,7 +18,7 @@ use super::version_and_crate; /// In addition to returning cached data from the index, this returns /// fields for `id`, `version_id`, and `downloads` (which appears to always /// be 0) -pub fn dependencies(req: &mut dyn Request) -> CargoResult { +pub fn dependencies(req: &mut dyn Request) -> AppResult { let (version, _) = version_and_crate(req)?; let conn = req.db_conn()?; let deps = version.dependencies(&*conn)?; @@ -35,7 +35,7 @@ pub fn dependencies(req: &mut dyn Request) -> CargoResult { } /// Handles the `GET /crates/:crate_id/:version/authors` route. -pub fn authors(req: &mut dyn Request) -> CargoResult { +pub fn authors(req: &mut dyn Request) -> AppResult { let (version, _) = version_and_crate(req)?; let conn = req.db_conn()?; let names = version_authors::table @@ -66,7 +66,7 @@ pub fn authors(req: &mut dyn Request) -> CargoResult { /// /// The frontend doesn't appear to hit this endpoint, but our tests do, and it seems to be a useful /// API route to have. -pub fn show(req: &mut dyn Request) -> CargoResult { +pub fn show(req: &mut dyn Request) -> AppResult { let (version, krate) = version_and_crate(req)?; let conn = req.db_conn()?; let published_by = version.published_by(&conn); diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index dd1105d3501..6fd6eccae0e 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -6,7 +6,7 @@ use super::version_and_crate; use crate::controllers::prelude::*; use crate::git; use crate::models::Rights; -use crate::util::CargoError; +use crate::util::AppError; /// Handles the `DELETE /crates/:crate_id/:version/yank` route. /// This does not delete a crate version, it makes the crate @@ -17,17 +17,17 @@ use crate::util::CargoError; /// Crate deletion is not implemented to avoid breaking builds, /// and the goal of yanking a crate is to prevent crates /// beginning to depend on the yanked crate version. -pub fn yank(req: &mut dyn Request) -> CargoResult { +pub fn yank(req: &mut dyn Request) -> AppResult { modify_yank(req, true) } /// Handles the `PUT /crates/:crate_id/:version/unyank` route. -pub fn unyank(req: &mut dyn Request) -> CargoResult { +pub fn unyank(req: &mut dyn Request) -> AppResult { modify_yank(req, false) } /// Changes `yanked` flag on a crate version record -fn modify_yank(req: &mut dyn Request, yanked: bool) -> CargoResult { +fn modify_yank(req: &mut dyn Request, yanked: bool) -> AppResult { let (version, krate) = version_and_crate(req)?; let user = req.user()?; let conn = req.db_conn()?; @@ -38,7 +38,7 @@ fn modify_yank(req: &mut dyn Request, yanked: bool) -> CargoResult { git::yank(krate.name, version, yanked) .enqueue(&conn) - .map_err(|e| CargoError::from_std_error(e))?; + .map_err(|e| AppError::from_std_error(e))?; #[derive(Serialize)] struct R { diff --git a/src/db.rs b/src/db.rs index d0548ff6f5a..7ece465f3f0 100644 --- a/src/db.rs +++ b/src/db.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use url::Url; use crate::middleware::app::RequestApp; -use crate::util::CargoResult; +use crate::util::AppResult; use crate::Env; #[allow(missing_debug_implementations)] @@ -18,7 +18,7 @@ pub enum DieselPool { } impl DieselPool { - pub fn get(&self) -> CargoResult> { + pub fn get(&self) -> AppResult> { match self { DieselPool::Pool(pool) => Ok(DieselPooledConn::Pool(pool.get()?)), DieselPool::Test(conn) => Ok(DieselPooledConn::Test(conn.lock())), @@ -89,11 +89,11 @@ pub trait RequestTransaction { /// /// The connection will live for the lifetime of the request. // FIXME: This description does not match the implementation below. - fn db_conn(&self) -> CargoResult>; + fn db_conn(&self) -> AppResult>; } impl RequestTransaction for T { - fn db_conn(&self) -> CargoResult> { + fn db_conn(&self) -> AppResult> { self.app().diesel_database.get().map_err(Into::into) } } diff --git a/src/email.rs b/src/email.rs index e32ef012438..dbf7ae7bf51 100644 --- a/src/email.rs +++ b/src/email.rs @@ -1,6 +1,6 @@ use std::path::Path; -use crate::util::{bad_request, CargoResult}; +use crate::util::{bad_request, AppResult}; use failure::Fail; use lettre::file::FileTransport; @@ -37,7 +37,7 @@ fn build_email( subject: &str, body: &str, mailgun_config: &Option, -) -> CargoResult { +) -> AppResult { let sender = mailgun_config .as_ref() .map(|s| s.smtp_login.as_str()) @@ -70,7 +70,7 @@ pub fn send_user_confirm_email(email: &str, user_name: &str, token: &str) { /// For use in cases where we want to fail if an email is bad because the user is directly trying /// to set their email correctly, as opposed to us silently trying to use the email from their /// GitHub profile during signup. -pub fn try_send_user_confirm_email(email: &str, user_name: &str, token: &str) -> CargoResult<()> { +pub fn try_send_user_confirm_email(email: &str, user_name: &str, token: &str) -> AppResult<()> { // Create a URL with token string as path to send to user // If user clicks on path, look email/user up in database, // make sure tokens match @@ -103,7 +103,7 @@ this invitation.", let _ = send_email(email, subject, &body); } -fn send_email(recipient: &str, subject: &str, body: &str) -> CargoResult<()> { +fn send_email(recipient: &str, subject: &str, body: &str) -> AppResult<()> { let mailgun_config = init_config_vars(); let email = build_email(recipient, subject, body, &mailgun_config)?; diff --git a/src/git.rs b/src/git.rs index c01b6812696..e5bd05da724 100644 --- a/src/git.rs +++ b/src/git.rs @@ -10,7 +10,7 @@ use url::Url; use crate::background_jobs::Environment; use crate::models::{DependencyKind, Version}; use crate::schema::versions; -use crate::util::errors::{std_error_no_send, CargoResult}; +use crate::util::errors::{std_error_no_send, AppResult}; static DEFAULT_GIT_SSH_USERNAME: &str = "git"; @@ -148,7 +148,7 @@ pub struct Repository { } impl Repository { - pub fn open(repository_config: &RepositoryConfig) -> CargoResult { + pub fn open(repository_config: &RepositoryConfig) -> AppResult { let checkout_path = TempDir::new("git")?; let repository = git2::build::RepoBuilder::new() @@ -224,7 +224,7 @@ impl Repository { ref_status } - pub fn reset_head(&self) -> CargoResult<()> { + pub fn reset_head(&self) -> AppResult<()> { let mut origin = self.repository.find_remote("origin")?; origin.fetch( &["refs/heads/*:refs/heads/*"], diff --git a/src/github.rs b/src/github.rs index 428ee60fa1e..4b9ca577eea 100644 --- a/src/github.rs +++ b/src/github.rs @@ -8,12 +8,12 @@ use serde::de::DeserializeOwned; use std::str; use crate::app::App; -use crate::util::{errors::NotFound, human, internal, CargoError, CargoResult}; +use crate::util::{errors::NotFound, human, internal, AppError, AppResult}; /// Does all the nonsense for sending a GET to Github. Doesn't handle parsing /// because custom error-code handling may be desirable. Use /// `parse_github_response` to handle the "common" processing of responses. -pub fn github_api(app: &App, url: &str, auth: &AccessToken) -> CargoResult +pub fn github_api(app: &App, url: &str, auth: &AccessToken) -> AppResult where T: DeserializeOwned, { @@ -31,7 +31,7 @@ where .map_err(Into::into) } -fn handle_error_response(error: &reqwest::Error) -> Box { +fn handle_error_response(error: &reqwest::Error) -> Box { use reqwest::StatusCode as Status; match error.status() { diff --git a/src/middleware/current_user.rs b/src/middleware/current_user.rs index a6c72695263..4fb42a3fb23 100644 --- a/src/middleware/current_user.rs +++ b/src/middleware/current_user.rs @@ -4,7 +4,7 @@ use conduit_cookie::RequestSession; use diesel::prelude::*; use crate::db::RequestTransaction; -use crate::util::errors::{std_error, CargoResult, ChainError, Unauthorized}; +use crate::util::errors::{std_error, AppResult, ChainError, Unauthorized}; use crate::models::user::{UserNoEmailType, ALL_COLUMNS}; use crate::models::User; @@ -72,18 +72,18 @@ impl Middleware for CurrentUser { } pub trait RequestUser { - fn user(&self) -> CargoResult<&User>; - fn authentication_source(&self) -> CargoResult; + fn user(&self) -> AppResult<&User>; + fn authentication_source(&self) -> AppResult; } impl<'a> RequestUser for dyn Request + 'a { - fn user(&self) -> CargoResult<&User> { + fn user(&self) -> AppResult<&User> { self.extensions() .find::() .chain_error(|| Unauthorized) } - fn authentication_source(&self) -> CargoResult { + fn authentication_source(&self) -> AppResult { self.extensions() .find::() .cloned() diff --git a/src/models/dependency.rs b/src/models/dependency.rs index c82f56a4892..44f22074c37 100644 --- a/src/models/dependency.rs +++ b/src/models/dependency.rs @@ -1,7 +1,7 @@ use diesel::prelude::*; use crate::git; -use crate::util::{human, CargoResult}; +use crate::util::{human, AppResult}; use crate::models::{Crate, Version}; use crate::schema::*; @@ -73,7 +73,7 @@ pub fn add_dependencies( conn: &PgConnection, deps: &[EncodableCrateDependency], target_version_id: i32, -) -> CargoResult> { +) -> AppResult> { use self::dependencies::dsl::*; use diesel::insert_into; diff --git a/src/models/krate.rs b/src/models/krate.rs index 969de414a98..a65632ece4b 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -10,7 +10,7 @@ use crate::app::App; use crate::email; use crate::models::user; use crate::models::user::UserNoEmailType; -use crate::util::{human, CargoResult}; +use crate::util::{human, AppResult}; use crate::models::{ Badge, Category, CrateOwner, CrateOwnerInvitation, Keyword, NewCrateOwnerInvitation, Owner, @@ -106,7 +106,7 @@ impl<'a> NewCrate<'a> { conn: &PgConnection, uploader: i32, rate_limit: Option<&PublishRateLimit>, - ) -> CargoResult { + ) -> AppResult { use diesel::update; self.validate()?; @@ -131,8 +131,8 @@ impl<'a> NewCrate<'a> { }) } - fn validate(&self) -> CargoResult<()> { - fn validate_url(url: Option<&str>, field: &str) -> CargoResult<()> { + fn validate(&self) -> AppResult<()> { + fn validate_url(url: Option<&str>, field: &str) -> AppResult<()> { let url = match url { Some(s) => s, None => return Ok(()), @@ -159,7 +159,7 @@ impl<'a> NewCrate<'a> { Ok(()) } - fn ensure_name_not_reserved(&self, conn: &PgConnection) -> CargoResult<()> { + fn ensure_name_not_reserved(&self, conn: &PgConnection) -> AppResult<()> { use crate::schema::reserved_crate_names::dsl::*; use diesel::dsl::exists; use diesel::select; @@ -385,7 +385,7 @@ impl Crate { } } - pub fn max_version(&self, conn: &PgConnection) -> CargoResult { + pub fn max_version(&self, conn: &PgConnection) -> AppResult { use crate::schema::versions::dsl::*; let vs = self @@ -397,7 +397,7 @@ impl Crate { Ok(Version::max(vs)) } - pub fn owners(&self, conn: &PgConnection) -> CargoResult> { + pub fn owners(&self, conn: &PgConnection) -> AppResult> { let users = CrateOwner::by_owner_kind(OwnerKind::User) .filter(crate_owners::crate_id.eq(self.id)) .inner_join(users::table) @@ -423,7 +423,7 @@ impl Crate { conn: &PgConnection, req_user: &User, login: &str, - ) -> CargoResult { + ) -> AppResult { use diesel::insert_into; let owner = Owner::find_or_create_by_login(app, conn, req_user, login)?; @@ -486,7 +486,7 @@ impl Crate { conn: &PgConnection, req_user: &User, login: &str, - ) -> CargoResult<()> { + ) -> AppResult<()> { let owner = Owner::find_or_create_by_login(app, conn, req_user, login)?; let target = crate_owners::table.find((self.id(), owner.id(), owner.kind() as i32)); @@ -507,7 +507,7 @@ impl Crate { &self, conn: &PgConnection, params: &IndexMap, - ) -> CargoResult<(Vec, i64)> { + ) -> AppResult<(Vec, i64)> { use crate::controllers::helpers::pagination::*; use diesel::sql_query; use diesel::sql_types::{BigInt, Integer}; diff --git a/src/models/owner.rs b/src/models/owner.rs index 88d8c7afb92..60288b81ce7 100644 --- a/src/models/owner.rs +++ b/src/models/owner.rs @@ -3,7 +3,7 @@ use diesel::prelude::*; use crate::app::App; use crate::github; -use crate::util::{human, CargoResult}; +use crate::util::{human, AppResult}; use crate::models::user::{UserNoEmailType, ALL_COLUMNS}; use crate::models::{Crate, Team, User}; @@ -65,7 +65,7 @@ impl Owner { conn: &PgConnection, req_user: &User, name: &str, - ) -> CargoResult { + ) -> AppResult { if name.contains(':') { Ok(Owner::Team(Team::create_or_update( app, conn, name, req_user, diff --git a/src/models/team.rs b/src/models/team.rs index 831b3a93390..2f41a8e945d 100644 --- a/src/models/team.rs +++ b/src/models/team.rs @@ -2,7 +2,7 @@ use diesel::prelude::*; use crate::app::App; use crate::github::{github_api, team_url}; -use crate::util::{errors::NotFound, human, CargoResult}; +use crate::util::{errors::NotFound, human, AppResult}; use oauth2::{prelude::*, AccessToken}; @@ -73,7 +73,7 @@ impl Team { conn: &PgConnection, login: &str, req_user: &User, - ) -> CargoResult { + ) -> AppResult { // must look like system:xxxxxxx let mut chunks = login.split(':'); match chunks.next().unwrap() { @@ -113,7 +113,7 @@ impl Team { org_name: &str, team_name: &str, req_user: &User, - ) -> CargoResult { + ) -> AppResult { // GET orgs/:org/teams // check that `team` is the `slug` in results, and grab its data @@ -177,11 +177,11 @@ impl Team { /// Note that we're assuming that the given user is the one interested in /// the answer. If this is not the case, then we could accidentally leak /// private membership information here. - pub fn contains_user(&self, app: &App, user: &User) -> CargoResult { + pub fn contains_user(&self, app: &App, user: &User) -> AppResult { team_with_gh_id_contains_user(app, self.github_id, user) } - pub fn owning(krate: &Crate, conn: &PgConnection) -> CargoResult> { + pub fn owning(krate: &Crate, conn: &PgConnection) -> AppResult> { let base_query = CrateOwner::belonging_to(krate).filter(crate_owners::deleted.eq(false)); let teams = base_query .inner_join(teams::table) @@ -214,7 +214,7 @@ impl Team { } } -fn team_with_gh_id_contains_user(app: &App, github_id: i32, user: &User) -> CargoResult { +fn team_with_gh_id_contains_user(app: &App, github_id: i32, user: &User) -> AppResult { // GET teams/:team_id/memberships/:user_name // check that "state": "active" diff --git a/src/models/user.rs b/src/models/user.rs index 5ff3cb987d5..b6c079534bc 100644 --- a/src/models/user.rs +++ b/src/models/user.rs @@ -3,7 +3,7 @@ use diesel::prelude::*; use std::borrow::Cow; use crate::app::App; -use crate::util::CargoResult; +use crate::util::AppResult; use crate::models::{Crate, CrateOwner, Email, NewEmail, Owner, OwnerKind, Rights}; use crate::schema::{crate_owners, emails, users}; @@ -168,7 +168,7 @@ impl User { .map(User::from) } - pub fn owning(krate: &Crate, conn: &PgConnection) -> CargoResult> { + pub fn owning(krate: &Crate, conn: &PgConnection) -> AppResult> { let users = CrateOwner::by_owner_kind(OwnerKind::User) .inner_join(users::table) .select(ALL_COLUMNS) @@ -189,7 +189,7 @@ impl User { /// `Publish` as well, but this is a non-obvious invariant so we don't bother. /// Sweet free optimization if teams are proving burdensome to check. /// More than one team isn't really expected, though. - pub fn rights(&self, app: &App, owners: &[Owner]) -> CargoResult { + pub fn rights(&self, app: &App, owners: &[Owner]) -> AppResult { let mut best = Rights::None; for owner in owners { match *owner { @@ -208,7 +208,7 @@ impl User { Ok(best) } - pub fn verified_email(&self, conn: &PgConnection) -> CargoResult> { + pub fn verified_email(&self, conn: &PgConnection) -> AppResult> { Ok(Email::belonging_to(self) .select(emails::email) .filter(emails::verified.eq(true)) diff --git a/src/models/version.rs b/src/models/version.rs index 0194b55e096..e3fa74e510b 100644 --- a/src/models/version.rs +++ b/src/models/version.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use chrono::NaiveDateTime; use diesel::prelude::*; -use crate::util::{human, CargoResult}; +use crate::util::{human, AppResult}; use crate::models::user; use crate::models::user::UserNoEmailType; @@ -138,7 +138,7 @@ impl NewVersion { license_file: Option<&str>, crate_size: i32, published_by: i32, - ) -> CargoResult { + ) -> AppResult { let features = serde_json::to_value(features)?; let mut new_version = NewVersion { @@ -160,7 +160,7 @@ impl NewVersion { conn: &PgConnection, authors: &[String], published_by_email: &str, - ) -> CargoResult { + ) -> AppResult { use crate::schema::version_authors::{name, version_id}; use crate::schema::versions::dsl::*; use diesel::dsl::exists; @@ -201,7 +201,7 @@ impl NewVersion { }) } - fn validate_license(&mut self, license_file: Option<&str>) -> CargoResult<()> { + fn validate_license(&mut self, license_file: Option<&str>) -> AppResult<()> { if let Some(ref license) = self.license { for part in license.split('/') { license_exprs::validate_license_expr(part).map_err(|e| { diff --git a/src/publish_rate_limit.rs b/src/publish_rate_limit.rs index d21d47ab574..f919628a249 100644 --- a/src/publish_rate_limit.rs +++ b/src/publish_rate_limit.rs @@ -4,7 +4,7 @@ use diesel::prelude::*; use std::time::Duration; use crate::schema::{publish_limit_buckets, publish_rate_overrides}; -use crate::util::errors::{CargoResult, TooManyRequests}; +use crate::util::errors::{AppResult, TooManyRequests}; #[derive(Debug, Clone, Copy)] pub struct PublishRateLimit { @@ -31,7 +31,7 @@ struct Bucket { } impl PublishRateLimit { - pub fn check_rate_limit(&self, uploader: i32, conn: &PgConnection) -> CargoResult<()> { + pub fn check_rate_limit(&self, uploader: i32, conn: &PgConnection) -> AppResult<()> { let bucket = self.take_token(uploader, Utc::now().naive_utc(), conn)?; if bucket.tokens >= 1 { Ok(()) @@ -55,7 +55,7 @@ impl PublishRateLimit { uploader: i32, now: NaiveDateTime, conn: &PgConnection, - ) -> CargoResult { + ) -> AppResult { use self::publish_limit_buckets::dsl::*; use diesel::sql_types::{Double, Interval, Text, Timestamp}; @@ -108,7 +108,7 @@ mod tests { use crate::test_util::*; #[test] - fn take_token_with_no_bucket_creates_new_one() -> CargoResult<()> { + fn take_token_with_no_bucket_creates_new_one() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -139,7 +139,7 @@ mod tests { } #[test] - fn take_token_with_existing_bucket_modifies_existing_bucket() -> CargoResult<()> { + fn take_token_with_existing_bucket_modifies_existing_bucket() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -159,7 +159,7 @@ mod tests { } #[test] - fn take_token_after_delay_refills() -> CargoResult<()> { + fn take_token_after_delay_refills() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -180,7 +180,7 @@ mod tests { } #[test] - fn refill_subsecond_rate() -> CargoResult<()> { + fn refill_subsecond_rate() -> AppResult<()> { let conn = pg_connection(); // Subsecond rates have floating point rounding issues, so use a known // timestamp that rounds fine @@ -204,7 +204,7 @@ mod tests { } #[test] - fn last_refill_always_advanced_by_multiple_of_rate() -> CargoResult<()> { + fn last_refill_always_advanced_by_multiple_of_rate() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -225,7 +225,7 @@ mod tests { } #[test] - fn zero_tokens_returned_when_user_has_no_tokens_left() -> CargoResult<()> { + fn zero_tokens_returned_when_user_has_no_tokens_left() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -248,7 +248,7 @@ mod tests { } #[test] - fn a_user_with_no_tokens_gets_a_token_after_exactly_rate() -> CargoResult<()> { + fn a_user_with_no_tokens_gets_a_token_after_exactly_rate() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -270,7 +270,7 @@ mod tests { } #[test] - fn tokens_never_refill_past_burst() -> CargoResult<()> { + fn tokens_never_refill_past_burst() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -292,7 +292,7 @@ mod tests { } #[test] - fn override_is_used_instead_of_global_burst_if_present() -> CargoResult<()> { + fn override_is_used_instead_of_global_burst_if_present() -> AppResult<()> { let conn = pg_connection(); let now = now(); @@ -318,7 +318,7 @@ mod tests { Ok(()) } - fn new_user(conn: &PgConnection, gh_login: &str) -> CargoResult { + fn new_user(conn: &PgConnection, gh_login: &str) -> AppResult { use crate::models::NewUser; let user = NewUser { @@ -329,11 +329,7 @@ mod tests { Ok(user.id) } - fn new_user_bucket( - conn: &PgConnection, - tokens: i32, - now: NaiveDateTime, - ) -> CargoResult { + fn new_user_bucket(conn: &PgConnection, tokens: i32, now: NaiveDateTime) -> AppResult { diesel::insert_into(publish_limit_buckets::table) .values(Bucket { user_id: new_user(conn, "new_user")?, diff --git a/src/router.rs b/src/router.rs index d324228889c..6806f1ac84a 100644 --- a/src/router.rs +++ b/src/router.rs @@ -5,7 +5,7 @@ use conduit::{Handler, Request, Response}; use conduit_router::{RequestParams, RouteBuilder}; use crate::controllers::*; -use crate::util::errors::{std_error, CargoError, CargoResult, NotFound}; +use crate::util::errors::{std_error, AppError, AppResult, NotFound}; use crate::util::RequestProxy; use crate::{App, Env}; @@ -129,7 +129,7 @@ pub fn build_router(app: &App) -> R404 { R404(router) } -struct C(pub fn(&mut dyn Request) -> CargoResult); +struct C(pub fn(&mut dyn Request) -> AppResult); impl Handler for C { fn call(&self, req: &mut dyn Request) -> Result> { @@ -179,7 +179,7 @@ mod tests { use conduit_test::MockRequest; use diesel::result::Error as DieselError; - fn err(err: E) -> CargoResult { + fn err(err: E) -> AppResult { Err(Box::new(err)) } diff --git a/src/tests/all.rs b/src/tests/all.rs index b3c0b067208..5ede0328de8 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -19,7 +19,7 @@ use crate::util::{Bad, RequestHelper, TestApp}; use cargo_registry::{ models::{Crate, CrateOwner, Dependency, NewCategory, NewTeam, NewUser, Team, User, Version}, schema::crate_owners, - util::CargoResult, + util::AppResult, views::{ EncodableCategory, EncodableCategoryWithSubcategories, EncodableCrate, EncodableKeyword, EncodableOwner, EncodableVersion, GoodCrate, @@ -236,7 +236,7 @@ fn new_team(login: &str) -> NewTeam<'_> { } } -fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) -> CargoResult<()> { +fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) -> AppResult<()> { let crate_owner = CrateOwner { crate_id: krate.id, owner_id: t.id, diff --git a/src/tests/builders.rs b/src/tests/builders.rs index 17c71581f0e..9f7d25d0260 100644 --- a/src/tests/builders.rs +++ b/src/tests/builders.rs @@ -3,7 +3,7 @@ use cargo_registry::{ models::{Crate, Keyword, NewCrate, NewVersion, Version}, schema::{crates, dependencies, version_downloads, versions}, - util::CargoResult, + util::AppResult, views::krate_publish as u, }; use std::{collections::HashMap, io::Read}; @@ -72,7 +72,7 @@ impl<'a> VersionBuilder<'a> { crate_id: i32, published_by: i32, connection: &PgConnection, - ) -> CargoResult { + ) -> AppResult { use diesel::{insert_into, update}; let license = match self.license { @@ -229,7 +229,7 @@ impl<'a> CrateBuilder<'a> { self } - fn build(mut self, connection: &PgConnection) -> CargoResult { + fn build(mut self, connection: &PgConnection) -> AppResult { use diesel::{insert_into, select, update}; let mut krate = self diff --git a/src/uploaders.rs b/src/uploaders.rs index fca3c98c036..974d469aa10 100644 --- a/src/uploaders.rs +++ b/src/uploaders.rs @@ -4,7 +4,7 @@ use openssl::hash::{Hasher, MessageDigest}; use reqwest::header; use crate::util::LimitErrorReader; -use crate::util::{human, internal, CargoResult, ChainError, Maximums}; +use crate::util::{human, internal, AppResult, ChainError, Maximums}; use std::env; use std::fs::{self, File}; @@ -96,7 +96,7 @@ impl Uploader { content_length: u64, content_type: &str, extra_headers: header::HeaderMap, - ) -> CargoResult> { + ) -> AppResult> { match *self { Uploader::S3 { ref bucket, .. } => { bucket @@ -129,7 +129,7 @@ impl Uploader { krate: &Crate, maximums: Maximums, vers: &semver::Version, - ) -> CargoResult> { + ) -> AppResult> { let app = Arc::clone(req.app()); let path = Uploader::crate_path(&krate.name, &vers.to_string()); let mut body = Vec::new(); @@ -157,7 +157,7 @@ impl Uploader { crate_name: &str, vers: &str, readme: String, - ) -> CargoResult<()> { + ) -> AppResult<()> { let path = Uploader::readme_path(crate_name, vers); let content_length = readme.len() as u64; let content = Cursor::new(readme); @@ -183,7 +183,7 @@ fn verify_tarball( vers: &semver::Version, tarball: &[u8], max_unpack: u64, -) -> CargoResult<()> { +) -> AppResult<()> { // All our data is currently encoded with gzip let decoder = GzDecoder::new(tarball); diff --git a/src/util.rs b/src/util.rs index 383e13b0efc..8ab7eee2692 100644 --- a/src/util.rs +++ b/src/util.rs @@ -6,7 +6,7 @@ use conduit::Response; use serde::Serialize; pub use self::errors::ChainError; -pub use self::errors::{bad_request, human, internal, internal_error, CargoError, CargoResult}; +pub use self::errors::{bad_request, human, internal, internal_error, AppError, AppResult}; pub use self::io_util::{read_fill, read_le_u32, LimitErrorReader}; pub use self::request_helpers::*; pub use self::request_proxy::RequestProxy; diff --git a/src/util/errors.rs b/src/util/errors.rs index 481b97881a4..d89e530c49a 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -18,11 +18,11 @@ struct Bad { } // ============================================================================= -// CargoError trait +// AppError trait -pub trait CargoError: Send + fmt::Display + fmt::Debug + 'static { +pub trait AppError: Send + fmt::Display + fmt::Debug + 'static { fn description(&self) -> &str; - fn cause(&self) -> Option<&(dyn CargoError)> { + fn cause(&self) -> Option<&(dyn AppError)> { None } @@ -34,7 +34,7 @@ pub trait CargoError: Send + fmt::Display + fmt::Debug + 'static { }], })) } else { - self.cause().and_then(CargoError::response) + self.cause().and_then(AppError::response) } } fn human(&self) -> bool { @@ -46,12 +46,12 @@ pub trait CargoError: Send + fmt::Display + fmt::Debug + 'static { } } -impl dyn CargoError { +impl dyn AppError { pub fn is(&self) -> bool { self.get_type_id() == TypeId::of::() } - pub fn from_std_error(err: Box) -> Box { + pub fn from_std_error(err: Box) -> Box { Self::try_convert(&*err).unwrap_or_else(|| internal(&err)) } @@ -68,11 +68,11 @@ impl dyn CargoError { } } -impl CargoError for Box { +impl AppError for Box { fn description(&self) -> &str { (**self).description() } - fn cause(&self) -> Option<&dyn CargoError> { + fn cause(&self) -> Option<&dyn AppError> { (**self).cause() } fn human(&self) -> bool { @@ -83,56 +83,56 @@ impl CargoError for Box { } } -pub type CargoResult = Result>; +pub type AppResult = Result>; // ============================================================================= // Chaining errors pub trait ChainError { - fn chain_error(self, callback: F) -> CargoResult + fn chain_error(self, callback: F) -> AppResult where - E: CargoError, + E: AppError, F: FnOnce() -> E; } #[derive(Debug)] struct ChainedError { error: E, - cause: Box, + cause: Box, } impl ChainError for F where - F: FnOnce() -> CargoResult, + F: FnOnce() -> AppResult, { - fn chain_error(self, callback: C) -> CargoResult + fn chain_error(self, callback: C) -> AppResult where - E: CargoError, + E: AppError, C: FnOnce() -> E, { self().chain_error(callback) } } -impl ChainError for Result { - fn chain_error(self, callback: C) -> CargoResult +impl ChainError for Result { + fn chain_error(self, callback: C) -> AppResult where - E2: CargoError, + E2: AppError, C: FnOnce() -> E2, { self.map_err(move |err| { Box::new(ChainedError { error: callback(), cause: Box::new(err), - }) as Box + }) as Box }) } } impl ChainError for Option { - fn chain_error(self, callback: C) -> CargoResult + fn chain_error(self, callback: C) -> AppResult where - E: CargoError, + E: AppError, C: FnOnce() -> E, { match self { @@ -142,11 +142,11 @@ impl ChainError for Option { } } -impl CargoError for ChainedError { +impl AppError for ChainedError { fn description(&self) -> &str { self.error.description() } - fn cause(&self) -> Option<&dyn CargoError> { + fn cause(&self) -> Option<&dyn AppError> { Some(&*self.cause) } fn response(&self) -> Option { @@ -157,7 +157,7 @@ impl CargoError for ChainedError { } } -impl fmt::Display for ChainedError { +impl fmt::Display for ChainedError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{} caused by {}", self.error, self.cause) } @@ -166,29 +166,29 @@ impl fmt::Display for ChainedError { // ============================================================================= // Error impls -impl CargoError for E { +impl AppError for E { fn description(&self) -> &str { Error::description(self) } } -impl From for Box { - fn from(err: E) -> Box { - CargoError::try_convert(&err).unwrap_or_else(|| Box::new(err)) +impl From for Box { + fn from(err: E) -> Box { + AppError::try_convert(&err).unwrap_or_else(|| Box::new(err)) } } // ============================================================================= // Concrete errors #[derive(Debug)] -struct ConcreteCargoError { +struct ConcreteAppError { description: String, detail: Option, - cause: Option>, + cause: Option>, human: bool, } -impl fmt::Display for ConcreteCargoError { +impl fmt::Display for ConcreteAppError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.description)?; if let Some(ref s) = self.detail { @@ -198,11 +198,11 @@ impl fmt::Display for ConcreteCargoError { } } -impl CargoError for ConcreteCargoError { +impl AppError for ConcreteAppError { fn description(&self) -> &str { &self.description } - fn cause(&self) -> Option<&dyn CargoError> { + fn cause(&self) -> Option<&dyn AppError> { self.cause.as_ref().map(|c| &**c) } fn human(&self) -> bool { @@ -213,7 +213,7 @@ impl CargoError for ConcreteCargoError { #[derive(Debug, Clone, Copy)] pub struct NotFound; -impl CargoError for NotFound { +impl AppError for NotFound { fn description(&self) -> &str { "not found" } @@ -238,7 +238,7 @@ impl fmt::Display for NotFound { #[derive(Debug, Clone, Copy)] pub struct Unauthorized; -impl CargoError for Unauthorized { +impl AppError for Unauthorized { fn description(&self) -> &str { "unauthorized" } @@ -263,7 +263,7 @@ impl fmt::Display for Unauthorized { #[derive(Debug)] struct BadRequest(String); -impl CargoError for BadRequest { +impl AppError for BadRequest { fn description(&self) -> &str { self.0.as_ref() } @@ -285,8 +285,8 @@ impl fmt::Display for BadRequest { } } -pub fn internal_error(error: &str, detail: &str) -> Box { - Box::new(ConcreteCargoError { +pub fn internal_error(error: &str, detail: &str) -> Box { + Box::new(ConcreteAppError { description: error.to_string(), detail: Some(detail.to_string()), cause: None, @@ -294,8 +294,8 @@ pub fn internal_error(error: &str, detail: &str) -> Box { }) } -pub fn internal(error: &S) -> Box { - Box::new(ConcreteCargoError { +pub fn internal(error: &S) -> Box { + Box::new(ConcreteAppError { description: error.to_string(), detail: None, cause: None, @@ -303,8 +303,8 @@ pub fn internal(error: &S) -> Box { }) } -pub fn human(error: &S) -> Box { - Box::new(ConcreteCargoError { +pub fn human(error: &S) -> Box { + Box::new(ConcreteAppError { description: error.to_string(), detail: None, cause: None, @@ -319,20 +319,20 @@ pub fn human(error: &S) -> Box { /// /// Since this is going back to the UI these errors are treated the same as /// `human` errors, other than the HTTP status code. -pub fn bad_request(error: &S) -> Box { +pub fn bad_request(error: &S) -> Box { Box::new(BadRequest(error.to_string())) } #[derive(Debug)] -pub struct CargoErrToStdErr(pub Box); +pub struct AppErrToStdErr(pub Box); -impl Error for CargoErrToStdErr { +impl Error for AppErrToStdErr { fn description(&self) -> &str { self.0.description() } } -impl fmt::Display for CargoErrToStdErr { +impl fmt::Display for AppErrToStdErr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.0)?; @@ -346,18 +346,18 @@ impl fmt::Display for CargoErrToStdErr { } } -pub(crate) fn std_error(e: Box) -> Box { - Box::new(CargoErrToStdErr(e)) +pub(crate) fn std_error(e: Box) -> Box { + Box::new(AppErrToStdErr(e)) } -pub(crate) fn std_error_no_send(e: Box) -> Box { - Box::new(CargoErrToStdErr(e)) +pub(crate) fn std_error_no_send(e: Box) -> Box { + Box::new(AppErrToStdErr(e)) } #[derive(Debug, Clone, Copy)] pub struct ReadOnlyMode; -impl CargoError for ReadOnlyMode { +impl AppError for ReadOnlyMode { fn description(&self) -> &str { "tried to write in read only mode" } @@ -390,7 +390,7 @@ pub struct TooManyRequests { pub retry_after: NaiveDateTime, } -impl CargoError for TooManyRequests { +impl AppError for TooManyRequests { fn description(&self) -> &str { "too many requests" } From a947ff256025ca0661f1ecbfaf665474690111c4 Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Sat, 23 Nov 2019 12:42:17 -0500 Subject: [PATCH 2/5] Rename `human` to `cargo_err` --- src/bin/enqueue-job.rs | 4 +-- src/controllers.rs | 2 +- src/controllers/crate_owner_invitation.rs | 2 +- src/controllers/helpers/pagination.rs | 4 +-- src/controllers/krate/owners.rs | 12 ++++----- src/controllers/krate/publish.rs | 18 ++++++------- src/controllers/user/me.rs | 12 ++++----- src/controllers/user/session.rs | 4 +-- src/controllers/version.rs | 4 +-- src/controllers/version/yank.rs | 2 +- src/github.rs | 4 +-- src/models/dependency.rs | 8 +++--- src/models/krate.rs | 8 +++--- src/models/owner.rs | 4 +-- src/models/team.rs | 12 ++++----- src/models/version.rs | 6 ++--- src/router.rs | 6 ++--- src/uploaders.rs | 8 +++--- src/util.rs | 2 +- src/util/errors.rs | 32 +++++++++++------------ 20 files changed, 77 insertions(+), 77 deletions(-) diff --git a/src/bin/enqueue-job.rs b/src/bin/enqueue-job.rs index f91bc270705..f5df8b7cc3c 100644 --- a/src/bin/enqueue-job.rs +++ b/src/bin/enqueue-job.rs @@ -1,4 +1,4 @@ -use cargo_registry::util::{human, AppError, AppResult}; +use cargo_registry::util::{cargo_err, AppError, AppResult}; use cargo_registry::{db, env, tasks}; use diesel::PgConnection; @@ -14,7 +14,7 @@ fn main() -> AppResult<()> { .unwrap_or_else(|| String::from("db-dump.tar.gz")); tasks::dump_db(database_url, target_name).enqueue(&conn) } - other => Err(human(&format!("Unrecognized job type `{}`", other))), + other => Err(cargo_err(&format!("Unrecognized job type `{}`", other))), } } diff --git a/src/controllers.rs b/src/controllers.rs index f8f5d672b2f..d1437c8f69b 100644 --- a/src/controllers.rs +++ b/src/controllers.rs @@ -6,7 +6,7 @@ mod prelude { pub use conduit_router::RequestParams; pub use crate::db::RequestTransaction; - pub use crate::util::{human, AppResult}; + pub use crate::util::{cargo_err, AppResult}; pub use crate::middleware::app::RequestApp; pub use crate::middleware::current_user::RequestUser; diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index 28ab7c2c135..1cf8ded7ef4 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -38,7 +38,7 @@ pub fn handle_invite(req: &mut dyn Request) -> AppResult { let conn = &*req.db_conn()?; let crate_invite: OwnerInvitation = - serde_json::from_str(&body).map_err(|_| human("invalid json request"))?; + serde_json::from_str(&body).map_err(|_| cargo_err("invalid json request"))?; let crate_invite = crate_invite.crate_owner_invite; diff --git a/src/controllers/helpers/pagination.rs b/src/controllers/helpers/pagination.rs index 50ddf6cb171..8454d40c25a 100644 --- a/src/controllers/helpers/pagination.rs +++ b/src/controllers/helpers/pagination.rs @@ -18,7 +18,7 @@ impl Page { if let Some(s) = params.get("page") { let numeric_page = s.parse()?; if numeric_page < 1 { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "page indexing starts from 1, page {} is invalid", numeric_page, ))); @@ -48,7 +48,7 @@ impl PaginationOptions { .unwrap_or(Ok(DEFAULT_PER_PAGE))?; if per_page > MAX_PER_PAGE { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "cannot request more than {} items", MAX_PER_PAGE, ))); diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index b372c504712..581d68b06d0 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -82,11 +82,11 @@ fn parse_owners_request(req: &mut dyn Request) -> AppResult> { owners: Option>, } let request: Request = - serde_json::from_str(&body).map_err(|_| human("invalid json request"))?; + serde_json::from_str(&body).map_err(|_| cargo_err("invalid json request"))?; request .owners .or(request.users) - .ok_or_else(|| human("invalid json request")) + .ok_or_else(|| cargo_err("invalid json request")) } fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { @@ -104,10 +104,10 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { Rights::Full => {} // Yes! Rights::Publish => { - return Err(human("team members don't have permission to modify owners")); + return Err(cargo_err("team members don't have permission to modify owners")); } Rights::None => { - return Err(human("only owners have permission to modify owners")); + return Err(cargo_err("only owners have permission to modify owners")); } } @@ -117,7 +117,7 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { let login_test = |owner: &Owner| owner.login().to_lowercase() == *login.to_lowercase(); if owners.iter().any(login_test) { - return Err(human(&format_args!("`{}` is already an owner", login))); + return Err(cargo_err(&format_args!("`{}` is already an owner", login))); } let msg = krate.owner_add(app, &conn, user, login)?; msgs.push(msg); @@ -128,7 +128,7 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { krate.owner_remove(app, &conn, user, login)?; } if User::owning(&krate, &conn)?.is_empty() { - return Err(human( + return Err(cargo_err( "cannot remove all individual owners of a crate. \ Team member don't have permission to modify owners, so \ at least one individual owner is required.", diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 0af7a61aa61..9faeee0694a 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -42,7 +42,7 @@ pub fn publish(req: &mut dyn Request) -> AppResult { let verified_email_address = user.verified_email(&conn)?; let verified_email_address = verified_email_address.ok_or_else(|| { - human( + cargo_err( "A verified email address is required to publish crates to crates.io. \ Visit https://crates.io/me to set and verify your email address.", ) @@ -88,7 +88,7 @@ pub fn publish(req: &mut dyn Request) -> AppResult { let owners = krate.owners(&conn)?; if user.rights(req.app(), &owners)? < Rights::Publish { - return Err(human( + return Err(cargo_err( "this crate exists but you don't seem to be an owner. \ If you believe this is a mistake, perhaps you need \ to accept an invitation to be an owner before \ @@ -97,7 +97,7 @@ pub fn publish(req: &mut dyn Request) -> AppResult { } if krate.name != *name { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "crate was previously named `{}`", krate.name ))); @@ -111,7 +111,7 @@ pub fn publish(req: &mut dyn Request) -> AppResult { let content_length = req .content_length() - .chain_error(|| human("missing header: Content-Length"))?; + .chain_error(|| cargo_err("missing header: Content-Length"))?; let maximums = Maximums::new( krate.max_upload_size, @@ -120,7 +120,7 @@ pub fn publish(req: &mut dyn Request) -> AppResult { ); if content_length > maximums.max_upload_size { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "max upload size is: {}", maximums.max_upload_size ))); @@ -221,13 +221,13 @@ fn parse_new_headers(req: &mut dyn Request) -> AppResult<(EncodableCrateUpload, let max = req.app().config.max_upload_size; if metadata_length > max { - return Err(human(&format_args!("max upload size is: {}", max))); + return Err(cargo_err(&format_args!("max upload size is: {}", max))); } let mut json = vec![0; metadata_length as usize]; read_fill(req.body(), &mut json)?; - let json = String::from_utf8(json).map_err(|_| human("json body was not valid utf-8"))?; + let json = String::from_utf8(json).map_err(|_| cargo_err("json body was not valid utf-8"))?; let new: EncodableCrateUpload = serde_json::from_str(&json) - .map_err(|e| human(&format_args!("invalid upload request: {}", e)))?; + .map_err(|e| cargo_err(&format_args!("invalid upload request: {}", e)))?; // Make sure required fields are provided fn empty(s: Option<&String>) -> bool { @@ -245,7 +245,7 @@ fn parse_new_headers(req: &mut dyn Request) -> AppResult<(EncodableCrateUpload, missing.push("authors"); } if !missing.is_empty() { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "missing or empty metadata fields: {}. Please \ see https://doc.rust-lang.org/cargo/reference/manifest.html for \ how to upload metadata", diff --git a/src/controllers/user/me.rs b/src/controllers/user/me.rs index 3e9ace1b5e0..381e02452e9 100644 --- a/src/controllers/user/me.rs +++ b/src/controllers/user/me.rs @@ -122,7 +122,7 @@ pub fn update_user(req: &mut dyn Request) -> AppResult { // need to check if current user matches user to be updated if &user.id.to_string() != name { - return Err(human("current user does not match requested user")); + return Err(cargo_err("current user does not match requested user")); } #[derive(Deserialize)] @@ -136,17 +136,17 @@ pub fn update_user(req: &mut dyn Request) -> AppResult { } let user_update: UserUpdate = - serde_json::from_str(&body).map_err(|_| human("invalid json request"))?; + serde_json::from_str(&body).map_err(|_| cargo_err("invalid json request"))?; if user_update.user.email.is_none() { - return Err(human("empty email rejected")); + return Err(cargo_err("empty email rejected")); } let user_email = user_update.user.email.unwrap(); let user_email = user_email.trim(); if user_email == "" { - return Err(human("empty email rejected")); + return Err(cargo_err("empty email rejected")); } conn.transaction::<_, Box, _>(|| { @@ -166,7 +166,7 @@ pub fn update_user(req: &mut dyn Request) -> AppResult { .set(&new_email) .returning(emails::token) .get_result::(&*conn) - .map_err(|_| human("Error in creating token"))?; + .map_err(|_| cargo_err("Error in creating token"))?; crate::email::send_user_confirm_email(user_email, &user.gh_login, &token); @@ -213,7 +213,7 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> AppResult { // need to check if current user matches user to be updated if &user.id != name { - return Err(human("current user does not match requested user")); + return Err(cargo_err("current user does not match requested user")); } conn.transaction(|| { diff --git a/src/controllers/user/session.rs b/src/controllers/user/session.rs index 43d33b682c7..05b924409aa 100644 --- a/src/controllers/user/session.rs +++ b/src/controllers/user/session.rs @@ -85,7 +85,7 @@ pub fn github_access_token(req: &mut dyn Request) -> AppResult { 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(human("invalid state parameter")); + return Err(cargo_err("invalid state parameter")); } } @@ -96,7 +96,7 @@ pub fn github_access_token(req: &mut dyn Request) -> AppResult { .app() .github .exchange_code(code) - .map_err(|s| human(&s))?; + .map_err(|s| cargo_err(&s))?; let token = token.access_token(); let ghuser = github::github_api::(req.app(), "/user", token)?; let user = ghuser.save_to_database(&token.secret(), &*req.db_conn()?)?; diff --git a/src/controllers/version.rs b/src/controllers/version.rs index bffdd3ec50d..2970d78ca8a 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -12,7 +12,7 @@ fn version_and_crate(req: &mut dyn Request) -> AppResult<(Version, Crate)> { let crate_name = &req.params()["crate_id"]; let semver = &req.params()["version"]; if semver::Version::parse(semver).is_err() { - return Err(human(&format_args!("invalid semver: {}", semver))); + return Err(cargo_err(&format_args!("invalid semver: {}", semver))); }; let conn = req.db_conn()?; let krate = Crate::by_name(crate_name).first::(&*conn)?; @@ -21,7 +21,7 @@ fn version_and_crate(req: &mut dyn Request) -> AppResult<(Version, Crate)> { .filter(versions::num.eq(semver)) .first(&*conn) .map_err(|_| { - human(&format_args!( + cargo_err(&format_args!( "crate `{}` does not have a version `{}`", crate_name, semver )) diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 6fd6eccae0e..f29da798d67 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -33,7 +33,7 @@ fn modify_yank(req: &mut dyn Request, yanked: bool) -> AppResult { let conn = req.db_conn()?; let owners = krate.owners(&conn)?; if user.rights(req.app(), &owners)? < Rights::Publish { - return Err(human("must already be an owner to yank or unyank")); + return Err(cargo_err("must already be an owner to yank or unyank")); } git::yank(krate.name, version, yanked) diff --git a/src/github.rs b/src/github.rs index 4b9ca577eea..569a00943bf 100644 --- a/src/github.rs +++ b/src/github.rs @@ -8,7 +8,7 @@ use serde::de::DeserializeOwned; use std::str; use crate::app::App; -use crate::util::{errors::NotFound, human, internal, AppError, AppResult}; +use crate::util::{errors::NotFound, cargo_err, internal, AppError, AppResult}; /// Does all the nonsense for sending a GET to Github. Doesn't handle parsing /// because custom error-code handling may be desirable. Use @@ -35,7 +35,7 @@ fn handle_error_response(error: &reqwest::Error) -> Box { use reqwest::StatusCode as Status; match error.status() { - Some(Status::UNAUTHORIZED) | Some(Status::FORBIDDEN) => human( + Some(Status::UNAUTHORIZED) | Some(Status::FORBIDDEN) => cargo_err( "It looks like you don't have permission \ to query a necessary property from Github \ to complete this request. \ diff --git a/src/models/dependency.rs b/src/models/dependency.rs index 44f22074c37..bab9d921bff 100644 --- a/src/models/dependency.rs +++ b/src/models/dependency.rs @@ -1,7 +1,7 @@ use diesel::prelude::*; use crate::git; -use crate::util::{human, AppResult}; +use crate::util::{cargo_err, AppResult}; use crate::models::{Crate, Version}; use crate::schema::*; @@ -82,16 +82,16 @@ pub fn add_dependencies( .map(|dep| { if let Some(registry) = &dep.registry { if !registry.is_empty() { - return Err(human(&format_args!("Dependency `{}` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.", &*dep.name))); + return Err(cargo_err(&format_args!("Dependency `{}` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.", &*dep.name))); } } // Match only identical names to ensure the index always references the original crate name let krate = Crate::by_exact_name(&dep.name) .first::(&*conn) - .map_err(|_| human(&format_args!("no known crate named `{}`", &*dep.name)))?; + .map_err(|_| cargo_err(&format_args!("no known crate named `{}`", &*dep.name)))?; if dep.version_req == semver::VersionReq::parse("*").unwrap() { - return Err(human( + return Err(cargo_err( "wildcard (`*`) dependency constraints are not allowed \ on crates.io. See https://doc.rust-lang.org/cargo/faq.html#can-\ libraries-use--as-a-version-for-their-dependencies for more \ diff --git a/src/models/krate.rs b/src/models/krate.rs index a65632ece4b..7638a3fa483 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -10,7 +10,7 @@ use crate::app::App; use crate::email; use crate::models::user; use crate::models::user::UserNoEmailType; -use crate::util::{human, AppResult}; +use crate::util::{cargo_err, AppResult}; use crate::models::{ Badge, Category, CrateOwner, CrateOwnerInvitation, Keyword, NewCrateOwnerInvitation, Owner, @@ -141,7 +141,7 @@ impl<'a> NewCrate<'a> { // Manually check the string, as `Url::parse` may normalize relative URLs // making it difficult to ensure that both slashes are present. if !url.starts_with("http://") && !url.starts_with("https://") { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "URL for field `{}` must begin with http:// or https:// (url: {})", field, url ))); @@ -149,7 +149,7 @@ impl<'a> NewCrate<'a> { // Ensure the entire URL parses as well Url::parse(url) - .map_err(|_| human(&format_args!("`{}` is not a valid url: `{}`", field, url)))?; + .map_err(|_| cargo_err(&format_args!("`{}` is not a valid url: `{}`", field, url)))?; Ok(()) } @@ -169,7 +169,7 @@ impl<'a> NewCrate<'a> { )) .get_result::(conn)?; if reserved_name { - Err(human("cannot upload a crate with a reserved name")) + Err(cargo_err("cannot upload a crate with a reserved name")) } else { Ok(()) } diff --git a/src/models/owner.rs b/src/models/owner.rs index 60288b81ce7..c3847c91884 100644 --- a/src/models/owner.rs +++ b/src/models/owner.rs @@ -3,7 +3,7 @@ use diesel::prelude::*; use crate::app::App; use crate::github; -use crate::util::{human, AppResult}; +use crate::util::{cargo_err, AppResult}; use crate::models::user::{UserNoEmailType, ALL_COLUMNS}; use crate::models::{Crate, Team, User}; @@ -77,7 +77,7 @@ impl Owner { .first::(conn) .map(User::from) .map(Owner::User) - .map_err(|_| human(&format_args!("could not find user with login `{}`", name))) + .map_err(|_| cargo_err(&format_args!("could not find user with login `{}`", name))) } } diff --git a/src/models/team.rs b/src/models/team.rs index 2f41a8e945d..1d243a21062 100644 --- a/src/models/team.rs +++ b/src/models/team.rs @@ -2,7 +2,7 @@ use diesel::prelude::*; use crate::app::App; use crate::github::{github_api, team_url}; -use crate::util::{errors::NotFound, human, AppResult}; +use crate::util::{errors::NotFound, cargo_err, AppResult}; use oauth2::{prelude::*, AccessToken}; @@ -82,7 +82,7 @@ impl Team { // Ok to unwrap since we know one ":" is contained let org = chunks.next().unwrap(); let team = chunks.next().ok_or_else(|| { - human( + cargo_err( "missing github team argument; \ format is github:org:team", ) @@ -96,7 +96,7 @@ impl Team { req_user, ) } - _ => Err(human( + _ => Err(cargo_err( "unknown organization handler, \ only 'github:org:team' is supported", )), @@ -126,7 +126,7 @@ impl Team { } if let Some(c) = org_name.chars().find(|c| whitelist(*c)) { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "organization cannot contain special \ characters like {}", c @@ -150,14 +150,14 @@ impl Team { .into_iter() .find(|team| team.slug.to_lowercase() == team_name.to_lowercase()) .ok_or_else(|| { - human(&format_args!( + cargo_err(&format_args!( "could not find the github team {}/{}", org_name, team_name )) })?; if !team_with_gh_id_contains_user(app, team.id, req_user)? { - return Err(human("only members of a team can add it as an owner")); + return Err(cargo_err("only members of a team can add it as an owner")); } #[derive(Deserialize)] diff --git a/src/models/version.rs b/src/models/version.rs index e3fa74e510b..e685967e1a9 100644 --- a/src/models/version.rs +++ b/src/models/version.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use chrono::NaiveDateTime; use diesel::prelude::*; -use crate::util::{human, AppResult}; +use crate::util::{cargo_err, AppResult}; use crate::models::user; use crate::models::user::UserNoEmailType; @@ -171,7 +171,7 @@ impl NewVersion { .filter(crate_id.eq(self.crate_id)) .filter(num.eq(&self.num)); if select(exists(already_uploaded)).get_result(conn)? { - return Err(human(&format_args!( + return Err(cargo_err(&format_args!( "crate version `{}` is already \ uploaded", self.num @@ -205,7 +205,7 @@ impl NewVersion { if let Some(ref license) = self.license { for part in license.split('/') { license_exprs::validate_license_expr(part).map_err(|e| { - human(&format_args!( + cargo_err(&format_args!( "{}; see http://opensource.org/licenses \ for options, and http://spdx.org/licenses/ \ for their identifiers", diff --git a/src/router.rs b/src/router.rs index 6806f1ac84a..2c94ae62451 100644 --- a/src/router.rs +++ b/src/router.rs @@ -174,7 +174,7 @@ impl Handler for R404 { #[cfg(test)] mod tests { use super::*; - use crate::util::errors::{bad_request, human, internal, NotFound, Unauthorized}; + use crate::util::errors::{bad_request, cargo_err, internal, NotFound, Unauthorized}; use conduit_test::MockRequest; use diesel::result::Error as DieselError; @@ -206,8 +206,8 @@ mod tests { ); assert_eq!(C(|_| err(NotFound)).call(&mut req).unwrap().status.0, 404); - // Human errors are returned as 200 so that cargo displays this nicely on the command line - assert_eq!(C(|_| Err(human(""))).call(&mut req).unwrap().status.0, 200); + // cargo_err errors are returned as 200 so that cargo displays this nicely on the command line + assert_eq!(C(|_| Err(cargo_err(""))).call(&mut req).unwrap().status.0, 200); // All other error types are propogated up the middleware, eventually becoming status 500 assert!(C(|_| Err(internal(""))).call(&mut req).is_err()); diff --git a/src/uploaders.rs b/src/uploaders.rs index 974d469aa10..90412ce7482 100644 --- a/src/uploaders.rs +++ b/src/uploaders.rs @@ -4,7 +4,7 @@ use openssl::hash::{Hasher, MessageDigest}; use reqwest::header; use crate::util::LimitErrorReader; -use crate::util::{human, internal, AppResult, ChainError, Maximums}; +use crate::util::{cargo_err, internal, AppResult, ChainError, Maximums}; use std::env; use std::fs::{self, File}; @@ -196,7 +196,7 @@ fn verify_tarball( let prefix = format!("{}-{}", krate.name, vers); for entry in archive.entries()? { let entry = entry.chain_error(|| { - human("uploaded tarball is malformed or too large when decompressed") + cargo_err("uploaded tarball is malformed or too large when decompressed") })?; // Verify that all entries actually start with `$name-$vers/`. @@ -205,7 +205,7 @@ fn verify_tarball( // as `bar-0.1.0/` source code, and this could overwrite other crates in // the registry! if !entry.path()?.starts_with(&prefix) { - return Err(human("invalid tarball uploaded")); + return Err(cargo_err("invalid tarball uploaded")); } // Historical versions of the `tar` crate which Cargo uses internally @@ -215,7 +215,7 @@ fn verify_tarball( // generate a tarball with these file types so this should work for now. let entry_type = entry.header().entry_type(); if entry_type.is_hard_link() || entry_type.is_symlink() { - return Err(human("invalid tarball uploaded")); + return Err(cargo_err("invalid tarball uploaded")); } } Ok(()) diff --git a/src/util.rs b/src/util.rs index 8ab7eee2692..c5cc33ceed0 100644 --- a/src/util.rs +++ b/src/util.rs @@ -6,7 +6,7 @@ use conduit::Response; use serde::Serialize; pub use self::errors::ChainError; -pub use self::errors::{bad_request, human, internal, internal_error, AppError, AppResult}; +pub use self::errors::{bad_request, cargo_err, internal, internal_error, AppError, AppResult}; pub use self::io_util::{read_fill, read_le_u32, LimitErrorReader}; pub use self::request_helpers::*; pub use self::request_proxy::RequestProxy; diff --git a/src/util/errors.rs b/src/util/errors.rs index d89e530c49a..ca1f810355c 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -27,7 +27,7 @@ pub trait AppError: Send + fmt::Display + fmt::Debug + 'static { } fn response(&self) -> Option { - if self.human() { + if self.cargo_err() { Some(json_response(&Bad { errors: vec![StringError { detail: self.description().to_string(), @@ -37,7 +37,7 @@ pub trait AppError: Send + fmt::Display + fmt::Debug + 'static { self.cause().and_then(AppError::response) } } - fn human(&self) -> bool { + fn cargo_err(&self) -> bool { false } @@ -75,8 +75,8 @@ impl AppError for Box { fn cause(&self) -> Option<&dyn AppError> { (**self).cause() } - fn human(&self) -> bool { - (**self).human() + fn cargo_err(&self) -> bool { + (**self).cargo_err() } fn response(&self) -> Option { (**self).response() @@ -152,8 +152,8 @@ impl AppError for ChainedError { fn response(&self) -> Option { self.error.response() } - fn human(&self) -> bool { - self.error.human() + fn cargo_err(&self) -> bool { + self.error.cargo_err() } } @@ -185,7 +185,7 @@ struct ConcreteAppError { description: String, detail: Option, cause: Option>, - human: bool, + cargo_err: bool, } impl fmt::Display for ConcreteAppError { @@ -205,8 +205,8 @@ impl AppError for ConcreteAppError { fn cause(&self) -> Option<&dyn AppError> { self.cause.as_ref().map(|c| &**c) } - fn human(&self) -> bool { - self.human + fn cargo_err(&self) -> bool { + self.cargo_err } } @@ -290,7 +290,7 @@ pub fn internal_error(error: &str, detail: &str) -> Box { description: error.to_string(), detail: Some(detail.to_string()), cause: None, - human: false, + cargo_err: false, }) } @@ -299,16 +299,16 @@ pub fn internal(error: &S) -> Box { description: error.to_string(), detail: None, cause: None, - human: false, + cargo_err: false, }) } -pub fn human(error: &S) -> Box { +pub fn cargo_err(error: &S) -> Box { Box::new(ConcreteAppError { description: error.to_string(), detail: None, cause: None, - human: true, + cargo_err: true, }) } @@ -318,7 +318,7 @@ pub fn human(error: &S) -> Box { /// non-200 response codes for its stores to work properly. /// /// Since this is going back to the UI these errors are treated the same as -/// `human` errors, other than the HTTP status code. +/// `cargo_err` errors, other than the HTTP status code. pub fn bad_request(error: &S) -> Box { Box::new(BadRequest(error.to_string())) } @@ -374,7 +374,7 @@ impl AppError for ReadOnlyMode { Some(response) } - fn human(&self) -> bool { + fn cargo_err(&self) -> bool { true } } @@ -416,7 +416,7 @@ impl AppError for TooManyRequests { Some(response) } - fn human(&self) -> bool { + fn cargo_err(&self) -> bool { true } } From 683cd9ff97a5912e01f8f807346c214fa25226ea Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Sat, 23 Nov 2019 12:45:47 -0500 Subject: [PATCH 3/5] `cargo fmt` --- src/controllers/krate/owners.rs | 4 +++- src/github.rs | 2 +- src/models/krate.rs | 5 +++-- src/models/team.rs | 2 +- src/router.rs | 5 ++++- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 581d68b06d0..1ddf032433d 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -104,7 +104,9 @@ fn modify_owners(req: &mut dyn Request, add: bool) -> AppResult { Rights::Full => {} // Yes! Rights::Publish => { - return Err(cargo_err("team members don't have permission to modify owners")); + return Err(cargo_err( + "team members don't have permission to modify owners", + )); } Rights::None => { return Err(cargo_err("only owners have permission to modify owners")); diff --git a/src/github.rs b/src/github.rs index 569a00943bf..86bc321c611 100644 --- a/src/github.rs +++ b/src/github.rs @@ -8,7 +8,7 @@ use serde::de::DeserializeOwned; use std::str; use crate::app::App; -use crate::util::{errors::NotFound, cargo_err, internal, AppError, AppResult}; +use crate::util::{cargo_err, errors::NotFound, internal, AppError, AppResult}; /// Does all the nonsense for sending a GET to Github. Doesn't handle parsing /// because custom error-code handling may be desirable. Use diff --git a/src/models/krate.rs b/src/models/krate.rs index 7638a3fa483..f290e4a03f6 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -148,8 +148,9 @@ impl<'a> NewCrate<'a> { } // Ensure the entire URL parses as well - Url::parse(url) - .map_err(|_| cargo_err(&format_args!("`{}` is not a valid url: `{}`", field, url)))?; + Url::parse(url).map_err(|_| { + cargo_err(&format_args!("`{}` is not a valid url: `{}`", field, url)) + })?; Ok(()) } diff --git a/src/models/team.rs b/src/models/team.rs index 1d243a21062..64fe9e173e1 100644 --- a/src/models/team.rs +++ b/src/models/team.rs @@ -2,7 +2,7 @@ use diesel::prelude::*; use crate::app::App; use crate::github::{github_api, team_url}; -use crate::util::{errors::NotFound, cargo_err, AppResult}; +use crate::util::{cargo_err, errors::NotFound, AppResult}; use oauth2::{prelude::*, AccessToken}; diff --git a/src/router.rs b/src/router.rs index 2c94ae62451..86bcce0ea81 100644 --- a/src/router.rs +++ b/src/router.rs @@ -207,7 +207,10 @@ mod tests { assert_eq!(C(|_| err(NotFound)).call(&mut req).unwrap().status.0, 404); // cargo_err errors are returned as 200 so that cargo displays this nicely on the command line - assert_eq!(C(|_| Err(cargo_err(""))).call(&mut req).unwrap().status.0, 200); + assert_eq!( + C(|_| Err(cargo_err(""))).call(&mut req).unwrap().status.0, + 200 + ); // All other error types are propogated up the middleware, eventually becoming status 500 assert!(C(|_| Err(internal(""))).call(&mut req).is_err()); From 1f290c71a0736af243852c8490141e60c8de4a40 Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Sat, 23 Nov 2019 13:37:38 -0500 Subject: [PATCH 4/5] Remove unused `internal_err` --- src/util.rs | 2 +- src/util/errors.rs | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/util.rs b/src/util.rs index c5cc33ceed0..e16471a41ca 100644 --- a/src/util.rs +++ b/src/util.rs @@ -6,7 +6,7 @@ use conduit::Response; use serde::Serialize; pub use self::errors::ChainError; -pub use self::errors::{bad_request, cargo_err, internal, internal_error, AppError, AppResult}; +pub use self::errors::{bad_request, cargo_err, internal, AppError, AppResult}; pub use self::io_util::{read_fill, read_le_u32, LimitErrorReader}; pub use self::request_helpers::*; pub use self::request_proxy::RequestProxy; diff --git a/src/util/errors.rs b/src/util/errors.rs index ca1f810355c..184b32500f1 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -285,15 +285,6 @@ impl fmt::Display for BadRequest { } } -pub fn internal_error(error: &str, detail: &str) -> Box { - Box::new(ConcreteAppError { - description: error.to_string(), - detail: Some(detail.to_string()), - cause: None, - cargo_err: false, - }) -} - pub fn internal(error: &S) -> Box { Box::new(ConcreteAppError { description: error.to_string(), From 53e5ef2080591186f4e875358327f48035fec219 Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Sat, 23 Nov 2019 14:35:52 -0500 Subject: [PATCH 5/5] Extract error `fallback_response` behavior Errors that implement their own `response` method do not need to impl any of the fallback methods. This change should help avoid adding new errors that rely on this fallback behavior. --- src/util/errors.rs | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/util/errors.rs b/src/util/errors.rs index 184b32500f1..aca5bdef5d3 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -26,8 +26,14 @@ pub trait AppError: Send + fmt::Display + fmt::Debug + 'static { None } - fn response(&self) -> Option { - if self.cargo_err() { + /// Generate an HTTP response for the error + fn response(&self) -> Option; + + /// Fallback logic for generating a cargo friendly response + /// + /// This behavior is deprecated and no new calls or impls should be added. + fn fallback_response(&self) -> Option { + if self.fallback_with_description_as_bad_200() { Some(json_response(&Bad { errors: vec![StringError { detail: self.description().to_string(), @@ -37,7 +43,13 @@ pub trait AppError: Send + fmt::Display + fmt::Debug + 'static { self.cause().and_then(AppError::response) } } - fn cargo_err(&self) -> bool { + + /// Determines if the `fallback_response` method should send the description as a status 200 + /// error to cargo, or send the cause response (if applicable). + /// + /// This is only to be used by the `fallback_response` method. If your error type impls + /// `response`, then there is no need to impl this method. + fn fallback_with_description_as_bad_200(&self) -> bool { false } @@ -75,8 +87,8 @@ impl AppError for Box { fn cause(&self) -> Option<&dyn AppError> { (**self).cause() } - fn cargo_err(&self) -> bool { - (**self).cargo_err() + fn fallback_with_description_as_bad_200(&self) -> bool { + (**self).fallback_with_description_as_bad_200() } fn response(&self) -> Option { (**self).response() @@ -152,8 +164,8 @@ impl AppError for ChainedError { fn response(&self) -> Option { self.error.response() } - fn cargo_err(&self) -> bool { - self.error.cargo_err() + fn fallback_with_description_as_bad_200(&self) -> bool { + self.error.fallback_with_description_as_bad_200() } } @@ -170,6 +182,9 @@ impl AppError for E { fn description(&self) -> &str { Error::description(self) } + fn response(&self) -> Option { + self.fallback_response() + } } impl From for Box { @@ -205,7 +220,10 @@ impl AppError for ConcreteAppError { fn cause(&self) -> Option<&dyn AppError> { self.cause.as_ref().map(|c| &**c) } - fn cargo_err(&self) -> bool { + fn response(&self) -> Option { + self.fallback_response() + } + fn fallback_with_description_as_bad_200(&self) -> bool { self.cargo_err } } @@ -364,10 +382,6 @@ impl AppError for ReadOnlyMode { response.status = (503, "Service Unavailable"); Some(response) } - - fn cargo_err(&self) -> bool { - true - } } impl fmt::Display for ReadOnlyMode { @@ -406,10 +420,6 @@ impl AppError for TooManyRequests { .insert("Retry-After".into(), vec![retry_after.to_string()]); Some(response) } - - fn cargo_err(&self) -> bool { - true - } } impl fmt::Display for TooManyRequests {