Skip to content

util/errors: Extract CustomApiError struct #7812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/controllers/metrics.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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() {
Expand Down
6 changes: 4 additions & 2 deletions src/middleware/block_traffic.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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());
}
}

Expand Down
13 changes: 10 additions & 3 deletions src/models/crate_owner_invitation.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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| {
Expand Down
28 changes: 14 additions & 14 deletions src/util/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand All @@ -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
Expand All @@ -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")
}

// =============================================================================
Expand Down
183 changes: 14 additions & 169 deletions src/util/errors/json.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use axum::response::{IntoResponse, Response};
use axum::Json;
use std::borrow::Cow;
use std::fmt;

use super::{AppError, BoxedAppError, InternalAppErrorStatic};
Expand All @@ -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;

Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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()
}
}