Skip to content

Commit 072c20a

Browse files
serchersercher
committed
node-api: enable uncaught exceptions policy by default
This enables the option `--force-node-api-uncaught-exceptions-policy` for a specific Node-API addon when it is compiled with `NAPI_EXPERIMENTAL` (and this would be the default behavior when `NAPI_VERSION` 10 releases). This would not break existing Node-API addons. PR-URL: nodejs/node#49313 Refs: nodejs/node#36510 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
1 parent 4241988 commit 072c20a

16 files changed

+307
-133
lines changed

‎graal-nodejs/doc/api/n-api.md

+8
Original file line numberDiff line numberDiff line change
@@ -6241,6 +6241,13 @@ napi_create_threadsafe_function(napi_env env,
62416241
[`napi_threadsafe_function_call_js`][] provides more details.
62426242
* `[out] result`: The asynchronous thread-safe JavaScript function.
62436243

6244+
**Change History:**
6245+
6246+
* Experimental (`NAPI_EXPERIMENTAL` is defined):
6247+
6248+
Uncaught exceptions thrown in `call_js_cb` are handled with the
6249+
[`'uncaughtException'`][] event, instead of being ignored.
6250+
62446251
### `napi_get_threadsafe_function_context`
62456252

62466253
<!-- YAML
@@ -6471,6 +6478,7 @@ the add-on's file name during loading.
64716478
[Visual Studio]: https://visualstudio.microsoft.com
64726479
[Working with JavaScript properties]: #working-with-javascript-properties
64736480
[Xcode]: https://developer.apple.com/xcode/
6481+
[`'uncaughtException'`]: process.md#event-uncaughtexception
64746482
[`Number.MAX_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.max_safe_integer
64756483
[`Number.MIN_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.min_safe_integer
64766484
[`Worker`]: worker_threads.md#class-worker

‎graal-nodejs/src/node_api.cc

+10-6
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,8 @@ void node_napi_env__::trigger_fatal_exception(v8::Local<v8::Value> local_err) {
8282
node::errors::TriggerUncaughtException(isolate, local_err, local_msg);
8383
}
8484

85-
// option enforceUncaughtExceptionPolicy is added for not breaking existing
86-
// running n-api add-ons, and should be deprecated in the next major Node.js
87-
// release.
85+
// The option enforceUncaughtExceptionPolicy is added for not breaking existing
86+
// running Node-API add-ons.
8887
template <bool enforceUncaughtExceptionPolicy, typename T>
8988
void node_napi_env__::CallbackIntoModule(T&& call) {
9089
CallIntoModule(call, [](napi_env env_, v8::Local<v8::Value> local_err) {
@@ -93,19 +92,24 @@ void node_napi_env__::CallbackIntoModule(T&& call) {
9392
return;
9493
}
9594
node::Environment* node_env = env->node_env();
96-
if (!node_env->options()->force_node_api_uncaught_exceptions_policy &&
95+
// If the module api version is less than NAPI_VERSION_EXPERIMENTAL,
96+
// and the option --force-node-api-uncaught-exceptions-policy is not
97+
// specified, emit a warning about the uncaught exception instead of
98+
// triggering uncaught exception event.
99+
if (env->module_api_version < NAPI_VERSION_EXPERIMENTAL &&
100+
!node_env->options()->force_node_api_uncaught_exceptions_policy &&
97101
!enforceUncaughtExceptionPolicy) {
98102
ProcessEmitDeprecationWarning(
99103
node_env,
100104
"Uncaught N-API callback exception detected, please run node "
101-
"with option --force-node-api-uncaught-exceptions-policy=true"
105+
"with option --force-node-api-uncaught-exceptions-policy=true "
102106
"to handle those exceptions properly.",
103107
"DEP0168");
104108
return;
105109
}
106110
// If there was an unhandled exception in the complete callback,
107111
// report it as a fatal exception. (There is no JavaScript on the
108-
// callstack that can possibly handle it.)
112+
// call stack that can possibly handle it.)
109113
env->trigger_fatal_exception(local_err);
110114
});
111115
}

‎graal-nodejs/test/js-native-api/test_reference/binding.gyp

+6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@
55
"sources": [
66
"test_reference.c"
77
]
8+
},
9+
{
10+
"target_name": "test_finalizer",
11+
"sources": [
12+
"test_finalizer.c"
13+
]
814
}
915
]
1016
}

