Skip to content

Commit 78af363

Browse files
legendecasmhdawson
authored andcommitted
node-api: rtn pending excep on napi_new_instance
When there are any JavaScript exceptions pending, `napi_pending_exception` should be returned. PR-URL: #38798 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 7612d82 commit 78af363

File tree

3 files changed

+79
-1
lines changed

3 files changed

+79
-1
lines changed

src/js_native_api_v8.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,7 @@ napi_status napi_new_instance(napi_env env,
26602660
auto maybe = ctor->NewInstance(context, argc,
26612661
reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)));
26622662

2663-
CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure);
2663+
CHECK_MAYBE_EMPTY(env, maybe, napi_pending_exception);
26642664

26652665
*result = v8impl::JsValueFromV8LocalValue(maybe.ToLocalChecked());
26662666
return GET_RETURN_STATUS(env);

test/js-native-api/test_exception/test.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,34 @@ const test_exception = (function() {
4848
` thrown, but ${returnedError} was passed`);
4949
}
5050

51+
52+
{
53+
const throwTheError = class { constructor() { throw theError; } };
54+
55+
// Test that the native side successfully captures the exception
56+
let returnedError = test_exception.constructReturnException(throwTheError);
57+
assert.strictEqual(returnedError, theError);
58+
59+
// Test that the native side passes the exception through
60+
assert.throws(
61+
() => { test_exception.constructAllowException(throwTheError); },
62+
(err) => err === theError
63+
);
64+
65+
// Test that the exception thrown above was marked as pending
66+
// before it was handled on the JS side
67+
const exception_pending = test_exception.wasPending();
68+
assert.strictEqual(exception_pending, true,
69+
'Exception not pending as expected,' +
70+
` .wasPending() returned ${exception_pending}`);
71+
72+
// Test that the native side does not capture a non-existing exception
73+
returnedError = test_exception.constructReturnException(common.mustCall());
74+
assert.strictEqual(returnedError, undefined,
75+
'Returned error should be undefined when no exception is' +
76+
` thrown, but ${returnedError} was passed`);
77+
}
78+
5179
{
5280
// Test that no exception appears that was not thrown by us
5381
let caughtError;
@@ -66,3 +94,22 @@ const test_exception = (function() {
6694
'Exception state did not remain clear as expected,' +
6795
` .wasPending() returned ${exception_pending}`);
6896
}
97+
98+
{
99+
// Test that no exception appears that was not thrown by us
100+
let caughtError;
101+
try {
102+
test_exception.constructAllowException(common.mustCall());
103+
} catch (anError) {
104+
caughtError = anError;
105+
}
106+
assert.strictEqual(caughtError, undefined,
107+
'No exception originated on the native side, but' +
108+
` ${caughtError} was passed`);
109+
110+
// Test that the exception state remains clear when no exception is thrown
111+
const exception_pending = test_exception.wasPending();
112+
assert.strictEqual(exception_pending, false,
113+
'Exception state did not remain clear as expected,' +
114+
` .wasPending() returned ${exception_pending}`);
115+
}

test/js-native-api/test_exception/test_exception.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@ static napi_value returnException(napi_env env, napi_callback_info info) {
2222
return NULL;
2323
}
2424

25+
static napi_value constructReturnException(napi_env env, napi_callback_info info) {
26+
size_t argc = 1;
27+
napi_value args[1];
28+
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
29+
30+
napi_value result;
31+
napi_status status = napi_new_instance(env, args[0], 0, 0, &result);
32+
if (status == napi_pending_exception) {
33+
napi_value ex;
34+
NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &ex));
35+
return ex;
36+
}
37+
38+
return NULL;
39+
}
40+
2541
static napi_value allowException(napi_env env, napi_callback_info info) {
2642
size_t argc = 1;
2743
napi_value args[1];
@@ -38,6 +54,19 @@ static napi_value allowException(napi_env env, napi_callback_info info) {
3854
return NULL;
3955
}
4056

57+
static napi_value constructAllowException(napi_env env, napi_callback_info info) {
58+
size_t argc = 1;
59+
napi_value args[1];
60+
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
61+
62+
napi_value result;
63+
napi_new_instance(env, args[0], 0, 0, &result);
64+
// Ignore status and check napi_is_exception_pending() instead.
65+
66+
NODE_API_CALL(env, napi_is_exception_pending(env, &exceptionWasPending));
67+
return NULL;
68+
}
69+
4170
static napi_value wasPending(napi_env env, napi_callback_info info) {
4271
napi_value result;
4372
NODE_API_CALL(env, napi_get_boolean(env, exceptionWasPending, &result));
@@ -64,6 +93,8 @@ napi_value Init(napi_env env, napi_value exports) {
6493
napi_property_descriptor descriptors[] = {
6594
DECLARE_NODE_API_PROPERTY("returnException", returnException),
6695
DECLARE_NODE_API_PROPERTY("allowException", allowException),
96+
DECLARE_NODE_API_PROPERTY("constructReturnException", constructReturnException),
97+
DECLARE_NODE_API_PROPERTY("constructAllowException", constructAllowException),
6798
DECLARE_NODE_API_PROPERTY("wasPending", wasPending),
6899
DECLARE_NODE_API_PROPERTY("createExternal", createExternal),
69100
};

0 commit comments

Comments
 (0)