Merge pull request #7812 from Turbo87/custom-api-error

util/errors: Extract `CustomApiError` struct

Author: Turbo87

src/controllers/metrics.rs

@@ -1,5 +1,5 @@
 use crate::controllers::frontend_prelude::*;
-use crate::util::errors::{forbidden, not_found, MetricsDisabled};
+use crate::util::errors::{custom, forbidden, not_found};
 use prometheus::TextEncoder;
 
 /// Handles the `GET /api/private/metrics/:kind` endpoint.
@@ -17,7 +17,8 @@ pub async fn prometheus(app: AppState, Path(kind): Path<String>, req: Parts) ->
     } else {
         // To avoid accidentally leaking metrics if the environment variable is not set, prevent
         // access to any metrics endpoint if the authorization token is not configured.
-        return Err(Box::new(MetricsDisabled));
+        let detail = "Metrics are disabled on this crates.io instance";
+        return Err(custom(StatusCode::NOT_FOUND, detail));
     }
 
     let metrics = spawn_blocking(move || match kind.as_str() {

src/middleware/block_traffic.rs

@@ -1,7 +1,7 @@
 use crate::app::AppState;
 use crate::middleware::log_request::RequestLogExt;
 use crate::middleware::real_ip::RealIp;
-use crate::util::errors::RouteBlocked;
+use crate::util::errors::custom;
 use axum::extract::{Extension, MatchedPath, Request};
 use axum::middleware::Next;
 use axum::response::{IntoResponse, Response};
@@ -87,7 +87,9 @@ fn rejection_response_from(state: &AppState, headers: &HeaderMap) -> Response {
 pub fn block_routes(matched_path: Option<&MatchedPath>, state: &AppState) -> Result<(), Response> {
     if let Some(matched_path) = matched_path {
         if state.config.blocked_routes.contains(matched_path.as_str()) {
-            return Err(RouteBlocked.into_response());
+            let body = "This route is temporarily blocked. See https://status.crates.io.";
+            let error = custom(StatusCode::SERVICE_UNAVAILABLE, body);
+            return Err(error.into_response());
         }
     }
 

src/models/crate_owner_invitation.rs

@@ -1,11 +1,12 @@
 use chrono::{NaiveDateTime, Utc};
 use diesel::prelude::*;
+use http::StatusCode;
 use secrecy::SecretString;
 
 use crate::config;
 use crate::models::{CrateOwner, OwnerKind};
 use crate::schema::{crate_owner_invitations, crate_owners, crates};
-use crate::util::errors::{AppResult, OwnershipInvitationExpired};
+use crate::util::errors::{custom, AppResult};
 
 #[derive(Debug)]
 pub enum NewCrateOwnerInvitationOutcome {
@@ -97,11 +98,17 @@ impl CrateOwnerInvitation {
 
     pub fn accept(self, conn: &mut PgConnection, config: &config::Server) -> AppResult<()> {
         if self.is_expired(config) {
-            let crate_name = crates::table
+            let crate_name: String = crates::table
                 .find(self.crate_id)
                 .select(crates::name)
                 .first(conn)?;
-            return Err(Box::new(OwnershipInvitationExpired { crate_name }));
+
+            let detail = format!(
+                "The invitation to become an owner of the {crate_name} crate expired. \
+                Please reach out to an owner of the crate to request a new invitation.",
+            );
+
+            return Err(custom(StatusCode::GONE, detail));
         }
 
         conn.transaction(|conn| {

src/util/errors.rs

@@ -32,10 +32,7 @@ mod json;
 use crate::email::EmailError;
 use crates_io_github::GitHubError;
 pub use json::TOKEN_FORMAT_ERROR;
-pub(crate) use json::{
-    InsecurelyGeneratedTokenRevoked, MetricsDisabled, OwnershipInvitationExpired, ReadOnlyMode,
-    RouteBlocked, TooManyRequests,
-};
+pub(crate) use json::{custom, InsecurelyGeneratedTokenRevoked, ReadOnlyMode, TooManyRequests};
 
 pub type BoxedAppError = Box<dyn AppError>;
 
@@ -45,7 +42,7 @@ pub type BoxedAppError = Box<dyn AppError>;
 /// endpoints, use helpers like `bad_request` or `server_error` which set a
 /// correct status code.
 pub fn cargo_err<S: ToString>(error: S) -> BoxedAppError {
-    Box::new(json::Ok(error.to_string()))
+    custom(StatusCode::OK, error.to_string())
 }
 
 // The following are intended to be used for errors being sent back to the Ember
@@ -55,32 +52,35 @@ pub fn cargo_err<S: ToString>(error: S) -> BoxedAppError {
 
 /// Return an error with status 400 and the provided description as JSON
 pub fn bad_request<S: ToString>(error: S) -> BoxedAppError {
-    Box::new(json::BadRequest(error.to_string()))
+    custom(StatusCode::BAD_REQUEST, error.to_string())
 }
 
 pub fn account_locked(reason: &str, until: Option<NaiveDateTime>) -> BoxedAppError {
-    Box::new(json::AccountLocked {
-        reason: reason.to_string(),
-        until,
-    })
+    let detail = until
+        .map(|until| until.format("%Y-%m-%d at %H:%M:%S UTC"))
+        .map(|until| format!("This account is locked until {until}. Reason: {reason}"))
+        .unwrap_or_else(|| format!("This account is indefinitely locked. Reason: {reason}"));
+
+    custom(StatusCode::FORBIDDEN, detail)
 }
 
 pub fn forbidden() -> BoxedAppError {
-    Box::new(json::Forbidden)
+    let detail = "must be logged in to perform that action";
+    custom(StatusCode::FORBIDDEN, detail)
 }
 
 pub fn not_found() -> BoxedAppError {
-    Box::new(json::NotFound)
+    custom(StatusCode::NOT_FOUND, "Not Found")
 }
 
 /// Returns an error with status 500 and the provided description as JSON
 pub fn server_error<S: ToString>(error: S) -> BoxedAppError {
-    Box::new(json::ServerError(error.to_string()))
+    custom(StatusCode::INTERNAL_SERVER_ERROR, error.to_string())
 }
 
 /// Returns an error with status 503 and the provided description as JSON
 pub fn service_unavailable() -> BoxedAppError {
-    Box::new(json::ServiceUnavailable)
+    custom(StatusCode::SERVICE_UNAVAILABLE, "Service unavailable")
 }
 
 // =============================================================================

src/util/errors/json.rs

@@ -1,5 +1,6 @@
 use axum::response::{IntoResponse, Response};
 use axum::Json;
+use std::borrow::Cow;
 use std::fmt;
 
 use super::{AppError, BoxedAppError, InternalAppErrorStatic};
@@ -16,37 +17,6 @@ fn json_error(detail: &str, status: StatusCode) -> Response {
 
 // The following structs are empty and do not provide a custom message to the user
 
-#[derive(Debug)]
-pub(crate) struct NotFound;
-
-impl AppError for NotFound {
-    fn response(&self) -> Response {
-        json_error("Not Found", StatusCode::NOT_FOUND)
-    }
-}
-
-impl fmt::Display for NotFound {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        "Not Found".fmt(f)
-    }
-}
-
-#[derive(Debug)]
-pub(super) struct Forbidden;
-
-impl AppError for Forbidden {
-    fn response(&self) -> Response {
-        let detail = "must be logged in to perform that action";
-        json_error(detail, StatusCode::FORBIDDEN)
-    }
-}
-
-impl fmt::Display for Forbidden {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        "must be logged in to perform that action".fmt(f)
-    }
-}
-
 #[derive(Debug)]
 pub(crate) struct ReadOnlyMode;
 
@@ -66,63 +36,28 @@ impl fmt::Display for ReadOnlyMode {
 
 // The following structs wrap owned data and provide a custom message to the user
 
-#[derive(Debug)]
-pub(super) struct Ok(pub(super) String);
-
-impl AppError for Ok {
-    fn response(&self) -> Response {
-        json_error(&self.0, StatusCode::OK)
-    }
+pub fn custom(status: StatusCode, detail: impl Into<Cow<'static, str>>) -> BoxedAppError {
+    Box::new(CustomApiError {
+        status,
+        detail: detail.into(),
+    })
 }
 
-impl fmt::Display for Ok {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.fmt(f)
-    }
-}
-
-#[derive(Debug)]
-pub(super) struct BadRequest(pub(super) String);
-
-impl AppError for BadRequest {
-    fn response(&self) -> Response {
-        json_error(&self.0, StatusCode::BAD_REQUEST)
-    }
+#[derive(Debug, Clone)]
+pub struct CustomApiError {
+    status: StatusCode,
+    detail: Cow<'static, str>,
 }
 
-impl fmt::Display for BadRequest {
+impl fmt::Display for CustomApiError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.fmt(f)
+        self.detail.fmt(f)
     }
 }
 
-#[derive(Debug)]
-pub(super) struct ServerError(pub(super) String);
-
-impl AppError for ServerError {
+impl AppError for CustomApiError {
     fn response(&self) -> Response {
-        json_error(&self.0, StatusCode::INTERNAL_SERVER_ERROR)
-    }
-}
-
-impl fmt::Display for ServerError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.fmt(f)
-    }
-}
-
-#[derive(Debug)]
-pub(crate) struct ServiceUnavailable;
-
-impl AppError for ServiceUnavailable {
-    fn response(&self) -> Response {
-        json_error("Service unavailable", StatusCode::SERVICE_UNAVAILABLE)
-    }
-}
-
-impl fmt::Display for ServiceUnavailable {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        "Service unavailable".fmt(f)
+        json_error(&self.detail, self.status)
     }
 }
 
@@ -198,93 +133,3 @@ impl fmt::Display for InsecurelyGeneratedTokenRevoked {
         Result::Ok(())
     }
 }
-
-#[derive(Debug)]
-pub(super) struct AccountLocked {
-    pub(super) reason: String,
-    pub(super) until: Option<NaiveDateTime>,
-}
-
-impl AppError for AccountLocked {
-    fn response(&self) -> Response {
-        json_error(&self.to_string(), StatusCode::FORBIDDEN)
-    }
-}
-
-impl fmt::Display for AccountLocked {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if let Some(until) = self.until {
-            let until = until.format("%Y-%m-%d at %H:%M:%S UTC");
-            write!(
-                f,
-                "This account is locked until {}. Reason: {}",
-                until, self.reason
-            )
-        } else {
-            write!(
-                f,
-                "This account is indefinitely locked. Reason: {}",
-                self.reason
-            )
-        }
-    }
-}
-
-#[derive(Debug)]
-pub(crate) struct OwnershipInvitationExpired {
-    pub(crate) crate_name: String,
-}
-
-impl AppError for OwnershipInvitationExpired {
-    fn response(&self) -> Response {
-        json_error(&self.to_string(), StatusCode::GONE)
-    }
-}
-
-impl fmt::Display for OwnershipInvitationExpired {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "The invitation to become an owner of the {} crate expired. \
-             Please reach out to an owner of the crate to request a new invitation.",
-            self.crate_name
-        )
-    }
-}
-
-#[derive(Debug)]
-pub(crate) struct MetricsDisabled;
-
-impl AppError for MetricsDisabled {
-    fn response(&self) -> Response {
-        json_error(&self.to_string(), StatusCode::NOT_FOUND)
-    }
-}
-
-impl fmt::Display for MetricsDisabled {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str("Metrics are disabled on this crates.io instance")
-    }
-}
-
-#[derive(Debug)]
-pub(crate) struct RouteBlocked;
-
-impl AppError for RouteBlocked {
-    fn response(&self) -> Response {
-        json_error(&self.to_string(), StatusCode::SERVICE_UNAVAILABLE)
-    }
-}
-
-impl fmt::Display for RouteBlocked {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str("This route is temporarily blocked. See https://status.crates.io.")
-    }
-}
-
-impl IntoResponse for RouteBlocked {
-    fn into_response(self) -> Response {
-        let body = Json(json!({ "errors": [{ "detail": self.to_string() }] }));
-        (StatusCode::SERVICE_UNAVAILABLE, body).into_response()
-    }
-}