Skip to content

Commit

Permalink
node-api: rtn pending excep on napi_new_instance
Browse files Browse the repository at this point in the history
When there are any JavaScript exceptions pending,
`napi_pending_exception` should be returned.

PR-URL: nodejs#38798
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
  • Loading branch information
legendecas committed Jul 25, 2021
1 parent 64140af commit f89cf6a
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2662,7 +2662,7 @@ napi_status napi_new_instance(napi_env env,
auto maybe = ctor->NewInstance(context, argc,
reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)));

CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure);
CHECK_MAYBE_EMPTY(env, maybe, napi_pending_exception);

*result = v8impl::JsValueFromV8LocalValue(maybe.ToLocalChecked());
return GET_RETURN_STATUS(env);
Expand Down
47 changes: 47 additions & 0 deletions test/js-native-api/test_exception/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,34 @@ const test_exception = (function() {
` thrown, but ${returnedError} was passed`);
}


{
const throwTheError = class { constructor() { throw theError; } };

// Test that the native side successfully captures the exception
let returnedError = test_exception.constructReturnException(throwTheError);
assert.strictEqual(returnedError, theError);

// Test that the native side passes the exception through
assert.throws(
() => { test_exception.constructAllowException(throwTheError); },
(err) => err === theError
);

// Test that the exception thrown above was marked as pending
// before it was handled on the JS side
const exception_pending = test_exception.wasPending();
assert.strictEqual(exception_pending, true,
'Exception not pending as expected,' +
` .wasPending() returned ${exception_pending}`);

// Test that the native side does not capture a non-existing exception
returnedError = test_exception.constructReturnException(common.mustCall());
assert.strictEqual(returnedError, undefined,
'Returned error should be undefined when no exception is' +
` thrown, but ${returnedError} was passed`);
}

{
// Test that no exception appears that was not thrown by us
let caughtError;
Expand All @@ -66,3 +94,22 @@ const test_exception = (function() {
'Exception state did not remain clear as expected,' +
` .wasPending() returned ${exception_pending}`);
}

{
// Test that no exception appears that was not thrown by us
let caughtError;
try {
test_exception.constructAllowException(common.mustCall());
} catch (anError) {
caughtError = anError;
}
assert.strictEqual(caughtError, undefined,
'No exception originated on the native side, but' +
` ${caughtError} was passed`);

// Test that the exception state remains clear when no exception is thrown
const exception_pending = test_exception.wasPending();
assert.strictEqual(exception_pending, false,
'Exception state did not remain clear as expected,' +
` .wasPending() returned ${exception_pending}`);
}
31 changes: 31 additions & 0 deletions test/js-native-api/test_exception/test_exception.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,22 @@ static napi_value returnException(napi_env env, napi_callback_info info) {
return NULL;
}

static napi_value constructReturnException(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

napi_value result;
napi_status status = napi_new_instance(env, args[0], 0, 0, &result);
if (status == napi_pending_exception) {
napi_value ex;
NAPI_CALL(env, napi_get_and_clear_last_exception(env, &ex));
return ex;
}

return NULL;
}

static napi_value allowException(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
Expand All @@ -38,6 +54,19 @@ static napi_value allowException(napi_env env, napi_callback_info info) {
return NULL;
}

static napi_value constructAllowException(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

napi_value result;
napi_new_instance(env, args[0], 0, 0, &result);
// Ignore status and check napi_is_exception_pending() instead.

NAPI_CALL(env, napi_is_exception_pending(env, &exceptionWasPending));
return NULL;
}

static napi_value wasPending(napi_env env, napi_callback_info info) {
napi_value result;
NAPI_CALL(env, napi_get_boolean(env, exceptionWasPending, &result));
Expand All @@ -64,6 +93,8 @@ napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor descriptors[] = {
DECLARE_NAPI_PROPERTY("returnException", returnException),
DECLARE_NAPI_PROPERTY("allowException", allowException),
DECLARE_NAPI_PROPERTY("constructReturnException", constructReturnException),
DECLARE_NAPI_PROPERTY("constructAllowException", constructAllowException),
DECLARE_NAPI_PROPERTY("wasPending", wasPending),
DECLARE_NAPI_PROPERTY("createExternal", createExternal),
};
Expand Down

0 comments on commit f89cf6a

Please sign in to comment.