fix(error): Bubble error up to main when connecting to Redis fails

limitador-server/src/main.rs

@@ -109,7 +109,13 @@ impl Limiter {
     }
 
     async fn storage_using_async_redis(redis_url: &str) -> AsyncRedisStorage {
-        AsyncRedisStorage::new(redis_url).await
+        match AsyncRedisStorage::new(redis_url).await {
+            Ok(storage) => storage,
+            Err(err) => {
+                eprintln!("Failed to connect to Redis at {}: {}", redis_url, err);
+                process::exit(1)
+            }
+        }
     }
 
     async fn storage_using_redis_and_local_cache(
@@ -134,7 +140,13 @@ impl Limiter {
         cached_redis_storage = cached_redis_storage.ttl_ratio_cached_counters(cache_cfg.ttl_ratio);
         cached_redis_storage = cached_redis_storage.max_cached_counters(cache_cfg.max_counters);
 
-        cached_redis_storage.build().await
+        match cached_redis_storage.build().await {
+            Ok(storage) => storage,
+            Err(err) => {
+                eprintln!("Failed to connect to Redis at {}: {}", redis_url, err);
+                process::exit(1)
+            }
+        }
     }
 
     #[cfg(feature = "infinispan")]

limitador/src/lib.rs

@@ -25,7 +25,7 @@
 //!
 //! // Custom redis URL
 //! let rate_limiter = RateLimiter::new_with_storage(
-//!     Box::new(RedisStorage::new("redis://127.0.0.1:7777"))
+//!     Box::new(RedisStorage::new("redis://127.0.0.1:7777").unwrap())
 //! );
 //! # }
 //! ```
@@ -146,7 +146,7 @@
 //!
 //! async {
 //!     let rate_limiter = AsyncRateLimiter::new_with_storage(
-//!         Box::new(AsyncRedisStorage::new("redis://127.0.0.1:7777").await)
+//!         Box::new(AsyncRedisStorage::new("redis://127.0.0.1:7777").await.unwrap())
 //!     );
 //! };
 //! # }
@@ -171,7 +171,7 @@
 //!
 //! async {
 //!     let rate_limiter = AsyncRateLimiter::new_with_storage(
-//!         Box::new(AsyncRedisStorage::new("redis://127.0.0.1:7777").await)
+//!         Box::new(AsyncRedisStorage::new("redis://127.0.0.1:7777").await.unwrap())
 //!     );
 //!     rate_limiter.add_limit(limit);
 //! };

limitador/src/storage/redis/redis_async.rs

@@ -8,7 +8,7 @@ use crate::storage::keys::*;
 use crate::storage::redis::scripts::SCRIPT_UPDATE_COUNTER;
 use crate::storage::{AsyncCounterStorage, Authorization, StorageErr};
 use async_trait::async_trait;
-use redis::AsyncCommands;
+use redis::{AsyncCommands, RedisError};
 use std::collections::HashSet;
 use std::str::FromStr;
 use std::time::Duration;
@@ -145,14 +145,14 @@ impl AsyncCounterStorage for AsyncRedisStorage {
 }
 
 impl AsyncRedisStorage {
-    pub async fn new(redis_url: &str) -> Self {
-        Self {
+    pub async fn new(redis_url: &str) -> Result<Self, RedisError> {
+        let info = ConnectionInfo::from_str(redis_url)?;
+        Ok(Self {
             conn_manager: ConnectionManager::new(
-                redis::Client::open(ConnectionInfo::from_str(redis_url).unwrap()).unwrap(),
+                redis::Client::open(info).expect("Somehow couldn't create Redis client!"),
             )
-            .await
-            .unwrap(),
-        }
+            .await?,
+        })
     }
 
     pub fn new_with_conn_manager(conn_manager: ConnectionManager) -> Self {
@@ -173,3 +173,25 @@ impl AsyncRedisStorage {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::storage::redis::AsyncRedisStorage;
+    use redis::ErrorKind;
+
+    #[tokio::test]
+    async fn errs_on_bad_url() {
+        let result = AsyncRedisStorage::new("cassandra://127.0.0.1:6379").await;
+        assert!(result.is_err());
+        assert_eq!(result.err().unwrap().kind(), ErrorKind::InvalidClientConfig);
+    }
+
+    #[tokio::test]
+    async fn errs_on_connection_issue() {
+        let result = AsyncRedisStorage::new("redis://127.0.0.1:21").await;
+        assert!(result.is_err());
+        let error = result.err().unwrap();
+        assert_eq!(error.kind(), ErrorKind::IoError);
+        assert!(error.is_connection_refusal())
+    }
+}

limitador/src/storage/redis/redis_cached.rs

@@ -12,7 +12,7 @@ use crate::storage::redis::{
 use crate::storage::{AsyncCounterStorage, Authorization, StorageErr};
 use async_trait::async_trait;
 use redis::aio::ConnectionManager;
-use redis::ConnectionInfo;
+use redis::{ConnectionInfo, RedisError};
 use std::collections::HashSet;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -174,7 +174,7 @@ impl AsyncCounterStorage for CachedRedisStorage {
 }
 
 impl CachedRedisStorage {
-    pub async fn new(redis_url: &str) -> Self {
+    pub async fn new(redis_url: &str) -> Result<Self, RedisError> {
         Self::new_with_options(
             redis_url,
             Some(Duration::from_secs(DEFAULT_FLUSHING_PERIOD_SEC)),
@@ -191,12 +191,9 @@ impl CachedRedisStorage {
         max_cached_counters: usize,
         ttl_cached_counters: Duration,
         ttl_ratio_cached_counters: u64,
-    ) -> Self {
-        let redis_conn_manager = ConnectionManager::new(
-            redis::Client::open(ConnectionInfo::from_str(redis_url).unwrap()).unwrap(),
-        )
-        .await
-        .unwrap();
+    ) -> Result<Self, RedisError> {
+        let info = ConnectionInfo::from_str(redis_url)?;
+        let redis_conn_manager = ConnectionManager::new(redis::Client::open(info).unwrap()).await?;
 
         let async_redis_storage =
             AsyncRedisStorage::new_with_conn_manager(redis_conn_manager.clone());
@@ -222,13 +219,13 @@ impl CachedRedisStorage {
             .ttl_ratio_cached_counter(ttl_ratio_cached_counters)
             .build();
 
-        Self {
+        Ok(Self {
             cached_counters: Mutex::new(cached_counters),
             batcher_counter_updates: batcher,
             redis_conn_manager,
             async_redis_storage,
             batching_is_enabled: flushing_period.is_some(),
-        }
+        })
     }
 
     async fn values_with_ttls(
@@ -302,7 +299,7 @@ impl CachedRedisStorageBuilder {
         self
     }
 
-    pub async fn build(self) -> CachedRedisStorage {
+    pub async fn build(self) -> Result<CachedRedisStorage, RedisError> {
         CachedRedisStorage::new_with_options(
             &self.redis_url,
             self.flushing_period,
@@ -313,3 +310,25 @@ impl CachedRedisStorageBuilder {
         .await
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::storage::redis::CachedRedisStorage;
+    use redis::ErrorKind;
+
+    #[tokio::test]
+    async fn errs_on_bad_url() {
+        let result = CachedRedisStorage::new("cassandra://127.0.0.1:6379").await;
+        assert!(result.is_err());
+        assert_eq!(result.err().unwrap().kind(), ErrorKind::InvalidClientConfig);
+    }
+
+    #[tokio::test]
+    async fn errs_on_connection_issue() {
+        let result = CachedRedisStorage::new("redis://127.0.0.1:21").await;
+        assert!(result.is_err());
+        let error = result.err().unwrap();
+        assert_eq!(error.kind(), ErrorKind::IoError);
+        assert!(error.is_connection_refusal())
+    }
+}

limitador/src/storage/redis/redis_sync.rs

@@ -139,15 +139,21 @@ impl CounterStorage for RedisStorage {
 }
 
 impl RedisStorage {
-    pub fn new(redis_url: &str) -> Self {
-        let conn_manager = RedisConnectionManager::new(redis_url).unwrap();
-        let conn_pool = Pool::builder()
+    pub fn new(redis_url: &str) -> Result<Self, String> {
+        let conn_manager = match RedisConnectionManager::new(redis_url) {
+            Ok(conn_manager) => conn_manager,
+            Err(err) => {
+                return Err(err.to_string());
+            }
+        };
+        match Pool::builder()
             .connection_timeout(Duration::from_secs(3))
             .max_size(MAX_REDIS_CONNS)
             .build(conn_manager)
-            .unwrap();
-
-        Self { conn_pool }
+        {
+            Ok(conn_pool) => Ok(Self { conn_pool }),
+            Err(err) => Err(err.to_string()),
+        }
     }
 }
 
@@ -190,7 +196,7 @@ impl ManageConnection for RedisConnectionManager {
 
 impl Default for RedisStorage {
     fn default() -> Self {
-        Self::new(DEFAULT_REDIS_URL)
+        Self::new(DEFAULT_REDIS_URL).unwrap()
     }
 }
 
@@ -199,8 +205,21 @@ mod test {
     use crate::storage::redis::RedisStorage;
 
     #[test]
-    fn create_default() {
-        let _ = RedisStorage::default();
+    fn errs_on_bad_url() {
+        let result = RedisStorage::new("cassandra://127.0.0.1:6379");
+        assert!(result.is_err());
+        assert_eq!(result.err().unwrap(), "Redis URL did not parse".to_string())
+    }
+
+    #[test]
+    fn errs_on_connection_issue() {
+        // this panic!s And I really don't see how to bubble the redis error back up:
+        // r2d2 consumes it
+        // RedisError are not publicly constructable
+        // So using String as error type… sad
+        let result = RedisStorage::new("redis://127.0.0.1:21");
+        assert!(result.is_err());
+        assert!(result.err().unwrap().contains("Connection refused"));
     }
 
     #[test]

limitador/tests/integration_tests.rs

@@ -38,12 +38,12 @@ macro_rules! test_with_all_storage_impls {
             #[tokio::test]
             #[serial]
             async fn [<$function _with_async_redis>]() {
-                let storage = AsyncRedisStorage::new("redis://127.0.0.1:6379").await;
+                let storage = AsyncRedisStorage::new("redis://127.0.0.1:6379").await.expect("We need a Redis running locally");
                 storage.clear().await.unwrap();
                 let rate_limiter = AsyncRateLimiter::new_with_storage(
                     Box::new(storage)
                 );
-                AsyncRedisStorage::new("redis://127.0.0.1:6379").await.clear().await.unwrap();
+                AsyncRedisStorage::new("redis://127.0.0.1:6379").await.expect("We need a Redis running locally").clear().await.unwrap();
                 $function(&mut TestsLimiter::new_from_async_impl(rate_limiter)).await;
             }