Common error type and admin error type that uses it

author: Alex Auvolat <alex@adnab.me> 2022-05-13 14:30:30 +0200
committer: Alex Auvolat <alex@adnab.me> 2022-05-13 14:30:30 +0200
commit: c0fb9fd0fe553e5eda39dcb1a09f059bcd631b6c (patch)
tree: 4d73c67a540e032190543fc319fad12c409e1e16
parent: 983037d965fdcdf089b09fa90fac31501defae9e (diff)
download: garage-c0fb9fd0fe553e5eda39dcb1a09f059bcd631b6c.tar.gz
garage-c0fb9fd0fe553e5eda39dcb1a09f059bcd631b6c.zip
11 files changed, 249 insertions, 29 deletions
diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs
index bffffd72..b344a51b 100644
--- a/src/api/admin/api_server.rs
+++ b/src/api/admin/api_server.rs
@@ -15,9 +15,9 @@ use prometheus::{Encoder, TextEncoder};
 use garage_model::garage::Garage;
 use garage_util::error::Error as GarageError;
 
-use crate::error::*;
 use crate::generic_server::*;
 
+use crate::admin::error::*;
 use crate::admin::bucket::*;
 use crate::admin::cluster::*;
 use crate::admin::key::*;
diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs
index 2a25bb18..1ecb66ab 100644
--- a/src/api/admin/bucket.rs
+++ b/src/api/admin/bucket.rs
@@ -17,8 +17,8 @@ use garage_model::permission::*;
 use garage_model::s3::object_table::ObjectFilter;
 
 use crate::admin::key::ApiBucketKeyPerm;
