Remove anyhow

Signed-off-by: Javier G. Sogo <[email protected]>

Author: jgsogo

Cargo.toml

@@ -15,7 +15,6 @@ tungstenite = { version = "0.24.0", features = ["native-tls"] }
 url = "2.5.2"
 http = "1.1.0"
 thiserror = "2.0.4"
-anyhow = "1.0.94"
 
 [dev-dependencies]
 dotenvy = "0.15.7"

src/errors.rs

@@ -1,7 +1,12 @@
-use std::sync::PoisonError;
+use std::{str::ParseBoolError, sync::PoisonError};
 
 use thiserror::Error;
 
+use crate::{
+    models::{Segment, SegmentRule},
+    AttrValue,
+};
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 #[derive(Debug, Error)]
@@ -42,12 +47,22 @@ pub enum Error {
     ConfigurationAccessError(#[from] ConfigurationAccessError),
 
     #[error("Failed to evaluate entity: {0}")]
-    EntityEvaluationError(String),
+    EntityEvaluationError(EntityEvaluationError),
 
     #[error("{0}")]
     Other(String),
 }
 
+#[derive(Debug, Error)]
+#[error("sdaf '{0}'")]
+pub struct EntityEvaluationError(pub(crate) SegmentEvaluationError);
+
+impl From<SegmentEvaluationError> for Error {
+    fn from(value: SegmentEvaluationError) -> Self {
+        Self::EntityEvaluationError(EntityEvaluationError(value))
+    }
+}
+
 impl<T> From<PoisonError<T>> for Error {
     fn from(_value: PoisonError<T>) -> Self {
         Error::CannotAcquireLock
@@ -94,3 +109,52 @@ impl<T> From<PoisonError<T>> for ConfigurationAccessError {
         ConfigurationAccessError::LockAcquisitionError
     }
 }
+
+#[derive(Debug, Error)]
+pub(crate) enum CheckOperatorErrorDetail {
+    #[error("Entity attribute is not a string.")]
+    StringExpected,
+
+    #[error("Entity attribute has unexpected type: Boolean.")]
+    BooleanExpected(#[from] std::str::ParseBoolError),
+
+    #[error("Entity attribute has unexpected type: Number.")]
+    NumberExpected(#[from] std::num::ParseFloatError),
+
+    #[error("Entity attribute is not a number.")]
+    EntityAttrNotANumber,
+
+    #[error("Operator not implemented.")]
+    OperatorNotImplemented,
+}
+
+#[derive(Debug, Error)]
+pub(crate) enum SegmentEvaluationError {
+    #[error("Operation")]
+    SegmentEvaluationFailed(#[from] SegmentEvaluationErrorKind),
+
+    #[error("Segment ID '{0}' not found")]
+    SegmentIdNotFound(String),
+}
+#[derive(Debug, Error)]
+#[error("Operation")]
+pub(crate) struct SegmentEvaluationErrorKind {
+    pub(crate) segment: Segment,
+    pub(crate) segment_rule: SegmentRule,
+    pub(crate) attr_value: AttrValue,
+    pub(crate) source: CheckOperatorErrorDetail,
+}
+
+impl From<(CheckOperatorErrorDetail, &Segment, &SegmentRule, &AttrValue)>
+    for SegmentEvaluationError
+{
+    fn from(value: (CheckOperatorErrorDetail, &Segment, &SegmentRule, &AttrValue)) -> Self {
+        let (source, segment, segment_rule, attr_value) = value;
+        Self::SegmentEvaluationFailed(SegmentEvaluationErrorKind {
+            segment: segment.clone(),
+            segment_rule: segment_rule.clone(),
+            attr_value: attr_value.clone(),
+            source,
+        })
+    }
+}

src/segment_evaluation.rs

@@ -14,17 +14,13 @@
 
 use std::collections::HashMap;
 
-use crate::errors::{Error, Result};
+use crate::errors::{CheckOperatorErrorDetail, Result, SegmentEvaluationError};
 use crate::models::Segment;
 use crate::{
     entity::{AttrValue, Entity},
     models::TargetingRule,
 };
 
-// For chaining errors creating useful error messages:
-use anyhow::anyhow;
-use anyhow::{Context, Result as AnyhowResult};
-
 pub(crate) fn find_applicable_segment_rule_for_entity(
     segments: &HashMap<String, Segment>,
     segment_rules: impl Iterator<Item = TargetingRule>,
@@ -34,26 +30,18 @@ pub(crate) fn find_applicable_segment_rule_for_entity(
     targeting_rules.sort_by(|a, b| a.order.cmp(&b.order));
 
     for targeting_rule in targeting_rules.into_iter() {
-        if targeting_rule_applies_to_entity(segments, &targeting_rule, entity).map_err(|e| {
-            // This terminates the use of anyhow in this module, converting all errors:
-            let cause: String = e.chain().map(|c| format!("\nCaused by: {c}")).collect();
-            Error::EntityEvaluationError(format!(
-                "Failed to evaluate entity '{}' against targeting rule '{}'.{cause}",
-                entity.get_id(),
-                targeting_rule.order
-            ))
-        })? {
+        if targeting_rule_applies_to_entity(segments, &targeting_rule, entity)? {
             return Ok(Some(targeting_rule));
         }
     }
-    return Ok(None);
+    Ok(None)
 }
 
 fn targeting_rule_applies_to_entity(
     segments: &HashMap<String, Segment>,
     targeting_rule: &TargetingRule,
     entity: &impl Entity,
-) -> AnyhowResult<bool> {
+) -> std::result::Result<bool, SegmentEvaluationError> {
     // TODO: we need to get the naming correct here to distinguish between rules, segments, segment_ids, targeting_rules etc. correctly
     let rules = &targeting_rule.rules;
     for rule in rules.iter() {
@@ -69,21 +57,25 @@ fn segment_applies_to_entity(
     segments: &HashMap<String, Segment>,
     segment_ids: &[String],
     entity: &impl Entity,
-) -> AnyhowResult<bool> {
+) -> std::result::Result<bool, SegmentEvaluationError> {
     for segment_id in segment_ids.iter() {
-        let segment = segments.get(segment_id).ok_or(Error::Other(
-            format!("Segment '{segment_id}' not found.").into(),
-        ))?;
-        let applies = belong_to_segment(segment, entity.get_attributes())
-            .context(format!("Failed to evaluate segment '{segment_id}'"))?;
+        let segment = segments
+            .get(segment_id)
+            .ok_or(SegmentEvaluationError::SegmentIdNotFound(
+                segment_id.clone(),
+            ))?;
+        let applies = belong_to_segment(segment, entity.get_attributes())?;
         if applies {
             return Ok(true);
         }
     }
     Ok(false)
 }
 
-fn belong_to_segment(segment: &Segment, attrs: HashMap<String, AttrValue>) -> AnyhowResult<bool> {
+fn belong_to_segment(
+    segment: &Segment,
+    attrs: HashMap<String, AttrValue>,
+) -> std::result::Result<bool, SegmentEvaluationError> {
     for rule in segment.rules.iter() {
         let operator = &rule.operator;
         let attr_name = &rule.attribute_name;
@@ -94,31 +86,26 @@ fn belong_to_segment(segment: &Segment, attrs: HashMap<String, AttrValue>) -> An
         let rule_result = match attr_value {
             None => {
                 println!("Warning: Operation '{attr_name}' '{operator}' '[...]' failed to evaluate: '{attr_name}' not found in entity");
-                Ok(false)
+                false
             }
             Some(attr_value) => {
                 // FIXME: the following algorithm is too hard to read. Is it just me or do we need to simplify this?
                 // One of the values needs to match.
                 // Find a candidate (a candidate corresponds to a value which matches or which might match but the operator failed):
-                let candidate = rule.values.iter().find_map(|value| {
-                    let result_for_value =
-                        check_operator(attr_value, operator, value).context(format!(
-                            "Operation '{attr_name}' '{operator}' '{value}' failed to evaluate."
-                        ));
-                    match result_for_value {
+                let candidate = rule
+                    .values
+                    .iter()
+                    .find_map(|value| match check_operator(attr_value, operator, value) {
                         Ok(true) => Some(Ok(())),
                         Ok(false) => None,
                         Err(e) => Some(Err(e)),
-                    }
-                });
+                    })
+                    .transpose()
+                    .map_err(|e| (e, segment, rule, attr_value))?;
                 // check if the candidate is good, or if the operator failed:
-                match candidate {
-                    None => Ok(false),
-                    Some(Ok(())) => Ok(true),
-                    Some(Err(e)) => Err(e),
-                }
+                candidate.is_some()
             }
-        }?;
+        };
         // All rules must match:
         if !rule_result {
             return Ok(false);
@@ -131,83 +118,47 @@ fn check_operator(
     attribute_value: &AttrValue,
     operator: &str,
     reference_value: &str,
-) -> AnyhowResult<bool> {
+) -> std::result::Result<bool, CheckOperatorErrorDetail> {
     match operator {
         "is" => match attribute_value {
             AttrValue::String(data) => Ok(*data == reference_value),
-            AttrValue::Boolean(data) => {
-                let result = *data
-                    == reference_value
-                        .parse::<bool>()
-                        .map_err(|_| anyhow!("Entity attribute has unexpected type: Boolean."))?;
-                Ok(result)
-            }
-            AttrValue::Numeric(data) => {
-                let result = *data
-                    == reference_value
-                        .parse::<f64>()
-                        .map_err(|_| anyhow!("Entity attribute has unexpected type: Number."))?;
-                Ok(result)
-            }
+            AttrValue::Boolean(data) => Ok(*data == reference_value.parse::<bool>()?),
+            AttrValue::Numeric(data) => Ok(*data == reference_value.parse::<f64>()?),
         },
         "contains" => match attribute_value {
             AttrValue::String(data) => Ok(data.contains(reference_value)),
-            _ => Err(anyhow!("Entity attribute is not a string.")),
+            _ => Err(CheckOperatorErrorDetail::StringExpected),
         },
         "startsWith" => match attribute_value {
             AttrValue::String(data) => Ok(data.starts_with(reference_value)),
-            _ => Err(anyhow!("Entity attribute is not a string.")),
+            _ => Err(CheckOperatorErrorDetail::StringExpected),
         },
         "endsWith" => match attribute_value {
             AttrValue::String(data) => Ok(data.ends_with(reference_value)),
-            _ => Err(anyhow!("Entity attribute is not a string.")),
+            _ => Err(CheckOperatorErrorDetail::StringExpected),
         },
         "greaterThan" => match attribute_value {
             // TODO: Go implementation also compares strings (by parsing them as floats). Do we need this?
             //       https://github.com/IBM/appconfiguration-go-sdk/blob/master/lib/internal/models/Rule.go#L82
             // TODO: we could have numbers not representable as f64, maybe we should try to parse it to i64 and u64 too?
             // TODO: we should have a different nesting style here: match the reference_value first and error out when given
             //       entity attr does not match. This would yield more natural error messages
-            AttrValue::Numeric(data) => {
-                let result = *data
-                    > reference_value
-                        .parse()
-                        .map_err(|_| Error::Other("Value cannot convert into f64.".into()))?;
-                Ok(result)
-            }
-            _ => Err(anyhow!("Entity attribute is not a number.")),
+            AttrValue::Numeric(data) => Ok(*data > reference_value.parse()?),
+            _ => Err(CheckOperatorErrorDetail::EntityAttrNotANumber),
         },
         "lesserThan" => match attribute_value {
-            AttrValue::Numeric(data) => {
-                let result = *data
-                    < reference_value
-                        .parse()
-                        .map_err(|_| Error::Other("Value cannot convert into f64.".into()))?;
-                Ok(result)
-            }
-            _ => Err(anyhow!("Entity attribute is not a number.")),
+            AttrValue::Numeric(data) => Ok(*data < reference_value.parse()?),
+            _ => Err(CheckOperatorErrorDetail::EntityAttrNotANumber),
         },
         "greaterThanEquals" => match attribute_value {
-            AttrValue::Numeric(data) => {
-                let result = *data
-                    >= reference_value
-                        .parse()
-                        .map_err(|_| Error::Other("Value cannot convert into f64.".into()))?;
-                Ok(result)
-            }
-            _ => Err(anyhow!("Entity attribute is not a number.")),
+            AttrValue::Numeric(data) => Ok(*data >= reference_value.parse()?),
+            _ => Err(CheckOperatorErrorDetail::EntityAttrNotANumber),
         },
         "lesserThanEquals" => match attribute_value {
-            AttrValue::Numeric(data) => {
-                let result = *data
-                    <= reference_value
-                        .parse()
-                        .map_err(|_| Error::Other("Value cannot convert into f64.".into()))?;
-                Ok(result)
-            }
-            _ => Err(anyhow!("Entity attribute is not a number.")),
+            AttrValue::Numeric(data) => Ok(*data <= reference_value.parse()?),
+            _ => Err(CheckOperatorErrorDetail::EntityAttrNotANumber),
         },
-        _ => Err(anyhow!("Operator not implemented")),
+        _ => Err(CheckOperatorErrorDetail::OperatorNotImplemented),
     }
 }
 
@@ -218,6 +169,7 @@ pub mod tests {
         models::{ConfigValue, Segment, SegmentRule, Segments, TargetingRule},
         AttrValue,
     };
+    use crate::errors::Error;
     use rstest::*;
 
     #[fixture]
@@ -316,7 +268,15 @@ pub mod tests {
         //  Caused by: Operation 'name' 'is' 'heinz' failed to evaluate.
         //  Caused by: Entity attribute has unexpected type: Number.
         // We are checking here that the parts are present to allow debugging of config by the user:
-        let msg = rule.unwrap_err().to_string();
+
+        let e = rule.unwrap_err();
+        assert!(matches!(
+            e,
+            Error::EntityEvaluationError(ref v)
+        ));
+
+        let msg = e.to_string();
+        assert_eq!(msg, "lol");
         assert!(msg.contains("'a2'"));
         assert!(msg.contains("'0'"));
         assert!(msg.contains("'some_segment_id_1'"));