feat(deployer): split up deployer error enum (#339)

* feat(deployer): split up deployer error enum * fix: unsafe unwrap
shuttle-hq · Sep 16, 2022 · d4bf86c · d4bf86c
1 parent 4e00a03
commit d4bf86c
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 52 deletions.
diff --git a/deployer/src/error.rs b/deployer/src/error.rs
@@ -1,12 +1,6 @@
 use std::error::Error as StdError;
 use std::io;
 
-use axum::http::{header, HeaderValue, StatusCode};
-use axum::response::{IntoResponse, Response};
-use axum::Json;
-
-use serde::{ser::SerializeMap, Serialize};
-use serde_json::json;
 use shuttle_service::loader::LoaderError;
 
 use cargo::util::errors::CargoTestError;
@@ -31,34 +25,6 @@ pub enum Error {
     SecretsParse(#[from] toml::de::Error),
     #[error("Failed to set secrets: {0}")]
     SecretsSet(#[source] Box<dyn StdError + Send>),
-    #[error("Failed to parse secrets: {0}")]
-    Persistence(#[from] crate::persistence::PersistenceError),
-}
-
-impl Serialize for Error {
-    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        let mut map = serializer.serialize_map(Some(2))?;
-        map.serialize_entry("type", &format!("{:?}", self))?;
-        map.serialize_entry("msg", &self.source().unwrap().to_string())?;
-        map.end()
-    }
-}
-
-impl IntoResponse for Error {
-    fn into_response(self) -> Response {
-        (
-            StatusCode::INTERNAL_SERVER_ERROR,
-            [(
-                header::CONTENT_TYPE,
-                HeaderValue::from_static("application/json"),
-            )],
-            Json(json!({ "message": self })),
-        )
-            .into_response()
-    }
 }
 
 pub type Result<T> = std::result::Result<T, Error>;
diff --git a/deployer/src/handlers/deployment.rs b/deployer/src/handlers/deployment.rs
@@ -1,9 +1,6 @@
 use std::sync::Arc;
 
-use crate::{
-    error::Result,
-    persistence::{Deployment, State},
-};
+use crate::persistence::{Deployment, State};
 use async_trait::async_trait;
 use axum::{
     extract::{FromRequest, Path},
@@ -15,7 +12,6 @@ use serde::Serialize;
 use uuid::Uuid;
 
 use super::user::UserGuard;
-
 /// Guard used to make sure a request has a valid api key set on the Basic Auth and that it owns a service's deployment
 ///
 /// *Note*
@@ -94,7 +90,7 @@ pub trait DeploymentAuthorizer: Sync + Send {
         &self,
         api_key: &str,
         deployment_id: &Uuid,
-    ) -> Result<Option<Deployment>>;
+    ) -> super::Result<Option<Deployment>>;
 }
 
 impl From<Deployment> for DeploymentGuard {

diff --git a/deployer/src/handlers/error.rs b/deployer/src/handlers/error.rs
@@ -0,0 +1,43 @@
+use std::error::Error as StdError;
+
+use axum::http::{header, HeaderValue, StatusCode};
+use axum::response::{IntoResponse, Response};
+use axum::Json;
+
+use serde::{ser::SerializeMap, Serialize};
+use serde_json::json;
+
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    #[error("Persistence failure: {0}")]
+    Persistence(#[from] crate::persistence::PersistenceError),
+}
+
+impl Serialize for Error {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        let mut map = serializer.serialize_map(Some(2))?;
+        map.serialize_entry("type", &format!("{:?}", self))?;
+        // use the error source if available, if not use display implementation
+        map.serialize_entry("msg", &self.source().unwrap_or(self).to_string())?;
+        map.end()
+    }
+}
+
+impl IntoResponse for Error {
+    fn into_response(self) -> Response {
+        (
+            StatusCode::INTERNAL_SERVER_ERROR,
+            [(
+                header::CONTENT_TYPE,
+                HeaderValue::from_static("application/json"),
+            )],
+            Json(json!({ "message": self })),
+        )
+            .into_response()
+    }
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
diff --git a/deployer/src/handlers/mod.rs b/deployer/src/handlers/mod.rs
@@ -1,5 +1,6 @@
 mod admin;
 mod deployment;
+mod error;
 mod service;
 mod user;
 
@@ -18,7 +19,6 @@ use tracing::{debug, debug_span, error, field, Span};
 use uuid::Uuid;
 
 use crate::deployment::{DeploymentManager, Queued};
-use crate::error::{Error, Result};
 use crate::persistence::{self, Deployment, Log, Persistence, SecretGetter, State};
 
 use std::collections::HashMap;
@@ -32,6 +32,7 @@ pub use self::service::ServiceAuthorizer;
 use self::service::ServiceGuard;
 use self::user::UserGuard;
 pub use self::user::UserValidator;
+pub use {self::error::Error, self::error::Result};
 
 pub fn make_router(
     persistence: Persistence,
@@ -185,7 +186,7 @@ async fn post_service(
         id,
         service_name: service.name,
         service_id: service.id,
-        data_stream: Box::pin(stream.map_err(Error::Streaming)),
+        data_stream: Box::pin(stream.map_err(crate::error::Error::Streaming)),
         will_run_tests: !params.contains_key("no-test"),
     };
 

diff --git a/deployer/src/handlers/service.rs b/deployer/src/handlers/service.rs
@@ -1,6 +1,6 @@
 use std::sync::Arc;
 
-use crate::{error::Result, persistence::Service};
+use crate::persistence::Service;
 use async_trait::async_trait;
 use axum::{
     extract::{FromRequest, Path},
@@ -88,7 +88,7 @@ pub trait ServiceAuthorizer: Sync + Send {
         &self,
         api_key: &str,
         service_name: &str,
-    ) -> Result<Option<Service>>;
+    ) -> super::Result<Option<Service>>;
 }
 
 impl From<Service> for ServiceGuard {

diff --git a/deployer/src/handlers/user.rs b/deployer/src/handlers/user.rs
@@ -1,6 +1,6 @@
 use std::sync::Arc;
 
-use crate::{error::Result, persistence::User};
+use crate::persistence::User;
 use async_trait::async_trait;
 use axum::{
     extract::FromRequest,
@@ -78,7 +78,7 @@ pub struct UserGuardError {
 
 #[async_trait::async_trait]
 pub trait UserValidator: Sync + Send {
-    async fn is_user_valid(&self, api_key: &str) -> Result<Option<User>>;
+    async fn is_user_valid(&self, api_key: &str) -> super::Result<Option<User>>;
 }
 
 impl From<User> for UserGuard {

diff --git a/deployer/src/persistence/mod.rs b/deployer/src/persistence/mod.rs
@@ -463,13 +463,13 @@ impl SecretGetter for Persistence {
 
 #[async_trait::async_trait]
 impl UserValidator for Persistence {
-    async fn is_user_valid(&self, api_key: &str) -> crate::error::Result<Option<User>> {
+    async fn is_user_valid(&self, api_key: &str) -> crate::handlers::Result<Option<User>> {
         sqlx::query_as("SELECT * FROM users WHERE api_key = ?")
             .bind(api_key)
             .fetch_optional(&self.pool)
             .await
             .map_err(Error::from)
-            .map_err(crate::error::Error::Persistence)
+            .map_err(crate::handlers::Error::Persistence)
     }
 }
 
@@ -479,14 +479,14 @@ impl ServiceAuthorizer for Persistence {
         &self,
         api_key: &str,
         service_name: &str,
-    ) -> crate::error::Result<Option<Service>> {
+    ) -> crate::handlers::Result<Option<Service>> {
         sqlx::query_as("SELECT * FROM services WHERE user_id = ? AND name = ?")
             .bind(api_key)
             .bind(service_name)
             .fetch_optional(&self.pool)
             .await
             .map_err(Error::from)
-            .map_err(crate::error::Error::Persistence)
+            .map_err(crate::handlers::Error::Persistence)
     }
 }
 
@@ -496,7 +496,7 @@ impl DeploymentAuthorizer for Persistence {
         &self,
         api_key: &str,
         deployment_id: &Uuid,
-    ) -> crate::error::Result<Option<Deployment>> {
+    ) -> crate::handlers::Result<Option<Deployment>> {
         sqlx::query_as(
             r#"SELECT d.id AS id, service_id, state, last_update
                 FROM deployments AS d
@@ -508,7 +508,7 @@ impl DeploymentAuthorizer for Persistence {
         .fetch_optional(&self.pool)
         .await
         .map_err(Error::from)
-        .map_err(crate::error::Error::Persistence)
+        .map_err(crate::handlers::Error::Persistence)
     }
 }