FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: fix process exit listeners not receiving unsettled tla codes · nodejs/node@5e51c62 · GitHub

/ node Public

Commit 5e51c62

Browse files
dario-piotrowicz
authored and
targos
committed
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 <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0ad450f commit 5e51c62

11 files changed

Lines changed: 112 additions & 32 deletions

File tree

‎doc/api/process.md‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1893,8 +1893,28 @@ A number which will be the process exit code, when the process either
18931893
exits gracefully, or is exited via [`process.exit()`][] without specifying
18941894
a code.
18951895
1896-
Specifying a code to [`process.exit(code)`][`process.exit()`] will override any
1897-
previous setting of `process.exitCode`.
1896+
The value of `process.exitCode` can be updated by either assigning a value to
1897+
`process.exitCode` or by passing an argument to [`process.exit()`][]:
1898+
1899+
```console
1900+
$ node -e 'process.exitCode = 9'; echo $?
1901+
9
1902+
$ node -e 'process.exit(42)'; echo $?
1903+
42
1904+
$ node -e 'process.exitCode = 9; process.exit(42)'; echo $?
1905+
42
1906+
```
1907+
1908+
The value can also be set implicitly by Node.js when unrecoverable errors occur (e.g.
1909+
such as the encountering of an unsettled top-level await). However explicit
1910+
manipulations of the exit code always take precedence over implicit ones:
1911+
1912+
```console
1913+
$ node --input-type=module -e 'await new Promise(() => {})'; echo $?
1914+
13
1915+
$ node --input-type=module -e 'process.exitCode = 9; await new Promise(() => {})'; echo $?
1916+
9
1917+
```
18981918
18991919
## `process.features.cached_builtins`
19001920

‎lib/internal/bootstrap/node.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,10 @@ process.domain = null;
104104
configurable: true,
105105
});
106106

107-
let exitCode;
108107
ObjectDefineProperty(process, 'exitCode', {
109108
__proto__: null,
110109
get() {
111-
return exitCode;
110+
return fields[kHasExitCode] ? fields[kExitCode] : undefined;
112111
},
113112
set(code) {
114113
if (code !== null && code !== undefined) {
@@ -123,7 +122,6 @@ process.domain = null;
123122
} else {
124123
fields[kHasExitCode] = 0;
125124
}
126-
exitCode = code;
127125
},
128126
enumerable: true,
129127
configurable: false,

‎src/api/embed_helpers.cc‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,7 @@ Maybe<ExitCode> SpinEventLoopInternal(Environment* env) {
7373

7474
env->PrintInfoForSnapshotIfDebug();
7575
env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
76-
Maybe<ExitCode> exit_code = EmitProcessExitInternal(env);
77-
if (exit_code.FromMaybe(ExitCode::kGenericUserError) !=
78-
ExitCode::kNoFailure) {
79-
return exit_code;
80-
}
81-
82-
auto unsettled_tla = env->CheckUnsettledTopLevelAwait();
83-
if (unsettled_tla.IsNothing()) {
84-
return Nothing<ExitCode>();
85-
}
86-
if (!unsettled_tla.FromJust()) {
87-
return Just(ExitCode::kUnsettledTopLevelAwait);
88-
}
89-
return Just(ExitCode::kNoFailure);
76+
return EmitProcessExitInternal(env);
9077
}
9178

9279
struct CommonEnvironmentSetup::Impl {

‎src/api/hooks.cc‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,26 @@ Maybe<ExitCode> EmitProcessExitInternal(Environment* env) {
7070
return Nothing<ExitCode>();
7171
}
7272

73-
Local<Integer> exit_code = Integer::New(
74-
isolate, static_cast<int32_t>(env->exit_code(ExitCode::kNoFailure)));
73+
ExitCode exit_code = env->exit_code(ExitCode::kNoFailure);
74+
75+
// the exit code wasn't already set, so let's check for unsettled tlas
76+
if (exit_code == ExitCode::kNoFailure) {
77+
auto unsettled_tla = env->CheckUnsettledTopLevelAwait();
78+
if (!unsettled_tla.FromJust()) {
79+
exit_code = ExitCode::kUnsettledTopLevelAwait;
80+
env->set_exit_code(exit_code);
81+
}
82+
}
7583

76-
if (ProcessEmit(env, "exit", exit_code).IsEmpty()) {
84+
Local<Integer> exit_code_int =
85+
Integer::New(isolate, static_cast<int32_t>(exit_code));
86+
87+
if (ProcessEmit(env, "exit", exit_code_int).IsEmpty()) {
7788
return Nothing<ExitCode>();
7889
}
90+
7991
// Reload exit code, it may be changed by `emit('exit')`
80-
return Just(env->exit_code(ExitCode::kNoFailure));
92+
return Just(env->exit_code(exit_code));
8193
}
8294

8395
Maybe<int> EmitProcessExit(Environment* env) {

‎src/env-inl.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,11 @@ inline ExitCode Environment::exit_code(const ExitCode default_code) const {
341341
: static_cast<ExitCode>(exit_info_[kExitCode]);
342342
}
343343

344+
inline void Environment::set_exit_code(const ExitCode code) {
345+
exit_info_[kExitCode] = static_cast<int>(code);
346+
exit_info_[kHasExitCode] = 1;
347+
}
348+
344349
inline AliasedInt32Array& Environment::exit_info() {
345350
return exit_info_;
346351
}

‎src/env.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,8 @@ class Environment final : public MemoryRetainer {
739739
bool exiting() const;
740740
inline ExitCode exit_code(const ExitCode default_code) const;
741741

742+
inline void set_exit_code(const ExitCode code);
743+
742744
// This stores whether the --abort-on-uncaught-exception flag was passed
743745
// to Node.
744746
inline bool abort_on_uncaught_exception() const;

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

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES
7676
fixtures.path('es-modules/tla/unresolved.mjs'),
7777
]);
7878

79-
assert.match(stderr, /Warning: Detected unsettled top-level await at.+unresolved\.mjs:1/);
79+
assert.match(stderr, /Warning: Detected unsettled top-level await at.+unresolved\.mjs:5\b/);
8080
assert.match(stderr, /await new Promise/);
81-
assert.strictEqual(stdout, '');
81+
assert.strictEqual(stdout, 'the exit listener received code: 13\n');
8282
assert.strictEqual(code, 13);
8383
});
8484

@@ -88,9 +88,11 @@ describe('ESM: unsettled and rejected promises', { concurrency: !process.env.TES
8888
fixtures.path('es-modules/tla/unresolved.mjs'),
8989
]);
9090

91-
assert.strictEqual(stderr, '');
92-
assert.strictEqual(stdout, '');
93-
assert.strictEqual(code, 13);
91+
assert.deepStrictEqual({ code, stdout, stderr }, {
92+
code: 13,
93+
stdout: 'the exit listener received code: 13\n',
94+
stderr: '',
95+
});
9496
});
9597

