Skip to content

Commit

Permalink
module: handle Top-Level Await non-fulfills better
Browse files Browse the repository at this point in the history
Handle situations in which the main `Promise` from a TLA module
is not fulfilled better:

- When not resolving the `Promise` at all, set a non-zero exit code
  (unless another one has been requested explicitly) to distinguish
  the result from a successful completion.
- When rejecting the `Promise`, always treat it like an uncaught
  exception. In particular, this also ensures a non-zero exit code.

Refs: #34558
  • Loading branch information
addaleax committed Aug 6, 2020
1 parent 7343272 commit 6dd288e
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 17 deletions.
2 changes: 2 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -2588,6 +2588,8 @@ cases:
and generally can only happen during development of Node.js itself.
* `12` **Invalid Debug Argument**: The `--inspect` and/or `--inspect-brk`
options were set, but the port number chosen was invalid or unavailable.
* `13` **Unfinished Top-Level Await**: `await` was used outside of a function
in the top-level code, but the passed `Promise` never resolved.
* `>128` **Signal Exits**: If Node.js receives a fatal signal such as
`SIGKILL` or `SIGHUP`, then its exit code will be `128` plus the
value of the signal code. This is a standard POSIX practice, since
Expand Down
24 changes: 22 additions & 2 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,29 @@ function runMainESM(mainPath) {
esmLoader.loadESM((ESMLoader) => {
const main = path.isAbsolute(mainPath) ?
pathToFileURL(mainPath).href : mainPath;
return ESMLoader.import(main);
handleMainPromise(ESMLoader.import(main));
});
}

function handleMainPromise(promise) {
// Handle a Promise from running code that potentially does Top-Level Await.
// In that case, it makes sense to:
// - Treat a rejection as an unhandled exception
// - 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;
}
process.on('exit', handler);
promise
.finally(() => process.off('exit', handler))
.catch((err) => {
internalBinding('errors').triggerUncaughtException(
err, true /* fromPromise */);
});
}

// For backwards compatibility, we have to run a bunch of
// monkey-patchable code that belongs to the CJS loader (exposed by
// `require('module')`) even when the entry point is ESM.
Expand All @@ -62,5 +81,6 @@ function executeUserEntryPoint(main = process.argv[1]) {
}

module.exports = {
executeUserEntryPoint
executeUserEntryPoint,
handleMainPromise,
};
15 changes: 5 additions & 10 deletions lib/internal/process/execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const {
JSONStringify,
PromiseResolve,
} = primordials;

const path = require('path');
Expand Down Expand Up @@ -43,20 +42,16 @@ function evalModule(source, print) {
if (print) {
throw new ERR_EVAL_ESM_CANNOT_PRINT();
}
const { log, error } = require('internal/console/global');
const { decorateErrorStack } = require('internal/util');
const { log } = require('internal/console/global');
const asyncESM = require('internal/process/esm_loader');
PromiseResolve(asyncESM.ESMLoader).then(async (loader) => {
const { handleMainPromise } = require('internal/modules/run_main');
handleMainPromise((async () => {
const loader = await asyncESM.ESMLoader;
const { result } = await loader.eval(source);
if (print) {
log(result);
}
})
.catch((e) => {
decorateErrorStack(e);
error(e);
process.exit(1);
});
})());
}

function evalScript(name, body, breakFirstLine, print) {
Expand Down
82 changes: 82 additions & 0 deletions test/es-module/test-esm-tla-unfinished.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import '../common/index.mjs';
import assert from 'assert';
import child_process from 'child_process';
import fixtures from '../common/fixtures.js';

{
// Unresolved TLA promise, --eval
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
['--input-type=module', '--eval', 'await new Promise(() => {})'],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout, stderr], [13, '', '']);
}

{
// Rejected TLA promise, --eval
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
['--input-type=module', '-e', 'await Promise.reject(new Error("Xyz"))'],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout], [1, '']);
assert.match(stderr, /Error: Xyz/);
}

{
// Unresolved TLA promise with explicit exit code, --eval
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
['--input-type=module', '--eval',
'process.exitCode = 42;await new Promise(() => {})'],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout, stderr], [42, '', '']);
}

{
// Rejected TLA promise with explicit exit code, --eval
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
['--input-type=module', '-e',
'process.exitCode = 42;await Promise.reject(new Error("Xyz"))'],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout], [1, '']);
assert.match(stderr, /Error: Xyz/);
}

{
// Unresolved TLA promise, module file
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
[fixtures.path('es-modules/tla/unresolved.mjs')],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout, stderr], [13, '', '']);
}

{
// Rejected TLA promise, module file
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
[fixtures.path('es-modules/tla/rejected.mjs')],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout], [1, '']);
assert.match(stderr, /Error: Xyz/);
}

{
// Unresolved TLA promise, module file
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
[fixtures.path('es-modules/tla/unresolved-withexitcode.mjs')],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout, stderr], [42, '', '']);
}

{
// Rejected TLA promise, module file
const { status, stdout, stderr } = child_process.spawnSync(
process.execPath,
[fixtures.path('es-modules/tla/rejected-withexitcode.mjs')],
{ encoding: 'utf8' });
assert.deepStrictEqual([status, stdout], [1, '']);
assert.match(stderr, /Error: Xyz/);
}
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/tla/rejected-withexitcode.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
process.exitCode = 42;
await Promise.reject(new Error('Xyz'));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/tla/rejected.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
await Promise.reject(new Error('Xyz'));
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/tla/unresolved-withexitcode.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
process.exitCode = 42;
await new Promise(() => {});
1 change: 1 addition & 0 deletions test/fixtures/es-modules/tla/unresolved.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
await new Promise(() => {});
1 change: 0 additions & 1 deletion test/message/esm_display_syntax_error_import.out
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-ex
at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*)
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:*:*)
1 change: 0 additions & 1 deletion test/message/esm_display_syntax_error_import_module.out
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ SyntaxError: The requested module './module-named-exports.mjs' does not provide
at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*)
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:*:*)
6 changes: 3 additions & 3 deletions test/message/esm_loader_not_found_cjs_hint_bare.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
internal/process/esm_loader.js:*
internalBinding('errors').triggerUncaughtException(
^
internal/modules/run_main.js:*
internalBinding('errors').triggerUncaughtException(
^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '*test*fixtures*node_modules*some_module*obj' imported from *test*fixtures*esm_loader_not_found_cjs_hint_bare.mjs
Did you mean to import some_module/obj.js?
Expand Down

0 comments on commit 6dd288e

Please sign in to comment.