From 48e43703dc070ed87ef5616c157f48b83cf78980 Mon Sep 17 00:00:00 2001 From: Balakrishna Avulapati Date: Mon, 30 Mar 2026 21:07:18 +0530 Subject: [PATCH 1/2] feat: port test_exception to CTS ports [test_exception](https://github.com/nodejs/node/tree/main/test/js-native-api/test_exception) from Node.js test suite to the CTS. Signed-off-by: Balakrishna Avulapati --- eslint.config.js | 1 + implementors/node/assert.js | 26 ++-- implementors/node/child_process.js | 66 ++++++++ implementors/node/tests.ts | 29 ++-- .../test_exception/CMakeLists.txt | 1 + tests/js-native-api/test_exception/test.js | 146 ++++++++++++++++++ .../test_exception/testFinalizerException.js | 5 + .../testFinalizerException_child.mjs | 14 ++ .../test_exception/test_exception.c | 116 ++++++++++++++ 9 files changed, 377 insertions(+), 27 deletions(-) create mode 100644 implementors/node/child_process.js create mode 100644 tests/js-native-api/test_exception/CMakeLists.txt create mode 100644 tests/js-native-api/test_exception/test.js create mode 100644 tests/js-native-api/test_exception/testFinalizerException.js create mode 100644 tests/js-native-api/test_exception/testFinalizerException_child.mjs create mode 100644 tests/js-native-api/test_exception/test_exception.c diff --git a/eslint.config.js b/eslint.config.js index fff73c6..3d600ef 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -16,6 +16,7 @@ export default defineConfig([ mustCall: "readonly", mustNotCall: "readonly", gcUntil: "readonly", + spawnTest: "readonly", experimentalFeatures: "readonly", }, }, diff --git a/implementors/node/assert.js b/implementors/node/assert.js index de9934c..c49a264 100644 --- a/implementors/node/assert.js +++ b/implementors/node/assert.js @@ -1,24 +1,22 @@ - import { ok, strictEqual, notStrictEqual, deepStrictEqual, throws, + match, } from "node:assert/strict"; -const assert = Object.assign( - (value, message) => ok(value, message), - { - ok: (value, message) => ok(value, message), - strictEqual: (actual, expected, message) => - strictEqual(actual, expected, message), - notStrictEqual: (actual, expected, message) => - notStrictEqual(actual, expected, message), - deepStrictEqual: (actual, expected, message) => - deepStrictEqual(actual, expected, message), - throws: (fn, error, message) => throws(fn, error, message), - }, -); +const assert = Object.assign((value, message) => ok(value, message), { + ok: (value, message) => ok(value, message), + strictEqual: (actual, expected, message) => + strictEqual(actual, expected, message), + notStrictEqual: (actual, expected, message) => + notStrictEqual(actual, expected, message), + deepStrictEqual: (actual, expected, message) => + deepStrictEqual(actual, expected, message), + throws: (fn, error, message) => throws(fn, error, message), + match: (string, regex, message) => match(string, regex, message), +}); Object.assign(globalThis, { assert }); diff --git a/implementors/node/child_process.js b/implementors/node/child_process.js new file mode 100644 index 0000000..9ddcb64 --- /dev/null +++ b/implementors/node/child_process.js @@ -0,0 +1,66 @@ +import { spawnSync } from "node:child_process"; +import path from "node:path"; + +const ROOT_PATH = path.resolve(import.meta.dirname, "..", ".."); +const FEATURES_MODULE_PATH = path.join( + ROOT_PATH, + "implementors", + "node", + "features.js", +); +const ASSERT_MODULE_PATH = path.join( + ROOT_PATH, + "implementors", + "node", + "assert.js", +); +const LOAD_ADDON_MODULE_PATH = path.join( + ROOT_PATH, + "implementors", + "node", + "load-addon.js", +); +const GC_MODULE_PATH = path.join(ROOT_PATH, "implementors", "node", "gc.js"); +const MUST_CALL_MODULE_PATH = path.join( + ROOT_PATH, + "implementors", + "node", + "must-call.js", +); +const CHILD_PROCESS_MODULE_PATH = path.join( + ROOT_PATH, + "implementors", + "node", + "child_process.js", +); + +const spawnTest = (filePath, options = {}) => { + const result = spawnSync( + process.execPath, + [ + "--expose-gc", + "--import", + "file://" + FEATURES_MODULE_PATH, + "--import", + "file://" + ASSERT_MODULE_PATH, + "--import", + "file://" + LOAD_ADDON_MODULE_PATH, + "--import", + "file://" + GC_MODULE_PATH, + "--import", + "file://" + MUST_CALL_MODULE_PATH, + "--import", + "file://" + CHILD_PROCESS_MODULE_PATH, + filePath, + ], + { cwd: options.cwd || process.cwd() }, + ); + return { + status: result.status, + signal: result.signal, + stderr: result.stderr?.toString() ?? "", + stdout: result.stdout?.toString() ?? "", + }; +}; + +Object.assign(globalThis, { spawnTest }); diff --git a/implementors/node/tests.ts b/implementors/node/tests.ts index d4f8b2c..c8b814e 100644 --- a/implementors/node/tests.ts +++ b/implementors/node/tests.ts @@ -5,7 +5,7 @@ import path from "node:path"; assert( typeof import.meta.dirname === "string", - "Expecting a recent Node.js runtime API version" + "Expecting a recent Node.js runtime API version", ); const ROOT_PATH = path.resolve(import.meta.dirname, "..", ".."); @@ -14,31 +14,32 @@ const FEATURES_MODULE_PATH = path.join( ROOT_PATH, "implementors", "node", - "features.js" + "features.js", ); const ASSERT_MODULE_PATH = path.join( ROOT_PATH, "implementors", "node", - "assert.js" + "assert.js", ); const LOAD_ADDON_MODULE_PATH = path.join( ROOT_PATH, "implementors", "node", - "load-addon.js" + "load-addon.js", ); -const GC_MODULE_PATH = path.join( +const GC_MODULE_PATH = path.join(ROOT_PATH, "implementors", "node", "gc.js"); +const MUST_CALL_MODULE_PATH = path.join( ROOT_PATH, "implementors", "node", - "gc.js" + "must-call.js", ); -const MUST_CALL_MODULE_PATH = path.join( +const CHILD_PROCESS_MODULE_PATH = path.join( ROOT_PATH, "implementors", "node", - "must-call.js" + "child_process.js", ); export function listDirectoryEntries(dir: string) { @@ -62,7 +63,7 @@ export function listDirectoryEntries(dir: string) { export function runFileInSubprocess( cwd: string, - filePath: string + filePath: string, ): Promise { return new Promise((resolve, reject) => { const child = spawn( @@ -80,9 +81,11 @@ export function runFileInSubprocess( "file://" + GC_MODULE_PATH, "--import", "file://" + MUST_CALL_MODULE_PATH, + "--import", + "file://" + CHILD_PROCESS_MODULE_PATH, filePath, ], - { cwd } + { cwd }, ); let stderrOutput = ""; @@ -111,9 +114,9 @@ export function runFileInSubprocess( new Error( `Test file ${path.relative( TESTS_ROOT_PATH, - filePath - )} failed (${reason})${stderrSuffix}` - ) + filePath, + )} failed (${reason})${stderrSuffix}`, + ), ); }); }); diff --git a/tests/js-native-api/test_exception/CMakeLists.txt b/tests/js-native-api/test_exception/CMakeLists.txt new file mode 100644 index 0000000..5ecd96a --- /dev/null +++ b/tests/js-native-api/test_exception/CMakeLists.txt @@ -0,0 +1 @@ +add_node_api_cts_addon(test_exception test_exception.c) diff --git a/tests/js-native-api/test_exception/test.js b/tests/js-native-api/test_exception/test.js new file mode 100644 index 0000000..bff4aab --- /dev/null +++ b/tests/js-native-api/test_exception/test.js @@ -0,0 +1,146 @@ +"use strict"; +// Flags: --expose-gc + +const theError = new Error("Some error"); + +// The test module throws an error during Init, but in order for its exports to +// not be lost, it attaches them to the error's "bindings" property. This way, +// we can make sure that exceptions thrown during the module initialization +// phase are propagated through require() into JavaScript. +// https://github.com/nodejs/node/issues/19437 +const test_exception = (function () { + let resultingException; + try { + loadAddon("test_exception"); + } catch (anException) { + resultingException = anException; + } + assert.strictEqual(resultingException.message, "Error during Init"); + return resultingException.binding; +})(); + +{ + const throwTheError = () => { + throw theError; + }; + + // Test that the native side successfully captures the exception + let returnedError = test_exception.returnException(throwTheError); + assert.strictEqual(returnedError, theError); + + // Test that the native side passes the exception through + assert.throws( + () => { + test_exception.allowException(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.returnException(mustCall()); + assert.strictEqual( + returnedError, + undefined, + "Returned error should be undefined when no exception is" + + ` 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(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; + try { + test_exception.allowException(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}`, + ); +} + +{ + // Test that no exception appears that was not thrown by us + let caughtError; + try { + test_exception.constructAllowException(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}`, + ); +} diff --git a/tests/js-native-api/test_exception/testFinalizerException.js b/tests/js-native-api/test_exception/testFinalizerException.js new file mode 100644 index 0000000..7b804fc --- /dev/null +++ b/tests/js-native-api/test_exception/testFinalizerException.js @@ -0,0 +1,5 @@ +// This test verifies that exceptions thrown from C finalizers during GC +// are propagated as uncaught exceptions (printed to stderr). +// It spawns a child process that triggers the finalizer and checks its stderr. +const result = spawnTest("testFinalizerException_child.mjs"); +assert.match(result.stderr, /Error during Finalize/); diff --git a/tests/js-native-api/test_exception/testFinalizerException_child.mjs b/tests/js-native-api/test_exception/testFinalizerException_child.mjs new file mode 100644 index 0000000..828c67f --- /dev/null +++ b/tests/js-native-api/test_exception/testFinalizerException_child.mjs @@ -0,0 +1,14 @@ +// This file is spawned as a child process by testFinalizerException.js. +// It loads the addon, creates an external with a C finalizer that throws, +// then runs GC until the finalizer fires and crashes the process. +try { + loadAddon("test_exception"); +} catch (anException) { + anException.binding.createExternal(); +} + +// Collect garbage 10 times. At least one of those should throw the exception +// and cause the whole process to bail with it, its text printed to stderr and +// asserted by the parent process to match expectations. +let gcCount = 10; +await gcUntil("test", () => --gcCount <= 0); diff --git a/tests/js-native-api/test_exception/test_exception.c b/tests/js-native-api/test_exception/test_exception.c new file mode 100644 index 0000000..84b9919 --- /dev/null +++ b/tests/js-native-api/test_exception/test_exception.c @@ -0,0 +1,116 @@ +#include +#include "../common.h" +#include "../entry_point.h" + +static bool exceptionWasPending = false; +static int num = 0x23432; + +static napi_value returnException(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + + napi_value global; + NODE_API_CALL(env, napi_get_global(env, &global)); + + napi_value result; + napi_status status = napi_call_function(env, global, args[0], 0, 0, &result); + if (status == napi_pending_exception) { + napi_value ex; + NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &ex)); + return ex; + } + + return NULL; +} + +static napi_value constructReturnException(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_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; + NODE_API_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]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + + napi_value global; + NODE_API_CALL(env, napi_get_global(env, &global)); + + napi_value result; + napi_call_function(env, global, args[0], 0, 0, &result); + // Ignore status and check napi_is_exception_pending() instead. + + NODE_API_CALL(env, napi_is_exception_pending(env, &exceptionWasPending)); + return NULL; +} + +static napi_value constructAllowException(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_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. + + NODE_API_CALL(env, napi_is_exception_pending(env, &exceptionWasPending)); + return NULL; +} + +static napi_value wasPending(napi_env env, napi_callback_info info) { + napi_value result; + NODE_API_CALL(env, napi_get_boolean(env, exceptionWasPending, &result)); + + return result; +} + +static void finalizer(napi_env env, void *data, void *hint) { + NODE_API_CALL_RETURN_VOID(env, + napi_throw_error(env, NULL, "Error during Finalize")); +} + +static napi_value createExternal(napi_env env, napi_callback_info info) { + napi_value external; + + NODE_API_CALL(env, + napi_create_external(env, &num, finalizer, NULL, &external)); + + return external; +} + +EXTERN_C_START +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor descriptors[] = { + DECLARE_NODE_API_PROPERTY("returnException", returnException), + DECLARE_NODE_API_PROPERTY("allowException", allowException), + DECLARE_NODE_API_PROPERTY("constructReturnException", constructReturnException), + DECLARE_NODE_API_PROPERTY("constructAllowException", constructAllowException), + DECLARE_NODE_API_PROPERTY("wasPending", wasPending), + DECLARE_NODE_API_PROPERTY("createExternal", createExternal), + }; + NODE_API_CALL(env, napi_define_properties( + env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors)); + + napi_value error, code, message; + NODE_API_CALL(env, napi_create_string_utf8(env, "Error during Init", + NAPI_AUTO_LENGTH, &message)); + NODE_API_CALL(env, napi_create_string_utf8(env, "", NAPI_AUTO_LENGTH, &code)); + NODE_API_CALL(env, napi_create_error(env, code, message, &error)); + NODE_API_CALL(env, napi_set_named_property(env, error, "binding", exports)); + NODE_API_CALL(env, napi_throw(env, error)); + + return exports; +} +EXTERN_C_END From 8af7deee1a2caa55bc95ac443055e136e1d47bde Mon Sep 17 00:00:00 2001 From: Balakrishna Avulapati Date: Mon, 30 Mar 2026 21:11:32 +0530 Subject: [PATCH 2/2] docs: update port status Signed-off-by: Balakrishna Avulapati --- PORTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PORTING.md b/PORTING.md index 657cbb3..315507f 100644 --- a/PORTING.md +++ b/PORTING.md @@ -55,7 +55,7 @@ Tests covering the engine-specific part of Node-API, defined in `js_native_api.h | `test_dataview` | Not ported | Medium | | `test_date` | Ported ✅ | Easy | | `test_error` | Not ported | Medium | -| `test_exception` | Not ported | Medium | +| `test_exception` | Ported ✅ | Medium | | `test_finalizer` | Not ported | Medium | | `test_function` | Not ported | Medium | | `test_general` | Not ported | Hard |