Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v14.x-backport] node-api: rtn pending excep on napi_new_instance #39516

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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