From fd00c73350f29d58fa604802f484e6c6b8961488 Mon Sep 17 00:00:00 2001 From: chesedo Date: Wed, 9 Nov 2022 11:11:48 +0200 Subject: [PATCH 1/3] refactor: base client error off response status code --- cargo-shuttle/src/client.rs | 31 ++++++++++++++++++------------ common/src/models/error.rs | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/cargo-shuttle/src/client.rs b/cargo-shuttle/src/client.rs index a2233f10e..a84371026 100644 --- a/cargo-shuttle/src/client.rs +++ b/cargo-shuttle/src/client.rs @@ -3,7 +3,7 @@ use std::fmt::Write; use anyhow::{Context, Result}; use async_trait::async_trait; use headers::{Authorization, HeaderMapExt}; -use reqwest::{Body, Response}; +use reqwest::{Body, Response, StatusCode}; use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, RequestBuilder}; use reqwest_retry::policies::ExponentialBackoff; use reqwest_retry::RetryTransientMiddleware; @@ -31,23 +31,30 @@ trait ToJson { #[async_trait] impl ToJson for Response { async fn to_json(self) -> Result { + let status_code = self.status(); let full = self.bytes().await?; trace!( response = std::str::from_utf8(&full).unwrap_or_default(), "parsing response to json" ); - // try to deserialize into calling function response model - match serde_json::from_slice(&full) { - Ok(res) => Ok(res), - Err(_) => { - trace!("parsing response to common error"); - // 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()) - } + + if matches!( + status_code, + StatusCode::OK | StatusCode::SWITCHING_PROTOCOLS + ) { + serde_json::from_slice(&full).context("failed to parse a successfull response") + } else { + trace!("parsing response to common error"); + let res: error::ApiError = match serde_json::from_slice(&full) { + Ok(res) => res, + _ => { + trace!("getting error from status code"); + status_code.into() + } + }; + + Err(res.into()) } } } diff --git a/common/src/models/error.rs b/common/src/models/error.rs index da5b77fa5..a90e7f941 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -4,6 +4,7 @@ use comfy_table::Color; use crossterm::style::Stylize; use http::StatusCode; use serde::{Deserialize, Serialize}; +use tracing::{error, log::warn}; #[derive(Serialize, Deserialize, Debug)] pub struct ApiError { @@ -90,3 +91,40 @@ impl From for ApiError { } } } + +impl From 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 successfull status code") + } + StatusCode::FORBIDDEN => "this request is not allowed", + StatusCode::UNAUTHORIZED => { + "we we're able to authorize your request. Is your key still valid?" + }, + 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 a deployer"); + "response from deployer is invalid. Please create a ticket to report this" + }, + _ => { + error!(%code, "got an unexpected status code"); + "an unexpected error occured. Please create a ticket to report this" + }, + }; + + Self { + message: message.to_string(), + status_code: code.as_u16(), + } + } +} From a7787499d88df6d2bf211848b5f7b0bbfe012d34 Mon Sep 17 00:00:00 2001 From: Pieter Date: Fri, 11 Nov 2022 07:58:46 +0200 Subject: [PATCH 2/3] Update common/src/models/error.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Oddbjørn Grødem <29732646+oddgrd@users.noreply.github.com> --- common/src/models/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/models/error.rs b/common/src/models/error.rs index a90e7f941..dc5b103f5 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -100,7 +100,7 @@ impl From for ApiError { } StatusCode::FORBIDDEN => "this request is not allowed", StatusCode::UNAUTHORIZED => { - "we we're able to authorize your request. Is your key still valid?" + "we were unable to authorize your request. Is your key still valid?" }, 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", From 69a425ceb52691638a1d02ea72c1932fb0f88d98 Mon Sep 17 00:00:00 2001 From: chesedo Date: Fri, 11 Nov 2022 10:07:17 +0200 Subject: [PATCH 3/3] refactor: use tracing --- common/src/models/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/models/error.rs b/common/src/models/error.rs index dc5b103f5..e8f28c913 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -4,7 +4,7 @@ use comfy_table::Color; use crossterm::style::Stylize; use http::StatusCode; use serde::{Deserialize, Serialize}; -use tracing::{error, log::warn}; +use tracing::{error, warn}; #[derive(Serialize, Deserialize, Debug)] pub struct ApiError {