From b2a2d3859fefd53dab0b87274d5aed1f6bb608a3 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 24 May 2022 12:48:05 +0200 Subject: [PATCH] K2V client improvements (#307) - [x] Better distinguish error types - [x] Parse error messages received from server - [x] Remove `src/` folder layer, we don't have that for other crates Co-authored-by: Alex Auvolat Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/307 Co-authored-by: Alex Co-committed-by: Alex --- Cargo.lock | 1 + Cargo.nix | 5 ++- src/k2v-client/Cargo.toml | 5 +++ src/k2v-client/{src => }/bin/k2v-cli.rs | 0 src/k2v-client/{src => }/error.rs | 7 ++++ src/k2v-client/{src => }/lib.rs | 53 +++++++++++++++++++++++-- 6 files changed, 65 insertions(+), 6 deletions(-) rename src/k2v-client/{src => }/bin/k2v-cli.rs (100%) rename src/k2v-client/{src => }/error.rs (81%) rename src/k2v-client/{src => }/lib.rs (91%) diff --git a/Cargo.lock b/Cargo.lock index fcf3030a..630642ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1584,6 +1584,7 @@ dependencies = [ "clap 3.1.18", "garage_util 0.7.0", "http", + "log", "rusoto_core", "rusoto_credential", "rusoto_signature", diff --git a/Cargo.nix b/Cargo.nix index 371ce8d3..d100e7bb 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -688,7 +688,7 @@ in registry = "registry+https://github.com/rust-lang/crates.io-index"; src = fetchCratesIo { inherit name version; sha256 = "59a6001667ab124aebae2a495118e11d30984c3a653e99d86d58971708cf5e4b"; }; dependencies = { - ${ if hostPlatform.config == "aarch64-linux-android" || hostPlatform.parsed.cpu.name == "aarch64" && hostPlatform.parsed.kernel.name == "linux" || hostPlatform.config == "aarch64-apple-darwin" then "libc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".libc."0.2.121" { inherit profileName; }; + ${ if hostPlatform.config == "aarch64-linux-android" || hostPlatform.config == "aarch64-apple-darwin" || hostPlatform.parsed.cpu.name == "aarch64" && hostPlatform.parsed.kernel.name == "linux" then "libc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".libc."0.2.121" { inherit profileName; }; }; }); @@ -2117,6 +2117,7 @@ in clap = rustPackages."registry+https://github.com/rust-lang/crates.io-index".clap."3.1.18" { inherit profileName; }; garage_util = rustPackages."unknown".garage_util."0.7.0" { inherit profileName; }; http = rustPackages."registry+https://github.com/rust-lang/crates.io-index".http."0.2.6" { inherit profileName; }; + log = rustPackages."registry+https://github.com/rust-lang/crates.io-index".log."0.4.16" { inherit profileName; }; rusoto_core = rustPackages."registry+https://github.com/rust-lang/crates.io-index".rusoto_core."0.48.0" { inherit profileName; }; rusoto_credential = rustPackages."registry+https://github.com/rust-lang/crates.io-index".rusoto_credential."0.48.0" { inherit profileName; }; rusoto_signature = rustPackages."registry+https://github.com/rust-lang/crates.io-index".rusoto_signature."0.48.0" { inherit profileName; }; @@ -5029,7 +5030,7 @@ in [ "default" ] ]; dependencies = { - ${ if hostPlatform.config == "aarch64-uwp-windows-msvc" || hostPlatform.config == "aarch64-pc-windows-msvc" then "windows_aarch64_msvc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_aarch64_msvc."0.32.0" { inherit profileName; }; + ${ if hostPlatform.config == "aarch64-pc-windows-msvc" || hostPlatform.config == "aarch64-uwp-windows-msvc" then "windows_aarch64_msvc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_aarch64_msvc."0.32.0" { inherit profileName; }; ${ if hostPlatform.config == "i686-pc-windows-gnu" || hostPlatform.config == "i686-uwp-windows-gnu" then "windows_i686_gnu" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_i686_gnu."0.32.0" { inherit profileName; }; ${ if hostPlatform.config == "i686-uwp-windows-msvc" || hostPlatform.config == "i686-pc-windows-msvc" then "windows_i686_msvc" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_i686_msvc."0.32.0" { inherit profileName; }; ${ if hostPlatform.config == "x86_64-pc-windows-gnu" || hostPlatform.config == "x86_64-uwp-windows-gnu" then "windows_x86_64_gnu" else null } = rustPackages."registry+https://github.com/rust-lang/crates.io-index".windows_x86_64_gnu."0.32.0" { inherit profileName; }; diff --git a/src/k2v-client/Cargo.toml b/src/k2v-client/Cargo.toml index 84c6b8b2..224414ab 100644 --- a/src/k2v-client/Cargo.toml +++ b/src/k2v-client/Cargo.toml @@ -6,6 +6,7 @@ edition = "2018" [dependencies] base64 = "0.13.0" http = "0.2.6" +log = "0.4" rusoto_core = "0.48.0" rusoto_credential = "0.48.0" rusoto_signature = "0.48.0" @@ -22,6 +23,10 @@ garage_util = { path = "../util", optional = true } [features] cli = ["clap", "tokio/fs", "tokio/io-std", "garage_util"] +[lib] +path = "lib.rs" + [[bin]] name = "k2v-cli" +path = "bin/k2v-cli.rs" required-features = ["cli"] diff --git a/src/k2v-client/src/bin/k2v-cli.rs b/src/k2v-client/bin/k2v-cli.rs similarity index 100% rename from src/k2v-client/src/bin/k2v-cli.rs rename to src/k2v-client/bin/k2v-cli.rs diff --git a/src/k2v-client/src/error.rs b/src/k2v-client/error.rs similarity index 81% rename from src/k2v-client/src/error.rs rename to src/k2v-client/error.rs index 62357934..37c221f2 100644 --- a/src/k2v-client/src/error.rs +++ b/src/k2v-client/error.rs @@ -5,6 +5,13 @@ use thiserror::Error; /// Errors returned by this crate #[derive(Error, Debug)] pub enum Error { + #[error("{0}, {1}: {2} (path = {3})")] + Remote( + http::StatusCode, + Cow<'static, str>, + Cow<'static, str>, + Cow<'static, str>, + ), #[error("received invalid response: {0}")] InvalidResponse(Cow<'static, str>), #[error("not found")] diff --git a/src/k2v-client/src/lib.rs b/src/k2v-client/lib.rs similarity index 91% rename from src/k2v-client/src/lib.rs rename to src/k2v-client/lib.rs index ba1cd6ea..95974d7a 100644 --- a/src/k2v-client/src/lib.rs +++ b/src/k2v-client/lib.rs @@ -4,6 +4,7 @@ use std::time::Duration; use http::header::{ACCEPT, CONTENT_LENGTH, CONTENT_TYPE}; use http::status::StatusCode; use http::HeaderMap; +use log::{debug, error}; use rusoto_core::{ByteStream, DispatchSignedRequest, HttpClient}; use rusoto_credential::AwsCredentials; @@ -310,12 +311,47 @@ impl K2vClient { StatusCode::NO_CONTENT => Vec::new(), StatusCode::NOT_FOUND => return Err(Error::NotFound), StatusCode::NOT_MODIFIED => Vec::new(), - _ => { - return Err(Error::InvalidResponse( - format!("invalid error code: {}", res.status).into(), - )) + s => { + let err_body = read_body(&mut res.headers, res.body) + .await + .unwrap_or_default(); + let err_body_str = std::str::from_utf8(&err_body) + .map(String::from) + .unwrap_or_else(|_| base64::encode(&err_body)); + + if s.is_client_error() || s.is_server_error() { + error!("Error response {}: {}", res.status, err_body_str); + let err = match serde_json::from_slice::(&err_body) { + Ok(err) => Error::Remote( + res.status, + err.code.into(), + err.message.into(), + err.path.into(), + ), + Err(_) => Error::Remote( + res.status, + "unknown".into(), + err_body_str.into(), + "?".into(), + ), + }; + return Err(err); + } else { + let msg = format!( + "Unexpected response code {}. Response body: {}", + res.status, err_body_str + ); + error!("{}", msg); + return Err(Error::InvalidResponse(msg.into())); + } } }; + debug!( + "Response body: {}", + std::str::from_utf8(&body) + .map(String::from) + .unwrap_or_else(|_| base64::encode(&body)) + ); Ok(Response { body, @@ -558,6 +594,15 @@ struct BatchDeleteResponse<'a> { deleted_items: u64, } +#[derive(Deserialize)] +struct ErrorResponse { + code: String, + message: String, + #[allow(dead_code)] + region: String, + path: String, +} + struct Response { body: Vec, status: StatusCode,