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

Additionally:
* Replace `TryFromJs::from_js` implementations with a default
* Add a `json::Error` wrapper to hide implementation details

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

@@ -1,13 +1,60 @@
-use std::{error, fmt};
+use std::{convert::Infallible, error, fmt, marker::PhantomData};
 
+use crate::types_impl::JsValue;
 use crate::{
     context::{Context, Cx},
     result::JsResult,
-    types::{extract::TryIntoJs, JsError},
+    types::{
+        extract::{private, TryIntoJs},
+        JsError, 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,24 +99,27 @@
 //! 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::{error::Error, with::With};
+pub use self::{
+    error::{Error, TypeExpected},
+    with::With,
+};
 
 #[cfg(feature = "serde")]
 #[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
 pub use self::json::Json;
 
-mod error;
 #[cfg(feature = "serde")]
-mod json;
+#[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
+pub mod json;
+
+mod error;
 mod private;
 mod try_from_js;
 mod try_into_js;
@@ -127,10 +130,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(
@@ -139,7 +139,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
@@ -165,38 +174,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

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

@@ -9,7 +9,7 @@ use crate::{
     context::{internal::ContextInternal, Cx},
     handle::{Handle, Root},
     object::Object,
-    result::{NeonResult, ResultExt, Throw},
+    result::{NeonResult, Throw},
     sys,
     types::{
         buffer::{Binary, TypedArray},
@@ -22,14 +22,6 @@ use crate::{
 #[cfg(feature = "napi-5")]
 use crate::types::JsDate;
 
-macro_rules! from_js {
-    () => {
-        fn from_js(cx: &mut Cx<'cx>, v: Handle<'cx, JsValue>) -> NeonResult<Self> {
-            Self::try_from_js(cx, v)?.or_throw(cx)
-        }
-    };
-}
-
 impl<'cx, V> TryFromJs<'cx> for Handle<'cx, V>
 where
     V: Value,
@@ -42,8 +34,6 @@ where
     ) -> NeonResult<Result<Self, Self::Error>> {
         Ok(v.downcast(cx).map_err(|_| TypeExpected::new()))
     }
-
-    from_js!();
 }
 
 impl<'cx, O> TryFromJs<'cx> for Root<O>
@@ -61,8 +51,6 @@ where
             Err(_) => Err(TypeExpected::new()),
         })
     }
-
-    from_js!();
 }
 
 impl<'cx, T> TryFromJs<'cx> for Option<T>
@@ -81,14 +69,6 @@ where
 
         T::try_from_js(cx, v).map(|v| v.map(Some))
     }
-
-    fn from_js(cx: &mut Cx<'cx>, v: Handle<'cx, JsValue>) -> NeonResult<Self> {
-        if is_null_or_undefined(cx, v)? {
-            return Ok(None);
-        }
-
-        T::from_js(cx, v).map(Some)
-    }
 }
 
 impl<'cx> TryFromJs<'cx> for f64 {
@@ -110,8 +90,6 @@ impl<'cx> TryFromJs<'cx> for f64 {
 
         Ok(Ok(n))
     }
-
-    from_js!();
 }
 
 impl<'cx> TryFromJs<'cx> for bool {
@@ -133,8 +111,6 @@ impl<'cx> TryFromJs<'cx> for bool {
 
         Ok(Ok(b))
     }
-
-    from_js!();
 }
 
 impl<'cx> TryFromJs<'cx> for String {
@@ -178,8 +154,6 @@ impl<'cx> TryFromJs<'cx> for String {
             Ok(Ok(String::from_utf8_unchecked(buf)))
         }
     }
-
-    from_js!();
 }
 
 #[cfg_attr(docsrs, doc(cfg(feature = "napi-5")))]
@@ -203,8 +177,6 @@ impl<'cx> TryFromJs<'cx> for Date {
 
         Ok(Ok(Date(d)))
     }
-
-    from_js!();
 }
 
 // This implementation primarily exists for macro authors. It is infallible, rather
@@ -226,10 +198,6 @@ impl<'cx> TryFromJs<'cx> for () {
     ) -> NeonResult<Result<Self, Self::Error>> {
         Ok(Ok(()))
     }
-
-    fn from_js(_cx: &mut Cx<'cx>, _v: Handle<'cx, JsValue>) -> NeonResult<Self> {
-        Ok(())
-    }
 }
 
 impl<'cx, T> TryFromJs<'cx> for Vec<T>
@@ -250,8 +218,6 @@ where
 
         Ok(Ok(v.as_slice(cx).to_vec()))
     }
-
-    from_js!();
 }
 
 impl<'cx> TryFromJs<'cx> for Buffer {
@@ -268,8 +234,6 @@ impl<'cx> TryFromJs<'cx> for Buffer {
 
         Ok(Ok(Buffer(v.as_slice(cx).to_vec())))
     }
-
-    from_js!();
 }
 
 impl<'cx> TryFromJs<'cx> for ArrayBuffer {
@@ -286,8 +250,6 @@ impl<'cx> TryFromJs<'cx> for ArrayBuffer {
 
         Ok(Ok(ArrayBuffer(v.as_slice(cx).to_vec())))
     }
-
-    from_js!();
 }
 
 fn is_null_or_undefined<V>(cx: &mut Cx, v: Handle<V>) -> NeonResult<bool>

test/napi/src/js/objects.rs

@@ -113,12 +113,12 @@ pub fn call_methods_with_prop(mut cx: FunctionContext) -> JsResult<JsString> {
     obj.prop(&mut cx, "setName")
         .bind()?
         .arg("Wonder Woman")?
-        .call()?;
+        .exec()?;
     obj.prop(&mut cx, "toString").bind()?.call()
 }
 
 pub fn call_non_method_with_prop(mut cx: FunctionContext) -> JsResult<JsUndefined> {
     let obj: Handle<JsObject> = cx.argument::<JsObject>(0)?;
-    obj.prop(&mut cx, "number").bind()?.call()?;
+    obj.prop(&mut cx, "number").bind()?.exec()?;
     Ok(cx.undefined())
 }