Deal with syntax error of conditions

Signed-off-by: Alex Snaps <alex@wcgw.dev>

limitador-server/src/envoy_rls/server.rs

@@ -255,7 +255,8 @@ mod tests {
             60,
             vec!["req.method == 'GET'"],
             vec!["app_id"],
-        );
+        )
+        .expect("This must be a valid limit!");
 
         let limiter = RateLimiter::new(10_000);
         limiter.add_limit(limit);
@@ -394,8 +395,10 @@ mod tests {
         let namespace = "test_namespace";
 
         vec![
-            Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["z"]),
-            Limit::new(namespace, 0, 60, vec!["x == '1'", "y == '2'"], vec!["z"]),
+            Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["z"])
+                .expect("This must be a valid limit!"),
+            Limit::new(namespace, 0, 60, vec!["x == '1'", "y == '2'"], vec!["z"])
+                .expect("This must be a valid limit!"),
         ]
         .into_iter()
         .for_each(|limit| {
@@ -459,7 +462,8 @@ mod tests {
     #[tokio::test]
     async fn test_takes_into_account_the_hits_addend_param() {
         let namespace = "test_namespace";
-        let limit = Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["y"]);
+        let limit = Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["y"])
+            .expect("This must be a valid limit!");
 
         let limiter = RateLimiter::new(10_000);
         limiter.add_limit(limit);
@@ -528,7 +532,8 @@ mod tests {
         // "hits_addend" is optional according to the spec, and should default
         // to 1, However, with the autogenerated structs it defaults to 0.
         let namespace = "test_namespace";
-        let limit = Limit::new(namespace, 1, 60, vec!["x == '1'"], vec!["y"]);
+        let limit = Limit::new(namespace, 1, 60, vec!["x == '1'"], vec!["y"])
+            .expect("This must be a valid limit!");
 
         let limiter = RateLimiter::new(10_000);
         limiter.add_limit(limit);

limitador-server/src/http_api/request_types.rs

@@ -1,9 +1,9 @@
 use limitador::counter::Counter as LimitadorCounter;
+use limitador::errors::LimitadorError;
 use limitador::limit::Limit as LimitadorLimit;
 use paperclip::actix::Apiv2Schema;
 use serde::{Deserialize, Serialize};
 use std::collections::{BTreeMap, HashMap};
-
 // We need to define the Limit and Counter types. They're basically the same as
 // defined in the lib but with some modifications to be able to derive
 // Apiv2Schema (needed to generate the OpenAPI specs).
@@ -41,8 +41,10 @@ impl From<&LimitadorLimit> for Limit {
     }
 }
 
-impl From<Limit> for LimitadorLimit {
-    fn from(limit: Limit) -> Self {
+impl TryFrom<Limit> for LimitadorLimit {
+    type Error = LimitadorError;
+
+    fn try_from(limit: Limit) -> Result<Self, Self::Error> {
         let mut limitador_limit = if let Some(id) = limit.id {
             Self::with_id(
                 id,
@@ -51,22 +53,22 @@ impl From<Limit> for LimitadorLimit {
                 limit.seconds,
                 limit.conditions,
                 limit.variables,
-            )
+            )?
         } else {
             Self::new(
                 limit.namespace,
                 limit.max_value,
                 limit.seconds,
                 limit.conditions,
                 limit.variables,
-            )
+            )?
         };
 
         if let Some(name) = limit.name {
             limitador_limit.set_name(name)
         }
 
-        limitador_limit
+        Ok(limitador_limit)
     }
 }
 

limitador-server/src/http_api/server.rs

@@ -555,7 +555,8 @@ mod tests {
             60,
             vec!["req.method == 'GET'"],
             vec!["app_id"],
-        );
+        )
+        .expect("This must be a valid limit!");
 
         match &limiter {
             Limiter::Blocking(limiter) => limiter.add_limit(limit.clone()),

limitador/benches/bench.rs

@@ -547,13 +547,16 @@ fn generate_test_limits(scenario: &TestScenario) -> (Vec<Limit>, Vec<TestCallPar
         let namespace = idx_namespace.to_string();
 
         for limit_idx in 0..scenario.n_limits_per_ns {
-            test_limits.push(Limit::new(
-                namespace.clone(),
-                u64::MAX,
-                ((limit_idx * 60) + 10) as u64,
-                conditions.clone(),
-                variables.clone(),
-            ))
+            test_limits.push(
+                Limit::new(
+                    namespace.clone(),
+                    u64::MAX,
+                    ((limit_idx * 60) + 10) as u64,
+                    conditions.clone(),
+                    variables.clone(),
+                )
+                .expect("This must be a valid limit!"),
+            )
         }
 
         call_params.push(TestCallParams {

limitador/src/errors.rs

@@ -1,32 +1,51 @@
+use crate::limit::ConditionParsingError;
 use crate::storage::StorageErr;
+use std::convert::Infallible;
 use std::error::Error;
 use std::fmt::{Display, Formatter};
 
 #[derive(Debug)]
 pub enum LimitadorError {
-    Storage(StorageErr),
+    StorageError(StorageErr),
+    InterpreterError(ConditionParsingError),
 }
 
 impl Display for LimitadorError {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         match self {
-            LimitadorError::Storage(err) => {
+            LimitadorError::StorageError(err) => {
                 write!(f, "error while accessing the limits storage: {err:?}")
             }
+            LimitadorError::InterpreterError(err) => {
+                write!(f, "error parsing condition: {err:?}")
+            }
         }
     }
 }
 
 impl Error for LimitadorError {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         match self {
-            LimitadorError::Storage(err) => Some(err),
+            LimitadorError::StorageError(err) => Some(err),
+            LimitadorError::InterpreterError(err) => Some(err),
         }
     }
 }
 
 impl From<StorageErr> for LimitadorError {
     fn from(e: StorageErr) -> Self {
-        Self::Storage(e)
+        Self::StorageError(e)
+    }
+}
+
+impl From<ConditionParsingError> for LimitadorError {
+    fn from(err: ConditionParsingError) -> Self {
+        LimitadorError::InterpreterError(err)
+    }
+}
+
+impl From<Infallible> for LimitadorError {
+    fn from(value: Infallible) -> Self {
+        unreachable!("unexpected infallible value: {:?}", value)
     }
 }

limitador/src/lib.rs

@@ -73,7 +73,7 @@
 //!      60,
 //!      vec!["req.method == 'GET'"],
 //!      vec!["user_id"],
-//! );
+//! ).unwrap();
 //! let mut rate_limiter = RateLimiter::new(1000);
 //!
 //! // Add a limit
@@ -105,7 +105,7 @@
 //!      60,
 //!      vec!["req.method == 'GET'"],
 //!      vec!["user_id"],
-//! );
+//! ).unwrap();
 //! rate_limiter.add_limit(limit);
 //!
 //! // We've defined a limit of 2. So we can report 2 times before being
@@ -169,7 +169,7 @@
 //!      60,
 //!      vec!["req.method == 'GET'"],
 //!      vec!["user_id"],
-//! );
+//! ).unwrap();
 //!
 //! async {
 //!     let rate_limiter = AsyncRateLimiter::new_with_storage(
@@ -708,7 +708,8 @@ mod test {
             100,
             Vec::<String>::default(),
             Vec::<String>::default(),
-        );
+        )
+        .expect("This must be a valid limit!");
         rl.add_limit(l.clone());
         let limits = rl.get_limits(&namespace.into());
         assert_eq!(limits.len(), 1);