From eef4c1f2c9c6181ee073640b30df157d3dc40cf8 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sun, 2 Feb 2025 00:11:13 +0000 Subject: [PATCH] src: fix process exit listeners not receiving unsettled tla codes fix listeners registered via `process.on('exit', ...` not receiving error code 13 when an unsettled top-level-await is encountered in the code --- src/api/embed_helpers.cc | 15 +--------- src/api/hooks.cc | 19 +++++++++--- test/es-module/test-esm-tla-unfinished.mjs | 30 ++++++++++++++----- .../tla/unresolved-withexitcode.mjs | 5 ++++ test/fixtures/es-modules/tla/unresolved.mjs | 4 +++ 5 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc index 34de89a8dc0398..14d2a20c1c18d9 100644 --- a/src/api/embed_helpers.cc +++ b/src/api/embed_helpers.cc @@ -73,20 +73,7 @@ Maybe SpinEventLoopInternal(Environment* env) { env->PrintInfoForSnapshotIfDebug(); env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); }); - Maybe exit_code = EmitProcessExitInternal(env); - if (exit_code.FromMaybe(ExitCode::kGenericUserError) != - ExitCode::kNoFailure) { - return exit_code; - } - - auto unsettled_tla = env->CheckUnsettledTopLevelAwait(); - if (unsettled_tla.IsNothing()) { - return Nothing(); - } - if (!unsettled_tla.FromJust()) { - return Just(ExitCode::kUnsettledTopLevelAwait); - } - return Just(ExitCode::kNoFailure); + return EmitProcessExitInternal(env); } struct CommonEnvironmentSetup::Impl { diff --git a/src/api/hooks.cc b/src/api/hooks.cc index cf95d009d4a088..9955b15bc413bf 100644 --- a/src/api/hooks.cc +++ b/src/api/hooks.cc @@ -70,14 +70,25 @@ Maybe EmitProcessExitInternal(Environment* env) { return Nothing(); } - Local exit_code = Integer::New( - isolate, static_cast(env->exit_code(ExitCode::kNoFailure))); + ExitCode exit_code = env->exit_code(ExitCode::kNoFailure); + + // the exit code wasn't already set, so let's check for unsettled tlas + if (exit_code == ExitCode::kNoFailure) { + auto unsettled_tla = env->CheckUnsettledTopLevelAwait(); + if (!unsettled_tla.FromJust()) { + exit_code = ExitCode::kUnsettledTopLevelAwait; + } + } - if (ProcessEmit(env, "exit", exit_code).IsEmpty()) { + Local exit_code_int = Integer::New( + isolate, static_cast(exit_code)); + + if (ProcessEmit(env, "exit", exit_code_int).IsEmpty()) { return Nothing(); } + // Reload exit code, it may be changed by `emit('exit')` - return Just(env->exit_code(ExitCode::kNoFailure)); + return Just(env->exit_code(exit_code)); } Maybe EmitProcessExit(Environment* env) { diff --git a/test/es-module/test-esm-tla-unfinished.mjs b/test/es-module/test-esm-tla-unfinished.mjs index 4c6e0708daaf08..8061d6149cd08f 100644 --- a/test/es-module/test-esm-tla-unfinished.mjs +++ b/test/es-module/test-esm-tla-unfinished.mjs @@ -72,24 +72,31 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES }); it('should exit for an unsettled TLA promise with warning', async () => { - const { code, stderr, stdout } = await spawnPromisified(execPath, [ + const { code, stderr } = await spawnPromisified(execPath, [ fixtures.path('es-modules/tla/unresolved.mjs'), ]); - assert.match(stderr, /Warning: Detected unsettled top-level await at.+unresolved\.mjs:1/); + assert.match(stderr, /Warning: Detected unsettled top-level await at.+unresolved\.mjs:5/); assert.match(stderr, /await new Promise/); - assert.strictEqual(stdout, ''); + assert.strictEqual(code, 13); + }); + + it('the process exit event should provide the correct code for an unsettled TLA promise', async () => { + const { code, stdout } = await spawnPromisified(execPath, [ + fixtures.path('es-modules/tla/unresolved.mjs'), + ]); + + assert.strictEqual(stdout, 'the exit listener received code: 13\n'); assert.strictEqual(code, 13); }); it('should exit for an unsettled TLA promise without warning', async () => { - const { code, stderr, stdout } = await spawnPromisified(execPath, [ + const { code, stderr } = await spawnPromisified(execPath, [ '--no-warnings', fixtures.path('es-modules/tla/unresolved.mjs'), ]); assert.strictEqual(stderr, ''); - assert.strictEqual(stdout, ''); assert.strictEqual(code, 13); }); @@ -105,13 +112,22 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES }); it('should exit for an unsettled TLA promise and respect explicit exit code via stdin', async () => { - const { code, stderr, stdout } = await spawnPromisified(execPath, [ + const { code, stderr } = await spawnPromisified(execPath, [ '--no-warnings', fixtures.path('es-modules/tla/unresolved-withexitcode.mjs'), ]); assert.strictEqual(stderr, ''); - assert.strictEqual(stdout, ''); + assert.strictEqual(code, 42); + }); + + it('should exit for an unsettled TLA promise and respect explicit exit code in process exit event', async () => { + const { code, stdout } = await spawnPromisified(execPath, [ + '--no-warnings', + fixtures.path('es-modules/tla/unresolved-withexitcode.mjs'), + ]); + + assert.strictEqual(stdout, 'the exit listener received code: 42\n'); assert.strictEqual(code, 42); }); diff --git a/test/fixtures/es-modules/tla/unresolved-withexitcode.mjs b/test/fixtures/es-modules/tla/unresolved-withexitcode.mjs index 1cb982311080b8..0316dae1cd9a3c 100644 --- a/test/fixtures/es-modules/tla/unresolved-withexitcode.mjs +++ b/test/fixtures/es-modules/tla/unresolved-withexitcode.mjs @@ -1,2 +1,7 @@ +process.on('exit', (exitCode) => { + console.log(`the exit listener received code: ${exitCode}`); +}); + process.exitCode = 42; + await new Promise(() => {}); diff --git a/test/fixtures/es-modules/tla/unresolved.mjs b/test/fixtures/es-modules/tla/unresolved.mjs index 231a8cd634825c..37566bd5688fb5 100644 --- a/test/fixtures/es-modules/tla/unresolved.mjs +++ b/test/fixtures/es-modules/tla/unresolved.mjs @@ -1 +1,5 @@ +process.on('exit', (exitCode) => { + console.log(`the exit listener received code: ${exitCode}`); +}) + await new Promise(() => {});