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

PR-URL: #56872
Fixes: #53551
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: dario-piotrowicz

doc/api/process.md

@@ -1885,8 +1885,28 @@ A number which will be the process exit code, when the process either
 exits gracefully, or is exited via [`process.exit()`][] without specifying
 a code.
 
-Specifying a code to [`process.exit(code)`][`process.exit()`] will override any
-previous setting of `process.exitCode`.
+The value of `process.exitCode` can be updated by either assigning a value to
+`process.exitCode` or by passing an argument to [`process.exit()`][]:
+
+```console
+$ node -e 'process.exitCode = 9'; echo $?
+9
+$ node -e 'process.exit(42)'; echo $?
+42
+$ node -e 'process.exitCode = 9; process.exit(42)'; echo $?
+42
+```
+
+The value can also be set implicitly by Node.js when unrecoverable errors occur (e.g.
+such as the encountering of an unsettled top-level await). However explicit
+manipulations of the exit code always take precedence over implicit ones:
+
+```console
+$ node --input-type=module -e 'await new Promise(() => {})'; echo $?
+13
+$ node --input-type=module -e 'process.exitCode = 9; await new Promise(() => {})'; echo $?
+9
+```
 
 ## `process.features.cached_builtins`
 

lib/internal/bootstrap/node.js

@@ -107,11 +107,10 @@ process.domain = null;
     configurable: true,
   });
 
-  let exitCode;
   ObjectDefineProperty(process, 'exitCode', {
     __proto__: null,
     get() {
-      return exitCode;
+      return fields[kHasExitCode] ? fields[kExitCode] : undefined;
     },
     set(code) {
       if (code !== null && code !== undefined) {
@@ -126,7 +125,6 @@ process.domain = null;
       } else {
         fields[kHasExitCode] = 0;
       }
-      exitCode = code;
     },
     enumerable: true,
     configurable: false,

src/api/embed_helpers.cc

@@ -73,20 +73,7 @@ Maybe<ExitCode> SpinEventLoopInternal(Environment* env) {
 
   env->PrintInfoForSnapshotIfDebug();
   env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
-  Maybe<ExitCode> 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<ExitCode>();
-  }
-  if (!unsettled_tla.FromJust()) {
-    return Just(ExitCode::kUnsettledTopLevelAwait);
-  }
-  return Just(ExitCode::kNoFailure);
+  return EmitProcessExitInternal(env);
 }
 
 struct CommonEnvironmentSetup::Impl {

src/api/hooks.cc

@@ -70,14 +70,26 @@ Maybe<ExitCode> EmitProcessExitInternal(Environment* env) {
     return Nothing<ExitCode>();
   }
 
-  Local<Integer> exit_code = Integer::New(
-      isolate, static_cast<int32_t>(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;
+      env->set_exit_code(exit_code);
+    }
+  }
 
-  if (ProcessEmit(env, "exit", exit_code).IsEmpty()) {
+  Local<Integer> exit_code_int =
+      Integer::New(isolate, static_cast<int32_t>(exit_code));
+
+  if (ProcessEmit(env, "exit", exit_code_int).IsEmpty()) {
     return Nothing<ExitCode>();
   }
+
   // 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<int> EmitProcessExit(Environment* env) {

src/env-inl.h

@@ -376,6 +376,11 @@ inline ExitCode Environment::exit_code(const ExitCode default_code) const {
              : static_cast<ExitCode>(exit_info_[kExitCode]);
 }
 
+inline void Environment::set_exit_code(const ExitCode code) {
+  exit_info_[kExitCode] = static_cast<int>(code);
+  exit_info_[kHasExitCode] = 1;
+}
+
 inline AliasedInt32Array& Environment::exit_info() {
   return exit_info_;
 }

src/env.h

@@ -756,6 +756,8 @@ class Environment final : public MemoryRetainer {
   bool exiting() const;
   inline ExitCode exit_code(const ExitCode default_code) const;
 
+  inline void set_exit_code(const ExitCode code);
+
   // This stores whether the --abort-on-uncaught-exception flag was passed
   // to Node.
   inline bool abort_on_uncaught_exception() const;

test/es-module/test-esm-tla-unfinished.mjs

@@ -76,9 +76,9 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES
       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\b/);
     assert.match(stderr, /await new Promise/);
-    assert.strictEqual(stdout, '');
+    assert.strictEqual(stdout, 'the exit listener received code: 13\n');
     assert.strictEqual(code, 13);
   });
 
@@ -88,9 +88,11 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES
       fixtures.path('es-modules/tla/unresolved.mjs'),
     ]);
 
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(stdout, '');
-    assert.strictEqual(code, 13);
+    assert.deepStrictEqual({ code, stdout, stderr }, {
+      code: 13,
+      stdout: 'the exit listener received code: 13\n',
+      stderr: '',
+    });
   });
 
   it('should throw for a rejected TLA promise via stdin', async () => {
@@ -104,15 +106,17 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES
     assert.strictEqual(code, 1);
   });
 
