n-api: add missing exception checking

Add checks for a pending exception in napi_make_callback
after the callback has been invoked.  If there is a pending
exception then we need to avoid checking the result as that
will not be able to complete properly.

Add additional checks to the unit test for napi_make_callback
to catch this case.

PR-URL: https://github.com/nodejs/node/pull/19362
Fixes: https://github.com/nodejs/node-addon-api/issues/235
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

2 files changed, 22 insertions, 5 deletions

diff --git a/src/node_api.cc b/src/node_api.cc
index bb027822f1..2ee241badf 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -2801,11 +2801,15 @@ napi_status napi_make_callback(napi_env env,
       isolate, v8recv, v8func, argc,
       reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)),
       *node_async_context);
-  CHECK_MAYBE_EMPTY(env, callback_result, napi_generic_failure);
 
-  if (result != nullptr) {
-    *result = v8impl::JsValueFromV8LocalValue(
-        callback_result.ToLocalChecked());
+  if (try_catch.HasCaught()) {
+    return napi_set_last_error(env, napi_pending_exception);
+  } else {
+    CHECK_MAYBE_EMPTY(env, callback_result, napi_generic_failure);
+    if (result != nullptr) {
+      *result = v8impl::JsValueFromV8LocalValue(
+          callback_result.ToLocalChecked());
+    }
   }
 
   return GET_RETURN_STATUS(env);
diff --git a/test/addons-napi/test_make_callback_recurse/binding.cc b/test/addons-napi/test_make_callback_recurse/binding.cc
index 641f9f6423..bfe9a457d2 100644
--- a/test/addons-napi/test_make_callback_recurse/binding.cc
+++ b/test/addons-napi/test_make_callback_recurse/binding.cc
@@ -13,9 +13,22 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) {
   napi_value recv = args[0];
   napi_value func = args[1];
 
-  napi_make_callback(env, nullptr /* async_context */,
+  napi_status status = napi_make_callback(env, nullptr /* async_context */,
     recv, func, 0 /* argc */, nullptr /* argv */, nullptr /* result */);
 
+  bool isExceptionPending;
+  NAPI_CALL(env, napi_is_exception_pending(env, &isExceptionPending));
+  if (isExceptionPending && !(status == napi_pending_exception)) {
+    // if there is an exception pending we don't expect any
+    // other error
+    napi_value pending_error;
+    status = napi_get_and_clear_last_exception(env, &pending_error);
+    NAPI_CALL(env,
+      napi_throw_error((env),
+                        nullptr,
+                        "error when only pending exception expected"));
+  }
+
   return recv;
 }