Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: print location of unsettled top-level await in entry points #51999

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@
/doc/api/packages.md @nodejs/loaders
/lib/internal/bootstrap/realm.js @nodejs/loaders
/lib/internal/modules/* @nodejs/loaders
/lib/internal/process/esm_loader.js @nodejs/loaders
/lib/internal/process/execution.js @nodejs/loaders
/lib/module.js @nodejs/loaders
/src/module_wrap* @nodejs/loaders @nodejs/vm
Expand Down
4 changes: 2 additions & 2 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -3953,8 +3953,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.
* `13` **Unsettled Top-Level Await**: `await` was used outside of a function
in the top-level code, but the passed `Promise` never settled.
* `14` **Snapshot Failure**: Node.js was started to build a V8 startup
snapshot and it failed because certain requirements of the state of
the application were not met.
Expand Down
7 changes: 2 additions & 5 deletions lib/internal/main/check_syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ function loadESMIfNeeded(cb) {
const hasModulePreImport = getOptionValue('--import').length > 0;

if (hasModulePreImport) {
const { loadESM } = require('internal/process/esm_loader');
loadESM(cb);
require('internal/modules/run_main').runEntryPointWithESMLoader(cb);
return;
}
cb();
Expand All @@ -76,7 +75,5 @@ async function checkSyntax(source, filename) {
return;
}

const { loadESM } = require('internal/process/esm_loader');
const { handleMainPromise } = require('internal/modules/run_main');
handleMainPromise(loadESM((loader) => wrapSafe(filename, source)));
wrapSafe(filename, source);
}
8 changes: 4 additions & 4 deletions lib/internal/main/eval_stdin.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const {
const { getOptionValue } = require('internal/options');

const {
evalModule,
evalModuleEntryPoint,
evalScript,
readStdin,
} = require('internal/process/execution');
Expand All @@ -24,15 +24,15 @@ readStdin((code) => {
process._eval = code;

const print = getOptionValue('--print');
const loadESM = getOptionValue('--import').length > 0;
const shouldLoadESM = getOptionValue('--import').length > 0;
if (getOptionValue('--input-type') === 'module' ||
(getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
evalModule(code, print);
evalModuleEntryPoint(code, print);
} else {
evalScript('[stdin]',
code,
getOptionValue('--inspect-brk'),
print,
loadESM);
shouldLoadESM);
}
});
8 changes: 4 additions & 4 deletions lib/internal/main/eval_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const {
prepareMainThreadExecution,
markBootstrapComplete,
} = require('internal/process/pre_execution');
const { evalModule, evalScript } = require('internal/process/execution');
const { evalModuleEntryPoint, evalScript } = require('internal/process/execution');
const { addBuiltinLibsToObject } = require('internal/modules/helpers');

const { getOptionValue } = require('internal/options');
Expand All @@ -24,10 +24,10 @@ markBootstrapComplete();

const source = getOptionValue('--eval');
const print = getOptionValue('--print');
const loadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
const shouldLoadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
if (getOptionValue('--input-type') === 'module' ||
(getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
evalModule(source, print);
evalModuleEntryPoint(source, print);
} else {
// For backward compatibility, we want the identifier crypto to be the
// `node:crypto` module rather than WebCrypto.
Expand All @@ -54,5 +54,5 @@ if (getOptionValue('--input-type') === 'module' ||
) : source,
getOptionValue('--inspect-brk'),
print,
loadESM);
shouldLoadESM);
}
5 changes: 3 additions & 2 deletions lib/internal/main/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
process.exit(kInvalidCommandLineArgument);
}

const esmLoader = require('internal/process/esm_loader');
esmLoader.loadESM(() => {
require('internal/modules/run_main').runEntryPointWithESMLoader(() => {
console.log(`Welcome to Node.js ${process.version}.\n` +
'Type ".help" for more information.');

Expand Down Expand Up @@ -64,5 +63,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
getOptionValue('--inspect-brk'),
getOptionValue('--print'));
}
// The TLAs in the REPL are still run as scripts, just transformed as async
// IIFEs for the REPL code itself to await on.
});
}
4 changes: 2 additions & 2 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ port.on('message', (message) => {
}

case 'module': {
const { evalModule } = require('internal/process/execution');
PromisePrototypeThen(evalModule(filename), undefined, (e) => {
const { evalModuleEntryPoint } = require('internal/process/execution');
PromisePrototypeThen(evalModuleEntryPoint(filename), undefined, (e) => {
workerOnGlobalUncaughtException(e, true);
});
break;
Expand Down
16 changes: 0 additions & 16 deletions lib/internal/modules/esm/handle_process_exit.js

This file was deleted.

8 changes: 4 additions & 4 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const {
ERR_METHOD_NOT_IMPLEMENTED,
ERR_WORKER_UNSERIALIZABLE_ERROR,
} = require('internal/errors').codes;
const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors');
const { exitCodes: { kUnsettledTopLevelAwait } } = internalBinding('errors');
const { URL } = require('internal/url');
const { canParse: URLCanParse } = internalBinding('url');
const { receiveMessageOnPort } = require('worker_threads');
Expand Down Expand Up @@ -146,8 +146,8 @@ class Hooks {
* loader (user-land) to the worker.
*/
async register(urlOrSpecifier, parentURL, data) {
const moduleLoader = require('internal/process/esm_loader').esmLoader;
const keyedExports = await moduleLoader.import(
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
const keyedExports = await cascadedLoader.import(
urlOrSpecifier,
parentURL,
kEmptyObject,
Expand Down Expand Up @@ -615,7 +615,7 @@ class HooksProxy {
} while (response == null);
debug('got sync response from worker', { method, args });
if (response.message.status === 'never-settle') {
process.exit(kUnfinishedTopLevelAwait);
process.exit(kUnsettledTopLevelAwait);
} else if (response.message.status === 'exit') {
process.exit(response.message.body);
}
Expand Down
40 changes: 25 additions & 15 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const {
ERR_UNKNOWN_MODULE_FORMAT,
} = require('internal/errors').codes;
const { getOptionValue } = require('internal/options');
const { pathToFileURL, isURL } = require('internal/url');
const { isURL } = require('internal/url');
const { emitExperimentalWarning } = require('internal/util');
const {
getDefaultConditions,
Expand Down Expand Up @@ -85,11 +85,6 @@ class ModuleLoader {
*/
#defaultConditions = getDefaultConditions();

/**
* The index for assigning unique URLs to anonymous module evaluation
*/
evalIndex = 0;

/**
* Registry of resolved specifiers
*/
Expand Down Expand Up @@ -187,10 +182,7 @@ class ModuleLoader {
}
}

async eval(
source,
url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href,
) {
async eval(source, url, isEntryPoint = false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this method lacks a JSDoc, do you mind adding one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, didn't see this, feel free to follow up and add a comment if you like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird! I just looked through the history, and it seems this method never got a jsdoc when pretty much everything else did.

const evalInstance = (url) => {
const { ModuleWrap } = internalBinding('module_wrap');
const { registerModule } = require('internal/modules/esm/utils');
Expand All @@ -209,11 +201,12 @@ class ModuleLoader {
const job = new ModuleJob(
this, url, undefined, evalInstance, false, false);
this.loadCache.set(url, undefined, job);
const { module } = await job.run();
const { module } = await job.run(isEntryPoint);

return {
__proto__: null,
namespace: module.getNamespace(),
module,
};
}

Expand Down Expand Up @@ -318,9 +311,9 @@ class ModuleLoader {
* module import.
* @returns {Promise<ModuleExports>}
*/
async import(specifier, parentURL, importAttributes) {
async import(specifier, parentURL, importAttributes, isEntryPoint = false) {
const moduleJob = await this.getModuleJob(specifier, parentURL, importAttributes);
const { module } = await moduleJob.run();
const { module } = await moduleJob.run(isEntryPoint);
return module.getNamespace();
}

Expand Down Expand Up @@ -568,6 +561,23 @@ function getHooksProxy() {
return hooksProxy;
}

let cascadedLoader;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: why call it "cascaded" loader?


/**
* This is a singleton ESM loader that integrates the loader hooks, if any.
* It it used by other internal built-ins when they need to load ESM code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* It it used by other internal built-ins when they need to load ESM code
* It is used by other internal built-ins when they need to load ESM code

* while also respecting hooks.
* When built-ins need access to this loader, they should do
* require('internal/module/esm/loader').getOrInitializeCascadedLoader()
* lazily only right before the loader is actually needed, and don't do it
* in the top-level, to avoid circular dependencies.
* @returns {ModuleLoader}
*/
function getOrInitializeCascadedLoader() {
cascadedLoader ??= createModuleLoader();
return cascadedLoader;
}

/**
* Register a single loader programmatically.
* @param {string|import('url').URL} specifier
Expand Down Expand Up @@ -598,12 +608,11 @@ function getHooksProxy() {
* ```
*/
function register(specifier, parentURL = undefined, options) {
const moduleLoader = require('internal/process/esm_loader').esmLoader;
if (parentURL != null && typeof parentURL === 'object' && !isURL(parentURL)) {
options = parentURL;
parentURL = options.parentURL;
}
moduleLoader.register(
getOrInitializeCascadedLoader().register(
specifier,
parentURL ?? 'data:',
options?.data,
Expand All @@ -614,5 +623,6 @@ function register(specifier, parentURL = undefined, options) {
module.exports = {
createModuleLoader,
MoLow marked this conversation as resolved.
Show resolved Hide resolved
getHooksProxy,
getOrInitializeCascadedLoader,
register,
};
12 changes: 10 additions & 2 deletions lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@ const {
StringPrototypeIncludes,
StringPrototypeSplit,
StringPrototypeStartsWith,
globalThis,
} = primordials;

const { ModuleWrap } = internalBinding('module_wrap');

const {
privateSymbols: {
entry_point_module_private_symbol,
},
} = internalBinding('util');
const { decorateErrorStack, kEmptyObject } = require('internal/util');
const {
getSourceMapsEnabled,
Expand Down Expand Up @@ -213,8 +218,11 @@ class ModuleJob {
return { __proto__: null, module: this.module };
}

async run() {
async run(isEntryPoint = false) {
await this.instantiate();
if (isEntryPoint) {
globalThis[entry_point_module_private_symbol] = this.module;
}
const timeout = -1;
const breakOnSigint = false;
setHasStartedUserESMExecution();
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const {
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
const moduleWrap = internalBinding('module_wrap');
const { ModuleWrap } = moduleWrap;
const asyncESM = require('internal/process/esm_loader');
const { emitWarningSync } = require('internal/process/warning');
const { internalCompileFunction } = require('internal/vm');
const {
Expand Down Expand Up @@ -157,7 +156,8 @@ function errPath(url) {
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} The imported module.
*/
async function importModuleDynamically(specifier, { url }, attributes) {
return asyncESM.esmLoader.import(specifier, url, attributes);
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
return cascadedLoader.import(specifier, url, attributes);
}

// Strategy for loading a standard JavaScript module.
Expand Down Expand Up @@ -243,6 +243,7 @@ function loadCJSModule(module, source, url, filename) {

const compiledWrapper = compileResult.function;

const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
const __dirname = dirname(filename);
// eslint-disable-next-line func-name-matching,func-style
const requireFn = function require(specifier) {
Expand All @@ -261,7 +262,7 @@ function loadCJSModule(module, source, url, filename) {
}
specifier = `${pathToFileURL(path)}`;
}
const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAttributes);
const job = cascadedLoader.getModuleJobSync(specifier, url, importAttributes);
job.runSync();
return cjsCache.get(job.url).exports;
};
Expand All @@ -272,7 +273,7 @@ function loadCJSModule(module, source, url, filename) {
specifier = `${pathToFileURL(path)}`;
}
}
const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(specifier, url, kEmptyObject);
const { url: resolvedURL } = cascadedLoader.resolveSync(specifier, url, kEmptyObject);
return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL;
});
setOwnProperty(requireFn, 'main', process.mainModule);
Expand Down
11 changes: 4 additions & 7 deletions lib/internal/modules/esm/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const {
const {
emitExperimentalWarning,
getCWDURL,
getLazy,
} = require('internal/util');
const {
setImportModuleDynamicallyCallback,
Expand Down Expand Up @@ -181,9 +180,6 @@ function initializeImportMetaObject(symbol, meta) {
}
}
}
const getCascadedLoader = getLazy(
() => require('internal/process/esm_loader').esmLoader,
);

/**
* Proxy the dynamic import to the default loader.
Expand All @@ -194,7 +190,8 @@ const getCascadedLoader = getLazy(
*/
function defaultImportModuleDynamically(specifier, attributes, referrerName) {
const parentURL = normalizeReferrerURL(referrerName);
return getCascadedLoader().import(specifier, parentURL, attributes);
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
return cascadedLoader.import(specifier, parentURL, attributes);
}

/**
Expand Down Expand Up @@ -263,10 +260,10 @@ async function initializeHooks() {
const customLoaderURLs = getOptionValue('--experimental-loader');

const { Hooks } = require('internal/modules/esm/hooks');
const esmLoader = require('internal/process/esm_loader').esmLoader;
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();

const hooks = new Hooks();
esmLoader.setCustomizations(hooks);
cascadedLoader.setCustomizations(hooks);

// We need the loader customizations to be set _before_ we start invoking
// `--require`, otherwise loops can happen because a `--require` script
Expand Down
Loading