Merge pull request #1076 from neon-bindings/kv/try-from-error

fix(neon): Require `TryFromJs::Error` to implement `TryIntoJs`

crates/neon/src/types_impl/extract/boxed.rs

@@ -1,7 +1,7 @@
 use crate::{
     context::{Context, Cx},
     handle::Handle,
-    result::{JsResult, NeonResult, ResultExt},
+    result::{JsResult, NeonResult},
     types::{
         extract::{private, TryFromJs, TryIntoJs, TypeExpected},
         Finalize, JsBox, JsValue,
@@ -59,10 +59,6 @@ where
             Err(_) => Ok(Err(TypeExpected::new())),
         }
     }
-
-    fn from_js(cx: &mut Cx<'cx>, v: Handle<'cx, JsValue>) -> NeonResult<Self> {
-        Self::try_from_js(cx, v)?.or_throw(cx)
-    }
 }
 
 impl<'cx, T> TryIntoJs<'cx> for Boxed<T>

crates/neon/src/types_impl/extract/error.rs

@@ -1,13 +1,59 @@
-use std::{error, fmt};
+use std::{convert::Infallible, error, fmt, marker::PhantomData};
 
 use crate::{
     context::{Context, Cx},
     result::JsResult,
-    types::{extract::TryIntoJs, JsError},
+    types::{
+        extract::{private, TryIntoJs},
+        JsError, JsValue, Value,
+    },
 };
 
 type BoxError = Box<dyn error::Error + Send + Sync + 'static>;
 
