fix(napi): re-throw error in ThreadsafeFunction callback if we could

crates/napi/src/error.rs

@@ -81,7 +81,10 @@ impl From<JsUnknown> for Error {
     let mut result = std::ptr::null_mut();
     let status = unsafe { sys::napi_create_reference(value.0.env, value.0.value, 0, &mut result) };
     if status != sys::Status::napi_ok {
-      return Error::new(Status::from(status), "".to_owned());
+      return Error::new(
+        Status::from(status),
+        "Create Error reference failed".to_owned(),
+      );
     }
     Self {
       status: Status::GenericFailure,

crates/napi/src/threadsafe_function.rs

@@ -178,7 +178,7 @@ enum ThreadsafeFunctionCallVariant {
 struct ThreadsafeFunctionCallJsBackData<T> {
   data: T,
   call_variant: ThreadsafeFunctionCallVariant,
-  callback: Box<dyn FnOnce(JsUnknown) -> Result<()>>,
+  callback: Box<dyn FnOnce(Result<JsUnknown>) -> Result<()>>,
 }
 
 /// Communicate with the addon's main thread by invoking a JavaScript function from other threads.
@@ -434,7 +434,7 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
             ThreadsafeFunctionCallJsBackData {
               data,
               call_variant: ThreadsafeFunctionCallVariant::Direct,
-              callback: Box::new(|_d: JsUnknown| Ok(())),
+              callback: Box::new(|_d: Result<JsUnknown>| Ok(())),
             }
           })))
           .cast(),
@@ -463,8 +463,8 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
             ThreadsafeFunctionCallJsBackData {
               data,
               call_variant: ThreadsafeFunctionCallVariant::WithCallback,
-              callback: Box::new(move |d: JsUnknown| {
+              callback: Box::new(move |d: Result<JsUnknown>| {
-                D::from_napi_value(d.0.env, d.0.value).and_then(cb)
+                d.and_then(|d| D::from_napi_value(d.0.env, d.0.value).and_then(cb))
               }),
             }
           })))
@@ -478,7 +478,7 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
 
   #[cfg(feature = "tokio_rt")]
   pub async fn call_async<D: 'static + FromNapiValue>(&self, value: Result<T>) -> Result<D> {
-    let (sender, receiver) = tokio::sync::oneshot::channel::<D>();
+    let (sender, receiver) = tokio::sync::oneshot::channel::<Result<D>>();
 
     self.handle.with_read_aborted(|aborted| {
       if aborted {
@@ -492,12 +492,12 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
             ThreadsafeFunctionCallJsBackData {
               data,
               call_variant: ThreadsafeFunctionCallVariant::WithCallback,
-              callback: Box::new(move |d: JsUnknown| {
+              callback: Box::new(move |d: Result<JsUnknown>| {
-                D::from_napi_value(d.0.env, d.0.value).and_then(move |d| {
+                sender
-                  sender.send(d).map_err(|_| {
+                  .send(d.and_then(|d| D::from_napi_value(d.0.env, d.0.value)))
+                  .map_err(|_| {
                     crate::Error::from_reason("Failed to send return value to tokio sender")
                   })
-                })
               }),
             }
           })))
@@ -508,7 +508,13 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
     })?;
     receiver
       .await
-      .map_err(|err| crate::Error::new(Status::GenericFailure, format!("{}", err)))
+      .map_err(|_| {
+        crate::Error::new(
+          Status::GenericFailure,
+          "Receive value from threadsafe function sender failed",
+        )
+      })
+      .and_then(|ret| ret)
   }
 }
 
