K2V client improvements (#307)v0.7.2

- [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 <alex@adnab.me>
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/307
Co-authored-by: Alex <alex@adnab.me>
Co-committed-by: Alex <alex@adnab.me>

6 files changed, 65 insertions, 6 deletions

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
index 38c39361..38c39361 100644
--- a/src/k2v-client/src/bin/k2v-cli.rs
+++ b/src/k2v-client/bin/k2v-cli.rs
diff --git a/src/k2v-client/src/error.rs b/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
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::<ErrorResponse>(&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<u8>,
 	status: StatusCode,