-use crate::error::*;
-use crate::helpers::*;
+use crate::admin::error::*;
+use crate::admin::parse_json_body;
 
 pub async fn handle_list_buckets(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
 	let buckets = garage
@@ -97,9 +97,9 @@ pub async fn handle_get_bucket_info(
 			.await?
 			.ok_or_bad_request("Bucket not found")?,
 		_ => {
-			return Err(Error::BadRequest(
-				"Either id or globalAlias must be provided (but not both)".into(),
-			))
+			return Err(Error::bad_request(
+				"Either id or globalAlias must be provided (but not both)"
+			));
 		}
 	};
 
@@ -225,7 +225,7 @@ pub async fn handle_create_bucket(
 
 	if let Some(ga) = &req.global_alias {
 		if !is_valid_bucket_name(ga) {
-			return Err(Error::BadRequest(format!(
+			return Err(Error::bad_request(format!(
 				"{}: {}",
 				ga, INVALID_BUCKET_NAME_MESSAGE
 			)));
@@ -240,7 +240,7 @@ pub async fn handle_create_bucket(
 
 	if let Some(la) = &req.local_alias {
 		if !is_valid_bucket_name(&la.alias) {
-			return Err(Error::BadRequest(format!(
+			return Err(Error::bad_request(format!(
 				"{}: {}",
 				la.alias, INVALID_BUCKET_NAME_MESSAGE
 			)));
@@ -250,10 +250,10 @@ pub async fn handle_create_bucket(
 			.key_table
 			.get(&EmptyKey, &la.access_key_id)
 			.await?
-			.ok_or(Error::NoSuchKey)?;
-		let state = key.state.as_option().ok_or(Error::NoSuchKey)?;
+			.ok_or(Error::NoSuchAccessKey)?;
+		let state = key.state.as_option().ok_or(Error::NoSuchAccessKey)?;
 		if matches!(state.local_aliases.get(&la.alias), Some(_)) {
-			return Err(Error::BadRequest("Local alias already exists".into()));
+			return Err(Error::bad_request("Local alias already exists"));
 		}
 	}
 
@@ -333,7 +333,7 @@ pub async fn handle_delete_bucket(
 		)
 		.await?;
 	if !objects.is_empty() {
-		return Err(Error::BadRequest("Bucket is not empty".into()));
+		return Err(Error::bad_request("Bucket is not empty"));
 	}
 
 	// --- done checking, now commit ---
diff --git a/src/api/admin/cluster.rs b/src/api/admin/cluster.rs
index b8e9d96c..db4d968d 100644
--- a/src/api/admin/cluster.rs
+++ b/src/api/admin/cluster.rs
@@ -13,8 +13,8 @@ use garage_rpc::layout::*;
 
 use garage_model::garage::Garage;
 
-use crate::error::*;
-use crate::helpers::*;
+use crate::admin::error::*;
+use crate::admin::parse_json_body;
 
 pub async fn handle_get_cluster_status(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
 	let res = GetClusterStatusResponse {
diff --git a/src/api/admin/error.rs b/src/api/admin/error.rs
new file mode 100644
index 00000000..3e488d8d
--- /dev/null
+++ b/src/api/admin/error.rs
@@ -0,0 +1,94 @@
+use err_derive::Error;
+use hyper::header::HeaderValue;
+use hyper::{Body, HeaderMap, StatusCode};
+
+use garage_model::helper::error::Error as HelperError;
+use garage_util::error::Error as GarageError;
+
+use crate::generic_server::ApiError;
+pub use crate::common_error::*;
+
+/// Errors of this crate
+#[derive(Debug, Error)]
+pub enum Error {
+	#[error(display = "{}", _0)]
+	/// Error from common error
+	CommonError(CommonError),
+
+	// Category: cannot process
+	/// No proper api key was used, or the signature was invalid
+	#[error(display = "Forbidden: {}", _0)]
+	Forbidden(String),
+
+	/// The API access key does not exist
+	#[error(display = "Access key not found")]
+	NoSuchAccessKey,
+
+	/// The bucket requested don't exists
+	#[error(display = "Bucket not found")]
+	NoSuchBucket,
+
+	/// Tried to create a bucket that already exist
+	#[error(display = "Bucket already exists")]
+	BucketAlreadyExists,
+
+	/// Tried to delete a non-empty bucket
+	#[error(display = "Tried to delete a non-empty bucket")]
+	BucketNotEmpty,
+
+	// Category: bad request
+	/// Bucket name is not valid according to AWS S3 specs
+	#[error(display = "Invalid bucket name")]
+	InvalidBucketName,
+
+	/// The client sent a request for an action not supported by garage
+	#[error(display = "Unimplemented action: {}", _0)]
+	NotImplemented(String),
+}
+
+impl<T> From<T> for Error
+where CommonError: From<T> {
+	fn from(err: T) -> Self {
+		Error::CommonError(CommonError::from(err))
+	}
+}
+
+impl From<HelperError> for Error {
+	fn from(err: HelperError) -> Self {
+		match err {
+			HelperError::Internal(i) => Self::CommonError(CommonError::InternalError(i)),
+			HelperError::BadRequest(b) => Self::CommonError(CommonError::BadRequest(b)),
+			HelperError::InvalidBucketName(_) => Self::InvalidBucketName,
+			HelperError::NoSuchAccessKey(_) => Self::NoSuchAccessKey,
+			HelperError::NoSuchBucket(_) => Self::NoSuchBucket,
+		}
+	}
+}
+
+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 {
+		match self {
+			Error::CommonError(c) => c.http_status_code(),
+			Error::NoSuchAccessKey | Error::NoSuchBucket => StatusCode::NOT_FOUND,
+			Error::BucketNotEmpty | Error::BucketAlreadyExists => StatusCode::CONFLICT,
+			Error::Forbidden(_) => StatusCode::FORBIDDEN,
+			Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED,
+			Error::InvalidBucketName => StatusCode::BAD_REQUEST,
+		}
+	}
+
+	fn add_http_headers(&self, _header_map: &mut HeaderMap<HeaderValue>) {
+		// nothing
+	}
+
+	fn http_body(&self, garage_region: &str, path: &str) -> Body {
+		Body::from(format!("ERROR: {}\n\ngarage region: {}\npath: {}", self, garage_region, path))
+	}
+}
+
+impl Error {
+	pub fn bad_request<M: ToString>(msg: M) -> Self {
+		Self::CommonError(CommonError::BadRequest(msg.to_string()))
+	}
+}
diff --git a/src/api/admin/key.rs b/src/api/admin/key.rs
index 19ad5160..e5f25601 100644
--- a/src/api/admin/key.rs
+++ b/src/api/admin/key.rs
@@ -11,8 +11,8 @@ use garage_table::*;
 use garage_model::garage::Garage;
 use garage_model::key_table::*;
 
-use crate::error::*;
-use crate::helpers::*;
+use crate::admin::error::*;
+use crate::admin::parse_json_body;
 
 pub async fn handle_list_keys(garage: &Arc<Garage>) -> Result<Response<Body>, Error> {
 	let res = garage
@@ -54,13 +54,13 @@ pub async fn handle_get_key_info(
 			.key_table
 			.get(&EmptyKey, &id)
 			.await?
-			.ok_or(Error::NoSuchKey)?
+			.ok_or(Error::NoSuchAccessKey)?
 	} else if let Some(search) = search {
 		garage
 			.key_helper()
 			.get_existing_matching_key(&search)
 			.await
-			.map_err(|_| Error::NoSuchKey)?
+			.map_err(|_| Error::NoSuchAccessKey)?
 	} else {
 		unreachable!();
 	};
@@ -96,9 +96,9 @@ pub async fn handle_update_key(
 		.key_table
 		.get(&EmptyKey, &id)
 		.await?
-		.ok_or(Error::NoSuchKey)?;
+		.ok_or(Error::NoSuchAccessKey)?;
 
-	let key_state = key.state.as_option_mut().ok_or(Error::NoSuchKey)?;
+	let key_state = key.state.as_option_mut().ok_or(Error::NoSuchAccessKey)?;
 
 	if let Some(new_name) = req.name {
 		key_state.name.update(new_name);
@@ -131,9 +131,9 @@ pub async fn handle_delete_key(garage: &Arc<Garage>, id: String) -> Result<Respo
 		.key_table
 		.get(&EmptyKey, &id)
 		.await?
-		.ok_or(Error::NoSuchKey)?;
+		.ok_or(Error::NoSuchAccessKey)?;
 
-	key.state.as_option().ok_or(Error::NoSuchKey)?;
+	key.state.as_option().ok_or(Error::NoSuchAccessKey)?;
 
 	garage.key_helper().delete_key(&mut key).await?;
 
diff --git a/src/api/admin/mod.rs b/src/api/admin/mod.rs
index 05097c8b..68839039 100644
--- a/src/api/admin/mod.rs
+++ b/src/api/admin/mod.rs
@@ -1,6 +1,19 @@
 pub mod api_server;
 mod router;
+mod error;
 
 mod bucket;
 mod cluster;
 mod key;
+
+
+use serde::{Deserialize};
+use hyper::{Request, Body};
+
+use error::*;
+
+pub async fn parse_json_body<T: for<'de> Deserialize<'de>>(req: Request<Body>) -> Result<T, Error> {
+	let body = hyper::body::to_bytes(req.into_body()).await?;
+	let resp: T = serde_json::from_slice(&body).ok_or_bad_request("Invalid JSON")?;
+	Ok(resp)
+}
diff --git a/src/api/admin/router.rs b/src/api/admin/router.rs
index 6f787fe9..2a5098bf 100644
--- a/src/api/admin/router.rs
+++ b/src/api/admin/router.rs
@@ -2,7 +2,7 @@ use std::borrow::Cow;
 
 use hyper::{Method, Request};
 
-use crate::error::*;
+use crate::admin::error::*;
 use crate::router_macros::*;
 
 pub enum Authorization {
diff --git a/src/api/common_error.rs b/src/api/common_error.rs
new file mode 100644
index 00000000..8be85f97
--- /dev/null
+++ b/src/api/common_error.rs
@@ -0,0 +1,108 @@
+use err_derive::Error;
+use hyper::header::HeaderValue;
+use hyper::{Body, HeaderMap, StatusCode};
+
+use garage_model::helper::error::Error as HelperError;
+use garage_util::error::Error as GarageError;
+
+use crate::generic_server::ApiError;
+
+/// Errors of this crate
+#[derive(Debug, Error)]
+pub enum CommonError {
+	// Category: internal error
+	/// Error related to deeper parts of Garage
+	#[error(display = "Internal error: {}", _0)]
+	InternalError(#[error(source)] GarageError),
+
+	/// Error related to Hyper
+	#[error(display = "Internal error (Hyper error): {}", _0)]
+	Hyper(#[error(source)] hyper::Error),
+
+	/// Error related to HTTP
+	#[error(display = "Internal error (HTTP error): {}", _0)]
+	Http(#[error(source)] http::Error),
+
+	/// The client sent an invalid request
+	#[error(display = "Bad request: {}", _0)]
+	BadRequest(String),
+}
+
+impl CommonError {
+	pub fn http_status_code(&self) -> StatusCode {
+		match self {
+			CommonError::InternalError(
+				GarageError::Timeout
+				| GarageError::RemoteError(_)
+				| GarageError::Quorum(_, _, _, _),
+			) => StatusCode::SERVICE_UNAVAILABLE,
+			CommonError::InternalError(_) | CommonError::Hyper(_) | CommonError::Http(_) => 
+				StatusCode::INTERNAL_SERVER_ERROR,
+			CommonError::BadRequest(_) => StatusCode::BAD_REQUEST,
+		}
+	}
+}
+
+/// Trait to map error to the Bad Request error code
+pub trait OkOrBadRequest {
+	type S;
+	fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<Self::S, CommonError>;
+}
+
+impl<T, E> OkOrBadRequest for Result<T, E>
+where
+	E: std::fmt::Display,
+{
+	type S = T;
+	fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, CommonError> {
+		match self {
+			Ok(x) => Ok(x),
+			Err(e) => Err(CommonError::BadRequest(format!("{}: {}", reason.as_ref(), e))),
+		}
+	}
+}
+
+impl<T> OkOrBadRequest for Option<T> {
+	type S = T;
+	fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, CommonError> {
+		match self {
+			Some(x) => Ok(x),
+			None => Err(CommonError::BadRequest(reason.as_ref().to_string())),
+		}
+	}
+}
+
+/// Trait to map an error to an Internal Error code
+pub trait OkOrInternalError {
+	type S;
+	fn ok_or_internal_error<M: AsRef<str>>(self, reason: M) -> Result<Self::S, CommonError>;
+}
+
+impl<T, E> OkOrInternalError for Result<T, E>
+where
+	E: std::fmt::Display,
+{
+	type S = T;
+	fn ok_or_internal_error<M: AsRef<str>>(self, reason: M) -> Result<T, CommonError> {
+		match self {
+			Ok(x) => Ok(x),
+			Err(e) => Err(CommonError::InternalError(GarageError::Message(format!(
+				"{}: {}",
+				reason.as_ref(),
+				e
+			)))),
+		}
+	}
+}
+
+impl<T> OkOrInternalError for Option<T> {
+	type S = T;
+	fn ok_or_internal_error<M: AsRef<str>>(self, reason: M) -> Result<T, CommonError> {
+		match self {
+			Some(x) => Ok(x),
+			None => Err(CommonError::InternalError(GarageError::Message(
+				reason.as_ref().to_string(),
+			))),
+		}
+	}
+}
diff --git a/src/api/error.rs b/src/api/error.rs
index 5c33d763..90bfccef 100644
--- a/src/api/error.rs
+++ b/src/api/error.rs
@@ -166,6 +166,10 @@ impl Error {
 			_ => "InvalidRequest",
 		}
 	}
+
+	pub fn bad_request<M: ToString>(msg: M) -> Self {
+		Self::BadRequest(msg.to_string())
+	}
 }
 
 impl ApiError for Error {
diff --git a/src/api/lib.rs b/src/api/lib.rs
index f8165d2a..5bc2a18e 100644
--- a/src/api/lib.rs
+++ b/src/api/lib.rs
@@ -2,6 +2,7 @@
 #[macro_use]
 extern crate tracing;
 
+pub mod common_error;
 pub mod error;
 pub use error::Error;
 
diff --git a/src/api/router_macros.rs b/src/api/router_macros.rs
index a3e885e6..4c593300 100644
--- a/src/api/router_macros.rs
+++ b/src/api/router_macros.rs
@@ -38,7 +38,7 @@ macro_rules! router_match {
 					},
 				)*
 				(m, p) => {
-					return Err(Error::BadRequest(format!(
+					return Err(Error::bad_request(format!(
 						"Unknown API endpoint: {} {}",
 						m, p
 					)))
@@ -78,7 +78,7 @@ macro_rules! router_match {
                 )*)?
             }),
             )*
-            (kw, _) => Err(Error::BadRequest(format!("Invalid endpoint: {}", kw)))
+            (kw, _) => Err(Error::bad_request(format!("Invalid endpoint: {}", kw)))
         }
     }};
 
@@ -97,14 +97,14 @@ macro_rules! router_match {
             .take()
             .map(|param| param.parse())
             .transpose()
-            .map_err(|_| Error::BadRequest("Failed to parse query parameter".to_owned()))?
+            .map_err(|_| Error::bad_request("Failed to parse query parameter"))?
     }};
     (@@parse_param $query:expr, parse, $param:ident) => {{
         // extract and parse mandatory query parameter
         // both missing and un-parseable parameters are reported as errors
         $query.$param.take().ok_or_bad_request("Missing argument for endpoint")?
             .parse()
-            .map_err(|_| Error::BadRequest("Failed to parse query parameter".to_owned()))?
+            .map_err(|_| Error::bad_request("Failed to parse query parameter"))?
     }};
     (@func
     $(#[$doc:meta])*
@@ -173,7 +173,7 @@ macro_rules! generateQueryParameters {
                                 false
                             } else if v.as_ref().is_empty() {
                                 if res.keyword.replace(k).is_some() {
-                                    return Err(Error::BadRequest("Multiple keywords".to_owned()));
+                                    return Err(Error::bad_request("Multiple keywords"));
                                 }
                                 continue;
                             } else {
@@ -183,7 +183,7 @@ macro_rules! generateQueryParameters {
                         }
                     };
                     if repeated {
-                        return Err(Error::BadRequest(format!(
+                        return Err(Error::bad_request(format!(
                             "Query parameter repeated: '{}'",
                             k
                         )));
author	Alex Auvolat <alex@adnab.me>	2022-05-13 14:30:30 +0200
committer	Alex Auvolat <alex@adnab.me>	2022-05-13 14:30:30 +0200
commit	c0fb9fd0fe553e5eda39dcb1a09f059bcd631b6c (patch)
tree	4d73c67a540e032190543fc319fad12c409e1e16
parent	983037d965fdcdf089b09fa90fac31501defae9e (diff)
download	garage-c0fb9fd0fe553e5eda39dcb1a09f059bcd631b6c.tar.gz garage-c0fb9fd0fe553e5eda39dcb1a09f059bcd631b6c.zip