Auto merge of #3888 - Turbo87:503, r=pietroalbini

Cast `PoolError::UnhealthyPool` into `503 Service Unavailable` error

Resolves #1386

I'm not sure how to write a proper test for this unfortunately, but I've manually verified that this works as intended.

Author: bors

src/tests/unhealthy_database.rs

@@ -2,6 +2,7 @@ use crate::{
     builders::CrateBuilder,
     util::{MockAnonymousUser, RequestHelper, TestApp},
 };
+use http::StatusCode;
 use std::time::Duration;
 
 #[test]
@@ -64,3 +65,25 @@ fn assert_unconditional_redirects(anon: &MockAnonymousUser) {
     anon.get::<()>("/api/v1/crates/awesome-project/1.0.0/download")
         .assert_redirect_ends_with("/awesome-project/awesome-project-1.0.0.crate");
 }
+
+#[test]
+fn http_error_with_unhealthy_database() {
+    let (app, anon) = TestApp::init().with_slow_real_db_pool().empty();
+
+    let response = anon.get::<()>("/api/v1/summary");
+    assert_eq!(response.status(), StatusCode::OK);
+
+    app.db_chaosproxy().break_networking();
+
+    let response = anon.get::<()>("/api/v1/summary");
+    assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE);
+
+    app.db_chaosproxy().restore_networking();
+    app.as_inner()
+        .primary_database
+        .wait_until_healthy(Duration::from_millis(500))
+        .expect("the database did not return healthy");
+
+    let response = anon.get::<()>("/api/v1/summary");
+    assert_eq!(response.status(), StatusCode::OK);
+}

src/util/errors.rs

@@ -20,6 +20,7 @@ use std::fmt;
 use chrono::NaiveDateTime;
 use diesel::result::Error as DieselError;
 
+use crate::db::PoolError;
 use crate::util::AppResponse;
 
 mod json;
@@ -69,6 +70,11 @@ pub fn server_error<S: ToString + ?Sized>(error: &S) -> Box<dyn AppError> {
     Box::new(json::ServerError(error.to_string()))
 }
 
+/// Returns an error with status 503 and the provided description as JSON
+pub fn service_unavailable<S: ToString + ?Sized>(error: &S) -> Box<dyn AppError> {
+    Box::new(json::ServiceUnavailable(error.to_string()))
+}
+
 // =============================================================================
 // AppError trait
 
@@ -111,6 +117,10 @@ impl dyn AppError {
     }
 
     fn try_convert(err: &(dyn Error + Send + 'static)) -> Option<Box<Self>> {
+        if matches!(err.downcast_ref(), Some(PoolError::UnhealthyPool)) {
+            return Some(service_unavailable("Service unavailable"));
+        }
+
         match err.downcast_ref() {
             Some(DieselError::NotFound) => Some(not_found()),
             Some(DieselError::DatabaseError(_, info))

src/util/errors/json.rs

@@ -80,6 +80,8 @@ pub(super) struct BadRequest(pub(super) String);
 #[derive(Debug)]
 pub(super) struct ServerError(pub(super) String);
 #[derive(Debug)]
+pub(crate) struct ServiceUnavailable(pub(super) String);
+#[derive(Debug)]
 pub(crate) struct TooManyRequests {
     pub retry_after: NaiveDateTime,
 }
@@ -120,6 +122,18 @@ impl fmt::Display for ServerError {
     }
 }
 
+impl AppError for ServiceUnavailable {
+    fn response(&self) -> Option<AppResponse> {
+        Some(json_error(&self.0, StatusCode::SERVICE_UNAVAILABLE))
+    }
+}
+
+impl fmt::Display for ServiceUnavailable {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
 impl AppError for TooManyRequests {
     fn response(&self) -> Option<AppResponse> {
         use std::convert::TryInto;