Skip to content

Commit a90defe

Browse files
MoonBallrichardlau
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: #41388 Backport-PR-URL: #41508Fixes: #40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>
1 parent b050c65 commit a90defe

File tree

9 files changed

+57
-10
lines changed

9 files changed

+57
-10
lines changed
Lines changed: 12 additions & 0 deletions
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+
functionhandleProcessExit(){
7+
if(process.exitCode==null)process.exitCode=13;
8+
}
9+
10+
module.exports={
11+
handleProcessExit,
12+
};

‎lib/internal/modules/run_main.js‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
'use strict';
22

33
const{
4-
PromisePrototypeFinally,
54
StringPrototypeEndsWith,
65
}=primordials;
76
constCJSLoader=require('internal/modules/cjs/loader');
87
const{ Module, toRealPath, readPackageScope }=CJSLoader;
98
const{ getOptionValue }=require('internal/options');
109
constpath=require('path');
10+
const{
11+
handleProcessExit,
12+
}=require('internal/modules/esm/handle_process_exit');
1113

1214
functionresolveMainPath(main){
1315
// Note extension resolution for the main entry point can be deprecated in a
@@ -51,16 +53,13 @@ function runMainESM(mainPath){
5153
}));
5254
}
5355

54-
functionhandleMainPromise(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-
functionhandler(){
59-
if(process.exitCode===undefined)
60-
process.exitCode=13;
56+
asyncfunctionhandleMainPromise(promise){
57+
process.on('exit',handleProcessExit);
58+
try{
59+
returnawaitpromise;
60+
}finally{
61+
process.off('exit',handleProcessExit);
6162
}
62-
process.on('exit',handler);
63-
returnPromisePrototypeFinally(promise,()=>process.off('exit',handler));
6463
}
6564

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

‎lib/internal/process/per_thread.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ const{
3636
constformat=require('internal/util/inspect').format;
3737
constconstants=internalBinding('constants').os.signals;
3838

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

167171
functionexit(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‎

Lines changed: 18 additions & 0 deletions
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+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
process.exit();
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import{Worker,isMainThread}from'worker_threads';
2+
3+
if(isMainThread){
4+
newWorker(newURL(import.meta.url));
5+
awaitnewPromise(()=>{});
6+
}else{
7+
process.exit();
8+
}

‎test/message/esm_display_syntax_error_import.out‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-ex
66
at async ModuleJob.run (internal/modules/esm/module_job.js:*:*)
77
at async Loader.import (internal/modules/esm/loader.js:*:*)
88
at async Object.loadESM (internal/process/esm_loader.js:*:*)
9+
at async handleMainPromise (internal/modules/run_main.js:*:*)

‎test/message/esm_display_syntax_error_import_module.out‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ SyntaxError: The requested module './module-named-exports.mjs' does not provide
66
at async ModuleJob.run (internal/modules/esm/module_job.js:*:*)
77
at async Loader.import (internal/modules/esm/loader.js:*:*)
88
at async Object.loadESM (internal/process/esm_loader.js:*:*)
9+
at async handleMainPromise (internal/modules/run_main.js:*:*)

‎test/parallel/test-bootstrap-modules.js‎

Lines changed: 1 addition & 0 deletions
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)