chore: revise error handling for kv-store (#1015)

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

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<String, String>,
@@ -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<Option<String>, Error> {
+    pub fn get<'a>(&self, key: &'a str) -> Result<Option<String>, 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<String, Error> {
         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<String, Error> {
+    pub fn get_result<'a>(&self, key: &'a str) -> Result<String, Error<'a>> {
         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<bool, Error> {
+    pub fn remove(&mut self, key: &str) -> Result<Option<String>, 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> {

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<String, UnorderedSet<String>>,

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);

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))
     }
 }
 

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<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.collect_str(self)
+    }
+}