Add better provider parsing errors messages (#68)

* #67 Add cause of provider error in wrapper error

* Update changelog

* Fix failing test and semvar refactor to avoid major version upgrade

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 - Added derive Clone to RefreshRate struct. 
+- Add cause for entity and policy provider errors
 
 ### Added
 

src/public/file/entity_provider.rs

@@ -20,7 +20,7 @@ use std::io::Error;
 use std::sync::Arc;
 
 use async_trait::async_trait;
-use cedar_policy::{Entities, Request, Schema};
+use cedar_policy::{Entities, EntitiesError, Request, Schema};
 use derive_builder::Builder;
 use thiserror::Error;
 use tokio::sync::RwLock;
@@ -75,14 +75,15 @@ pub enum ProviderError {
     #[error("The Schema file failed to be parsed at path: {0}")]
     SchemaParseError(String),
     /// Entities file is malformed in some way
-    #[error("The Entities failed to be parsed at path: {0}")]
+    #[error("{0}")]
     EntitiesError(String),
 }
 
 /// A wrapper that wraps `EntitiesError` to map the error message
 struct EntitiesErrorWrapper {
     // This is the path to the file to load entities
     entity_file_path: String,
+    cause: EntitiesError,
 }
 
 /// A wrapper that wraps `SchemaParseError` to map the error message
@@ -94,10 +95,11 @@ struct SchemaParseErrorWrapper {
 /// Implements the constructor for the `EntitiesErrorWrapper`.
 impl EntitiesErrorWrapper {
     /// Creates a new wrapper of the `EntitiesError`
-    fn new(entity_file_path: String) -> Self {
+    fn new(entity_file_path: String, cause: EntitiesError) -> Self {
         Self {
             // This is the path to the file to load entities.
             entity_file_path,
+            cause,
         }
     }
 }
@@ -127,10 +129,17 @@ impl From<SchemaParseErrorWrapper> for ProviderError {
     }
 }
 
+fn create_entity_error_message(value: &EntitiesErrorWrapper) -> String {
+    format!(
+        "The Entities failed to be parsed at path: {}. Cause: {}",
+        value.entity_file_path, value.cause
+    )
+}
+
 /// Map the `EntitiesErrorWrapper` to the `ProvideError::EntitiesError`  with the file path
 impl From<EntitiesErrorWrapper> for ProviderError {
     fn from(value: EntitiesErrorWrapper) -> Self {
-        Self::EntitiesError(value.entity_file_path)
+        Self::EntitiesError(create_entity_error_message(&value))
     }
 }
 
@@ -165,16 +174,17 @@ impl EntityProvider {
                 let schema_file = File::open(schema_path)?;
                 let schema = Schema::from_file(schema_file)
                     .map_err(|_schema_error| SchemaParseErrorWrapper::new(schema_path.clone()))?;
-                let res = Entities::from_json_file(entities_file, Some(&schema))
-                    .map_err(|_entities_error| EntitiesErrorWrapper::new(entities_path.clone()))?;
+                let res = Entities::from_json_file(entities_file, Some(&schema)).map_err(
+                    |entities_error| {
+                        EntitiesErrorWrapper::new(entities_path.clone(), entities_error)
+                    },
+                )?;
                 debug!("Fetched Entities from file with Schema: entities_file_path={entities_path:?}: schema_file_path={schema_path:?}");
                 res
             } else {
                 let res =
-                    Entities::from_json_file(entities_file, None).map_err(|_entities_error| {
-                        EntitiesErrorWrapper {
-                            entity_file_path: entities_path.clone(),
-                        }
+                    Entities::from_json_file(entities_file, None).map_err(|entities_error| {
+                        EntitiesErrorWrapper::new(entities_path.clone(), entities_error)
                     })?;
                 debug!("Fetched Entities from file: entities_file_path={entities_path:?}");
                 res
@@ -222,19 +232,22 @@ impl UpdateProviderData for EntityProvider {
                         schema_path.to_string(),
                     )))
                 })?;
-                let res = Entities::from_json_file(entities_file, Some(&schema)).map_err(|_| {
-                    UpdateProviderDataError::General(Box::new(ProviderError::EntitiesError(
-                        entities_path.to_string(),
-                    )))
-                })?;
+                let res = Entities::from_json_file(entities_file, Some(&schema)).map_err(
+                    |entities_error| {
+                        UpdateProviderDataError::General(Box::new(ProviderError::from(
+                            EntitiesErrorWrapper::new(entities_path.clone(), entities_error),
+                        )))
+                    },
+                )?;
                 debug!("Updated Entities from file with Schema: entities_file_path={entities_path:?}: schema_file_path={schema_path:?}");
                 res
             } else {
-                let res = Entities::from_json_file(entities_file, None).map_err(|_| {
-                    UpdateProviderDataError::General(Box::new(ProviderError::EntitiesError(
-                        entities_path.to_string(),
-                    )))
-                })?;
+                let res =
+                    Entities::from_json_file(entities_file, None).map_err(|entities_error| {
+                        UpdateProviderDataError::General(Box::new(ProviderError::from(
+                            EntitiesErrorWrapper::new(entities_path.clone(), entities_error),
+                        )))
+                    })?;
                 debug!("Updated Entities from file: entities_file_path={entities_path:?}");
                 res
             };
