Skip to content

src: fix process exit listeners not receiving unsettled tla codes #56872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

24 changes: 22 additions & 2 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1893,8 +1893,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`

Expand Down
4 changes: 1 addition & 3 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,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) {
Expand All @@ -123,7 +122,6 @@ process.domain = null;
} else {
fields[kHasExitCode] = 0;
}
exitCode = code;
},
enumerable: true,
configurable: false,
Expand Down
15 changes: 1 addition & 14 deletions src/api/embed_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 16 additions & 4 deletions src/api/hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,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_;
}
Expand Down
2 changes: 2 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,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;
Expand Down
62 changes: 53 additions & 9 deletions test/es-module/test-esm-tla-unfinished.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,27 @@ 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, stdout, 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.strictEqual(stdout, 'the exit listener received code: 13\n');
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('should exit for an unsettled TLA promise without warning', async () => {
const { code, stderr, stdout } = await spawnPromisified(execPath, [
const { code, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
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 () => {
Expand All @@ -105,16 +107,29 @@ 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, stdout, stderr } = 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(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, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
fixtures.path('es-modules/tla/unresolved-withexitcode.mjs'),
]);

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 () => {
const { code, stderr, stdout } = await spawnPromisified(execPath, [
'--no-warnings',
Expand Down Expand Up @@ -158,4 +173,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, stdout, stderr } = await spawnPromisified(execPath, [
fixtures.path('es-modules/tla/unresolved-with-listener.mjs'),
]);

assert.strictEqual(stdout,
'the exit listener received code: 13\n' +
'process.exitCode inside the exist listener: 13\n'
);
assert.match(stderr, /Warning: Detected unsettled top-level await at/);
assert.strictEqual(code, 13);
});

it('should exit for an unsettled TLA promise and respect explicit exit code in process exit event', async () => {
const { code, stdout, stderr } = 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: '',
});
});
});
});
6 changes: 6 additions & 0 deletions test/fixtures/es-modules/tla/unresolved-with-listener.mjs
Original file line number Diff line number Diff line change
@@ -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(() => {});
Original file line number Diff line number Diff line change
@@ -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(() => {});
5 changes: 5 additions & 0 deletions test/fixtures/es-modules/tla/unresolved-withexitcode.mjs
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
process.on('exit', (exitCode) => {
console.log(`the exit listener received code: ${exitCode}`);
});

process.exitCode = 42;

await new Promise(() => {});
4 changes: 4 additions & 0 deletions test/fixtures/es-modules/tla/unresolved.mjs
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
process.on('exit', (exitCode) => {
console.log(`the exit listener received code: ${exitCode}`);
})

await new Promise(() => {});
Loading