fix: better error hints & formatting + nits (#1412)

* fix: better hint on unauthorized error, formatting * better error * deser as String * nit
shuttle-hq · Nov 22, 2023 · 2afaa16 · 2afaa16
1 parent 263fb0d
commit 2afaa16
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 37 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -1,2 +1,3 @@
 proto/src/generated/* linguist-generated=true
 proto/src/generated.rs linguist-generated=true
+CHANGELOG.md linguist-generated=true
diff --git a/cargo-shuttle/src/lib.rs b/cargo-shuttle/src/lib.rs
@@ -225,10 +225,10 @@ impl Shuttle {
                 self.resource_delete(&resource_type).await
             }
             Command::Project(ProjectCommand::Start(ProjectStartArgs { idle_minutes })) => {
-                self.project_create(idle_minutes).await
+                self.project_start(idle_minutes).await
             }
             Command::Project(ProjectCommand::Restart(ProjectStartArgs { idle_minutes })) => {
-                self.project_recreate(idle_minutes).await
+                self.project_restart(idle_minutes).await
             }
             Command::Project(ProjectCommand::Status { follow }) => {
                 self.project_status(follow).await
@@ -490,7 +490,7 @@ impl Shuttle {
             project_args.working_directory = path.clone();
 
             self.load_project(&project_args)?;
-            self.project_create(DEFAULT_IDLE_MINUTES).await?;
+            self.project_start(DEFAULT_IDLE_MINUTES).await?;
         }
 
         if std::env::current_dir().is_ok_and(|d| d != path) {
@@ -1667,7 +1667,7 @@ impl Shuttle {
         Ok(CommandOutcome::Ok)
     }
 
-    async fn project_create(&self, idle_minutes: u64) -> Result<CommandOutcome> {
+    async fn project_start(&self, idle_minutes: u64) -> Result<CommandOutcome> {
         let client = self.client.as_ref().unwrap();
         let config = &project::Config { idle_minutes };
 
@@ -1721,11 +1721,11 @@ impl Shuttle {
         Ok(CommandOutcome::Ok)
     }
 
-    async fn project_recreate(&self, idle_minutes: u64) -> Result<CommandOutcome> {
+    async fn project_restart(&self, idle_minutes: u64) -> Result<CommandOutcome> {
         self.project_stop()
             .await
             .map_err(suggestions::project::project_restart_failure)?;
-        self.project_create(idle_minutes)
+        self.project_start(idle_minutes)
             .await
             .map_err(suggestions::project::project_restart_failure)?;
 
@@ -2039,7 +2039,7 @@ fn is_dirty(repo: &Repository) -> Result<()> {
         }
 
         writeln!(error).expect("to append error");
-        writeln!(error, "To proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag").expect("to append error");
+        writeln!(error, "To proceed despite this and include the uncommitted changes, pass the `--allow-dirty` or `--ad` flag").expect("to append error");
 
         bail!(error);
     }

diff --git a/common/src/models/error.rs b/common/src/models/error.rs
@@ -65,27 +65,27 @@ pub enum ErrorKind {
 impl From<ErrorKind> for ApiError {
     fn from(kind: ErrorKind) -> Self {
         let (status, error_message) = match kind {
-            ErrorKind::Internal => (StatusCode::INTERNAL_SERVER_ERROR, "internal server error"),
-            ErrorKind::KeyMissing => (StatusCode::UNAUTHORIZED, "request is missing a key"),
+            ErrorKind::Internal => (StatusCode::INTERNAL_SERVER_ERROR, "Internal server error"),
+            ErrorKind::KeyMissing => (StatusCode::UNAUTHORIZED, "Request is missing a key"),
             ErrorKind::ServiceUnavailable => (
                 StatusCode::SERVICE_UNAVAILABLE,
-                "we're experiencing a high workload right now, please try again in a little bit",
+                "We're experiencing a high workload right now, please try again in a little bit",
             ),
-            ErrorKind::KeyMalformed => (StatusCode::BAD_REQUEST, "request has an invalid key"),
-            ErrorKind::BadHost => (StatusCode::BAD_REQUEST, "the 'Host' header is invalid"),
-            ErrorKind::UserNotFound => (StatusCode::NOT_FOUND, "user not found"),
-            ErrorKind::UserAlreadyExists => (StatusCode::BAD_REQUEST, "user already exists"),
+            ErrorKind::KeyMalformed => (StatusCode::BAD_REQUEST, "Request has an invalid key"),
+            ErrorKind::BadHost => (StatusCode::BAD_REQUEST, "The 'Host' header is invalid"),
+            ErrorKind::UserNotFound => (StatusCode::NOT_FOUND, "User not found"),
+            ErrorKind::UserAlreadyExists => (StatusCode::BAD_REQUEST, "User already exists"),
             ErrorKind::ProjectNotFound => (
                 StatusCode::NOT_FOUND,
-                "project not found. Make sure you are the owner of this project name. Run `cargo shuttle project start` to create a new project.",
+                "Project not found. Make sure you are the owner of this project name. Run `cargo shuttle project start` to create a new project.",
             ),
             ErrorKind::ProjectNotReady => (
                 StatusCode::SERVICE_UNAVAILABLE,
                 // "not ready" is matched against in cargo-shuttle for giving further instructions on project deletion
-                "project not ready. Try running `cargo shuttle project restart`.",
+                "Project not ready. Try running `cargo shuttle project restart`.",
             ),
             ErrorKind::ProjectUnavailable => {
-                (StatusCode::BAD_GATEWAY, "project returned invalid response")
+                (StatusCode::BAD_GATEWAY, "Project returned invalid response")
             },
             ErrorKind::TooManyProjects => {
                 (StatusCode::FORBIDDEN, "You cannot create more projects. Delete some projects first.")
@@ -107,21 +107,21 @@ impl From<ErrorKind> for ApiError {
                     status_code: StatusCode::BAD_REQUEST.as_u16(),
                 }
             }
-            ErrorKind::InvalidOperation => (StatusCode::BAD_REQUEST, "the requested operation is invalid"),
-            ErrorKind::ProjectAlreadyExists => (StatusCode::BAD_REQUEST, "a project with the same name already exists"),
+            ErrorKind::InvalidOperation => (StatusCode::BAD_REQUEST, "The requested operation is invalid"),
+            ErrorKind::ProjectAlreadyExists => (StatusCode::BAD_REQUEST, "A project with the same name already exists"),
             ErrorKind::OwnProjectAlreadyExists(message) => {
                 return Self {
                     message,
                     status_code: StatusCode::BAD_REQUEST.as_u16(),
                 }
             }
-            ErrorKind::InvalidCustomDomain => (StatusCode::BAD_REQUEST, "invalid custom domain"),
-            ErrorKind::CustomDomainNotFound => (StatusCode::NOT_FOUND, "custom domain not found"),
-            ErrorKind::CustomDomainAlreadyExists => (StatusCode::BAD_REQUEST, "custom domain already in use"),
-            ErrorKind::Unauthorized => (StatusCode::UNAUTHORIZED, "unauthorized"),
-            ErrorKind::Forbidden => (StatusCode::FORBIDDEN, "forbidden"),
-            ErrorKind::NotReady => (StatusCode::INTERNAL_SERVER_ERROR, "service not ready"),
-            ErrorKind::DeleteProjectFailed => (StatusCode::INTERNAL_SERVER_ERROR, "deleting project failed"),
+            ErrorKind::InvalidCustomDomain => (StatusCode::BAD_REQUEST, "Invalid custom domain"),
+            ErrorKind::CustomDomainNotFound => (StatusCode::NOT_FOUND, "Custom domain not found"),
+            ErrorKind::CustomDomainAlreadyExists => (StatusCode::BAD_REQUEST, "Custom domain already in use"),
+            ErrorKind::Unauthorized => (StatusCode::UNAUTHORIZED, "Unauthorized"),
+            ErrorKind::Forbidden => (StatusCode::FORBIDDEN, "Forbidden"),
+            ErrorKind::NotReady => (StatusCode::INTERNAL_SERVER_ERROR, "Service not ready"),
+            ErrorKind::DeleteProjectFailed => (StatusCode::INTERNAL_SERVER_ERROR, "Deleting project failed"),
             ErrorKind::RateLimited(message) => {
                 return Self {
                     message,
@@ -143,28 +143,28 @@ impl From<StatusCode> for ApiError {
             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::FORBIDDEN => "This request is not allowed",
             StatusCode::UNAUTHORIZED => {
-                "we were unable to authorize your request. Is your key still valid?"
+                "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::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"
+                "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"
+                "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"
+                "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"
+                "An unexpected error occurred. Please create a ticket to report this"
             },
         };
 

diff --git a/common/src/models/project.rs b/common/src/models/project.rs
@@ -331,8 +331,9 @@ pub mod name {
         where
             D: Deserializer<'de>,
         {
-            let s: String = String::deserialize(deserializer)?;
-            s.parse().map_err(DeError::custom)
+            String::deserialize(deserializer)?
+                .parse()
+                .map_err(DeError::custom)
         }
     }
 

diff --git a/service/src/builder.rs b/service/src/builder.rs
@@ -189,7 +189,7 @@ pub async fn clean_crate(project_path: &Path) -> anyhow::Result<()> {
         .await?
         .success()
     {
-        bail!("cargo clean failed");
+        bail!("`cargo clean` failed. Did you build anything yet?");
     }
 
     Ok(())