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: #19362 Fixes: nodejs/node-addon-api#235 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Mar 17, 2018 · d2b99e3 · d2b99e3
1 parent 425c589
commit d2b99e3
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 5 deletions.
diff --git a/src/node_api.cc b/src/node_api.cc
@@ -2811,11 +2811,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
@@ -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;
 }