Skip to content

Commit 968960d

Browse files
MoonBalladuh95
authored andcommitted
esm: make process.exit() default to exit code 0
Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: nodejs#41388 Fixes: nodejs#40808 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>
1 parent cede1f2 commit 968960d

File tree

7 files changed

+55
-9
lines changed

7 files changed

+55
-9
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
3+
// Handle a Promise from running code that potentially does Top-Level Await.
4+
// In that case, it makes sense to set the exit code to a specific non-zero
5+
// value if the main code never finishes running.
6+
function handleProcessExit() {
7+
process.exitCode ??= 13;
8+
}
9+
10+
module.exports = {
11+
handleProcessExit,
12+
};

lib/internal/modules/run_main.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ const CJSLoader = require('internal/modules/cjs/loader');
88
const { Module, toRealPath, readPackageScope } = CJSLoader;
99
const { getOptionValue } = require('internal/options');
1010
const path = require('path');
11+
const {
12+
handleProcessExit,
13+
} = require('internal/modules/esm/handle_process_exit');
1114

1215
function resolveMainPath(main) {
1316
// Note extension resolution for the main entry point can be deprecated in a
@@ -51,16 +54,13 @@ function runMainESM(mainPath) {
5154
}));
5255
}
5356

54-
function handleMainPromise(promise) {
55-
// Handle a Promise from running code that potentially does Top-Level Await.
56-
// In that case, it makes sense to set the exit code to a specific non-zero
57-
// value if the main code never finishes running.
58-
function handler() {
59-
if (process.exitCode === undefined)
60-
process.exitCode = 13;
57+
async function handleMainPromise(promise) {
58+
process.on('exit', handleProcessExit);
59+
try {
60+
return await promise;
61+
} finally {
62+
process.off('exit', handleProcessExit);
6163
}
62-
process.on('exit', handler);
63-
return PromisePrototypeFinally(promise, () => process.off('exit', handler));
6464
}
6565

6666
// For backwards compatibility, we have to run a bunch of

lib/internal/process/per_thread.js

+6
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ const {
3636
const format = require('internal/util/inspect').format;
3737
const constants = internalBinding('constants').os.signals;
3838

39+
const {
40+
handleProcessExit,
41+
} = require('internal/modules/esm/handle_process_exit');
42+
3943
function assert(x, msg) {
4044
if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
4145
}
@@ -165,6 +169,8 @@ function wrapProcessMethods(binding) {
165169
memoryUsage.rss = rss;
166170

167171
function exit(code) {
172+
process.off('exit', handleProcessExit);
173+
168174
if (code || code === 0)
169175
process.exitCode = code;
170176

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

+18
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,21 @@ import fixtures from '../common/fixtures.js';
8080
assert.deepStrictEqual([status, stdout], [1, '']);
8181
assert.match(stderr, /Error: Xyz/);
8282
}
83+
84+
{
85+
// Calling process.exit() in .mjs should return status 0
86+
const { status, stdout, stderr } = child_process.spawnSync(
87+
process.execPath,
88+
[fixtures.path('es-modules/tla/process-exit.mjs')],
89+
{ encoding: 'utf8' });
90+
assert.deepStrictEqual([status, stdout, stderr], [0, '', '']);
91+
}
92+
93+
{
94+
// Calling process.exit() in worker thread shouldn't influence main thread
95+
const { status, stdout, stderr } = child_process.spawnSync(
96+
process.execPath,
97+
[fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs')],
98+
{ encoding: 'utf8' });
99+
assert.deepStrictEqual([status, stdout, stderr], [13, '', '']);
100+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
process.exit();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { Worker, isMainThread } from 'worker_threads';
2+
3+
if (isMainThread) {
4+
new Worker(new URL(import.meta.url));
5+
await new Promise(() => {});
6+
} else {
7+
process.exit();
8+
}

test/parallel/test-bootstrap-modules.js

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ const expectedModules = new Set([
6767
'NativeModule internal/modules/esm/resolve',
6868
'NativeModule internal/modules/esm/transform_source',
6969
'NativeModule internal/modules/esm/translators',
70+
'NativeModule internal/modules/esm/handle_process_exit',
7071
'NativeModule internal/process/esm_loader',
7172
'NativeModule internal/options',
7273
'NativeModule internal/priority_queue',

0 commit comments

Comments
 (0)