‎graal-nodejs/test/js-native-api/test_reference/test_finalizer.c

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
#include <assert.h>
2+
#include <js_native_api.h>
3+
#include <stdlib.h>
4+
#include "../common.h"
5+
#include "../entry_point.h"
6+
7+
static int test_value = 1;
8+
static int finalize_count = 0;
9+
10+
static void FinalizeExternalCallJs(napi_env env, void* data, void* hint) {
11+
int* actual_value = data;
12+
NODE_API_ASSERT_RETURN_VOID(
13+
env,
14+
actual_value == &test_value,
15+
"The correct pointer was passed to the finalizer");
16+
17+
napi_ref finalizer_ref = (napi_ref)hint;
18+
napi_value js_finalizer;
19+
napi_value recv;
20+
NODE_API_CALL_RETURN_VOID(
21+
env, napi_get_reference_value(env, finalizer_ref, &js_finalizer));
22+
NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv));
23+
NODE_API_CALL_RETURN_VOID(
24+
env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL));
25+
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref));
26+
}
27+
28+
static napi_value CreateExternalWithJsFinalize(napi_env env,
29+
napi_callback_info info) {
30+
size_t argc = 1;
31+
napi_value args[1];
32+
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
33+
NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments");
34+
napi_value finalizer = args[0];
35+
napi_valuetype finalizer_valuetype;
36+
NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype));
37+
NODE_API_ASSERT(env,
38+
finalizer_valuetype == napi_function,
39+
"Wrong type of first argument");
40+
napi_ref finalizer_ref;
41+
NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref));
42+
43+
napi_value result;
44+
NODE_API_CALL(env,
45+
napi_create_external(env,
46+
&test_value,
47+
FinalizeExternalCallJs,
48+
finalizer_ref, /* finalize_hint */
49+
&result));
50+
51+
finalize_count = 0;
52+
return result;
53+
}
54+
55+
EXTERN_C_START
56+
napi_value Init(napi_env env, napi_value exports) {
57+
napi_property_descriptor descriptors[] = {
58+
DECLARE_NODE_API_PROPERTY("createExternalWithJsFinalize",
59+
CreateExternalWithJsFinalize),
60+
};
61+
62+
NODE_API_CALL(
63+
env,
64+
napi_define_properties(env,
65+
exports,
66+
sizeof(descriptors) / sizeof(*descriptors),
67+
descriptors));
68+
69+
return exports;
70+
}
71+
EXTERN_C_END

‎graal-nodejs/test/js-native-api/test_reference/test_finalizer.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Flags: --expose-gc --force-node-api-uncaught-exceptions-policy
33

44
const common = require('../../common');
5-
const test_reference = require(`./build/${common.buildType}/test_reference`);
5+
const binding = require(`./build/${common.buildType}/test_finalizer`);
66
const assert = require('assert');
77

