From 1d25e48d4a35de5ee782a96961ae2a9727efe475 Mon Sep 17 00:00:00 2001 From: Miraculous Owonubi Date: Thu, 12 Dec 2024 00:38:30 +0100 Subject: [PATCH] chore: revise error handling for kv-store (#1015) --- Cargo.lock | 1 + apps/kv-store/Cargo.toml | 1 + apps/kv-store/src/lib.rs | 34 +++++++++++++++---------- apps/visited/src/lib.rs | 2 +- crates/merod/src/cli/relay.rs | 8 +----- crates/storage/src/address.rs | 2 +- crates/storage/src/collections/error.rs | 10 ++++++++ 7 files changed, 36 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d3b35ca8d..bb82b9871 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4034,6 +4034,7 @@ version = "0.1.0" dependencies = [ "calimero-sdk", "calimero-storage", + "thiserror 1.0.69", ] [[package]] diff --git a/apps/kv-store/Cargo.toml b/apps/kv-store/Cargo.toml index e753ec2c5..0423adbfe 100644 --- a/apps/kv-store/Cargo.toml +++ b/apps/kv-store/Cargo.toml @@ -10,5 +10,6 @@ license.workspace = true crate-type = ["cdylib"] [dependencies] +thiserror.workspace = true calimero-sdk.workspace = true calimero-storage.workspace = true diff --git a/apps/kv-store/src/lib.rs b/apps/kv-store/src/lib.rs index 807b0b521..97f4a7df9 100644 --- a/apps/kv-store/src/lib.rs +++ b/apps/kv-store/src/lib.rs @@ -3,12 +3,13 @@ use std::collections::BTreeMap; use calimero_sdk::borsh::{BorshDeserialize, BorshSerialize}; -use calimero_sdk::types::Error; +use calimero_sdk::serde::Serialize; use calimero_sdk::{app, env}; -use calimero_storage::collections::UnorderedMap; +use calimero_storage::collections::{StoreError, UnorderedMap}; +use thiserror::Error; #[app::state(emits = for<'a> Event<'a>)] -#[derive(Debug, PartialEq, PartialOrd, BorshSerialize, BorshDeserialize)] +#[derive(Debug, BorshSerialize, BorshDeserialize)] #[borsh(crate = "calimero_sdk::borsh")] pub struct KvStore { items: UnorderedMap, @@ -22,6 +23,16 @@ pub enum Event<'a> { Cleared, } +#[derive(Debug, Error, Serialize)] +#[serde(crate = "calimero_sdk::serde")] +#[serde(tag = "kind", content = "data")] +pub enum Error<'a> { + #[error("key not found: {0}")] + NotFound(&'a str), + #[error("store error: {0}")] + StoreError(#[from] StoreError), +} + #[app::logic] impl KvStore { #[app::init] @@ -34,7 +45,7 @@ impl KvStore { pub fn set(&mut self, key: String, value: String) -> Result<(), Error> { env::log(&format!("Setting key: {:?} to value: {:?}", key, value)); - if self.items.get(&key)?.is_some() { + if self.items.contains(&key)? { app::emit!(Event::Updated { key: &key, value: &value, @@ -63,7 +74,7 @@ impl KvStore { Ok(self.items.len()?) } - pub fn get(&self, key: &str) -> Result, Error> { + pub fn get<'a>(&self, key: &'a str) -> Result, Error<'a>> { env::log(&format!("Getting key: {:?}", key)); self.items.get(key).map_err(Into::into) @@ -72,24 +83,21 @@ impl KvStore { pub fn get_unchecked(&self, key: &str) -> Result { env::log(&format!("Getting key without checking: {:?}", key)); - Ok(self.items.get(key)?.expect("Key not found.")) + Ok(self.items.get(key)?.expect("key not found")) } - pub fn get_result(&self, key: &str) -> Result { + pub fn get_result<'a>(&self, key: &'a str) -> Result> { env::log(&format!("Getting key, possibly failing: {:?}", key)); - self.get(key)?.ok_or_else(|| Error::msg("Key not found.")) + self.get(key)?.ok_or_else(|| Error::NotFound(key)) } - pub fn remove(&mut self, key: &str) -> Result { + pub fn remove(&mut self, key: &str) -> Result, Error> { env::log(&format!("Removing key: {:?}", key)); app::emit!(Event::Removed { key }); - self.items - .remove(key) - .map(|v| v.is_some()) - .map_err(Into::into) + self.items.remove(key).map_err(Into::into) } pub fn clear(&mut self) -> Result<(), Error> { diff --git a/apps/visited/src/lib.rs b/apps/visited/src/lib.rs index 22638befd..6fb6fb286 100644 --- a/apps/visited/src/lib.rs +++ b/apps/visited/src/lib.rs @@ -8,7 +8,7 @@ use calimero_sdk::types::Error; use calimero_storage::collections::{UnorderedMap, UnorderedSet}; #[app::state] -#[derive(Debug, PartialEq, PartialOrd, BorshSerialize, BorshDeserialize)] +#[derive(Debug, BorshSerialize, BorshDeserialize)] #[borsh(crate = "calimero_sdk::borsh")] pub struct VisitedCities { visited: UnorderedMap>, diff --git a/crates/merod/src/cli/relay.rs b/crates/merod/src/cli/relay.rs index 059f18177..b099520f7 100644 --- a/crates/merod/src/cli/relay.rs +++ b/crates/merod/src/cli/relay.rs @@ -10,7 +10,7 @@ use calimero_config::ConfigFile; use calimero_context_config::client::relayer::{RelayRequest, ServerError}; use calimero_context_config::client::transport::{Transport, TransportArguments, TransportRequest}; use calimero_context_config::client::Client; -use clap::{Parser, ValueEnum}; +use clap::Parser; use eyre::{bail, Result as EyreResult}; use futures_util::FutureExt; use tokio::net::TcpListener; @@ -33,12 +33,6 @@ pub struct RelayCommand { pub listen: SocketAddr, } -#[derive(Clone, Debug, ValueEnum)] -pub enum CallType { - Query, - Mutate, -} - impl RelayCommand { pub async fn run(self, root_args: RootArgs) -> EyreResult<()> { let path = root_args.home.join(root_args.node_name); diff --git a/crates/storage/src/address.rs b/crates/storage/src/address.rs index 4ad9cc3f2..1c9f23061 100644 --- a/crates/storage/src/address.rs +++ b/crates/storage/src/address.rs @@ -113,7 +113,7 @@ impl Id { impl Display for Id { #[expect(clippy::use_debug, reason = "fine for now")] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{self:?}") + write!(f, "{}", hex::encode(self.bytes)) } } diff --git a/crates/storage/src/collections/error.rs b/crates/storage/src/collections/error.rs index 1181db385..7f213de33 100644 --- a/crates/storage/src/collections/error.rs +++ b/crates/storage/src/collections/error.rs @@ -1,5 +1,6 @@ //! Error types for storage operations. +use serde::Serialize; use thiserror::Error; use crate::address::PathError; @@ -16,3 +17,12 @@ pub enum StoreError { #[error(transparent)] PathError(#[from] PathError), } + +impl Serialize for StoreError { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.collect_str(self) + } +}