From cb4f0cebe522bffae6564efa6f3f31e937a456f2 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 16 Nov 2024 20:16:47 +0100 Subject: [PATCH 1/4] util/diesel: Extract `is_read_only_error()` fn --- src/util/diesel.rs | 5 +++++ src/util/errors.rs | 7 ++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/util/diesel.rs b/src/util/diesel.rs index 743633dbf2f..aa9e587cfd4 100644 --- a/src/util/diesel.rs +++ b/src/util/diesel.rs @@ -1,10 +1,15 @@ use diesel::connection::LoadConnection; use diesel::pg::Pg; +use diesel::result::Error; pub trait Conn: LoadConnection {} impl Conn for T where T: LoadConnection {} +pub fn is_read_only_error(error: &Error) -> bool { + matches!(error, Error::DatabaseError(_, info) if info.message().ends_with("read-only transaction")) +} + pub mod prelude { //! Inline diesel prelude pub use diesel::associations::{Associations, GroupedBy, Identifiable}; diff --git a/src/util/errors.rs b/src/util/errors.rs index 9dc05fb8894..8d70ebdb237 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -30,6 +30,7 @@ use crate::middleware::log_request::ErrorField; mod json; use crate::email::EmailError; +use crate::util::diesel::is_read_only_error; use crates_io_github::GitHubError; pub use json::TOKEN_FORMAT_ERROR; pub(crate) use json::{custom, InsecurelyGeneratedTokenRevoked, ReadOnlyMode, TooManyRequests}; @@ -145,11 +146,7 @@ impl From for BoxedAppError { fn from(err: DieselError) -> BoxedAppError { match err { DieselError::NotFound => not_found(), - DieselError::DatabaseError(_, info) - if info.message().ends_with("read-only transaction") => - { - Box::new(ReadOnlyMode) - } + e if is_read_only_error(&e) => Box::new(ReadOnlyMode), DieselError::DatabaseError(DatabaseErrorKind::ClosedConnection, _) => { service_unavailable() } From 490deef45c7961e5fcdd59125ef8f2656b7b08fc Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 16 Nov 2024 20:18:40 +0100 Subject: [PATCH 2/4] controllers/user/session: Simplify `save_user_to_database()` error handling --- src/controllers/user/session.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/controllers/user/session.rs b/src/controllers/user/session.rs index 1d317e07f91..170af3c9139 100644 --- a/src/controllers/user/session.rs +++ b/src/controllers/user/session.rs @@ -15,8 +15,8 @@ use crate::middleware::session::SessionExtension; use crate::models::{NewUser, User}; use crate::schema::users; use crate::tasks::spawn_blocking; -use crate::util::diesel::Conn; -use crate::util::errors::{bad_request, server_error, AppResult, BoxedAppError, ReadOnlyMode}; +use crate::util::diesel::{is_read_only_error, Conn}; +use crate::util::errors::{bad_request, server_error, AppResult}; use crate::views::EncodableMe; use crates_io_github::GithubUser; @@ -145,11 +145,10 @@ fn save_user_to_database( access_token, ) .create_or_update(user.email.as_deref(), emails, conn) - .map_err(Into::into) - .or_else(|e: BoxedAppError| { + .or_else(|e| { // If we're in read only mode, we can't update their details // just look for an existing user - if e.is::() { + if is_read_only_error(&e) { users::table .filter(users::gh_id.eq(user.id)) .first(conn) @@ -159,6 +158,7 @@ fn save_user_to_database( Err(e) } }) + .map_err(Into::into) } /// Handles the `DELETE /api/private/session` route. From a14017596086f795696d1c702ed6ea67bd0e2d23 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 16 Nov 2024 20:21:29 +0100 Subject: [PATCH 3/4] util/errors: Replace `ReadOnlyMode` with `custom()` There is no need for a dedicated error struct anymore. --- src/util/errors.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/errors.rs b/src/util/errors.rs index 8d70ebdb237..92bf00a2681 100644 --- a/src/util/errors.rs +++ b/src/util/errors.rs @@ -33,7 +33,7 @@ use crate::email::EmailError; use crate::util::diesel::is_read_only_error; use crates_io_github::GitHubError; pub use json::TOKEN_FORMAT_ERROR; -pub(crate) use json::{custom, InsecurelyGeneratedTokenRevoked, ReadOnlyMode, TooManyRequests}; +pub(crate) use json::{custom, InsecurelyGeneratedTokenRevoked, TooManyRequests}; pub type BoxedAppError = Box; @@ -146,7 +146,10 @@ impl From for BoxedAppError { fn from(err: DieselError) -> BoxedAppError { match err { DieselError::NotFound => not_found(), - e if is_read_only_error(&e) => Box::new(ReadOnlyMode), + e if is_read_only_error(&e) => { + let detail = "crates.io is currently in read-only mode for maintenance. Please try again later."; + custom(StatusCode::SERVICE_UNAVAILABLE, detail) + } DieselError::DatabaseError(DatabaseErrorKind::ClosedConnection, _) => { service_unavailable() } From f8ee4b60d787268fc6cc828d8e1b64d185952a9d Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 16 Nov 2024 20:21:51 +0100 Subject: [PATCH 4/4] util/errors/json: Remove obsolete `ReadOnlyMode` struct This is not used anywhere anymore. --- src/util/errors/json.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/util/errors/json.rs b/src/util/errors/json.rs index 78e83c92c20..6ae0c3ce3fa 100644 --- a/src/util/errors/json.rs +++ b/src/util/errors/json.rs @@ -17,25 +17,6 @@ fn json_error(detail: &str, status: StatusCode) -> Response { (status, json).into_response() } -// The following structs are empty and do not provide a custom message to the user - -#[derive(Debug)] -pub(crate) struct ReadOnlyMode; - -impl AppError for ReadOnlyMode { - fn response(&self) -> Response { - let detail = "crates.io is currently in read-only mode for maintenance. \ - Please try again later."; - json_error(detail, StatusCode::SERVICE_UNAVAILABLE) - } -} - -impl fmt::Display for ReadOnlyMode { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - "Tried to write in read only mode".fmt(f) - } -} - // The following structs wrap owned data and provide a custom message to the user pub fn custom(status: StatusCode, detail: impl Into>) -> BoxedAppError {