Throw proper JS errors

crates/csl/Cargo.toml

@@ -15,7 +15,7 @@ readme = "README.md"
 repository = "https://github.com/cormacrelf/citeproc-rs"
 
 [dependencies]
-roxmltree = "0.7.0"
+roxmltree = "0.13.0"
 fnv = "1.0.6"
 strum = "0.15.0"
 strum_macros = "0.19"
@@ -26,3 +26,4 @@ serde_derive = "1.0.100"
 string_cache = "0.7.3"
 semver = "0.9.0"
 log = "0.4.8"
+thiserror = "1.0.20"

crates/csl/src/attr.rs

@@ -132,7 +132,7 @@ pub(crate) fn attribute_var_type<T: GetAttribute>(
 
 pub(crate) fn attribute_option<'a, 'd: 'a, T: GetAttribute>(
     node: &Node<'a, 'd>,
-    attr: impl Into<ExpandedName<'a>> + Clone,
+    attr: impl Into<ExpandedName<'a, 'd>> + Clone,
     info: &ParseInfo,
 ) -> Result<Option<T>, InvalidCsl> {
     match node.attribute(attr.clone()) {

crates/csl/src/error.rs

@@ -31,22 +31,36 @@ where
     s.serialize_str(&ToString::to_string(x))
 }
 
-#[derive(Debug, Serialize)]
+#[derive(thiserror::Error, Debug, Serialize)]
 pub enum StyleError {
-    Invalid(CslError),
-    ParseError(#[serde(serialize_with = "rox_error_serialize")] roxmltree::Error),
+    #[error("invalid style: {0}")]
+    Invalid(#[from] CslError),
+    #[error("could not parse style")]
+    ParseError(#[from] #[serde(serialize_with = "rox_error_serialize")] roxmltree::Error),
 }
 
 #[derive(Debug, Serialize)]
 pub struct CslError(pub Vec<InvalidCsl>);
 
+impl std::error::Error for CslError {}
+use std::fmt;
+impl fmt::Display for CslError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for i in &self.0 {
+            writeln!(f, "bytes {}..{} {}", i.range.start, i.range.end, i)?;
+        }
+        Ok(())
+    }
+}
+
 #[derive(Debug, PartialEq, Copy, Clone, Serialize)]
 pub enum Severity {
     Error,
     Warning,
 }
 
-#[derive(Debug, PartialEq, Clone, Serialize)]
+#[derive(thiserror::Error, Debug, PartialEq, Clone, Serialize)]
+#[error("[{severity:?}] {message} ({hint})")]
 pub struct InvalidCsl {
     pub severity: Severity,
     // TODO: serialize_with or otherwise get this into the output
@@ -203,18 +217,6 @@ impl InvalidCsl {
     }
 }
 
-impl From<roxmltree::Error> for StyleError {
-    fn from(err: roxmltree::Error) -> StyleError {
-        StyleError::ParseError(err)
-    }
-}
-
-impl From<CslError> for StyleError {
-    fn from(err: CslError) -> StyleError {
-        StyleError::Invalid(err)
-    }
-}
-
 impl From<Vec<CslError>> for CslError {
     fn from(errs: Vec<CslError>) -> CslError {
         // concat all of the sub-vecs into one

crates/wasm/Cargo.toml

@@ -14,6 +14,20 @@ edition = "2018"
 repository = "https://github.com/cormacrelf/citeproc-rs"
 description = "citeproc-rs, compiled to WebAssembly"
 
+# TODO: Set the opt level on new wasm-pack 0.6.0 config
+# when it's released, so as not to interfere with the
+# other native binary targets (cargo only lets you set it
+# on the workspace root)
+
+# [profile.release]
+# # Tell `rustc` to optimize for small code size.
+# opt-level = "s"
+# lto = true
+
+# https://github.com/rustwasm/wasm-pack/issues/886#issuecomment-667669802
+[package.metadata.wasm-pack.profile.release]
+wasm-opt = ["-O3", "--enable-mutable-globals"]
+
 [lib]
 crate-type = ["cdylib", "rlib"]
 
@@ -43,6 +57,7 @@ js-sys = "0.3.44"
 serde = "1.0.100"
 serde_derive = "1.0.100"
 serde_json = "1.0.40"
+anyhow = "1.0.32"
 
 [dependencies.wasm-bindgen]
 version = "0.2.67"
@@ -57,17 +72,3 @@ features = ["release_max_level_error"]
 [dev-dependencies]
 wasm-bindgen-test = "0.3.17"
 
-# TODO: Set the opt level on new wasm-pack 0.6.0 config
-# when it's released, so as not to interfere with the
-# other native binary targets (cargo only lets you set it
-# on the workspace root)
-
-# [profile.release]
-# # Tell `rustc` to optimize for small code size.
-# opt-level = "s"
-# lto = true
-
-# https://github.com/rustwasm/wasm-pack/issues/886#issuecomment-667669802
-[package.metadata.wasm-pack.profile.release]
-wasm-opt = ["-O3", "--enable-mutable-globals"]
-

crates/wasm/src/lib.rs

@@ -4,6 +4,7 @@
 //
 // Copyright © 2018 Corporation for Digital Scholarship
 
+#[macro_use]
 mod utils;
 
 #[macro_use]
@@ -12,12 +13,9 @@ extern crate serde_derive;
 #[macro_use]
 extern crate log;
 
-use self::utils::ErrorPlaceholder;
-
 use js_sys::{Error as JsError, Promise};
 use serde::Serialize;
 use std::cell::RefCell;
-use std::error::Error;
 use std::rc::Rc;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -49,11 +47,10 @@ impl Driver {
         // The Processor gets a "only has en-US, otherwise empty" fetcher.
         let us_fetcher = Arc::new(utils::USFetcher);
         let format = SupportedFormat::from_str(format)
-            .map_err(|_| JsError::new(&format!("unknown format `{}`", format)))?;
-        let engine = Processor::new(style, us_fetcher, true, format)
-            .map(RefCell::new)
-            .map(Rc::new)
-            .map_err(|e| JsError::new(&serde_json::to_string(&e).unwrap()))?;
+            .map_err(|_| anyhow::anyhow!("unknown format `{}`", format));
+        let format = js_err!(format);
+        let engine = js_err!(Processor::new(style, us_fetcher, true, format));
+        let engine = Rc::new(RefCell::new(engine));
 
         // The Driver manually adds locales fetched via Lifecycle, which asks the consumer
         // asynchronously.
@@ -65,13 +62,10 @@ impl Driver {
 
     /// Sets the style (which will also cause everything to be recomputed)
     #[wasm_bindgen(js_name = "setStyle")]
-    pub fn set_style(&mut self, style_text: &str) -> JsValue {
-        self.engine
-            .borrow_mut()
-            .set_style_text(style_text)
-            .map_err(|e| JsValue::from_serde(&e).unwrap())
-            .err()
-            .unwrap_or(JsValue::UNDEFINED)
+    pub fn set_style(&mut self, style_text: &str) -> Result<(), JsValue> {
+        let mut eng = self.engine.borrow_mut();
+        js_err!(eng.set_style_text(style_text));
+        Ok(())
     }
 
     /// Completely overwrites the references library.
@@ -98,9 +92,7 @@ impl Driver {
     /// * `refr` is a Reference object.
     #[wasm_bindgen(js_name = "insertReference")]
     pub fn insert_reference(&mut self, refr: TReference) -> Result<(), JsValue> {
-        let refr = refr
-            .into_serde()
-            .map_err(|_| ErrorPlaceholder::throw("could not parse Reference from host"))?;
+        let refr = js_err!(refr.into_serde());
         // inserting & replacing are the same
         self.engine.borrow_mut().insert_reference(refr);
         Ok(())
@@ -120,9 +112,7 @@ impl Driver {
     /// * `refr` is a
     #[wasm_bindgen(js_name = "includeUncited")]
     pub fn include_uncited(&mut self, uncited: TIncludeUncited) -> Result<(), JsValue> {
-        let uncited = uncited
-            .into_serde()
-            .map_err(|_| ErrorPlaceholder::throw("could not parse IncludeUncited from host"))?;
+        let uncited = js_err!(uncited.into_serde());
         self.engine.borrow_mut().include_uncited(uncited);
         Ok(())
     }
@@ -153,9 +143,7 @@ impl Driver {
     /// Inserts or replaces a cluster with a matching `id`.
     #[wasm_bindgen(js_name = "insertCluster")]
     pub fn insert_cluster(&mut self, cluster_id: JsValue) -> Result<(), JsValue> {
-        let cluster = cluster_id.into_serde().map_err(|e| {
-            ErrorPlaceholder::throw(&format!("could not parse cluster from host: {}", e))
-        })?;
+        let cluster = js_err!(cluster_id.into_serde());
         let mut eng = self.engine.borrow_mut();
         eng.insert_cluster(cluster);
         Ok(())
@@ -194,7 +182,7 @@ impl Driver {
                     id
                 ))
             })
-            .and_then(|b| JsValue::from_serde(&b).map_err(|e| JsError::new(e.description())))?)
+            .and_then(|b| JsValue::from_serde(&b).map_err(|e| JsError::new(&e.to_string())))?)
     }
 
     /// Previews a formatted citation cluster, in a particular position.
@@ -291,8 +279,7 @@ impl Driver {
     pub fn set_cluster_order(&mut self, positions: Box<[JsValue]>) -> Result<(), JsValue> {
         let positions: Vec<ClusterPosition> = utils::read_js_array(positions)?;
         let mut eng = self.engine.borrow_mut();
-        eng.set_cluster_order(&positions)
-            .map_err(|e| ErrorPlaceholder::throw(&format!("{:?}", e)))?;
+        js_err!(eng.set_cluster_order(&positions));
         Ok(())
     }
 

crates/wasm/src/utils.rs

@@ -56,13 +56,16 @@ cfg_if! {
     }
 }
 
-#[derive(Serialize)]
-pub struct ErrorPlaceholder(String);
-
-impl ErrorPlaceholder {
-    pub fn throw(msg: &str) -> JsValue {
-        JsValue::from_serde(&ErrorPlaceholder(msg.to_string())).unwrap()
-    }
+// https://github.com/rustwasm/wasm-bindgen/issues/1742
+macro_rules! js_err {
+    ($expression:expr) => {
+        match $expression {
+            Ok(a) => a,
+            Err(e) => {
+                return Err(js_sys::Error::new(&format!("{}", e)).into());
+            }
+        }
+    };
 }
 
 #[allow(clippy::boxed_local)]
@@ -71,9 +74,7 @@ where
     T: DeserializeOwned,
 {
     let xs: Result<Vec<T>, _> = js.iter().map(|x| x.into_serde()).collect();
-    xs
-        // TODO: remove Debug code
-        .map_err(|e| ErrorPlaceholder::throw(&format!("could not deserialize array: {:?}", e)))
+    Ok(js_err!(xs))
 }
 
 /// A `LocaleFetcher` that statically includes `en-US`, so it never has to be async-fetched, but