9698
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
104106
assert.strictEqual(code, 1);
105107
});
106108

107-
it('should exit for an unsettled TLA promise and respect explicit exit code via stdin', async () => {
109+
it('should exit for an unsettled TLA promise and respect explicit exit code', async () => {
108110
const { code, stderr, stdout } = await spawnPromisified(execPath, [
109111
'--no-warnings',
110112
fixtures.path('es-modules/tla/unresolved-withexitcode.mjs'),
111113
]);
112114

113-
assert.strictEqual(stderr, '');
114-
assert.strictEqual(stdout, '');
115-
assert.strictEqual(code, 42);
115+
assert.deepStrictEqual({ code, stdout, stderr }, {
116+
code: 42,
117+
stdout: 'the exit listener received code: 42\n',
118+
stderr: '',
119+
});
116120
});
117121

118122
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
158162
assert.strictEqual(stdout, '');
159163
assert.strictEqual(code, 13);
160164
});
165+
166+
describe('with exit listener', () => {
167+
it('the process exit event should provide the correct code', async () => {
168+
const { code, stderr, stdout } = await spawnPromisified(execPath, [
169+
fixtures.path('es-modules/tla/unresolved-with-listener.mjs'),
170+
]);
171+
172+
assert.match(stderr, /Warning: Detected unsettled top-level await at/);
173+
assert.strictEqual(stdout,
174+
'the exit listener received code: 13\n' +
175+
'process.exitCode inside the exist listener: 13\n'
176+
);
177+
assert.strictEqual(code, 13);
178+
});
179+
180+
it('should exit for an unsettled TLA promise and respect explicit exit code in process exit event', async () => {
181+
const { code, stderr, stdout } = await spawnPromisified(execPath, [
182+
'--no-warnings',
183+
fixtures.path('es-modules/tla/unresolved-withexitcode-and-listener.mjs'),
184+
]);
185+
186+
assert.deepStrictEqual({ code, stdout, stderr }, {
187+
code: 42,
188+
stdout: 'the exit listener received code: 42\n' +
189+
'process.exitCode inside the exist listener: 42\n',
190+
stderr: '',
191+
});
192+
});
193+
});
161194
});

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
process.on('exit', (exitCode) => {
2+
console.log(`the exit listener received code: ${exitCode}`);
3+
console.log(`process.exitCode inside the exist listener: ${process.exitCode}`);
4+
})
5+
6+
await new Promise(() => {});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
process.on('exit', (exitCode) => {
2+
console.log(`the exit listener received code: ${exitCode}`);
3+
console.log(`process.exitCode inside the exist listener: ${process.exitCode}`);
4+
});
5+
6+
process.exitCode = 42;
7+
8+
await new Promise(() => {});

‎test/fixtures/es-modules/tla/unresolved-withexitcode.mjs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,7 @@
1+
process.on('exit', (exitCode) => {
2+
console.log(`the exit listener received code: ${exitCode}`);
3+
});
4+
15
process.exitCode = 42;
6+
27
await new Promise(() => {});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL