refactor: errors

backends/src/client/permit.rs

@@ -32,7 +32,11 @@ use permit_pdp_client_rs::{
     models::{AuthorizationQuery, Resource, User, UserPermissionsQuery, UserPermissionsResult},
 };
 use serde::{Deserialize, Serialize};
-use shuttle_common::{claims::AccountTier, models::organization};
+use shuttle_common::{
+    claims::AccountTier,
+    models::{error::ApiError, organization},
+};
+use tracing::error;
 
 #[async_trait]
 pub trait PermissionsDal {
@@ -916,3 +920,23 @@ impl<T: Debug> From<PermitPDPClientError<T>> for Error {
         }
     }
 }
+impl From<Error> for ApiError {
+    fn from(error: Error) -> Self {
+        match error {
+            Error::ResponseError(value) => ApiError {
+                message: value.content.to_string(),
+                status_code: value.status.into(),
+            },
+            err => {
+                error!(
+                    error = &err as &dyn std::error::Error,
+                    "Internal error while talking to service"
+                );
+                ApiError {
+                    message: "Our server was unable to handle your request. A ticket should be created for us to fix this.".to_string(),
+                    status_code: StatusCode::INTERNAL_SERVER_ERROR.into(),
+                }
+            }
+        }
+    }
+}

common/src/models/deployment.rs

