From 47921b67f0efb8d493e44ef683e7dc4cc34cf677 Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Tue, 30 Apr 2024 18:14:44 +0200 Subject: [PATCH 1/2] feat(storey): more informative key decode errors --- crates/storey/src/containers/column.rs | 13 ++++++---- crates/storey/src/containers/item.rs | 11 ++++++--- crates/storey/src/containers/map.rs | 33 +++++++++++++++++++++----- crates/storey/src/containers/mod.rs | 19 +++++++-------- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/crates/storey/src/containers/column.rs b/crates/storey/src/containers/column.rs index 0ebac3f..a20ed96 100644 --- a/crates/storey/src/containers/column.rs +++ b/crates/storey/src/containers/column.rs @@ -7,7 +7,7 @@ use crate::encoding::{DecodableWith, EncodableWith}; use crate::storage::{IterableStorage, StorageBranch}; use crate::storage::{Storage, StorageMut}; -use super::{IterableAccessor, KeyDecodeError, Storable}; +use super::{IterableAccessor, Storable}; const META_NEXT_IX: &[u8] = &[0]; const META_LEN: &[u8] = &[1]; @@ -87,6 +87,7 @@ where { type AccessorT = ColumnAccess; type Key = u32; + type KeyDecodeError = ColumnKeyDecodeError; type Value = T; type ValueDecodeError = E::DecodeError; @@ -97,7 +98,7 @@ where } } - fn decode_key(key: &[u8]) -> Result { + fn decode_key(key: &[u8]) -> Result { let key = decode_ix(key)?; Ok(key) @@ -108,6 +109,10 @@ where } } +#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)] +#[error("invalid key length, expected 4 bytes of big-endian u32")] +pub struct ColumnKeyDecodeError; + /// An accessor for a `Column`. /// /// This type provides methods for interacting with the column in storage. @@ -216,9 +221,9 @@ where } } -fn decode_ix(key: &[u8]) -> Result { +fn decode_ix(key: &[u8]) -> Result { if key.len() != 4 { - return Err(KeyDecodeError); + return Err(ColumnKeyDecodeError); } let row_key = u32::from_be_bytes([key[0], key[1], key[2], key[3]]); diff --git a/crates/storey/src/containers/item.rs b/crates/storey/src/containers/item.rs index 3d338f3..68bba2a 100644 --- a/crates/storey/src/containers/item.rs +++ b/crates/storey/src/containers/item.rs @@ -4,7 +4,7 @@ use crate::encoding::{DecodableWith, EncodableWith, Encoding}; use crate::storage::StorageBranch; use crate::storage::{Storage, StorageMut}; -use super::{KeyDecodeError, Storable}; +use super::Storable; /// A single item in the storage. /// @@ -72,6 +72,7 @@ where { type AccessorT = ItemAccess; type Key = (); + type KeyDecodeError = ItemKeyDecodeError; type Value = T; type ValueDecodeError = E::DecodeError; @@ -82,11 +83,11 @@ where } } - fn decode_key(key: &[u8]) -> Result<(), KeyDecodeError> { + fn decode_key(key: &[u8]) -> Result<(), ItemKeyDecodeError> { if key.is_empty() { Ok(()) } else { - Err(KeyDecodeError) + Err(ItemKeyDecodeError) } } @@ -95,6 +96,10 @@ where } } +#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)] +#[error("invalid key length, expected empty key")] +pub struct ItemKeyDecodeError; + /// An accessor for an `Item`. /// /// This type provides methods to get and set the value of the item. diff --git a/crates/storey/src/containers/map.rs b/crates/storey/src/containers/map.rs index 0d0ca71..4f06a41 100644 --- a/crates/storey/src/containers/map.rs +++ b/crates/storey/src/containers/map.rs @@ -4,7 +4,7 @@ use crate::storage::IterableStorage; use crate::storage::StorageBranch; use super::IterableAccessor; -use super::{KeyDecodeError, Storable}; +use super::Storable; /// A map that stores values of type `V` under keys of type `K`. /// @@ -51,6 +51,7 @@ impl Map where K: OwnedKey, V: Storable, + ::KeyDecodeError: std::fmt::Display, { /// Creates a new map with the given prefix. /// @@ -92,9 +93,11 @@ impl Storable for Map where K: OwnedKey, V: Storable, + ::KeyDecodeError: std::fmt::Display, { type AccessorT = MapAccess; type Key = (K, V::Key); + type KeyDecodeError = MapKeyDecodeError; type Value = V::Value; type ValueDecodeError = V::ValueDecodeError; @@ -105,15 +108,16 @@ where } } - fn decode_key(key: &[u8]) -> Result { - let len = *key.first().ok_or(KeyDecodeError)? as usize; + fn decode_key(key: &[u8]) -> Result> { + let len = *key.first().ok_or(MapKeyDecodeError::EmptyKey)? as usize; if key.len() < len + 1 { - return Err(KeyDecodeError); + return Err(MapKeyDecodeError::KeyTooShort(len)); } - let map_key = K::from_bytes(&key[1..len + 1]).map_err(|_| KeyDecodeError)?; - let rest = V::decode_key(&key[len + 1..])?; + let map_key = + K::from_bytes(&key[1..len + 1]).map_err(|_| MapKeyDecodeError::InvalidUtf8)?; + let rest = V::decode_key(&key[len + 1..]).map_err(MapKeyDecodeError::Inner)?; Ok((map_key, rest)) } @@ -123,6 +127,22 @@ where } } +#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)] +#[error("invalid key length, expected empty key")] +pub enum MapKeyDecodeError { + #[error("empty key, expected length prefix (1 byte)")] + EmptyKey, + + #[error("key too short, expected {0} bytes after length prefix")] + KeyTooShort(usize), + + #[error("invalid UTF8")] + InvalidUtf8, + + #[error("sub key decode error: {0}")] + Inner(I), +} + /// An accessor for a map. /// /// The accessor provides methods for interacting with the map in storage. @@ -227,6 +247,7 @@ impl IterableAccessor for MapAccess where K: OwnedKey, V: Storable, + ::KeyDecodeError: std::fmt::Display, S: IterableStorage, { type StorableT = Map; diff --git a/crates/storey/src/containers/mod.rs b/crates/storey/src/containers/mod.rs index 91a1f68..9576d3d 100644 --- a/crates/storey/src/containers/mod.rs +++ b/crates/storey/src/containers/mod.rs @@ -31,6 +31,9 @@ pub trait Storable { /// Containers that store one item and don't manage subkeys should use the `()` type here. type Key; + /// The error type for decoding keys. + type KeyDecodeError; + /// The Value type for this collection/container. This is the type that will be used for /// value iteration. type Value; @@ -47,7 +50,7 @@ pub trait Storable { /// /// This method is used in key iteration to provide a typed key rather than raw bytes /// to the user. - fn decode_key(key: &[u8]) -> Result; + fn decode_key(key: &[u8]) -> Result; /// Decode a value from a byte slice. /// @@ -56,14 +59,10 @@ pub trait Storable { fn decode_value(value: &[u8]) -> Result; } -/// A key decoding error. -#[derive(Debug, PartialEq)] -pub struct KeyDecodeError; - /// A key-value pair decoding error. #[derive(Debug, PartialEq)] -pub enum KVDecodeError { - Key, +pub enum KVDecodeError { + Key(K), Value(V), } @@ -133,12 +132,12 @@ where S: Storable, B: IterableStorage + 'i, { - type Item = Result<(S::Key, S::Value), KVDecodeError>; + type Item = Result<(S::Key, S::Value), KVDecodeError>; fn next(&mut self) -> Option { self.inner.next().map(|(k, v)| -> Self::Item { match (S::decode_key(&k), S::decode_value(&v)) { - (Err(_), _) => Err(KVDecodeError::Key), + (Err(e), _) => Err(KVDecodeError::Key(e)), (_, Err(e)) => Err(KVDecodeError::Value(e)), (Ok(k), Ok(v)) => Ok((k, v)), } @@ -161,7 +160,7 @@ where S: Storable, B: IterableStorage + 'i, { - type Item = Result; + type Item = Result; fn next(&mut self) -> Option { self.inner.next().map(|k| S::decode_key(&k)) From d6987ca80c530c916c619da3dbcadc6bdefc0ee0 Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Mon, 6 May 2024 17:48:10 +0200 Subject: [PATCH 2/2] feat(storey): `Map`: more informative key decode error --- crates/storey/src/containers/map.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/storey/src/containers/map.rs b/crates/storey/src/containers/map.rs index 4f06a41..c91de66 100644 --- a/crates/storey/src/containers/map.rs +++ b/crates/storey/src/containers/map.rs @@ -263,7 +263,9 @@ pub trait Key { } pub trait OwnedKey: Key { - fn from_bytes(bytes: &[u8]) -> Result + type Error; + + fn from_bytes(bytes: &[u8]) -> Result where Self: Sized; } @@ -274,12 +276,20 @@ impl Key for String { } } +#[derive(Debug, PartialEq, Eq, Clone, Copy, thiserror::Error)] +#[error("invalid UTF8")] +pub struct InvalidUtf8; + impl OwnedKey for String { - fn from_bytes(bytes: &[u8]) -> Result + type Error = InvalidUtf8; + + fn from_bytes(bytes: &[u8]) -> Result where Self: Sized, { - std::str::from_utf8(bytes).map(String::from).map_err(|_| ()) + std::str::from_utf8(bytes) + .map(String::from) + .map_err(|_| InvalidUtf8) } }