Skip to content

Commit

Permalink
esm: make process.exit() default to exit code 0
Browse files Browse the repository at this point in the history
Due to a bug in top-level await implementation, it used to default to
exit code 13.

PR-URL: #41388
Backport-PR-URL: #41508
Fixes: #40808
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
  • Loading branch information
MoonBall authored and richardlau committed Jan 25, 2022
1 parent b050c65 commit a90defe
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 10 deletions.
12 changes: 12 additions & 0 deletions lib/internal/modules/esm/handle_process_exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

// Handle a Promise from running code that potentially does Top-Level Await.
// In that case, it makes sense to set the exit code to a specific non-zero
// value if the main code never finishes running.
function handleProcessExit() {
if (process.exitCode == null) process.exitCode = 13;
}

module.exports = {
handleProcessExit,
};
19 changes: 9 additions & 10 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
'use strict';

const {
PromisePrototypeFinally,
StringPrototypeEndsWith,
} = primordials;
const CJSLoader = require('internal/modules/cjs/loader');
const { Module, toRealPath, readPackageScope } = CJSLoader;
const { getOptionValue } = require('internal/options');
const path = require('path');
const {
handleProcessExit,
} = require('internal/modules/esm/handle_process_exit');

function resolveMainPath(main) {
// Note extension resolution for the main entry point can be deprecated in a
Expand Down Expand Up @@ -51,16 +53,13 @@ function runMainESM(mainPath) {
}));
}

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

// For backwards compatibility, we have to run a bunch of
Expand Down
6 changes: 6 additions & 0 deletions lib/internal/process/per_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ const {
const format = require('internal/util/inspect').format;
const constants = internalBinding('constants').os.signals;

const {
handleProcessExit,
} = require('internal/modules/esm/handle_process_exit');

function assert(x, msg) {
if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
}
Expand Down Expand Up @@ -165,6 +169,8 @@ function wrapProcessMethods(binding) {
memoryUsage.rss = rss;

function exit(code) {
process.off('exit', handleProcessExit);

if (code || code === 0)
process.exitCode = code;

Expand Down
18 changes: 18 additions & 0 deletions test/es-module/test-esm-tla-unfinished.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,21 @@ import fixtures from '../common/fixtures.js';
assert.deepStrictEqual([status, stdout], [1, '']);
assert.match(stderr, /Error: Xyz/);
}

{
// Calling process.exit() in .mjs should return status 0
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
[fixtures.path('es-modules/tla/process-exit.mjs')],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout, stderr], [0, '', '']);
}

{
// Calling process.exit() in worker thread shouldn't influence main thread
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
[fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs')],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout, stderr], [13, '', '']);
}
1 change: 1 addition & 0 deletions test/fixtures/es-modules/tla/process-exit.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
process.exit();
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Worker, isMainThread } from 'worker_threads';

if (isMainThread) {
new Worker(new URL(import.meta.url));
await new Promise(() => {});
} else {
process.exit();
}
1 change: 1 addition & 0 deletions test/message/esm_display_syntax_error_import.out
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-ex
at async ModuleJob.run (internal/modules/esm/module_job.js:*:*)
at async Loader.import (internal/modules/esm/loader.js:*:*)
at async Object.loadESM (internal/process/esm_loader.js:*:*)
at async handleMainPromise (internal/modules/run_main.js:*:*)
1 change: 1 addition & 0 deletions test/message/esm_display_syntax_error_import_module.out
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ SyntaxError: The requested module './module-named-exports.mjs' does not provide
at async ModuleJob.run (internal/modules/esm/module_job.js:*:*)
at async Loader.import (internal/modules/esm/loader.js:*:*)
at async Object.loadESM (internal/process/esm_loader.js:*:*)
at async handleMainPromise (internal/modules/run_main.js:*:*)
1 change: 1 addition & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const expectedModules = new Set([
'NativeModule internal/modules/esm/resolve',
'NativeModule internal/modules/esm/transform_source',
'NativeModule internal/modules/esm/translators',
'NativeModule internal/modules/esm/handle_process_exit',
'NativeModule internal/process/esm_loader',
'NativeModule internal/options',
'NativeModule internal/priority_queue',
Expand Down

0 comments on commit a90defe

Please sign in to comment.