Some movement of helper code and refactoring of error handling

7 files changed, 64 insertions, 19 deletions

diff --git a/src/model/Cargo.toml b/src/model/Cargo.toml
index 03881f5d..14e49557 100644
--- a/src/model/Cargo.toml
+++ b/src/model/Cargo.toml
@@ -21,6 +21,7 @@ garage_model_050 = { package = "garage_model", version = "0.5.1" }
 
 async-trait = "0.1.7"
 arc-swap = "1.0"
+err-derive = "0.3"
 hex = "0.4"
 log = "0.4"
 rand = "0.8"
diff --git a/src/model/block.rs b/src/model/block.rs
index 6df8e265..1173c7b3 100644
--- a/src/model/block.rs
+++ b/src/model/block.rs
@@ -594,10 +594,8 @@ impl BlockManager {
 							need_nodes.push(*node);
 						}
 					}
-					_ => {
-						return Err(Error::Message(
-							"Unexpected response to NeedBlockQuery RPC".to_string(),
-						));
+					m => {
+						return Err(Error::unexpected_rpc_message(m));
 					}
 				}
 			}
@@ -730,7 +728,7 @@ impl EndpointHandler<BlockRpc> for BlockManager {
 			BlockRpc::PutBlock { hash, data } => self.write_block(hash, data).await,
 			BlockRpc::GetBlock(h) => self.read_block(h).await,
 			BlockRpc::NeedBlockQuery(h) => self.need_block(h).await.map(BlockRpc::NeedBlockReply),
-			_ => Err(Error::BadRpc("Unexpected RPC message".to_string())),
+			m => Err(Error::unexpected_rpc_message(m)),
 		}
 	}
 }
diff --git a/src/model/garage.rs b/src/model/garage.rs
index 9db1843c..78b4433a 100644
--- a/src/model/garage.rs
+++ b/src/model/garage.rs
@@ -15,8 +15,8 @@ use garage_table::*;
 use crate::block::*;
 use crate::block_ref_table::*;
 use crate::bucket_alias_table::*;
-use crate::bucket_helper::*;
 use crate::bucket_table::*;
+use crate::helper;
 use crate::key_table::*;
 use crate::object_table::*;
 use crate::version_table::*;
@@ -162,7 +162,7 @@ impl Garage {
 		self.block_manager.garage.swap(None);
 	}
 
-	pub fn bucket_helper(&self) -> BucketHelper {
-		BucketHelper(self)
+	pub fn bucket_helper(&self) -> helper::bucket::BucketHelper {
+		helper::bucket::BucketHelper(self)
 	}
 }
diff --git a/src/model/bucket_helper.rs b/src/model/helper/bucket.rs
index b55ebc4b..e89a723d 100644
--- a/src/model/bucket_helper.rs
+++ b/src/model/helper/bucket.rs
@@ -1,10 +1,9 @@
-use garage_util::data::*;
-use garage_util::error::*;
-
 use garage_table::util::EmptyKey;
+use garage_util::data::*;
 
 use crate::bucket_table::Bucket;
 use crate::garage::Garage;
+use crate::helper::error::*;
 
 pub struct BucketHelper<'a>(pub(crate) &'a Garage);
 
@@ -52,12 +51,6 @@ impl<'a> BucketHelper<'a> {
 			.get(&bucket_id, &EmptyKey)
 			.await?
 			.filter(|b| !b.is_deleted())
-			.map(Ok)
-			.unwrap_or_else(|| {
-				Err(Error::BadRpc(format!(
-					"Bucket {:?} does not exist",
-					bucket_id
-				)))
-			})
+			.ok_or_bad_request(format!("Bucket {:?} does not exist", bucket_id))
 	}
 }
diff --git a/src/model/helper/error.rs b/src/model/helper/error.rs
new file mode 100644
index 00000000..b9b515f3
--- /dev/null
+++ b/src/model/helper/error.rs
@@ -0,0 +1,51 @@
+use err_derive::Error;
+use serde::{Deserialize, Serialize};
+
+use garage_util::error::Error as GarageError;
+
+#[derive(Debug, Error, Serialize, Deserialize)]
+pub enum Error {
+	#[error(display = "Internal error: {}", _0)]
+	Internal(#[error(source)] GarageError),
+
+	#[error(display = "Bad request: {}", _0)]
+	BadRequest(String),
+}
+
+impl From<netapp::error::Error> for Error {
+	fn from(e: netapp::error::Error) -> Self {
+		Error::Internal(GarageError::Netapp(e))
+	}
+}
+
+pub trait OkOrBadRequest {
+	type S;
+	fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<Self::S, Error>;
+}
+
+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, Error> {
+		match self {
+			Ok(x) => Ok(x),
+			Err(e) => Err(Error::BadRequest(format!(
+				"{}: {}",
+				reason.as_ref(),
+				e.to_string()
+			))),
+		}
+	}
+}
+
+impl<T> OkOrBadRequest for Option<T> {
+	type S = T;
+	fn ok_or_bad_request<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
+		match self {
+			Some(x) => Ok(x),
+			None => Err(Error::BadRequest(reason.as_ref().to_string())),
+		}
+	}
+}
diff --git a/src/model/helper/mod.rs b/src/model/helper/mod.rs
new file mode 100644
index 00000000..2f4e8898
--- /dev/null
+++ b/src/model/helper/mod.rs
@@ -0,0 +1,2 @@
+pub mod bucket;
+pub mod error;
diff --git a/src/model/lib.rs b/src/model/lib.rs
index e7d7e98b..9deaae9d 100644
--- a/src/model/lib.rs
+++ b/src/model/lib.rs
@@ -12,6 +12,6 @@ pub mod version_table;
 
 pub mod block;
 
-pub mod bucket_helper;
 pub mod garage;
+pub mod helper;
 pub mod migrate;