From 5a535788fc0a69950bbfdc6f189597c5e37a6e3b Mon Sep 17 00:00:00 2001
From: Alex Auvolat <alex@adnab.me>
Date: Fri, 13 May 2022 19:28:23 +0200
Subject: [PATCH] Json body for custom errors

---
 src/api/admin/error.rs | 30 +++++++++++++++++++++++++-----
 src/api/helpers.rs     | 10 +++++++++-
 src/api/k2v/error.rs   | 38 +++++++++++++++++++++++++++++++++-----
 3 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/src/api/admin/error.rs b/src/api/admin/error.rs
index bb35c16b..38dfe5b6 100644
--- a/src/api/admin/error.rs
+++ b/src/api/admin/error.rs
@@ -7,6 +7,7 @@ use garage_model::helper::error::Error as HelperError;
 use crate::common_error::CommonError;
 pub use crate::common_error::{CommonErrorDerivative, OkOrBadRequest, OkOrInternalError};
 use crate::generic_server::ApiError;
+use crate::helpers::CustomApiErrorBody;
 
 /// Errors of this crate
 #[derive(Debug, Error)]
@@ -44,6 +45,15 @@ impl From<HelperError> for Error {
 	}
 }
 
+impl Error {
+	fn code(&self) -> &'static str {
+		match self {
+			Error::CommonError(c) => c.aws_code(),
+			Error::NoSuchAccessKey => "NoSuchAccessKey",
+		}
+	}
+}
+
 impl ApiError for Error {
 	/// Get the HTTP status code that best represents the meaning of the error for the client
 	fn http_status_code(&self) -> StatusCode {
@@ -58,10 +68,20 @@ impl ApiError for Error {
 	}
 
 	fn http_body(&self, garage_region: &str, path: &str) -> Body {
-		// TODO nice json error
-		Body::from(format!(
-			"ERROR: {}\n\ngarage region: {}\npath: {}",
-			self, garage_region, path
-		))
+		let error = CustomApiErrorBody {
+			code: self.code().to_string(),
+			message: format!("{}", self),
+			path: path.to_string(),
+			region: garage_region.to_string(),
+		};
+		Body::from(serde_json::to_string_pretty(&error).unwrap_or_else(|_| {
+			r#"
+{
+	"code": "InternalError",
+	"message": "JSON encoding of error failed"
+}
+			"#
+			.into()
+		}))
 	}
 }
diff --git a/src/api/helpers.rs b/src/api/helpers.rs
index aa350e3c..9fb12dbe 100644
--- a/src/api/helpers.rs
+++ b/src/api/helpers.rs
@@ -1,6 +1,6 @@
 use hyper::{Body, Request};
 use idna::domain_to_unicode;
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 
 use crate::common_error::{CommonError as Error, *};
 
@@ -279,3 +279,11 @@ mod tests {
 		);
 	}
 }
+
+#[derive(Serialize)]
+pub(crate) struct CustomApiErrorBody {
+	pub(crate) code: String,
+	pub(crate) message: String,
+	pub(crate) region: String,
+	pub(crate) path: String,
+}
diff --git a/src/api/k2v/error.rs b/src/api/k2v/error.rs
index 4d8c1154..85d5de9d 100644
--- a/src/api/k2v/error.rs
+++ b/src/api/k2v/error.rs
@@ -7,6 +7,7 @@ use garage_model::helper::error::Error as HelperError;
 use crate::common_error::CommonError;
 pub use crate::common_error::{CommonErrorDerivative, OkOrBadRequest, OkOrInternalError};
 use crate::generic_server::ApiError;
+use crate::helpers::CustomApiErrorBody;
 use crate::signature::error::Error as SignatureError;
 
 /// Errors of this crate
@@ -78,6 +79,23 @@ impl From<SignatureError> for Error {
 	}
 }
 
+impl Error {
+	/// This returns a keyword for the corresponding error.
+	/// Here, these keywords are not necessarily those from AWS S3,
+	/// as we are building a custom API
+	fn code(&self) -> &'static str {
+		match self {
+			Error::CommonError(c) => c.aws_code(),
+			Error::NoSuchKey => "NoSuchKey",
+			Error::NotAcceptable(_) => "NotAcceptable",
+			Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed",
+			Error::InvalidBase64(_) => "InvalidBase64",
+			Error::InvalidHeader(_) => "InvalidHeaderValue",
+			Error::InvalidUtf8Str(_) => "InvalidUtf8String",
+		}
+	}
+}
+
 impl ApiError for Error {
 	/// Get the HTTP status code that best represents the meaning of the error for the client
 	fn http_status_code(&self) -> StatusCode {
@@ -97,10 +115,20 @@ impl ApiError for Error {
 	}
 
 	fn http_body(&self, garage_region: &str, path: &str) -> Body {
-		// TODO nice json error
-		Body::from(format!(
-			"ERROR: {}\n\ngarage region: {}\npath: {}",
-			self, garage_region, path
-		))
+		let error = CustomApiErrorBody {
+			code: self.code().to_string(),
+			message: format!("{}", self),
+			path: path.to_string(),
+			region: garage_region.to_string(),
+		};
+		Body::from(serde_json::to_string_pretty(&error).unwrap_or_else(|_| {
+			r#"
+{
+	"code": "InternalError",
+	"message": "JSON encoding of error failed"
+}
+			"#
+			.into()
+		}))
 	}
 }