@@ -527,7 +533,7 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::Fatal> {
           Box::into_raw(Box::new(ThreadsafeFunctionCallJsBackData {
             data: value,
             call_variant: ThreadsafeFunctionCallVariant::Direct,
-            callback: Box::new(|_d: JsUnknown| Ok(())),
+            callback: Box::new(|_d: Result<JsUnknown>| Ok(())),
           }))
           .cast(),
           mode.into(),
@@ -554,8 +560,8 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::Fatal> {
           Box::into_raw(Box::new(ThreadsafeFunctionCallJsBackData {
             data: value,
             call_variant: ThreadsafeFunctionCallVariant::WithCallback,
-            callback: Box::new(move |d: JsUnknown| {
+            callback: Box::new(move |d: Result<JsUnknown>| {
-              D::from_napi_value(d.0.env, d.0.value).and_then(cb)
+              d.and_then(|d| D::from_napi_value(d.0.env, d.0.value).and_then(cb))
             }),
           }))
           .cast(),
@@ -581,12 +587,14 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::Fatal> {
           Box::into_raw(Box::new(ThreadsafeFunctionCallJsBackData {
             data: value,
             call_variant: ThreadsafeFunctionCallVariant::WithCallback,
-            callback: Box::new(move |d: JsUnknown| {
+            callback: Box::new(move |d: Result<JsUnknown>| {
+              d.and_then(|d| {
                 D::from_napi_value(d.0.env, d.0.value).and_then(move |d| {
                   sender.send(d).map_err(|_| {
                     crate::Error::from_reason("Failed to send return value to tokio sender")
                   })
                 })
+              })
             }),
           }))
           .cast(),
@@ -677,7 +685,7 @@ unsafe extern "C" fn call_js_cb<T: 'static, V: ToNapiValue, R, ES>(
         values.collect()
       };
       let mut return_value = ptr::null_mut();
-      let status = match args {
+      let mut status = match args {
         Ok(args) => unsafe {
           sys::napi_call_function(
             raw_env,
@@ -705,11 +713,26 @@ unsafe extern "C" fn call_js_cb<T: 'static, V: ToNapiValue, R, ES>(
         },
       };
       if let ThreadsafeFunctionCallVariant::WithCallback = call_variant {
-        if let Err(err) = callback(JsUnknown(crate::Value {
+        // throw Error in JavaScript callback
+        let callback_arg = if status == sys::Status::napi_pending_exception {
+          let mut exception = ptr::null_mut();
+          status = unsafe { sys::napi_get_and_clear_last_exception(raw_env, &mut exception) };
+          Err(
+            JsUnknown(crate::Value {
+              env: raw_env,
+              value: exception,
+              value_type: crate::ValueType::Unknown,
+            })
+            .into(),
+          )
+        } else {
+          Ok(JsUnknown(crate::Value {
             env: raw_env,
             value: return_value,
             value_type: crate::ValueType::Unknown,
-        })) {
+          }))
+        };
+        if let Err(err) = callback(callback_arg) {
           let message = format!(
             "Failed to convert return value in ThreadsafeFunction callback into Rust value: {}",
             err
@@ -717,7 +740,7 @@ unsafe extern "C" fn call_js_cb<T: 'static, V: ToNapiValue, R, ES>(
           let message_length = message.len();
           unsafe {
             sys::napi_fatal_error(
-              "threadsafe_function.rs:642\0".as_ptr().cast(),
+              "threadsafe_function.rs:720\0".as_ptr().cast(),
               26,
               CString::new(message).unwrap().into_raw(),
               message_length,

examples/napi/__tests__/__snapshots__/typegen.spec.ts.md

@@ -518,6 +518,8 @@ Generated by [AVA](https://avajs.dev).
     ␊
     export function tsfnReturnPromiseTimeout(func: (err: Error | null, value: number) => any): Promise<number>␊
     ␊
+    export function tsfnThrowFromJs(tsfn: (err: Error | null, value: number) => any): Promise<number>␊
+    ␊
     export function tsRename(a: { foo: number }): string[]␊
     ␊
     export interface TsTypeChanged {␊

examples/napi/__tests__/values.spec.ts

@@ -55,6 +55,7 @@ import {
   threadsafeFunctionClosureCapture,
   tsfnCallWithCallback,
   tsfnAsyncCall,
+  tsfnThrowFromJs,
   asyncPlus100,
   getGlobal,
   getUndefined,
@@ -843,6 +844,19 @@ Napi4Test('async call ThreadsafeFunction', async (t) => {
   )
 })
 
+test('Throw from ThreadsafeFunction JavaScript callback', async (t) => {
+  const errMsg = 'ThrowFromJavaScriptRawCallback'
+  await t.throwsAsync(
+    () =>
+      tsfnThrowFromJs(() => {
+        throw new Error(errMsg)
+      }),
+    {
+      message: errMsg,
+    },
+  )
+})
+
 Napi4Test('accept ThreadsafeFunction', async (t) => {
   await new Promise<void>((resolve, reject) => {
     acceptThreadsafeFunction((err, value) => {
@@ -923,7 +937,7 @@ Napi4Test('object only from js', (t) => {
   })
 })
 
-Napi4Test('promise in either', async (t) => {
+test('promise in either', async (t) => {
   t.is(await promiseInEither(1), false)
   t.is(await promiseInEither(20), true)
   t.is(await promiseInEither(Promise.resolve(1)), false)

examples/napi/index.d.ts

@@ -508,6 +508,8 @@ export function tsfnReturnPromise(func: (err: Error | null, value: number) => an
 
 export function tsfnReturnPromiseTimeout(func: (err: Error | null, value: number) => any): Promise<number>
 
+export function tsfnThrowFromJs(tsfn: (err: Error | null, value: number) => any): Promise<number>
+
 export function tsRename(a: { foo: number }): string[]
 
 export interface TsTypeChanged {

examples/napi/src/threadsafe_function.rs

@@ -160,3 +160,8 @@ pub async fn tsfn_return_promise_timeout(func: ThreadsafeFunction<u32>) -> Resul
     }
   }
 }
+
+#[napi]
+pub async fn tsfn_throw_from_js(tsfn: ThreadsafeFunction<u32>) -> napi::Result<u32> {
+  tsfn.call_async::<Promise<u32>>(Ok(42)).await?.await
+}