Skip to content
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

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
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
51 changes: 42 additions & 9 deletions test/es-module/test-esm-tla-unfinished.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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: '',
});
});
});
});
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