@@ -267,12 +280,16 @@ impl SimpleEntityProvider for EntityProvider {
 
 #[cfg(test)]
 mod test {
-    use cedar_policy::{Context, Request};
+    use cedar_policy::{Context, Entities, Request};
+    use std::fs::File;
     use std::io::ErrorKind;
     use std::{fs, io};
     use tempfile::NamedTempFile;
 
-    use crate::public::file::entity_provider::{ConfigBuilder, EntityProvider, ProviderError};
+    use crate::public::file::entity_provider::{
+        create_entity_error_message, ConfigBuilder, EntitiesErrorWrapper, EntityProvider,
+        ProviderError,
+    };
     use crate::public::{SimpleEntityProvider, UpdateProviderData, UpdateProviderDataError};
 
     #[test]
@@ -382,17 +399,25 @@ mod test {
 
     #[test]
     fn entity_provider_malformed_entities() {
+        let entities_path = "tests/data/malformed_entities.json";
+        let entities_file = File::open(entities_path).unwrap();
         let error = EntityProvider::new(
             ConfigBuilder::default()
-                .entities_path("tests/data/malformed_entities.json")
+                .entities_path(entities_path)
                 .build()
                 .unwrap(),
         );
+        let cedar_error = Entities::from_json_file(entities_file, None);
 
         assert!(error.is_err());
+        assert!(cedar_error.is_err());
+
         assert_eq!(
             error.err().unwrap().to_string(),
-            "The Entities failed to be parsed at path: tests/data/malformed_entities.json"
+            create_entity_error_message(&EntitiesErrorWrapper::new(
+                entities_path.into(),
+                cedar_error.unwrap_err()
+            ))
         );
     }
 

src/public/file/policy_set_provider.rs

@@ -20,7 +20,7 @@ use std::str::FromStr;
 use std::sync::Arc;
 
 use async_trait::async_trait;
-use cedar_policy::{PolicySet, Request};
+use cedar_policy::{ParseErrors, PolicySet, Request};
 use derive_builder::Builder;
 use thiserror::Error;
 use tokio::sync::RwLock;
@@ -63,7 +63,7 @@ pub struct PolicySetProvider {
 #[derive(Error, Debug)]
 pub enum ProviderError {
     /// Policy set file is malformed in some way
-    #[error("The Policy Set failed to be parsed at path: {0}")]
+    #[error("{0}")]
     PolicySetParseError(String),
     /// Can't read from disk or find the file
     #[error("IO Error: {0}")]
@@ -74,15 +74,17 @@ pub enum ProviderError {
 struct ParseErrorWrapper {
     // This is the path to the file to load the policy set
     policy_set_path: String,
+    cause: ParseErrors,
 }
 
 /// Implements the constructor for the `ParseErrorWrapper`.
 impl ParseErrorWrapper {
     /// Creates a new wrapper of the `ParseErrors`
-    fn new(policy_set_path: String) -> Self {
+    fn new(policy_set_path: String, cause: ParseErrors) -> Self {
         Self {
             // This is the path to the file to load the policy set
             policy_set_path,
+            cause,
         }
     }
 }
@@ -94,10 +96,17 @@ impl From<std::io::Error> for ProviderError {
     }
 }
 
+fn create_policy_parse_error_message(value: &ParseErrorWrapper) -> String {
+    format!(
+        "The Policy Set failed to be parsed at path: {}. Cause: {}",
+        value.policy_set_path, value.cause
+    )
+}
+
 /// Map the `ParseErrorWrapper` to the `ProviderError::PolicySetParseError` with the file path
 impl From<ParseErrorWrapper> for ProviderError {
     fn from(value: ParseErrorWrapper) -> Self {
-        Self::PolicySetParseError(value.policy_set_path)
+        Self::PolicySetParseError(create_policy_parse_error_message(&value))
     }
 }
 
@@ -124,8 +133,9 @@ impl PolicySetProvider {
     pub fn new(configuration: Config) -> Result<Self, ProviderError> {
         let policy_set_path = configuration.policy_set_path;
         let policy_set_src = std::fs::read_to_string(Path::new(policy_set_path.as_str()))?;
-        let policy_set = PolicySet::from_str(&policy_set_src)
-            .map_err(|_parse_errors| ParseErrorWrapper::new(policy_set_path.clone()))?;
+        let policy_set = PolicySet::from_str(&policy_set_src).map_err(|parse_errors| {
+            ParseErrorWrapper::new(policy_set_path.clone(), parse_errors)
+        })?;
         let policy_ids = policy_set
             .policies()
             .map(cedar_policy::Policy::id)
@@ -147,11 +157,12 @@ impl UpdateProviderData for PolicySetProvider {
         let policy_set_path = self.policy_set_path.clone();
         let policy_set_src = std::fs::read_to_string(Path::new(policy_set_path.as_str()))
             .map_err(|e| UpdateProviderDataError::General(Box::new(ProviderError::IOError(e))))?;
-        let policy_set = PolicySet::from_str(&policy_set_src).map_err(|_| {
-            UpdateProviderDataError::General(Box::new(ProviderError::PolicySetParseError(
-                policy_set_path.clone(),
-            )))
-        })?;
+        let policy_set =
+            PolicySet::from_str(&policy_set_src).map_err(|parse_error: ParseErrors| {
+                UpdateProviderDataError::General(Box::new(ProviderError::from(
+                    ParseErrorWrapper::new(policy_set_path.clone(), parse_error),
+                )))
+            })?;
         {
             let mut policy_set_guard = self.policy_set.write().await;
             *policy_set_guard = Arc::new(policy_set.clone());
@@ -180,14 +191,17 @@ impl SimplePolicySetProvider for PolicySetProvider {
 
 #[cfg(test)]
 mod test {
-    use cedar_policy::{Context, Request};
+    use cedar_policy::{Context, PolicySet, Request};
     use std::io::ErrorKind;
+    use std::path::Path;
+    use std::str::FromStr;
     use std::{fs, io};
     use tempfile::NamedTempFile;
 
     use crate::public::file::policy_set_provider::ProviderError::PolicySetParseError;
     use crate::public::file::policy_set_provider::{
-        ConfigBuilder, PolicySetProvider, ProviderError,
+        create_policy_parse_error_message, ConfigBuilder, ParseErrorWrapper, PolicySetProvider,
+        ProviderError,
     };
     use crate::public::{SimplePolicySetProvider, UpdateProviderData, UpdateProviderDataError};
     #[test]
@@ -219,17 +233,25 @@ mod test {
 
     #[test]
     fn simple_policy_provider_is_malformed() {
+        let policy_path = "tests/data/malformed_policies.cedar";
+        let policy_set_src = std::fs::read_to_string(Path::new(policy_path)).unwrap();
         let error = PolicySetProvider::new(
             ConfigBuilder::default()
-                .policy_set_path("tests/data/malformed_policies.cedar")
+                .policy_set_path(policy_path)
                 .build()
                 .unwrap(),
         );
 
+        let cedar_error = PolicySet::from_str(&policy_set_src);
+
         assert!(error.is_err());
+        assert!(cedar_error.is_err());
         assert_eq!(
             error.err().unwrap().to_string(),
-            "The Policy Set failed to be parsed at path: tests/data/malformed_policies.cedar"
+            create_policy_parse_error_message(&ParseErrorWrapper::new(
+                policy_path.into(),
+                cedar_error.unwrap_err()
+            ))
         );
     }