From b5709fa139285b2f3d7e4bda70e336d5d7a36463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oddbj=C3=B8rn=20Gr=C3=B8dem?= <29732646+oddgrd@users.noreply.github.com> Date: Thu, 13 Oct 2022 12:26:15 +0200 Subject: [PATCH] feat(cargo-shuttle): better client errors (#394) * feat(cargo-shuttle): better CLI errors * refactor: move errorkind to common --- Cargo.lock | 20 ++++++---- common/Cargo.toml | 3 +- common/src/models/error.rs | 71 +++++++++++++++++++++++++++++++++- deployer/src/handlers/error.rs | 1 + gateway/Cargo.toml | 1 - gateway/src/lib.rs | 65 ++----------------------------- 6 files changed, 90 insertions(+), 71 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f27ea1b52..9e194faa7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1461,12 +1461,6 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" -[[package]] -name = "convert_case" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb4a24b1aaf0fd0ce8b45161144d6f42cd91677fd5940fd431183eb023b3a2b8" - [[package]] name = "cookie" version = "0.14.4" @@ -5054,6 +5048,7 @@ dependencies = [ "chrono", "comfy-table", "crossterm", + "http", "once_cell", "rustrict", "serde", @@ -5122,7 +5117,6 @@ dependencies = [ "chrono", "clap 4.0.4", "colored", - "convert_case", "fqdn", "futures", "http", @@ -7006,3 +7000,15 @@ name = "zeroize" version = "1.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94693807d016b2f2d2e14420eb3bfcca689311ff775dcf113d74ea624b7cdf07" + +[[patch.unused]] +name = "shuttle-aws-rds" +version = "0.5.2" + +[[patch.unused]] +name = "shuttle-persist" +version = "0.5.2" + +[[patch.unused]] +name = "shuttle-shared-db" +version = "0.5.2" diff --git a/common/Cargo.toml b/common/Cargo.toml index 08439730e..cd79925a6 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -10,6 +10,7 @@ description = "Common library for the shuttle platform (https://www.shuttle.rs/) chrono = { version = "0.4.22", features = ["serde"] } comfy-table = { version = "6.1.0", optional = true } crossterm = { version = "0.25.0", optional = true } +http = { version = "0.2.8", optional = true } once_cell = "1.13.1" rustrict = "0.5.0" serde = { version = "1.0.143", features = ["derive"] } @@ -21,5 +22,5 @@ uuid = { version = "1.1.1", features = ["v4", "serde"] } [features] default = ["models"] -models = ["display", "serde_json"] +models = ["display", "serde_json", "http"] display = ["comfy-table", "crossterm"] diff --git a/common/src/models/error.rs b/common/src/models/error.rs index ee939632d..5e5677925 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -2,17 +2,86 @@ use std::fmt::{Display, Formatter}; use comfy_table::Color; use crossterm::style::Stylize; +use http::StatusCode; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] pub struct ApiError { pub message: String, + pub status_code: u16, +} + +impl ApiError { + pub fn status(&self) -> StatusCode { + StatusCode::from_u16(self.status_code).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR) + } } impl Display for ApiError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.message.to_string().with(Color::Red)) + write!( + f, + "{}\nmessage: {}", + self.status().to_string().bold(), + self.message.to_string().with(Color::Red) + ) } } impl std::error::Error for ApiError {} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display)] +pub enum ErrorKind { + KeyMissing, + BadHost, + KeyMalformed, + Unauthorized, + Forbidden, + UserNotFound, + UserAlreadyExists, + ProjectNotFound, + InvalidProjectName, + ProjectAlreadyExists, + ProjectNotReady, + ProjectUnavailable, + InvalidOperation, + Internal, + NotReady, +} + +impl From 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::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. Run `cargo shuttle project new` to create a new project.", + ), + ErrorKind::ProjectNotReady => (StatusCode::SERVICE_UNAVAILABLE, "project not ready"), + ErrorKind::ProjectUnavailable => { + (StatusCode::BAD_GATEWAY, "project returned invalid response") + } + ErrorKind::InvalidProjectName => (StatusCode::BAD_REQUEST, "invalid project name"), + ErrorKind::InvalidOperation => ( + StatusCode::BAD_REQUEST, + "the requested operation is invalid", + ), + ErrorKind::ProjectAlreadyExists => ( + StatusCode::BAD_REQUEST, + "a project with the same name already exists", + ), + ErrorKind::Unauthorized => (StatusCode::UNAUTHORIZED, "unauthorized"), + ErrorKind::Forbidden => (StatusCode::FORBIDDEN, "forbidden"), + ErrorKind::NotReady => (StatusCode::INTERNAL_SERVER_ERROR, "service not ready"), + }; + Self { + message: error_message.to_string(), + status_code: status.as_u16(), + } + } +} diff --git a/deployer/src/handlers/error.rs b/deployer/src/handlers/error.rs index 3893ab949..b09fa24d1 100644 --- a/deployer/src/handlers/error.rs +++ b/deployer/src/handlers/error.rs @@ -49,6 +49,7 @@ impl IntoResponse for Error { )], Json(ApiError { message: self.to_string(), + status_code: code.as_u16(), }), ) .into_response() diff --git a/gateway/Cargo.toml b/gateway/Cargo.toml index 7b8625be8..7e2fd2367 100644 --- a/gateway/Cargo.toml +++ b/gateway/Cargo.toml @@ -11,7 +11,6 @@ base64 = "0.13" bollard = "0.13" chrono = "0.4" clap = { version = "4.0.0", features = [ "derive" ] } -convert_case = "0.5.0" fqdn = "0.2.2" futures = "0.3.21" http = "0.2.8" diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 702ca2683..d5d2980bd 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -8,16 +8,14 @@ use std::io; use std::pin::Pin; use std::str::FromStr; -use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use axum::Json; use bollard::Docker; -use convert_case::{Case, Casing}; use futures::prelude::*; use once_cell::sync::Lazy; use regex::Regex; use serde::{Deserialize, Deserializer, Serialize}; -use shuttle_common::models::error::ApiError; +use shuttle_common::models::error::{ApiError, ErrorKind}; use tokio::sync::mpsc::error::SendError; use tracing::error; @@ -33,32 +31,6 @@ use crate::service::{ContainerSettings, GatewayService}; static PROJECT_REGEX: Lazy = Lazy::new(|| Regex::new("^[a-zA-Z0-9\\-_]{3,64}$").unwrap()); -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ErrorKind { - KeyMissing, - BadHost, - KeyMalformed, - Unauthorized, - Forbidden, - UserNotFound, - UserAlreadyExists, - ProjectNotFound, - InvalidProjectName, - ProjectAlreadyExists, - ProjectNotReady, - ProjectUnavailable, - InvalidOperation, - Internal, - NotReady, -} - -impl std::fmt::Display for ErrorKind { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let formatted = format!("{:?}", self).to_case(Case::Snake); - write!(f, "{}", formatted) - } -} - /// Server-side errors that do not have to do with the user runtime /// should be [`Error`]s. /// @@ -116,38 +88,9 @@ impl IntoResponse for Error { fn into_response(self) -> Response { error!(error = %self, "request had an error"); - let (status, error_message) = match self.kind { - ErrorKind::Internal => (StatusCode::INTERNAL_SERVER_ERROR, "internal server error"), - ErrorKind::KeyMissing => (StatusCode::UNAUTHORIZED, "request is missing a key"), - 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"), - ErrorKind::ProjectNotReady => (StatusCode::SERVICE_UNAVAILABLE, "project not ready"), - ErrorKind::ProjectUnavailable => { - (StatusCode::BAD_GATEWAY, "project returned invalid response") - } - ErrorKind::InvalidProjectName => (StatusCode::BAD_REQUEST, "invalid project name"), - ErrorKind::InvalidOperation => ( - StatusCode::BAD_REQUEST, - "the requested operation is invalid", - ), - ErrorKind::ProjectAlreadyExists => ( - StatusCode::BAD_REQUEST, - "a project with the same name already exists", - ), - ErrorKind::Unauthorized => (StatusCode::UNAUTHORIZED, "unauthorized"), - ErrorKind::Forbidden => (StatusCode::FORBIDDEN, "forbidden"), - ErrorKind::NotReady => (StatusCode::INTERNAL_SERVER_ERROR, "service not ready"), - }; - ( - status, - Json(ApiError { - message: error_message.to_string(), - }), - ) - .into_response() + let error: ApiError = self.kind.into(); + + (error.status(), Json(error)).into_response() } }