feat(cargo-shuttle): better error messages

return an error message that is more relevant to the user than "failed to parse response to json" on non-200 responses
shuttle-hq · chesedo · Oct 11, 2022 · Oct 11, 2022 · Oct 11, 2022 · Oct 11, 2022
commit d90d32ac5b2b14e3802bb118fdac6ec9154a40ad
diff --git a/cargo-shuttle/src/client.rs b/cargo-shuttle/src/client.rs
@@ -11,7 +11,7 @@ use reqwest_retry::policies::ExponentialBackoff;
 use reqwest_retry::RetryTransientMiddleware;
 use serde::de::DeserializeOwned;
 use serde::Deserialize;
-use shuttle_common::models::{deployment, project, secret, service, user};
+use shuttle_common::models::{deployment, error, project, secret, service, user};
 use shuttle_common::project::ProjectName;
 use shuttle_common::{ApiKey, ApiUrl, LogItem};
 use tokio::net::TcpStream;
@@ -39,7 +39,17 @@ impl ToJson for Response {
             response = std::str::from_utf8(&full).unwrap_or_default(),
             "parsing response to json"
         );
-        serde_json::from_slice(&full).context("failed to parse response to JSON")
+        // try to deserialize into calling function response model
+        match serde_json::from_slice(&full) {
+            Ok(res) => Ok(res),
+            Err(_) => {
+                // if that doesn't work, try to deserialize into common error type
+                let res: error::ApiError =
+                    serde_json::from_slice(&full).context("failed to parse response to JSON")?;
+
+                Err(res.into())
+            }
+        }
     }
 }
 
@@ -63,7 +73,6 @@ impl Client {
             .context("failed to get API key from Shuttle server")?
             .to_json()
             .await
-            .context("could not parse server json response for create project request")
     }
 
     pub async fn deploy(
@@ -147,7 +156,6 @@ impl Client {
             .context("failed to make create project request")?
             .to_json()
             .await
-            .context("could not parse server json response for create project request")
     }
 
     pub async fn get_project(&self, project: &ProjectName) -> Result<project::Response> {
@@ -156,7 +164,7 @@ impl Client {
         self.get(path).await
     }
 
-    pub async fn delete_project(&self, project: &ProjectName) -> Result<()> {
+    pub async fn delete_project(&self, project: &ProjectName) -> Result<project::Response> {
         let path = format!("/projects/{}", project.as_str());
 
         self.delete(path).await
@@ -248,7 +256,6 @@ impl Client {
             .context("failed to make get request")?
             .to_json()
             .await
-            .context("could not parse server json response for get request")
     }
 
     async fn post<T: Into<Body>>(
@@ -285,7 +292,6 @@ impl Client {
             .context("failed to make delete request")?
             .to_json()
             .await
-            .context("could not parse server json response for delete request")
     }
 
     fn set_builder_auth(&self, builder: RequestBuilder) -> RequestBuilder {

diff --git a/cargo-shuttle/src/lib.rs b/cargo-shuttle/src/lib.rs
@@ -396,9 +396,9 @@ impl Shuttle {
     }
 
     async fn project_delete(&self, client: &Client) -> Result<()> {
-        client.delete_project(self.ctx.project_name()).await?;
+        let project = client.delete_project(self.ctx.project_name()).await?;
 
-        println!("Project has been deleted");
+        println!("{project}");
 
         Ok(())
     }

diff --git a/common/src/models/error.rs b/common/src/models/error.rs
@@ -0,0 +1,16 @@
+use std::fmt::{Display, Formatter};
+
+use serde::{Deserialize, Serialize};
+
+#[derive(Serialize, Deserialize, Debug)]
+pub struct ApiError {
+    pub message: String,
+}
+
+impl Display for ApiError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
+impl std::error::Error for ApiError {}
diff --git a/common/src/models/mod.rs b/common/src/models/mod.rs
@@ -1,4 +1,5 @@
 pub mod deployment;
+pub mod error;
 pub mod project;
 pub mod resource;
 pub mod secret;

diff --git a/deployer/src/handlers/error.rs b/deployer/src/handlers/error.rs
@@ -5,7 +5,7 @@ use axum::response::{IntoResponse, Response};
 use axum::Json;
 
 use serde::{ser::SerializeMap, Serialize};
-use serde_json::json;
+use shuttle_common::models::error::ApiError;
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -47,7 +47,9 @@ impl IntoResponse for Error {
                 header::CONTENT_TYPE,
                 HeaderValue::from_static("application/json"),
             )],
-            Json(json!({ "message": self })),
+            Json(ApiError {
+                message: self.to_string(),
+            }),
         )
             .into_response()
     }

diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs
@@ -103,10 +103,16 @@ async fn delete_project(
         user: User { name, .. },
     }: ScopedUser,
     Path(project): Path<ProjectName>,
-) -> Result<(), Error> {
+) -> Result<AxumJson<project::Response>, Error> {
     let work = service.destroy_project(project, name).await?;
 
-    sender.send(work).await.map_err(Error::from)
+    let name = work.project_name.to_string();
+    let state = work.work.clone().into();
+
+    sender.send(work).await?;
+
+    let response = project::Response { name, state };
+    Ok(AxumJson(response))
 }
 
 async fn route_project(

diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs
@@ -17,7 +17,7 @@ use futures::prelude::*;
 use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::{Deserialize, Deserializer, Serialize};
-use serde_json::json;
+use shuttle_common::models::error::ApiError;
 use tokio::sync::mpsc::error::SendError;
 use tracing::error;
 
@@ -141,7 +141,13 @@ impl IntoResponse for Error {
             ErrorKind::Forbidden => (StatusCode::FORBIDDEN, "forbidden"),
             ErrorKind::NotReady => (StatusCode::INTERNAL_SERVER_ERROR, "service not ready"),
         };
-        (status, Json(json!({ "error": error_message }))).into_response()
+        (
+            status,
+            Json(ApiError {
+                message: error_message.to_string(),
+            }),
+        )
+            .into_response()
     }
 }