Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
mhdawson committed Mar 16, 2018
1 parent 96cb4fb commit 6a5a9ad
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 5 deletions.
12 changes: 8 additions & 4 deletions src/node_api.cc
Expand Up @@ -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);
Expand Down
15 changes: 14 additions & 1 deletion test/addons-napi/test_make_callback_recurse/binding.cc
Expand Up @@ -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;
}

Expand Down

0 comments on commit 6a5a9ad

Please sign in to comment.