Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 74f5e30

Browse filesBrowse files
legendecastargos
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 <anna@addaleax.net> Reviewed-By: Michael Dawson <midawson@redhat.com>
1 parent 8115e6e commit 74f5e30
Copy full SHA for 74f5e30

File tree

Expand file treeCollapse file tree

3 files changed

+79
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+79
-1
lines changed
Open diff view settings
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+1-1Lines 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);
Collapse file

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

Copy file name to clipboardExpand all lines: test/js-native-api/test_exception/test.js
+47Lines 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/js-native-api/test_exception/test_exception.c
+31Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.