+/// Error returned when a JavaScript value is not the type expected
+pub struct TypeExpected<T: Value>(PhantomData<T>);
+
+impl<T: Value> TypeExpected<T> {
+    pub(super) fn new() -> Self {
+        Self(PhantomData)
+    }
+}
+
+impl<T: Value> fmt::Display for TypeExpected<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "expected {}", T::name())
+    }
+}
+
+impl<T: Value> fmt::Debug for TypeExpected<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("TypeExpected").field(&T::name()).finish()
+    }
+}
+
+impl<T: Value> error::Error for TypeExpected<T> {}
+
+impl<'cx, T: Value> TryIntoJs<'cx> for TypeExpected<T> {
+    type Value = JsError;
+
+    fn try_into_js(self, cx: &mut Cx<'cx>) -> JsResult<'cx, Self::Value> {
+        JsError::type_error(cx, self.to_string())
+    }
+}
+
+impl<T: Value> private::Sealed for TypeExpected<T> {}
+
+impl<'cx> TryIntoJs<'cx> for Infallible {
+    type Value = JsValue;
+
+    fn try_into_js(self, _: &mut Cx<'cx>) -> JsResult<'cx, Self::Value> {
+        unreachable!()
+    }
+}
+
+impl private::Sealed for Infallible {}
+
 #[derive(Debug)]
 /// Error that implements [`TryIntoJs`] and can produce specific error types.
 ///

crates/neon/src/types_impl/extract/json.rs

@@ -1,11 +1,29 @@
+//! Extract JavaScript values with JSON serialization
+//!
+//! For complex objects that implement [`serde::Serialize`] and [`serde::Deserialize`],
+//! it is more ergonomic--and often faster--to extract with JSON serialization. The [`Json`]
+//! extractor automatically calls `JSON.stringify` and `JSON.parse` as necessary.
+//!
+//! ```
+//! use neon::types::extract::Json;
+//!
+//! #[neon::export]
+//! fn sort(Json(mut strings): Json<Vec<String>>) -> Json<Vec<String>> {
+//!     strings.sort();
+//!     Json(strings)
+//! }
+//! ```
+
+use std::{error, fmt};
+
 use crate::{
     context::{Context, Cx},
     handle::Handle,
     object::Object,
     result::{JsResult, NeonResult},
     types::{
         extract::{private, TryFromJs, TryIntoJs},
-        JsFunction, JsObject, JsString, JsValue,
+        JsError, JsFunction, JsObject, JsString, JsValue,
     },
 };
 
@@ -73,17 +91,15 @@ impl<'cx, T> TryFromJs<'cx> for Json<T>
 where
     for<'de> T: serde::de::Deserialize<'de>,
 {
-    type Error = serde_json::Error;
+    type Error = Error;
 
     fn try_from_js(
         cx: &mut Cx<'cx>,
         v: Handle<'cx, JsValue>,
     ) -> NeonResult<Result<Self, Self::Error>> {
-        Ok(serde_json::from_str(&stringify(cx, v)?).map(Json))
-    }
-
-    fn from_js(cx: &mut Cx<'cx>, v: Handle<'cx, JsValue>) -> NeonResult<Self> {
-        Self::try_from_js(cx, v)?.or_else(|err| cx.throw_error(err.to_string()))
+        Ok(serde_json::from_str(&stringify(cx, v)?)
+            .map(Json)
+            .map_err(Error))
     }
 }
 
@@ -101,3 +117,30 @@ where
 }
 
 impl<T> private::Sealed for Json<T> {}
+
+/// Error returned when a value is invalid JSON
+pub struct Error(serde_json::Error);
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.0, f)
+    }
+}
+
+impl fmt::Debug for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(&self.0, f)
+    }
+}
+
+impl error::Error for Error {}
+
+impl<'cx> TryIntoJs<'cx> for Error {
+    type Value = JsError;
+
+    fn try_into_js(self, cx: &mut Cx<'cx>) -> JsResult<'cx, Self::Value> {
+        JsError::error(cx, self.to_string())
+    }
+}
+
+impl private::Sealed for Error {}

crates/neon/src/types_impl/extract/mod.rs

@@ -99,25 +99,29 @@
 //! Note well, in this example, type annotations are not required on the tuple because
 //! Rust is able to infer it from the type arguments on `add` and `concat`.
 
-use std::{fmt, marker::PhantomData};
-
 use crate::{
     context::{Context, Cx, FunctionContext},
     handle::Handle,
-    result::{JsResult, NeonResult, ResultExt},
+    result::{JsResult, NeonResult},
     types::{JsValue, Value},
 };
 
-pub use self::{boxed::Boxed, error::Error, with::With};
+pub use self::{
+    boxed::Boxed,
+    error::{Error, TypeExpected},
+    with::With,
+};
 
 #[cfg(feature = "serde")]
 #[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
 pub use self::json::Json;
 
+#[cfg(feature = "serde")]
+#[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
+pub mod json;
+
 mod boxed;
 mod error;
-#[cfg(feature = "serde")]
-mod json;
 mod private;
 mod try_from_js;
 mod try_into_js;
@@ -128,10 +132,7 @@ pub trait TryFromJs<'cx>
 where
     Self: private::Sealed + Sized,
 {
-    /// Error indicating non-JavaScript exception failure when extracting
-    // Consider adding a trait bound prior to unsealing `TryFromjs`
-    // https://github.com/neon-bindings/neon/issues/1026
-    type Error;
+    type Error: TryIntoJs<'cx>;
 
     /// Extract this Rust type from a JavaScript value
     fn try_from_js(
@@ -140,7 +141,16 @@ where
     ) -> NeonResult<Result<Self, Self::Error>>;
 
     /// Same as [`TryFromJs`], but all errors are converted to JavaScript exceptions
-    fn from_js(cx: &mut Cx<'cx>, v: Handle<'cx, JsValue>) -> NeonResult<Self>;
+    fn from_js(cx: &mut Cx<'cx>, v: Handle<'cx, JsValue>) -> NeonResult<Self> {
+        match Self::try_from_js(cx, v)? {
+            Ok(v) => Ok(v),
+            Err(err) => {
+                let err = err.try_into_js(cx)?;
+
+                cx.throw(err)
+            }
+        }
+    }
 }
 
 /// Convert Rust data into a JavaScript value
@@ -166,38 +176,6 @@ pub struct ArrayBuffer(pub Vec<u8>);
 /// Wrapper for converting between [`Vec<u8>`] and [`JsBuffer`](super::JsBuffer)
 pub struct Buffer(pub Vec<u8>);
 
-/// Error returned when a JavaScript value is not the type expected
-pub struct TypeExpected<T: Value>(PhantomData<T>);
-
-impl<T: Value> TypeExpected<T> {
-    fn new() -> Self {
-        Self(PhantomData)
-    }
-}
-
-impl<T: Value> fmt::Display for TypeExpected<T> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "expected {}", T::name())
-    }
-}
-
-impl<T: Value> fmt::Debug for TypeExpected<T> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_tuple("TypeExpected").field(&T::name()).finish()
-    }
-}
-
-impl<T: Value> std::error::Error for TypeExpected<T> {}
-
-impl<T, U: Value> ResultExt<T> for Result<T, TypeExpected<U>> {
-    fn or_throw<'a, C: Context<'a>>(self, cx: &mut C) -> NeonResult<T> {
-        match self {
-            Ok(v) => Ok(v),
-            Err(_) => cx.throw_type_error(format!("expected {}", U::name())),
-        }
-    }
-}
-
 /// Trait specifying values that may be extracted from function arguments.
 ///
 /// **Note:** This trait is implemented for tuples of up to 32 values, but for