@@ -63,7 +63,7 @@ impl State {
 }
 
 pub fn get_deployments_table(
-    deployments: &Vec<Response>,
+    deployments: &[Response],
     service_name: &str,
     page: u32,
     raw: bool,

common/src/models/error.rs

@@ -9,6 +9,8 @@ use tracing::{error, warn};
 #[cfg(feature = "axum")]
 impl axum::response::IntoResponse for ApiError {
     fn into_response(self) -> axum::response::Response {
+        warn!("{}", self.message);
+
         (self.status(), axum::Json(self)).into_response()
     }
 }
@@ -20,6 +22,27 @@ pub struct ApiError {
 }
 
 impl ApiError {
+    pub fn internal(message: &str) -> Self {
+        Self {
+            message: message.to_string(),
+            status_code: StatusCode::INTERNAL_SERVER_ERROR.as_u16(),
+        }
+    }
+
+    pub fn unavailable(error: impl std::error::Error) -> Self {
+        Self {
+            message: error.to_string(),
+            status_code: StatusCode::SERVICE_UNAVAILABLE.as_u16(),
+        }
+    }
+
+    fn bad_request(error: impl std::error::Error) -> Self {
+        Self {
+            message: error.to_string(),
+            status_code: StatusCode::BAD_REQUEST.as_u16(),
+        }
+    }
+
     pub fn status(&self) -> StatusCode {
         StatusCode::from_u16(self.status_code).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR)
     }
@@ -38,151 +61,9 @@ impl Display for ApiError {
 
 impl std::error::Error for ApiError {}
 
-#[derive(Debug, Clone, PartialEq, Error)]
-pub enum ErrorKind {
-    #[error("Request is missing a key")]
-    KeyMissing,
-    #[error("The 'Host' header is invalid")]
-    BadHost,
-    #[error("Request has an invalid key")]
-    KeyMalformed,
-    #[error("Unauthorized")]
-    Unauthorized,
-    #[error("Forbidden")]
-    Forbidden,
-    #[error("User not found")]
-    UserNotFound,
-    #[error("User already exists")]
-    UserAlreadyExists,
-    #[error("Project '{0}' not found. Make sure you are the owner of this project name. Run `cargo shuttle project start` to create a new project.")]
-    ProjectNotFound(String),
-    #[error("{0:?}")]
-    InvalidProjectName(InvalidProjectName),
-    #[error("A project with the same name already exists")]
-    ProjectAlreadyExists,
-    /// Contains a message describing a running state of the project.
-    /// Used if the project already exists but is owned
-    /// by the caller, which means they can modify the project.
-    #[error("{0}")]
-    OwnProjectAlreadyExists(String),
-    // "not ready" is matched against in cargo-shuttle for giving further instructions on project deletion
-    #[error("Project not ready. Try running `cargo shuttle project restart`.")]
-    ProjectNotReady,
-    #[error("Project returned invalid response")]
-    ProjectUnavailable,
-    #[error("You cannot create more projects. Delete some projects first.")]
-    TooManyProjects,
-    #[error("Could not automatically delete the following resources: {0:?}. Please reach out to Shuttle support for help.")]
-    ProjectHasResources(Vec<String>),
-    #[error("Could not automatically stop the running deployment for the project. Please reach out to Shuttle support for help.")]
-    ProjectHasRunningDeployment,
-    #[error("Project currently has a deployment that is busy building. Use `cargo shuttle deployment list` to see it and wait for it to finish")]
-    ProjectHasBuildingDeployment,
-    #[error("Tried to get project into a ready state for deletion but failed. Please reach out to Shuttle support for help.")]
-    ProjectCorrupted,
-    #[error("Custom domain not found")]
-    CustomDomainNotFound,
-    #[error("Invalid custom domain")]
-    InvalidCustomDomain,
-    #[error("Custom domain already in use")]
-    CustomDomainAlreadyExists,
-    #[error("The requested operation is invalid")]
-    InvalidOperation,
-    #[error("Internal server error")]
-    Internal,
-    #[error("Service not ready")]
-    NotReady,
-    #[error("We're experiencing a high workload right now, please try again in a little bit")]
-    ServiceUnavailable,
-    #[error("Deleting project failed")]
-    DeleteProjectFailed,
-    #[error("Our server is at capacity and cannot serve your request at this time. Please try again in a few minutes.")]
-    CapacityLimit,
-    #[error("{0:?}")]
-    InvalidOrganizationName(InvalidOrganizationName),
-}
-
-impl From<ErrorKind> for ApiError {
-    fn from(kind: ErrorKind) -> Self {
-        let status = match kind {
-            ErrorKind::Internal => StatusCode::INTERNAL_SERVER_ERROR,
-            ErrorKind::KeyMissing => StatusCode::UNAUTHORIZED,
-            ErrorKind::ServiceUnavailable => StatusCode::SERVICE_UNAVAILABLE,
-            ErrorKind::KeyMalformed => StatusCode::BAD_REQUEST,
-            ErrorKind::BadHost => StatusCode::BAD_REQUEST,
-            ErrorKind::UserNotFound => StatusCode::NOT_FOUND,
-            ErrorKind::UserAlreadyExists => StatusCode::BAD_REQUEST,
-            ErrorKind::ProjectNotFound(_) => StatusCode::NOT_FOUND,
-            ErrorKind::ProjectNotReady => StatusCode::SERVICE_UNAVAILABLE,
-            ErrorKind::ProjectUnavailable => StatusCode::BAD_GATEWAY,
-            ErrorKind::TooManyProjects => StatusCode::FORBIDDEN,
-            ErrorKind::ProjectHasRunningDeployment => StatusCode::INTERNAL_SERVER_ERROR,
-            ErrorKind::ProjectHasBuildingDeployment => StatusCode::BAD_REQUEST,
-            ErrorKind::ProjectCorrupted => StatusCode::BAD_REQUEST,
-            ErrorKind::ProjectHasResources(_) => StatusCode::INTERNAL_SERVER_ERROR,
-            ErrorKind::InvalidProjectName(_) => StatusCode::BAD_REQUEST,
-            ErrorKind::InvalidOperation => StatusCode::BAD_REQUEST,
-            ErrorKind::ProjectAlreadyExists => StatusCode::BAD_REQUEST,
-            ErrorKind::OwnProjectAlreadyExists(_) => StatusCode::BAD_REQUEST,
-            ErrorKind::InvalidCustomDomain => StatusCode::BAD_REQUEST,
-            ErrorKind::CustomDomainNotFound => StatusCode::NOT_FOUND,
-            ErrorKind::CustomDomainAlreadyExists => StatusCode::BAD_REQUEST,
-            ErrorKind::Unauthorized => StatusCode::UNAUTHORIZED,
-            ErrorKind::Forbidden => StatusCode::FORBIDDEN,
-            ErrorKind::NotReady => StatusCode::INTERNAL_SERVER_ERROR,
-            ErrorKind::DeleteProjectFailed => StatusCode::INTERNAL_SERVER_ERROR,
-            ErrorKind::CapacityLimit => StatusCode::SERVICE_UNAVAILABLE,
-            ErrorKind::InvalidOrganizationName(_) => StatusCode::BAD_REQUEST,
-        };
-        Self {
-            message: kind.to_string(),
-            status_code: status.as_u16(),
-        }
-    }
-}
-
-// Used as a fallback when an API response did not contain a serialized ApiError
-impl From<StatusCode> for ApiError {
-    fn from(code: StatusCode) -> Self {
-        let message = match code {
-            StatusCode::OK | StatusCode::ACCEPTED | StatusCode::FOUND | StatusCode::SWITCHING_PROTOCOLS => {
-                unreachable!("we should not have an API error with a successful status code")
-            }
-            StatusCode::FORBIDDEN => "This request is not allowed",
-            StatusCode::UNAUTHORIZED => {
-                "we were unable to authorize your request. Check that your API key is set correctly. Use `cargo shuttle login` to set it."
-            },
-            StatusCode::INTERNAL_SERVER_ERROR => "Our server was unable to handle your request. A ticket should be created for us to fix this.",
-            StatusCode::SERVICE_UNAVAILABLE => "We're experiencing a high workload right now, please try again in a little bit",
-            StatusCode::BAD_REQUEST => {
-                warn!("responding to a BAD_REQUEST request with an unhelpful message. Use ErrorKind instead");
-                "This request is invalid"
-            },
-            StatusCode::NOT_FOUND => {
-                warn!("responding to a NOT_FOUND request with an unhelpful message. Use ErrorKind instead");
-                "We don't serve this resource"
-            },
-            StatusCode::BAD_GATEWAY => {
-                warn!("got a bad response from the gateway");
-                // Gateway's default response when a request handler panicks is a 502 with some HTML.
-                "Response from gateway is invalid. Please create a ticket to report this"
-            },
-            _ => {
-                error!(%code, "got an unexpected status code");
-                "An unexpected error occurred. Please create a ticket to report this"
-            },
-        };
-
-        Self {
-            message: message.to_string(),
-            status_code: code.as_u16(),
-        }
-    }
-}
-
 // Note: The string "Invalid project name" is used by cargo-shuttle to determine what type of error was returned.
 // Changing it is breaking.
-#[derive(Debug, Clone, PartialEq, thiserror::Error)]
+#[derive(Debug, Clone, PartialEq, Error)]
 #[error(
     "Invalid project name. Project names must:
     1. only contain lowercase alphanumeric characters or dashes `-`.
@@ -194,6 +75,89 @@ impl From<StatusCode> for ApiError {
 )]
 pub struct InvalidProjectName;
 
-#[derive(Debug, Clone, PartialEq, thiserror::Error)]
+impl From<InvalidProjectName> for ApiError {
+    fn from(err: InvalidProjectName) -> Self {
+        Self::bad_request(err)
+    }
+}
+
+#[derive(Debug, Clone, PartialEq, Error)]
 #[error("Invalid organization name. Must not be more than 30 characters long.")]
 pub struct InvalidOrganizationName;
+
+impl From<InvalidOrganizationName> for ApiError {
+    fn from(err: InvalidOrganizationName) -> Self {
+        Self::bad_request(err)
+    }
+}
+
+#[derive(Debug, Error)]
+#[error("Project is not ready. Try to restart it")]
+pub struct ProjectNotReady;
+
+#[derive(Debug, Error)]
+#[error("Project is running but is not responding correctly. Try to restart it")]
+pub struct ProjectUnavailable;
+
+#[derive(Debug, Error)]
+#[error("Project '{0}' not found. Make sure you are the owner of this project name. Run `cargo shuttle project start` to create a new project.")]
+pub struct ProjectNotFound(pub String);
+
+impl From<ProjectNotFound> for ApiError {
+    fn from(err: ProjectNotFound) -> Self {
+        Self {
+            message: err.to_string(),
+            status_code: StatusCode::NOT_FOUND.as_u16(),
+        }
+    }
+}
+
+#[derive(Debug, Error)]
+#[error("Could not automatically delete the following resources: {0:?}. Please reach out to Shuttle support for help.")]
+pub struct ProjectHasResources(pub Vec<String>);
+
+impl From<ProjectHasResources> for ApiError {
+    fn from(err: ProjectHasResources) -> Self {
+        Self::bad_request(err)
+    }
+}
+
+#[derive(Debug, Error)]
+#[error("Could not automatically stop the running deployment for the project. Please reach out to Shuttle support for help.")]
+pub struct ProjectHasRunningDeployment;
+
+impl From<ProjectHasRunningDeployment> for ApiError {
+    fn from(err: ProjectHasRunningDeployment) -> Self {
+        Self::bad_request(err)
+    }
+}
+
+#[derive(Debug, Error)]
+#[error("Project currently has a deployment that is busy building. Use `cargo shuttle deployment list` to see it and wait for it to finish")]
+pub struct ProjectHasBuildingDeployment;
+
+impl From<ProjectHasBuildingDeployment> for ApiError {
+    fn from(err: ProjectHasBuildingDeployment) -> Self {
+        Self::bad_request(err)
+    }
+}
+
+#[derive(Debug, Error)]
+#[error("Tried to get project into a ready state for deletion but failed. Please reach out to Shuttle support for help.")]
+pub struct ProjectCorrupted;
+
+impl From<ProjectCorrupted> for ApiError {
+    fn from(err: ProjectCorrupted) -> Self {
+        Self::bad_request(err)
+    }
+}
+
+#[derive(Debug, Error)]
+#[error("Invalid custom domain")]
+pub struct InvalidCustomDomain;
+
+impl From<InvalidCustomDomain> for ApiError {
+    fn from(err: InvalidCustomDomain) -> Self {
+        Self::bad_request(err)
+    }
+}

common/src/models/mod.rs

@@ -41,8 +41,10 @@ impl ToJson for reqwest::Response {
             let res: error::ApiError = match serde_json::from_slice(&full) {
                 Ok(res) => res,
                 _ => {
-                    trace!("getting error from status code");
-                    status_code.into()
+                    error::ApiError {
+                        message: "Did not understand the response from the server. Please log a ticket for this".to_string(),
+                        status_code: status_code.as_u16(),
+                    }
                 }
             };
 

common/src/models/project.rs

@@ -164,12 +164,7 @@ pub struct Config {
     pub idle_minutes: u64,
 }
 
-pub fn get_projects_table(
-    projects: &Vec<Response>,
-    page: u32,
-    raw: bool,
-    page_hint: bool,
-) -> String {
+pub fn get_projects_table(projects: &[Response], page: u32, raw: bool, page_hint: bool) -> String {
     if projects.is_empty() {
         // The page starts at 1 in the CLI.
         let mut s = if page <= 1 {

common/src/models/resource.rs

@@ -14,7 +14,7 @@ use crate::{
 };
 
 pub fn get_resource_tables(
-    resources: &Vec<Response>,
+    resources: &[Response],
     service_name: &str,
     raw: bool,
     show_secrets: bool,

gateway/Cargo.toml

@@ -41,6 +41,7 @@ serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
 sqlx = { workspace = true, features = ["sqlite", "json", "migrate"] }
 strum = { workspace = true }
+thiserror = { workspace = true }
 tokio = { workspace = true, features = ["full"] }
 tonic = { workspace = true }
 tower = { workspace = true, features = ["steer"] }