Merge pull request #625 from sunng87/refactor/render-error-reason-2

refactor: improve RenderError and RenderErrorReason for 5.0 branch

examples/error.rs

@@ -30,14 +30,8 @@ pub fn error_helper(
         .param(0)
         .ok_or(RenderErrorReason::ParamNotFoundForIndex("error", 0))?;
     match param.value().as_str() {
-        Some("db") => Err(RenderError::from_error(
-            "helper error",
-            HelperError::DbError,
-        )),
-        Some("api") => Err(RenderError::from_error(
-            "helper error",
-            HelperError::ApiError,
-        )),
+        Some("db") => Err(RenderErrorReason::NestedError(Box::new(HelperError::DbError)).into()),
+        Some("api") => Err(RenderErrorReason::NestedError(Box::new(HelperError::ApiError)).into()),
         _ => Ok(()),
     }
 }
@@ -76,7 +70,7 @@ fn main() -> Result<(), Box<dyn StdError>> {
     let e2 = handlebars
         .render_template("{{err \"db\"}}", &json!({}))
         .unwrap_err();
-    // down-casting the error to user defined type
+    // get nested error to from `RenderError`
     match e2.source().and_then(|e| e.downcast_ref::<HelperError>()) {
         Some(HelperError::DbError) => {
             println!("Detected error from helper: db error",)

src/context.rs

@@ -4,7 +4,7 @@ use serde::Serialize;
 use serde_json::value::{to_value, Map, Value as Json};
 
 use crate::block::{BlockContext, BlockParamHolder};
-use crate::error::RenderError;
+use crate::error::{RenderError, RenderErrorReason};
 use crate::grammar::Rule;
 use crate::json::path::*;
 use crate::json::value::ScopedJson;
@@ -116,7 +116,10 @@ fn parse_json_visitor<'a>(
 
 fn get_data<'a>(d: Option<&'a Json>, p: &str) -> Result<Option<&'a Json>, RenderError> {
     let result = match d {
-        Some(Json::Array(l)) => p.parse::<usize>().map(|idx_u| l.get(idx_u))?,
+        Some(Json::Array(l)) => p
+            .parse::<usize>()
+            .map(|idx_u| l.get(idx_u))
+            .map_err(|_| RenderErrorReason::InvalidJsonIndex(p.to_owned()))?,
         Some(Json::Object(m)) => m.get(p),
         Some(_) => None,
         None => None,
@@ -160,7 +163,7 @@ impl Context {
     /// Create a context with given data
     pub fn wraps<T: Serialize>(e: T) -> Result<Context, RenderError> {
         to_value(e)
-            .map_err(RenderError::from)
+            .map_err(|e| RenderErrorReason::SerdeError(e).into())
             .map(|d| Context { data: d })
     }
 

src/error.rs

@@ -1,7 +1,6 @@
 use std::error::Error as StdError;
 use std::fmt::{self, Write};
 use std::io::Error as IOError;
-use std::num::ParseIntError;
 use std::string::FromUtf8Error;
 
 use serde_json::error::Error as SerdeError;
@@ -14,61 +13,49 @@ use walkdir::Error as WalkdirError;
 use rhai::{EvalAltResult, ParseError};
 
 /// Error when rendering data on template.
-#[derive(Debug, Default, Error)]
+#[derive(Debug)]
 pub struct RenderError {
-    pub desc: String,
     pub template_name: Option<String>,
     pub line_no: Option<usize>,
     pub column_no: Option<usize>,
-    #[source]
-    cause: Option<Box<dyn StdError + Send + Sync + 'static>>,
+    reason: Box<RenderErrorReason>,
     unimplemented: bool,
     // backtrace: Backtrace,
 }
 
 impl fmt::Display for RenderError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+        let desc = self.reason.to_string();
+
         match (self.line_no, self.column_no) {
             (Some(line), Some(col)) => write!(
                 f,
                 "Error rendering \"{}\" line {}, col {}: {}",
                 self.template_name.as_deref().unwrap_or("Unnamed template"),
                 line,
                 col,
-                self.desc
+                desc
             ),
-            _ => write!(f, "{}", self.desc),
+            _ => write!(f, "{}", desc),
         }
     }
 }
 
 impl From<IOError> for RenderError {
     fn from(e: IOError) -> RenderError {
-        RenderError::from_error("Cannot generate output.", e)
-    }
-}
-
-impl From<SerdeError> for RenderError {
-    fn from(e: SerdeError) -> RenderError {
-        RenderError::from_error("Failed to access JSON data.", e)
+        RenderErrorReason::IOError(e).into()
     }
 }
 
 impl From<FromUtf8Error> for RenderError {
-    fn from(e: FromUtf8Error) -> RenderError {
-        RenderError::from_error("Failed to generate bytes.", e)
-    }
-}
-
-impl From<ParseIntError> for RenderError {
-    fn from(e: ParseIntError) -> RenderError {
-        RenderError::from_error("Cannot access array/vector with string index.", e)
+    fn from(e: FromUtf8Error) -> Self {
+        RenderErrorReason::Utf8Error(e).into()
     }
 }
 
 impl From<TemplateError> for RenderError {
-    fn from(e: TemplateError) -> RenderError {
-        RenderError::from_error("Failed to parse template.", e)
+    fn from(e: TemplateError) -> Self {
+        RenderErrorReason::TemplateError(e).into()
     }
 }
 
@@ -77,6 +64,12 @@ impl From<TemplateError> for RenderError {
 pub enum RenderErrorReason {
     #[error("Template not found {0}")]
     TemplateNotFound(String),
+    #[error("Failed to parse template {0}")]
+    TemplateError(
+        #[from]
+        #[source]
+        TemplateError,
+    ),
     #[error("Failed to access variable in strict mode {0:?}")]
     MissingVariable(Option<String>),
     #[error("Partial not found {0}")]
@@ -103,64 +96,92 @@ pub enum RenderErrorReason {
     BlockContentRequired,
     #[error("Invalid json path {0}")]
     InvalidJsonPath(String),
+    #[error("Cannot access array/vector with string index, {0}")]
+    InvalidJsonIndex(String),
+    #[error("Failed to access JSON data: {0}")]
+    SerdeError(
+        #[from]
+        #[source]
+        SerdeError,
+    ),
+    #[error("IO Error: {0}")]
+    IOError(
+        #[from]
+        #[source]
+        IOError,
+    ),
+    #[error("FromUtf8Error: {0}")]
+    Utf8Error(
+        #[from]
+        #[source]
+        FromUtf8Error,
+    ),
+    #[error("Nested error: {0}")]
+    NestedError(#[source] Box<dyn StdError + Send + Sync + 'static>),
+    #[cfg(feature = "script_helper")]
+    #[error("Cannot convert data to Rhai dynamic: {0}")]
+    ScriptValueError(
+        #[from]
+        #[source]
+        Box<EvalAltResult>,
+    ),
+    #[cfg(feature = "script_helper")]
+    #[error("Failed to load rhai script: {0}")]
+    ScriptLoadError(
+        #[from]
+        #[source]
+        ScriptError,
+    ),
+    #[error("Unimplemented")]
+    Unimplemented,
     #[error("{0}")]
     Other(String),
 }
 
 impl From<RenderErrorReason> for RenderError {
     fn from(e: RenderErrorReason) -> RenderError {
-        RenderError::from_error(&e.to_string(), e)
-    }
-}
-
-#[cfg(feature = "script_helper")]
-impl From<Box<EvalAltResult>> for RenderError {
-    fn from(e: Box<EvalAltResult>) -> RenderError {
-        RenderError::from_error("Cannot convert data to Rhai dynamic", e)
-    }
-}
-
-#[cfg(feature = "script_helper")]
-impl From<ScriptError> for RenderError {
-    fn from(e: ScriptError) -> RenderError {
-        RenderError::from_error("Failed to load rhai script", e)
+        RenderError {
+            template_name: None,
+            line_no: None,
+            column_no: None,
+            reason: Box::new(e),
+            unimplemented: false,
+        }
     }
 }
 
 impl RenderError {
     #[deprecated(since = "5.0.0", note = "Use RenderErrorReason instead")]
     pub fn new<T: AsRef<str>>(desc: T) -> RenderError {
-        RenderError {
-            desc: desc.as_ref().to_owned(),
-            ..Default::default()
-        }
-    }
-
-    pub(crate) fn unimplemented() -> RenderError {
-        RenderError {
-            unimplemented: true,
-            ..Default::default()
-        }
+        RenderErrorReason::Other(desc.as_ref().to_string()).into()
     }
 
     pub fn strict_error(path: Option<&String>) -> RenderError {
         RenderErrorReason::MissingVariable(path.map(|p| p.to_owned())).into()
     }
 
-    pub fn from_error<E>(error_info: &str, cause: E) -> RenderError
+    #[deprecated(since = "5.0.0", note = "Use RenderErrorReason::NestedError instead")]
+    pub fn from_error<E>(_error_info: &str, cause: E) -> RenderError
     where
         E: StdError + Send + Sync + 'static,
     {
-        RenderError {
-            desc: error_info.to_owned(),
-            cause: Some(Box::new(cause)),
-            ..Default::default()
-        }
+        RenderErrorReason::NestedError(Box::new(cause)).into()
     }
 
     #[inline]
     pub(crate) fn is_unimplemented(&self) -> bool {
-        self.unimplemented
+        matches!(*self.reason, RenderErrorReason::Unimplemented)
+    }
+
+    /// Get `RenderErrorReason` for this error
+    pub fn reason(&self) -> &RenderErrorReason {
+        self.reason.as_ref()
+    }
+}
+
+impl StdError for RenderError {
+    fn source(&self) -> Option<&(dyn StdError + 'static)> {
+        Some(self.reason())
     }
 }
 

src/helpers/mod.rs

@@ -1,5 +1,5 @@
 use crate::context::Context;
-use crate::error::RenderError;
+use crate::error::{RenderError, RenderErrorReason};
 use crate::json::value::ScopedJson;
 use crate::output::Output;
 use crate::registry::Registry;
@@ -95,7 +95,7 @@ pub trait HelperDef {
         _: &'rc Context,
         _: &mut RenderContext<'reg, 'rc>,
     ) -> Result<ScopedJson<'rc>, RenderError> {
-        Err(RenderError::unimplemented())
+        Err(RenderErrorReason::Unimplemented.into())
     }
 
     /// A complex version of helper interface.

src/helpers/scripting.rs

@@ -1,7 +1,7 @@
 use std::collections::{BTreeMap, HashMap};
 
 use crate::context::Context;
-use crate::error::RenderError;
+use crate::error::{RenderError, RenderErrorReason};
 use crate::helpers::HelperDef;
 use crate::json::value::{PathAndJson, ScopedJson};
 use crate::registry::Registry;
@@ -23,23 +23,25 @@ fn call_script_helper<'reg: 'rc, 'rc>(
     engine: &Engine,
     script: &AST,
 ) -> Result<ScopedJson<'rc>, RenderError> {
-    let params: Dynamic = to_dynamic(params.iter().map(|p| p.value()).collect::<Vec<&Json>>())?;
+    let params: Dynamic = to_dynamic(params.iter().map(|p| p.value()).collect::<Vec<&Json>>())
+        .map_err(RenderErrorReason::from)?;
 
     let hash: Dynamic = to_dynamic(
         hash.iter()
             .map(|(k, v)| ((*k).to_owned(), v.value()))
             .collect::<HashMap<String, &Json>>(),
-    )?;
+    )
+    .map_err(RenderErrorReason::from)?;
 
     let mut scope = Scope::new();
     scope.push_dynamic("params", params);
     scope.push_dynamic("hash", hash);
 
     let result = engine
         .eval_ast_with_scope::<Dynamic>(&mut scope, script)
-        .map_err(RenderError::from)?;
+        .map_err(RenderErrorReason::from)?;
 
-    let result_json: Json = from_dynamic(&result)?;
+    let result_json: Json = from_dynamic(&result).map_err(RenderErrorReason::from)?;
 
     Ok(ScopedJson::Derived(result_json))
 }

src/helpers/string_helpers/mod.rs

@@ -117,19 +117,15 @@ mod tests {
     define_case_helpers_test_cases!("trainCase", test_train_case, ("train case", "Train-Case"),);
 
     #[test]
-    fn test_invlaid_input() {
+    fn test_invalid_input() {
         use crate::error::RenderErrorReason;
-        use std::error::Error;
 
         let hbs = crate::registry::Registry::new();
         let err = hbs
             .render_template("{{snakeCase 1}}", &json!({}))
             .unwrap_err();
         assert!(matches!(
-            err.source()
-                .unwrap()
-                .downcast_ref::<RenderErrorReason>()
-                .unwrap(),
+            err.reason(),
             RenderErrorReason::ParamTypeMismatchForName(_, _, _)
         ));
     }