More error refactoring

4 files changed, 29 insertions, 43 deletions

diff --git a/src/api/k2v/api_server.rs b/src/api/k2v/api_server.rs
index b70fcdff..eb0fbdd7 100644
--- a/src/api/k2v/api_server.rs
+++ b/src/api/k2v/api_server.rs
@@ -7,13 +7,12 @@ use hyper::{Body, Method, Request, Response};
 
 use opentelemetry::{trace::SpanRef, KeyValue};
 
-use garage_table::util::*;
 use garage_util::error::Error as GarageError;
 
 use garage_model::garage::Garage;
 
-use crate::k2v::error::*;
 use crate::generic_server::*;
+use crate::k2v::error::*;
 
 use crate::signature::payload::check_payload_signature;
 use crate::signature::streaming::*;
@@ -84,14 +83,14 @@ impl ApiHandler for K2VApiServer {
 
 		// The OPTIONS method is procesed early, before we even check for an API key
 		if let Endpoint::Options = endpoint {
-			return Ok(handle_options_s3api(garage, &req, Some(bucket_name)).await
+			return Ok(handle_options_s3api(garage, &req, Some(bucket_name))
+				.await
 				.ok_or_bad_request("Error handling OPTIONS")?);
 		}
 
 		let (api_key, mut content_sha256) = check_payload_signature(&garage, "k2v", &req).await?;
-		let api_key = api_key.ok_or_else(|| {
-			Error::Forbidden("Garage does not support anonymous access yet".to_string())
-		})?;
+		let api_key = api_key
+			.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;
 
 		let req = parse_streaming_body(
 			&api_key,
@@ -101,13 +100,14 @@ impl ApiHandler for K2VApiServer {
 			"k2v",
 		)?;
 
-		let bucket_id = garage.bucket_helper().resolve_bucket(&bucket_name, &api_key).await?;
+		let bucket_id = garage
+			.bucket_helper()
+			.resolve_bucket(&bucket_name, &api_key)
+			.await?;
 		let bucket = garage
-			.bucket_table
-			.get(&EmptyKey, &bucket_id)
-			.await?
-			.filter(|b| !b.state.is_deleted())
-			.ok_or(Error::NoSuchBucket)?;
+			.bucket_helper()
+			.get_existing_bucket(bucket_id)
+			.await?;
 
 		let allowed = match endpoint.authorization_type() {
 			Authorization::Read => api_key.allow_read(&bucket_id),
@@ -117,9 +117,7 @@ impl ApiHandler for K2VApiServer {
 		};
 
 		if !allowed {
-			return Err(Error::Forbidden(
-				"Operation is not allowed for this key.".to_string(),
-			));
+			return Err(Error::forbidden("Operation is not allowed for this key."));
 		}
 
 		// Look up what CORS rule might apply to response.
diff --git a/src/api/k2v/batch.rs b/src/api/k2v/batch.rs
index 8eae471c..db9901cf 100644
--- a/src/api/k2v/batch.rs
+++ b/src/api/k2v/batch.rs
@@ -12,8 +12,8 @@ use garage_model::garage::Garage;
 use garage_model::k2v::causality::*;
 use garage_model::k2v::item_table::*;
 
-use crate::k2v::error::*;
 use crate::helpers::*;
+use crate::k2v::error::*;
 use crate::k2v::range::read_range;
 
 pub async fn handle_insert_batch(
diff --git a/src/api/k2v/error.rs b/src/api/k2v/error.rs
index 6b9e81e6..4d8c1154 100644
--- a/src/api/k2v/error.rs
+++ b/src/api/k2v/error.rs
@@ -5,7 +5,7 @@ use hyper::{Body, HeaderMap, StatusCode};
 use garage_model::helper::error::Error as HelperError;
 
 use crate::common_error::CommonError;
-pub use crate::common_error::{OkOrBadRequest, OkOrInternalError};
+pub use crate::common_error::{CommonErrorDerivative, OkOrBadRequest, OkOrInternalError};
 use crate::generic_server::ApiError;
 use crate::signature::error::Error as SignatureError;
 
@@ -17,10 +17,6 @@ pub enum Error {
 	CommonError(CommonError),
 
 	// Category: cannot process
-	/// No proper api key was used, or the signature was invalid
-	#[error(display = "Forbidden: {}", _0)]
-	Forbidden(String),
-
 	/// Authorization Header Malformed
 	#[error(display = "Authorization header malformed, expected scope: {}", _0)]
 	AuthorizationHeaderMalformed(String),
@@ -29,10 +25,6 @@ pub enum Error {
 	#[error(display = "Key not found")]
 	NoSuchKey,
 
-	/// The bucket requested don't exists
-	#[error(display = "Bucket not found")]
-	NoSuchBucket,
-
 	/// Some base64 encoded data was badly encoded
 	#[error(display = "Invalid base64: {}", _0)]
 	InvalidBase64(#[error(source)] base64::DecodeError),
@@ -59,11 +51,15 @@ where
 	}
 }
 
+impl CommonErrorDerivative for Error {}
+
 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::CommonError(CommonError::InvalidBucketName),
+			HelperError::NoSuchBucket(_) => Self::CommonError(CommonError::NoSuchBucket),
 			e => Self::CommonError(CommonError::BadRequest(format!("{}", e))),
 		}
 	}
@@ -73,35 +69,26 @@ impl From<SignatureError> for Error {
 	fn from(err: SignatureError) -> Self {
 		match err {
 			SignatureError::CommonError(c) => Self::CommonError(c),
-			SignatureError::AuthorizationHeaderMalformed(c) => Self::AuthorizationHeaderMalformed(c),
-			SignatureError::Forbidden(f) => Self::Forbidden(f),
+			SignatureError::AuthorizationHeaderMalformed(c) => {
+				Self::AuthorizationHeaderMalformed(c)
+			}
 			SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i),
 			SignatureError::InvalidHeader(h) => Self::InvalidHeader(h),
 		}
 	}
 }
 
-impl Error {
-	//pub fn internal_error<M: ToString>(msg: M) -> Self {
-	//	Self::CommonError(CommonError::InternalError(GarageError::Message(
-	//		msg.to_string(),
-	//	)))
-	//}
-
-	pub fn bad_request<M: ToString>(msg: M) -> Self {
-		Self::CommonError(CommonError::BadRequest(msg.to_string()))
-	}
-}
-
 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::NoSuchKey | Error::NoSuchBucket => StatusCode::NOT_FOUND,
-			Error::Forbidden(_) => StatusCode::FORBIDDEN,
+			Error::NoSuchKey => StatusCode::NOT_FOUND,
 			Error::NotAcceptable(_) => StatusCode::NOT_ACCEPTABLE,
-			_ => StatusCode::BAD_REQUEST,
+			Error::AuthorizationHeaderMalformed(_)
+			| Error::InvalidBase64(_)
+			| Error::InvalidHeader(_)
+			| Error::InvalidUtf8Str(_) => StatusCode::BAD_REQUEST,
 		}
 	}
 
@@ -110,6 +97,7 @@ 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
diff --git a/src/api/k2v/range.rs b/src/api/k2v/range.rs
index 6aa5c90c..1f7dc4cd 100644
--- a/src/api/k2v/range.rs
+++ b/src/api/k2v/range.rs
@@ -7,8 +7,8 @@ use std::sync::Arc;
 use garage_table::replication::TableShardedReplication;
 use garage_table::*;
 
-use crate::k2v::error::*;
 use crate::helpers::key_after_prefix;
+use crate::k2v::error::*;
 
 /// Read range in a Garage table.
 /// Returns (entries, more?, nextStart)