From beeef4758e5ec0d521179a799a3237c2c0368911 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 3 Jan 2022 13:58:05 +0100 Subject: Some movement of helper code and refactoring of error handling --- Cargo.lock | 1 + src/api/error.rs | 10 ++++++++ src/garage/admin.rs | 64 ++++++++++++++++++++++++---------------------- src/garage/cli/cmd.rs | 14 +++++----- src/garage/main.rs | 8 +++++- src/model/Cargo.toml | 1 + src/model/block.rs | 8 +++--- src/model/bucket_helper.rs | 63 --------------------------------------------- src/model/garage.rs | 6 ++--- src/model/helper/bucket.rs | 56 ++++++++++++++++++++++++++++++++++++++++ src/model/helper/error.rs | 51 ++++++++++++++++++++++++++++++++++++ src/model/helper/mod.rs | 2 ++ src/model/lib.rs | 2 +- src/rpc/system.rs | 2 +- src/table/gc.rs | 2 +- src/table/sync.rs | 7 ++--- src/table/table.rs | 2 +- src/util/error.rs | 10 ++++++-- 18 files changed, 190 insertions(+), 119 deletions(-) delete mode 100644 src/model/bucket_helper.rs create mode 100644 src/model/helper/bucket.rs create mode 100644 src/model/helper/error.rs create mode 100644 src/model/helper/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 40d2a29f..033d157f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -474,6 +474,7 @@ version = "0.6.0" dependencies = [ "arc-swap", "async-trait", + "err-derive 0.3.0", "futures", "futures-util", "garage_model 0.5.1", diff --git a/src/api/error.rs b/src/api/error.rs index 828a2342..d6d4a1d7 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -4,6 +4,7 @@ use err_derive::Error; use hyper::header::HeaderValue; use hyper::{HeaderMap, StatusCode}; +use garage_model::helper::error::Error as HelperError; use garage_util::error::Error as GarageError; use crate::s3_xml; @@ -83,6 +84,15 @@ impl From for Error { } } +impl From for Error { + fn from(err: HelperError) -> Self { + match err { + HelperError::Internal(i) => Self::InternalError(i), + HelperError::BadRequest(b) => Self::BadRequest(b), + } + } +} + impl Error { /// Get the HTTP status code that best represents the meaning of the error for the client pub fn http_status_code(&self) -> StatusCode { diff --git a/src/garage/admin.rs b/src/garage/admin.rs index 49890189..740114c6 100644 --- a/src/garage/admin.rs +++ b/src/garage/admin.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use garage_util::crdt::*; use garage_util::data::*; -use garage_util::error::*; +use garage_util::error::{Error as GarageError, OkOrMessage}; use garage_util::time::*; use garage_table::replication::*; @@ -18,6 +18,7 @@ use garage_rpc::*; use garage_model::bucket_alias_table::*; use garage_model::bucket_table::*; use garage_model::garage::Garage; +use garage_model::helper::error::{Error, OkOrBadRequest}; use garage_model::key_table::*; use garage_model::migrate::Migrate; use garage_model::permission::*; @@ -91,7 +92,7 @@ impl AdminRpcHandler { .bucket_helper() .resolve_global_bucket_name(&query.name) .await? - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let bucket = self .garage @@ -137,7 +138,7 @@ impl AdminRpcHandler { let alias = match self.garage.bucket_alias_table.get(&EmptyKey, name).await? { Some(mut alias) => { if !alias.state.get().is_deleted() { - return Err(Error::BadRpc(format!("Bucket {} already exists", name))); + return Err(Error::BadRequest(format!("Bucket {} already exists", name))); } alias.state.update(Deletable::Present(AliasParams { bucket_id: bucket.id, @@ -145,7 +146,7 @@ impl AdminRpcHandler { alias } None => BucketAlias::new(name.clone(), bucket.id) - .ok_or_message(format!(INVALID_BUCKET_NAME_MESSAGE!(), name))?, + .ok_or_bad_request(format!(INVALID_BUCKET_NAME_MESSAGE!(), name))?, }; bucket.state.as_option_mut().unwrap().aliases.merge_raw( name, @@ -164,7 +165,7 @@ impl AdminRpcHandler { .get(&EmptyKey, &query.name) .await? .filter(|a| !a.is_deleted()) - .ok_or_message(format!("Bucket {} does not exist", query.name))?; + .ok_or_bad_request(format!("Bucket {} does not exist", query.name))?; let bucket_id = bucket_alias.state.get().as_option().unwrap().bucket_id; @@ -182,7 +183,7 @@ impl AdminRpcHandler { .filter(|(_, _, active)| *active) .any(|(name, _, _)| name != &query.name) { - return Err(Error::Message(format!("Bucket {} still has other global aliases. Use `bucket unalias` to delete them one by one.", query.name))); + return Err(Error::BadRequest(format!("Bucket {} still has other global aliases. Use `bucket unalias` to delete them one by one.", query.name))); } if bucket_state .local_aliases @@ -190,7 +191,7 @@ impl AdminRpcHandler { .iter() .any(|(_, _, active)| *active) { - return Err(Error::Message(format!("Bucket {} still has other local aliases. Use `bucket unalias` to delete them one by one.", query.name))); + return Err(Error::BadRequest(format!("Bucket {} still has other local aliases. Use `bucket unalias` to delete them one by one.", query.name))); } // Check bucket is empty @@ -200,11 +201,14 @@ impl AdminRpcHandler { .get_range(&bucket_id, None, Some(DeletedFilter::NotDeleted), 10) .await?; if !objects.is_empty() { - return Err(Error::BadRpc(format!("Bucket {} is not empty", query.name))); + return Err(Error::BadRequest(format!( + "Bucket {} is not empty", + query.name + ))); } if !query.yes { - return Err(Error::BadRpc( + return Err(Error::BadRequest( "Add --yes flag to really perform this operation".to_string(), )); } @@ -218,7 +222,7 @@ impl AdminRpcHandler { .await?; } } else { - return Err(Error::Message(format!("Key not found: {}", key_id))); + return Err(Error::BadRequest(format!("Key not found: {}", key_id))); } } // 2. delete bucket alias @@ -237,7 +241,7 @@ impl AdminRpcHandler { .bucket_helper() .resolve_global_bucket_name(&query.existing_bucket) .await? - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let mut bucket = self .garage .bucket_helper() @@ -257,12 +261,12 @@ impl AdminRpcHandler { query.new_name, bucket_id, key.key_id ))); } else { - return Err(Error::Message(format!("Alias {} already exists and points to different bucket: {:?} in namespace of key {}", query.new_name, existing_alias, key.key_id))); + return Err(Error::BadRequest(format!("Alias {} already exists and points to different bucket: {:?} in namespace of key {}", query.new_name, existing_alias, key.key_id))); } } if !is_valid_bucket_name(&query.new_name) { - return Err(Error::Message(format!( + return Err(Error::BadRequest(format!( INVALID_BUCKET_NAME_MESSAGE!(), query.new_name ))); @@ -312,7 +316,7 @@ impl AdminRpcHandler { query.new_name, bucket_id ))); } else { - return Err(Error::Message(format!( + return Err(Error::BadRequest(format!( "Alias {} already exists and points to different bucket: {:?}", query.new_name, p.bucket_id ))); @@ -330,7 +334,7 @@ impl AdminRpcHandler { let alias = match alias { None => BucketAlias::new(query.new_name.clone(), bucket_id) - .ok_or_message(format!(INVALID_BUCKET_NAME_MESSAGE!(), query.new_name))?, + .ok_or_bad_request(format!(INVALID_BUCKET_NAME_MESSAGE!(), query.new_name))?, Some(mut a) => { a.state = Lww::raw(alias_ts, Deletable::present(AliasParams { bucket_id })); a @@ -360,7 +364,7 @@ impl AdminRpcHandler { .get(&query.name) .map(|a| a.into_option()) .flatten() - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let mut bucket = self .garage .bucket_helper() @@ -379,7 +383,7 @@ impl AdminRpcHandler { .iter() .any(|((k, n), _, active)| *k == key.key_id && *n == query.name && *active); if !has_other_aliases { - return Err(Error::Message(format!("Bucket {} doesn't have other aliases, please delete it instead of just unaliasing.", query.name))); + return Err(Error::BadRequest(format!("Bucket {} doesn't have other aliases, please delete it instead of just unaliasing.", query.name))); } // Checks ok, remove alias @@ -410,7 +414,7 @@ impl AdminRpcHandler { .bucket_helper() .resolve_global_bucket_name(&query.name) .await? - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let mut bucket = self .garage .bucket_helper() @@ -429,7 +433,7 @@ impl AdminRpcHandler { .iter() .any(|(_, _, active)| *active); if !has_other_aliases { - return Err(Error::Message(format!("Bucket {} doesn't have other aliases, please delete it instead of just unaliasing.", query.name))); + return Err(Error::BadRequest(format!("Bucket {} doesn't have other aliases, please delete it instead of just unaliasing.", query.name))); } let mut alias = self @@ -461,7 +465,7 @@ impl AdminRpcHandler { .bucket_helper() .resolve_global_bucket_name(&query.bucket) .await? - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let bucket = self .garage .bucket_helper() @@ -491,7 +495,7 @@ impl AdminRpcHandler { .bucket_helper() .resolve_global_bucket_name(&query.bucket) .await? - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let bucket = self .garage .bucket_helper() @@ -521,7 +525,7 @@ impl AdminRpcHandler { .bucket_helper() .resolve_global_bucket_name(&query.bucket) .await? - .ok_or_message("Bucket not found")?; + .ok_or_bad_request("Bucket not found")?; let mut bucket = self .garage @@ -531,7 +535,7 @@ impl AdminRpcHandler { let bucket_state = bucket.state.as_option_mut().unwrap(); if !(query.allow ^ query.deny) { - return Err(Error::Message( + return Err(Error::BadRequest( "You must specify exactly one flag, either --allow or --deny".to_string(), )); } @@ -606,7 +610,7 @@ impl AdminRpcHandler { async fn handle_delete_key(&self, query: &KeyDeleteOpt) -> Result { let mut key = self.get_existing_key(&query.key_pattern).await?; if !query.yes { - return Err(Error::BadRpc( + return Err(Error::BadRequest( "Add --yes flag to really perform this operation".to_string(), )); } @@ -659,7 +663,7 @@ impl AdminRpcHandler { async fn handle_import_key(&self, query: &KeyImportOpt) -> Result { let prev_key = self.garage.key_table.get(&EmptyKey, &query.key_id).await?; if prev_key.is_some() { - return Err(Error::Message(format!("Key {} already exists in data store. Even if it is deleted, we can't let you create a new key with the same ID. Sorry.", query.key_id))); + return Err(Error::BadRequest(format!("Key {} already exists in data store. Even if it is deleted, we can't let you create a new key with the same ID. Sorry.", query.key_id))); } let imported_key = Key::import(&query.key_id, &query.secret_key, &query.name); self.garage.key_table.insert(&imported_key).await?; @@ -682,7 +686,7 @@ impl AdminRpcHandler { .filter(|k| !k.state.is_deleted()) .collect::>(); if candidates.len() != 1 { - Err(Error::Message(format!( + Err(Error::BadRequest(format!( "{} matching keys", candidates.len() ))) @@ -760,7 +764,7 @@ impl AdminRpcHandler { async fn handle_migrate(self: &Arc, opt: MigrateOpt) -> Result { if !opt.yes { - return Err(Error::BadRpc( + return Err(Error::BadRequest( "Please provide the --yes flag to initiate migration operation.".to_string(), )); } @@ -776,7 +780,7 @@ impl AdminRpcHandler { async fn handle_launch_repair(self: &Arc, opt: RepairOpt) -> Result { if !opt.yes { - return Err(Error::BadRpc( + return Err(Error::BadRequest( "Please provide the --yes flag to initiate repair operations.".to_string(), )); } @@ -803,7 +807,7 @@ impl AdminRpcHandler { if failures.is_empty() { Ok(AdminRpc::Ok("Repair launched on all nodes".to_string())) } else { - Err(Error::Message(format!( + Err(Error::BadRequest(format!( "Could not launch repair on nodes: {:?} (launched successfully on other nodes)", failures ))) @@ -946,7 +950,7 @@ impl EndpointHandler for AdminRpcHandler { AdminRpc::Migrate(opt) => self.handle_migrate(opt.clone()).await, AdminRpc::LaunchRepair(opt) => self.handle_launch_repair(opt.clone()).await, AdminRpc::Stats(opt) => self.handle_stats(opt.clone()).await, - _ => Err(Error::BadRpc("Invalid RPC".to_string())), + m => Err(GarageError::unexpected_rpc_message(m).into()), } } } diff --git a/src/garage/cli/cmd.rs b/src/garage/cli/cmd.rs index 834261e4..cca7c401 100644 --- a/src/garage/cli/cmd.rs +++ b/src/garage/cli/cmd.rs @@ -6,6 +6,8 @@ use garage_rpc::layout::*; use garage_rpc::system::*; use garage_rpc::*; +use garage_model::helper::error::Error as HelperError; + use crate::admin::*; use crate::cli::*; @@ -14,14 +16,14 @@ pub async fn cli_command_dispatch( system_rpc_endpoint: &Endpoint, admin_rpc_endpoint: &Endpoint, rpc_host: NodeID, -) -> Result<(), Error> { +) -> Result<(), HelperError> { match cmd { - Command::Status => cmd_status(system_rpc_endpoint, rpc_host).await, + Command::Status => Ok(cmd_status(system_rpc_endpoint, rpc_host).await?), Command::Node(NodeOperation::Connect(connect_opt)) => { - cmd_connect(system_rpc_endpoint, rpc_host, connect_opt).await + Ok(cmd_connect(system_rpc_endpoint, rpc_host, connect_opt).await?) } Command::Layout(layout_opt) => { - cli_layout_command_dispatch(layout_opt, system_rpc_endpoint, rpc_host).await + Ok(cli_layout_command_dispatch(layout_opt, system_rpc_endpoint, rpc_host).await?) } Command::Bucket(bo) => { cmd_admin(admin_rpc_endpoint, rpc_host, AdminRpc::BucketOperation(bo)).await @@ -149,7 +151,7 @@ pub async fn cmd_connect( println!("Success."); Ok(()) } - r => Err(Error::BadRpc(format!("Unexpected response: {:?}", r))), + m => Err(Error::unexpected_rpc_message(m)), } } @@ -157,7 +159,7 @@ pub async fn cmd_admin( rpc_cli: &Endpoint, rpc_host: NodeID, args: AdminRpc, -) -> Result<(), Error> { +) -> Result<(), HelperError> { match rpc_cli.call(&rpc_host, &args, PRIO_NORMAL).await?? { AdminRpc::Ok(msg) => { println!("{}", msg); diff --git a/src/garage/main.rs b/src/garage/main.rs index 69cd16e7..60a13ac7 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -22,6 +22,8 @@ use garage_util::error::*; use garage_rpc::system::*; use garage_rpc::*; +use garage_model::helper::error::Error as HelperError; + use admin::*; use cli::*; @@ -136,5 +138,9 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { let system_rpc_endpoint = netapp.endpoint::(SYSTEM_RPC_PATH.into()); let admin_rpc_endpoint = netapp.endpoint::(ADMIN_RPC_PATH.into()); - cli_command_dispatch(opt.cmd, &system_rpc_endpoint, &admin_rpc_endpoint, id).await + match cli_command_dispatch(opt.cmd, &system_rpc_endpoint, &admin_rpc_endpoint, id).await { + Err(HelperError::Internal(i)) => Err(i), + Err(HelperError::BadRequest(b)) => Err(Error::Message(format!("bad request: {}", b))), + Ok(x) => Ok(x), + } } 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 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/bucket_helper.rs b/src/model/bucket_helper.rs deleted file mode 100644 index b55ebc4b..00000000 --- a/src/model/bucket_helper.rs +++ /dev/null @@ -1,63 +0,0 @@ -use garage_util::data::*; -use garage_util::error::*; - -use garage_table::util::EmptyKey; - -use crate::bucket_table::Bucket; -use crate::garage::Garage; - -pub struct BucketHelper<'a>(pub(crate) &'a Garage); - -impl<'a> BucketHelper<'a> { - #[allow(clippy::ptr_arg)] - pub async fn resolve_global_bucket_name( - &self, - bucket_name: &String, - ) -> Result, Error> { - // Bucket names in Garage are aliases, true bucket identifiers - // are 32-byte UUIDs. This function resolves bucket names into - // their full identifier by looking up in the bucket_alias_table. - // This function also allows buckets to be identified by their - // full UUID (hex-encoded). Here, if the name to be resolved is a - // hex string of the correct length, it is directly parsed as a bucket - // identifier which is returned. There is no risk of this conflicting - // with an actual bucket name: bucket names are max 63 chars long by - // the AWS spec, and hex-encoded UUIDs are 64 chars long. - let hexbucket = hex::decode(bucket_name.as_str()) - .ok() - .map(|by| Uuid::try_from(&by)) - .flatten(); - if let Some(bucket_id) = hexbucket { - Ok(self - .0 - .bucket_table - .get(&bucket_id, &EmptyKey) - .await? - .filter(|x| !x.state.is_deleted()) - .map(|_| bucket_id)) - } else { - Ok(self - .0 - .bucket_alias_table - .get(&EmptyKey, bucket_name) - .await? - .map(|x| x.state.get().as_option().map(|x| x.bucket_id)) - .flatten()) - } - } - - pub async fn get_existing_bucket(&self, bucket_id: Uuid) -> Result { - self.0 - .bucket_table - .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 - ))) - }) - } -} 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/helper/bucket.rs b/src/model/helper/bucket.rs new file mode 100644 index 00000000..e89a723d --- /dev/null +++ b/src/model/helper/bucket.rs @@ -0,0 +1,56 @@ +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); + +impl<'a> BucketHelper<'a> { + #[allow(clippy::ptr_arg)] + pub async fn resolve_global_bucket_name( + &self, + bucket_name: &String, + ) -> Result, Error> { + // Bucket names in Garage are aliases, true bucket identifiers + // are 32-byte UUIDs. This function resolves bucket names into + // their full identifier by looking up in the bucket_alias_table. + // This function also allows buckets to be identified by their + // full UUID (hex-encoded). Here, if the name to be resolved is a + // hex string of the correct length, it is directly parsed as a bucket + // identifier which is returned. There is no risk of this conflicting + // with an actual bucket name: bucket names are max 63 chars long by + // the AWS spec, and hex-encoded UUIDs are 64 chars long. + let hexbucket = hex::decode(bucket_name.as_str()) + .ok() + .map(|by| Uuid::try_from(&by)) + .flatten(); + if let Some(bucket_id) = hexbucket { + Ok(self + .0 + .bucket_table + .get(&bucket_id, &EmptyKey) + .await? + .filter(|x| !x.state.is_deleted()) + .map(|_| bucket_id)) + } else { + Ok(self + .0 + .bucket_alias_table + .get(&EmptyKey, bucket_name) + .await? + .map(|x| x.state.get().as_option().map(|x| x.bucket_id)) + .flatten()) + } + } + + pub async fn get_existing_bucket(&self, bucket_id: Uuid) -> Result { + self.0 + .bucket_table + .get(&bucket_id, &EmptyKey) + .await? + .filter(|b| !b.is_deleted()) + .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 for Error { + fn from(e: netapp::error::Error) -> Self { + Error::Internal(GarageError::Netapp(e)) + } +} + +pub trait OkOrBadRequest { + type S; + fn ok_or_bad_request>(self, reason: M) -> Result; +} + +impl OkOrBadRequest for Result +where + E: std::fmt::Display, +{ + type S = T; + fn ok_or_bad_request>(self, reason: M) -> Result { + match self { + Ok(x) => Ok(x), + Err(e) => Err(Error::BadRequest(format!( + "{}: {}", + reason.as_ref(), + e.to_string() + ))), + } + } +} + +impl OkOrBadRequest for Option { + type S = T; + fn ok_or_bad_request>(self, reason: M) -> Result { + 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; diff --git a/src/rpc/system.rs b/src/rpc/system.rs index aa8947ea..6bca6e3e 100644 --- a/src/rpc/system.rs +++ b/src/rpc/system.rs @@ -576,7 +576,7 @@ impl EndpointHandler for System { self.clone().handle_advertise_cluster_layout(adv).await } SystemRpc::GetKnownNodes => Ok(self.handle_get_known_nodes()), - _ => Err(Error::BadRpc("Unexpected RPC message".to_string())), + m => Err(Error::unexpected_rpc_message(m)), } } } diff --git a/src/table/gc.rs b/src/table/gc.rs index 5cb8cb9b..8d0a5bef 100644 --- a/src/table/gc.rs +++ b/src/table/gc.rs @@ -315,7 +315,7 @@ where } Ok(GcRpc::Ok) } - _ => Err(Error::Message("Unexpected GC RPC".to_string())), + m => Err(Error::unexpected_rpc_message(m)), } } } diff --git a/src/table/sync.rs b/src/table/sync.rs index df9fb4d0..1df2b01d 100644 --- a/src/table/sync.rs +++ b/src/table/sync.rs @@ -514,10 +514,7 @@ where if let SyncRpc::Ok = rpc_resp { Ok(()) } else { - Err(Error::Message(format!( - "Unexpected response to RPC Update: {}", - debug_serialize(&rpc_resp) - ))) + Err(Error::unexpected_rpc_message(rpc_resp)) } } } @@ -545,7 +542,7 @@ where self.data.update_many(items)?; Ok(SyncRpc::Ok) } - _ => Err(Error::Message("Unexpected sync RPC".to_string())), + m => Err(Error::unexpected_rpc_message(m)), } } } diff --git a/src/table/table.rs b/src/table/table.rs index 396888c1..01789c11 100644 --- a/src/table/table.rs +++ b/src/table/table.rs @@ -311,7 +311,7 @@ where self.data.update_many(pairs)?; Ok(TableRpc::Ok) } - _ => Err(Error::BadRpc("Unexpected table RPC".to_string())), + m => Err(Error::unexpected_rpc_message(m)), } } } diff --git a/src/util/error.rs b/src/util/error.rs index ef5a76f2..bdb3a69b 100644 --- a/src/util/error.rs +++ b/src/util/error.rs @@ -59,8 +59,8 @@ pub enum Error { )] Quorum(usize, usize, usize, Vec), - #[error(display = "Bad RPC: {}", _0)] - BadRpc(String), + #[error(display = "Unexpected RPC message: {}", _0)] + UnexpectedRpcMessage(String), #[error(display = "Corrupt data: does not match hash {:?}", _0)] CorruptData(Hash), @@ -69,6 +69,12 @@ pub enum Error { Message(String), } +impl Error { + pub fn unexpected_rpc_message(v: T) -> Self { + Self::UnexpectedRpcMessage(debug_serialize(&v)) + } +} + impl From> for Error { fn from(e: sled::transaction::TransactionError) -> Error { match e { -- cgit v1.2.3