Added source to existing errors

Signed-off-by: Alex Snaps <[email protected]>
Kuadrant · Nov 25, 2024 · 7e85052 · 7e85052
1 parent c496e98
commit 7e85052
Show file tree

Hide file tree

Showing 8 changed files with 139 additions and 124 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/limitador/src/errors.rs b/limitador/src/errors.rs
@@ -1,14 +1,14 @@
 use crate::storage::StorageErr;
 use thiserror::Error;
 
-#[derive(Error, Debug, Eq, PartialEq)]
+#[derive(Error, Debug)]
 pub enum LimitadorError {
     #[error("error while accessing the limits storage: {0:?}")]
-    Storage(String),
+    Storage(StorageErr),
 }
 
 impl From<StorageErr> for LimitadorError {
     fn from(e: StorageErr) -> Self {
-        Self::Storage(e.msg().to_owned())
+        Self::Storage(e)
     }
 }
diff --git a/limitador/src/lib.rs b/limitador/src/lib.rs
@@ -221,6 +221,8 @@ pub struct RateLimiterBuilder {
     storage: Storage,
 }
 
+type LimitadorResult<T> = Result<T, LimitadorError>;
+
 pub struct CheckResult {
     pub limited: bool,
     pub counters: Vec<Counter>,
@@ -336,7 +338,7 @@ impl RateLimiter {
         self.storage.add_limit(limit)
     }
 
-    pub fn delete_limit(&self, limit: &Limit) -> Result<(), LimitadorError> {
+    pub fn delete_limit(&self, limit: &Limit) -> LimitadorResult<()> {
         self.storage.delete_limit(limit)?;
         Ok(())
     }
@@ -349,7 +351,7 @@ impl RateLimiter {
             .collect()
     }
 
-    pub fn delete_limits(&self, namespace: &Namespace) -> Result<(), LimitadorError> {
+    pub fn delete_limits(&self, namespace: &Namespace) -> LimitadorResult<()> {
         self.storage.delete_limits(namespace)?;
         Ok(())
     }
@@ -359,7 +361,7 @@ impl RateLimiter {
         namespace: &Namespace,
         values: &HashMap<String, String>,
         delta: u64,
-    ) -> Result<bool, LimitadorError> {
+    ) -> LimitadorResult<bool> {
         let counters = self.counters_that_apply(namespace, values)?;
 
         for counter in counters {
@@ -381,7 +383,7 @@ impl RateLimiter {
         namespace: &Namespace,
         values: &HashMap<String, String>,
         delta: u64,
-    ) -> Result<(), LimitadorError> {
+    ) -> LimitadorResult<()> {
         let counters = self.counters_that_apply(namespace, values)?;
 
         counters
@@ -396,7 +398,7 @@ impl RateLimiter {
         values: &HashMap<String, String>,
         delta: u64,
         load_counters: bool,
-    ) -> Result<CheckResult, LimitadorError> {
+    ) -> LimitadorResult<CheckResult> {
         let mut counters = self.counters_that_apply(namespace, values)?;
 
         if counters.is_empty() {
@@ -431,7 +433,7 @@ impl RateLimiter {
         }
     }
 
-    pub fn get_counters(&self, namespace: &Namespace) -> Result<HashSet<Counter>, LimitadorError> {
+    pub fn get_counters(&self, namespace: &Namespace) -> LimitadorResult<HashSet<Counter>> {
         self.storage
             .get_counters(namespace)
             .map_err(|err| err.into())
@@ -440,10 +442,7 @@ impl RateLimiter {
     // Deletes all the limits stored except the ones received in the params. For
     // every limit received, if it does not exist, it is created. If it already
     // exists, its associated counters are not reset.
-    pub fn configure_with(
-        &self,
-        limits: impl IntoIterator<Item = Limit>,
-    ) -> Result<(), LimitadorError> {
+    pub fn configure_with(&self, limits: impl IntoIterator<Item = Limit>) -> LimitadorResult<()> {
         let limits_to_keep_or_create = classify_limits_by_namespace(limits);
 
         let namespaces_limits_to_keep_or_create: HashSet<Namespace> =
@@ -479,7 +478,7 @@ impl RateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-    ) -> Result<Vec<Counter>, LimitadorError> {
+    ) -> LimitadorResult<Vec<Counter>> {
         let limits = self.storage.get_limits(namespace);
 
         let counters = limits
@@ -512,7 +511,7 @@ impl AsyncRateLimiter {
         self.storage.add_limit(limit)
     }
 
-    pub async fn delete_limit(&self, limit: &Limit) -> Result<(), LimitadorError> {
+    pub async fn delete_limit(&self, limit: &Limit) -> LimitadorResult<()> {
         self.storage.delete_limit(limit).await?;
         Ok(())
     }
@@ -525,7 +524,7 @@ impl AsyncRateLimiter {
             .collect()
     }
 
-    pub async fn delete_limits(&self, namespace: &Namespace) -> Result<(), LimitadorError> {
+    pub async fn delete_limits(&self, namespace: &Namespace) -> LimitadorResult<()> {
         self.storage.delete_limits(namespace).await?;
         Ok(())
     }
@@ -535,7 +534,7 @@ impl AsyncRateLimiter {
         namespace: &Namespace,
         values: &HashMap<String, String>,
         delta: u64,
-    ) -> Result<bool, LimitadorError> {
+    ) -> LimitadorResult<bool> {
         let counters = self.counters_that_apply(namespace, values).await?;
 
         for counter in counters {
@@ -556,7 +555,7 @@ impl AsyncRateLimiter {
         namespace: &Namespace,
         values: &HashMap<String, String>,
         delta: u64,
-    ) -> Result<(), LimitadorError> {
+    ) -> LimitadorResult<()> {
         let counters = self.counters_that_apply(namespace, values).await?;
 
         for counter in counters {
@@ -572,7 +571,7 @@ impl AsyncRateLimiter {
         values: &HashMap<String, String>,
         delta: u64,
         load_counters: bool,
-    ) -> Result<CheckResult, LimitadorError> {
+    ) -> LimitadorResult<CheckResult> {
         // the above where-clause is needed in order to call unwrap().
         let mut counters = self.counters_that_apply(namespace, values).await?;
 
@@ -609,10 +608,7 @@ impl AsyncRateLimiter {
         }
     }
 
-    pub async fn get_counters(
-        &self,
-        namespace: &Namespace,
-    ) -> Result<HashSet<Counter>, LimitadorError> {
+    pub async fn get_counters(&self, namespace: &Namespace) -> LimitadorResult<HashSet<Counter>> {
         self.storage
             .get_counters(namespace)
             .await
@@ -625,7 +621,7 @@ impl AsyncRateLimiter {
     pub async fn configure_with(
         &self,
         limits: impl IntoIterator<Item = Limit>,
-    ) -> Result<(), LimitadorError> {
+    ) -> LimitadorResult<()> {
         let limits_to_keep_or_create = classify_limits_by_namespace(limits);
 
         let namespaces_limits_to_keep_or_create: HashSet<Namespace> =
@@ -661,7 +657,7 @@ impl AsyncRateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-    ) -> Result<Vec<Counter>, LimitadorError> {
+    ) -> LimitadorResult<Vec<Counter>> {
         let limits = self.storage.get_limits(namespace);
 
         let counters = limits

diff --git a/limitador/src/storage/disk/expiring_value.rs b/limitador/src/storage/disk/expiring_value.rs
@@ -95,9 +95,10 @@ impl From<ExpiringValue> for Vec<u8> {
 }
 
 impl From<TryFromSliceError> for StorageErr {
-    fn from(_: TryFromSliceError) -> Self {
+    fn from(e: TryFromSliceError) -> Self {
         Self {
             msg: "Corrupted byte sequence while reading 8 bytes for 64-bit integer".to_owned(),
+            source: Some(Box::new(e)),
             transient: false,
         }
     }

diff --git a/limitador/src/storage/disk/mod.rs b/limitador/src/storage/disk/mod.rs
@@ -8,9 +8,11 @@ pub use rocksdb_storage::RocksDbStorage as DiskStorage;
 
 impl From<rocksdb::Error> for StorageErr {
     fn from(error: rocksdb::Error) -> Self {
+        let transient = error.kind() == ErrorKind::TimedOut || error.kind() == ErrorKind::TryAgain;
         Self {
             msg: format!("Underlying storage error: {error}"),
-            transient: error.kind() == ErrorKind::TimedOut || error.kind() == ErrorKind::TryAgain,
+            source: Some(Box::new(error)),
+            transient,
         }
     }
 }

diff --git a/limitador/src/storage/mod.rs b/limitador/src/storage/mod.rs
@@ -3,8 +3,9 @@ use crate::limit::{Limit, Namespace};
 use crate::InMemoryStorage;
 use async_trait::async_trait;
 use std::collections::{HashMap, HashSet};
+use std::error::Error;
+use std::fmt::{Display, Formatter};
 use std::sync::{Arc, RwLock};
-use thiserror::Error;
 
 #[cfg(feature = "disk_storage")]
 pub mod disk;
@@ -308,13 +309,25 @@ pub trait AsyncCounterStorage: Sync + Send {
     async fn clear(&self) -> Result<(), StorageErr>;
 }
 
-#[derive(Error, Debug)]
-#[error("error while accessing the limits storage: {msg}")]
+#[derive(Debug)]
 pub struct StorageErr {
     msg: String,
+    source: Option<Box<dyn Error + 'static>>,
     transient: bool,
 }
 
+impl Display for StorageErr {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "error while accessing the limits storage: {}", self.msg)
+    }
+}
+
+impl Error for StorageErr {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        self.source.as_ref().map(|source| source.as_ref())
+    }
+}
+
 impl StorageErr {
     pub fn msg(&self) -> &str {
         &self.msg

diff --git a/limitador/src/storage/redis/mod.rs b/limitador/src/storage/redis/mod.rs
@@ -21,12 +21,14 @@ pub use redis_sync::RedisStorage;
 
 impl From<RedisError> for StorageErr {
     fn from(e: RedisError) -> Self {
+        let transient = e.is_timeout()
+            || e.is_connection_dropped()
+            || e.is_cluster_error()
+            || e.is_connection_refusal();
         Self {
             msg: e.to_string(),
-            transient: e.is_timeout()
-                || e.is_connection_dropped()
-                || e.is_cluster_error()
-                || e.is_connection_refusal(),
+            source: Some(Box::new(e)),
+            transient,
         }
     }
 }

diff --git a/limitador/src/storage/redis/redis_sync.rs b/limitador/src/storage/redis/redis_sync.rs
@@ -236,6 +236,7 @@ impl From<::r2d2::Error> for StorageErr {
     fn from(e: ::r2d2::Error) -> Self {
         Self {
             msg: e.to_string(),
+            source: Some(Box::new(e)),
             transient: false,
         }
     }