88
process.on('uncaughtException', common.mustCall((err) => {
@@ -11,7 +11,7 @@ process.on('uncaughtException', common.mustCall((err) => {
1111

1212
(async function() {
1313
{
14-
test_reference.createExternalWithJsFinalize(
14+
binding.createExternalWithJsFinalize(
1515
common.mustCall(() => {
1616
throw new Error('finalizer error');
1717
}));

‎graal-nodejs/test/js-native-api/test_reference/test_reference.c

+18-57
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,6 @@ static void FinalizeExternal(napi_env env, void* data, void* hint) {
2222
finalize_count++;
2323
}
2424

25-
static void FinalizeExternalCallJs(napi_env env, void* data, void* hint) {
26-
int *actual_value = data;
27-
NODE_API_ASSERT_RETURN_VOID(env, actual_value == &test_value,
28-
"The correct pointer was passed to the finalizer");
29-
30-
napi_ref finalizer_ref = (napi_ref)hint;
31-
napi_value js_finalizer;
32-
napi_value recv;
33-
NODE_API_CALL_RETURN_VOID(env, napi_get_reference_value(env, finalizer_ref, &js_finalizer));
34-
NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv));
35-
NODE_API_CALL_RETURN_VOID(env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL));
36-
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref));
37-
}
38-
3925
static napi_value CreateExternal(napi_env env, napi_callback_info info) {
4026
int* data = &test_value;
4127

@@ -118,31 +104,6 @@ CreateExternalWithFinalize(napi_env env, napi_callback_info info) {
118104
return result;
119105
}
120106

121-
static napi_value
122-
CreateExternalWithJsFinalize(napi_env env, napi_callback_info info) {
123-
size_t argc = 1;
124-
napi_value args[1];
125-
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
126-
NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments");
127-
napi_value finalizer = args[0];
128-
napi_valuetype finalizer_valuetype;
129-
NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype));
130-
NODE_API_ASSERT(env, finalizer_valuetype == napi_function, "Wrong type of first argument");
131-
napi_ref finalizer_ref;
132-
NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref));
133-
134-
napi_value result;
135-
NODE_API_CALL(env,
136-
napi_create_external(env,
137-
&test_value,
138-
FinalizeExternalCallJs,
139-
finalizer_ref, /* finalize_hint */
140-
&result));
141-
142-
finalize_count = 0;
143-
return result;
144-
}
145-
146107
static napi_value CheckExternal(napi_env env, napi_callback_info info) {
147108
size_t argc = 1;
148109
napi_value arg;
@@ -263,24 +224,24 @@ static napi_value ValidateDeleteBeforeFinalize(napi_env env, napi_callback_info
263224
EXTERN_C_START
264225
napi_value Init(napi_env env, napi_value exports) {
265226
napi_property_descriptor descriptors[] = {
266-
DECLARE_NODE_API_GETTER("finalizeCount", GetFinalizeCount),
267-
DECLARE_NODE_API_PROPERTY("createExternal", CreateExternal),
268-
DECLARE_NODE_API_PROPERTY("createExternalWithFinalize",
269-
CreateExternalWithFinalize),
270-
DECLARE_NODE_API_PROPERTY("createExternalWithJsFinalize",
271-
CreateExternalWithJsFinalize),
272-
DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal),
273-
DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
274-
DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
275-
DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor),
276-
DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString", CreateSymbolForEmptyString),
277-
DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength", CreateSymbolForIncorrectLength),
278-
DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
279-
DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
280-
DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),
281-
DECLARE_NODE_API_GETTER("referenceValue", GetReferenceValue),
282-
DECLARE_NODE_API_PROPERTY("validateDeleteBeforeFinalize",
283-
ValidateDeleteBeforeFinalize),
227+
DECLARE_NODE_API_GETTER("finalizeCount", GetFinalizeCount),
228+
DECLARE_NODE_API_PROPERTY("createExternal", CreateExternal),
229+
DECLARE_NODE_API_PROPERTY("createExternalWithFinalize",
230+
CreateExternalWithFinalize),
231+
DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal),
232+
DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
233+
DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
234+
DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor),
235+
DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString",
236+
CreateSymbolForEmptyString),
237+
DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength",
238+
CreateSymbolForIncorrectLength),
239+
DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
240+
DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
241+
DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),
242+
DECLARE_NODE_API_GETTER("referenceValue", GetReferenceValue),
243+
DECLARE_NODE_API_PROPERTY("validateDeleteBeforeFinalize",
244+
ValidateDeleteBeforeFinalize),
284245
};
285246