-  it('should exit for an unsettled TLA promise and respect explicit exit code via stdin', async () => {
+  it('should exit for an unsettled TLA promise and respect explicit exit code', async () => {
     const { code, stderr, stdout } = await spawnPromisified(execPath, [
       '--no-warnings',
       fixtures.path('es-modules/tla/unresolved-withexitcode.mjs'),
     ]);
 
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(stdout, '');
-    assert.strictEqual(code, 42);
+    assert.deepStrictEqual({ code, stdout, stderr }, {
+      code: 42,
+      stdout: 'the exit listener received code: 42\n',
+      stderr: '',
+    });
   });
 
   it('should throw for a rejected TLA promise and ignore explicit exit code via stdin', async () => {
@@ -158,4 +162,33 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES
     assert.strictEqual(stdout, '');
     assert.strictEqual(code, 13);
   });
+
+  describe('with exit listener', () => {
+    it('the process exit event should provide the correct code', async () => {
+      const { code, stderr, stdout } = await spawnPromisified(execPath, [
+        fixtures.path('es-modules/tla/unresolved-with-listener.mjs'),
+      ]);
+
+      assert.match(stderr, /Warning: Detected unsettled top-level await at/);
+      assert.strictEqual(stdout,
+                         'the exit listener received code: 13\n' +
+                         'process.exitCode inside the exist listener: 13\n'
+      );
+      assert.strictEqual(code, 13);
+    });
+
+    it('should exit for an unsettled TLA promise and respect explicit exit code in process exit event', async () => {
+      const { code, stderr, stdout } = await spawnPromisified(execPath, [
+        '--no-warnings',
+        fixtures.path('es-modules/tla/unresolved-withexitcode-and-listener.mjs'),
+      ]);
+
+      assert.deepStrictEqual({ code, stdout, stderr }, {
+        code: 42,
+        stdout: 'the exit listener received code: 42\n' +
+                'process.exitCode inside the exist listener: 42\n',
+        stderr: '',
+      });
+    });
+  });
 });

test/fixtures/es-modules/tla/unresolved-with-listener.mjs

@@ -0,0 +1,6 @@
+process.on('exit', (exitCode) => {
+    console.log(`the exit listener received code: ${exitCode}`);
+    console.log(`process.exitCode inside the exist listener: ${process.exitCode}`);
+})
+
+await new Promise(() => {});

test/fixtures/es-modules/tla/unresolved-withexitcode-and-listener.mjs

@@ -0,0 +1,8 @@
+process.on('exit', (exitCode) => {
+    console.log(`the exit listener received code: ${exitCode}`);
+    console.log(`process.exitCode inside the exist listener: ${process.exitCode}`);
+});
+
+process.exitCode = 42;
+
+await new Promise(() => {});

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(() => {});

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(() => {});