286247
NODE_API_CALL(env, napi_define_properties(

‎graal-nodejs/test/node-api/test_buffer/binding.gyp

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
{
44
"target_name": "test_buffer",
55
"sources": [ "test_buffer.c" ]
6+
},
7+
{
8+
"target_name": "test_finalizer",
9+
"sources": [ "test_finalizer.c" ]
610
}
711
]
812
}

‎graal-nodejs/test/node-api/test_buffer/test_buffer.c

+7-43
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,6 @@ static void noopDeleter(napi_env env, void* data, void* finalize_hint) {
2222
deleterCallCount++;
2323
}
2424

25-
static void malignDeleter(napi_env env, void* data, void* finalize_hint) {
26-
NODE_API_ASSERT_RETURN_VOID(env, data != NULL && strcmp(data, theText) == 0, "invalid data");
27-
napi_ref finalizer_ref = (napi_ref)finalize_hint;
28-
napi_value js_finalizer;
29-
napi_value recv;
30-
NODE_API_CALL_RETURN_VOID(env, napi_get_reference_value(env, finalizer_ref, &js_finalizer));
31-
NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv));
32-
NODE_API_CALL_RETURN_VOID(env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL));
33-
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref));
34-
}
35-
3625
static napi_value newBuffer(napi_env env, napi_callback_info info) {
3726
napi_value theBuffer;
3827
char* theCopy;
@@ -118,30 +107,6 @@ static napi_value staticBuffer(napi_env env, napi_callback_info info) {
118107
return theBuffer;
119108
}
120109

121-
static napi_value malignFinalizerBuffer(napi_env env, napi_callback_info info) {
122-
size_t argc = 1;
123-
napi_value args[1];
124-
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
125-
NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments");
126-
napi_value finalizer = args[0];
127-
napi_valuetype finalizer_valuetype;
128-
NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype));
129-
NODE_API_ASSERT(env, finalizer_valuetype == napi_function, "Wrong type of first argument");
130-
napi_ref finalizer_ref;
131-
NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref));
132-
133-
napi_value theBuffer;
134-
NODE_API_CALL(
135-
env,
136-
napi_create_external_buffer(env,
137-
sizeof(theText),
138-
(void*)theText,
139-
malignDeleter,
140-
finalizer_ref, // finalize_hint
141-
&theBuffer));
142-
return theBuffer;
143-
}
144-
145110
static napi_value Init(napi_env env, napi_value exports) {
146111
napi_value theValue;
147112

@@ -151,14 +116,13 @@ static napi_value Init(napi_env env, napi_value exports) {
151116
napi_set_named_property(env, exports, "theText", theValue));
152117

153118
napi_property_descriptor methods[] = {
154-
DECLARE_NODE_API_PROPERTY("newBuffer", newBuffer),
155-
DECLARE_NODE_API_PROPERTY("newExternalBuffer", newExternalBuffer),
156-
DECLARE_NODE_API_PROPERTY("getDeleterCallCount", getDeleterCallCount),
157-
DECLARE_NODE_API_PROPERTY("copyBuffer", copyBuffer),
158-
DECLARE_NODE_API_PROPERTY("bufferHasInstance", bufferHasInstance),
159-
DECLARE_NODE_API_PROPERTY("bufferInfo", bufferInfo),
160-
DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer),
161-
DECLARE_NODE_API_PROPERTY("malignFinalizerBuffer", malignFinalizerBuffer),
119+
DECLARE_NODE_API_PROPERTY("newBuffer", newBuffer),
120+
DECLARE_NODE_API_PROPERTY("newExternalBuffer", newExternalBuffer),
121+
DECLARE_NODE_API_PROPERTY("getDeleterCallCount", getDeleterCallCount),
122+
DECLARE_NODE_API_PROPERTY("copyBuffer", copyBuffer),
123+
DECLARE_NODE_API_PROPERTY("bufferHasInstance", bufferHasInstance),
124+
DECLARE_NODE_API_PROPERTY("bufferInfo", bufferInfo),
125+
DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer),
162126
};
163127

164128
NODE_API_CALL(env, napi_define_properties(

0 commit comments

Comments
 (0)