From a61e5508a77805e336d219211ca18a081634d476 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 2 Mar 2024 03:23:22 +0100 Subject: [PATCH 1/5] module: refactor ESM loader initialization and entry point handling Split the `internal/process/esm_loader` file which contains the singleton cascaded loader: - The the singleton cascaded loader now directly resides in `internal/modules/esm/loader`, where the constructor also lives. This file is the root of most circular dependency of ESM code, (because components of the loader need the singleton itself), so this makes the dependency more obvious. Added comments about loading it lazily to avoid circular dependency. - The getter to the cascaded loader is also turned into a method to make the side effect explicit. - The sequence of `loadESM()` and `handleMainPromise` is now merged together into `runEntryPointWithESMLoader()` in `internal/modules/run_main` because this is intended to run entry points with the ESM loader and not just any module. - Documents how top-level await is handled. --- lib/internal/main/check_syntax.js | 7 +- lib/internal/main/eval_stdin.js | 8 +- lib/internal/main/eval_string.js | 8 +- lib/internal/main/repl.js | 5 +- lib/internal/main/worker_thread.js | 4 +- .../modules/esm/handle_process_exit.js | 16 ---- lib/internal/modules/esm/hooks.js | 4 +- lib/internal/modules/esm/loader.js | 34 +++++--- lib/internal/modules/esm/translators.js | 9 ++- lib/internal/modules/esm/utils.js | 11 +-- lib/internal/modules/run_main.js | 78 ++++++++++++++----- lib/internal/process/esm_loader.js | 40 ---------- lib/internal/process/execution.js | 38 +++++---- lib/internal/process/per_thread.js | 4 +- lib/internal/test_runner/utils.js | 4 +- lib/repl.js | 5 +- 16 files changed, 136 insertions(+), 139 deletions(-) delete mode 100644 lib/internal/modules/esm/handle_process_exit.js delete mode 100644 lib/internal/process/esm_loader.js diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js index 9a19c1809fe102..5a7ab5dc19e4e7 100644 --- a/lib/internal/main/check_syntax.js +++ b/lib/internal/main/check_syntax.js @@ -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(); @@ -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); } diff --git a/lib/internal/main/eval_stdin.js b/lib/internal/main/eval_stdin.js index d71751e781b9b5..3ee4bcdb1d853b 100644 --- a/lib/internal/main/eval_stdin.js +++ b/lib/internal/main/eval_stdin.js @@ -10,7 +10,7 @@ const { const { getOptionValue } = require('internal/options'); const { - evalModule, + evalModuleEntryPoint, evalScript, readStdin, } = require('internal/process/execution'); @@ -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); } }); diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js index 908532b0b1865a..1125aa8d98e5aa 100644 --- a/lib/internal/main/eval_string.js +++ b/lib/internal/main/eval_string.js @@ -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'); @@ -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. @@ -54,5 +54,5 @@ if (getOptionValue('--input-type') === 'module' || ) : source, getOptionValue('--inspect-brk'), print, - loadESM); + shouldLoadESM); } diff --git a/lib/internal/main/repl.js b/lib/internal/main/repl.js index da1764a9c80d95..f7aa3a3e2602fa 100644 --- a/lib/internal/main/repl.js +++ b/lib/internal/main/repl.js @@ -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.'); @@ -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. }); } diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index c14091ffe09ca7..c0b151a1eac9de 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -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; diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js deleted file mode 100644 index 4689ef6bb204c0..00000000000000 --- a/lib/internal/modules/esm/handle_process_exit.js +++ /dev/null @@ -1,16 +0,0 @@ -'use strict'; - -const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors'); - -/** - * 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() { - process.exitCode ??= kUnfinishedTopLevelAwait; -} - -module.exports = { - handleProcessExit, -}; diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 314c49a95a4ea1..60942130dd48b9 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -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, diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index c0e3cdb36e1c02..fd4faa3bd4af62 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -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, @@ -85,11 +85,6 @@ class ModuleLoader { */ #defaultConditions = getDefaultConditions(); - /** - * The index for assigning unique URLs to anonymous module evaluation - */ - evalIndex = 0; - /** * Registry of resolved specifiers */ @@ -187,10 +182,7 @@ class ModuleLoader { } } - async eval( - source, - url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href, - ) { + async eval(source, url) { const evalInstance = (url) => { const { ModuleWrap } = internalBinding('module_wrap'); const { registerModule } = require('internal/modules/esm/utils'); @@ -214,6 +206,7 @@ class ModuleLoader { return { __proto__: null, namespace: module.getNamespace(), + module, }; } @@ -568,6 +561,23 @@ function getHooksProxy() { return hooksProxy; } +let cascadedLoader; + +/** + * 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 + * 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 @@ -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, @@ -614,5 +623,6 @@ function register(specifier, parentURL = undefined, options) { module.exports = { createModuleLoader, getHooksProxy, + getOrInitializeCascadedLoader, register, }; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index ca547699d00ed1..61e489a6cedd7a 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -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 { @@ -157,7 +156,8 @@ function errPath(url) { * @returns {Promise} 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. @@ -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) { @@ -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; }; @@ -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); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 202159498edd47..7c1fb2a5745b5b 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -32,7 +32,6 @@ const { const { emitExperimentalWarning, getCWDURL, - getLazy, } = require('internal/util'); const { setImportModuleDynamicallyCallback, @@ -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. @@ -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); } /** @@ -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 diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 23268637e4fd58..6488bedb21cf6c 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -9,6 +9,15 @@ const { getNearestParentPackageJSONType } = internalBinding('modules'); const { getOptionValue } = require('internal/options'); const { checkPackageJSONIntegrity } = require('internal/modules/package_json_reader'); const path = require('path'); +const { pathToFileURL } = require('internal/url'); +const { kEmptyObject, getCWDURL } = require('internal/util'); +const { + hasUncaughtExceptionCaptureCallback, +} = require('internal/process/execution'); +const { + triggerUncaughtException, + exitCodes: { kUnfinishedTopLevelAwait }, +} = internalBinding('errors'); /** * Get the absolute path to the main entry point. @@ -94,35 +103,58 @@ function shouldUseESMLoader(mainPath) { } /** - * Run the main entry point through the ESM Loader. - * @param {string} mainPath - Absolute path for the main entry point + * 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 runMainESM(mainPath) { - const { loadESM } = require('internal/process/esm_loader'); - const { pathToFileURL } = require('internal/url'); - const main = pathToFileURL(mainPath).href; - - handleMainPromise(loadESM((esmLoader) => { - return esmLoader.import(main, undefined, { __proto__: null }); - })); +function handleProcessExit() { + process.exitCode ??= kUnfinishedTopLevelAwait; } /** - * Handle process exit events around the main entry point promise. - * @param {Promise} promise - Main entry point promise + * @param {function(ModuleLoader):ModuleWrap|undefined} callback */ -async function handleMainPromise(promise) { - const { - handleProcessExit, - } = require('internal/modules/esm/handle_process_exit'); +async function asyncRunEntryPointWithESMLoader(callback) { process.on('exit', handleProcessExit); + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); try { - return await promise; + const userImports = getOptionValue('--import'); + if (userImports.length > 0) { + const parentURL = getCWDURL().href; + for (let i = 0; i < userImports.length; i++) { + await cascadedLoader.import(userImports[i], parentURL, kEmptyObject); + } + } else { + cascadedLoader.forceLoadHooks(); + } + await callback(cascadedLoader); + } catch (err) { + if (hasUncaughtExceptionCaptureCallback()) { + process._fatalException(err); + return; + } + triggerUncaughtException( + err, + true, /* fromPromise */ + ); } finally { process.off('exit', handleProcessExit); } } +/** + * This initializes the ESM loader and runs --import (if any) before executing the + * callback to run the entry point. + * If the callback intends to evaluate a ESM module as entry point, it should return + * the corresponding ModuleWrap so that stalled TLA can be checked a process exit. + * @param {function(ModuleLoader):ModuleWrap|undefined} callback + * @returns {Promise} + */ +function runEntryPointWithESMLoader(callback) { + const promise = asyncRunEntryPointWithESMLoader(callback); + return promise; +} + /** * Parse the CLI main entry point string and run it. * For backwards compatibility, we have to run a bunch of monkey-patchable code that belongs to the CJS loader (exposed @@ -135,7 +167,14 @@ function executeUserEntryPoint(main = process.argv[1]) { const resolvedMain = resolveMainPath(main); const useESMLoader = shouldUseESMLoader(resolvedMain); if (useESMLoader) { - runMainESM(resolvedMain || main); + const mainPath = resolvedMain || main; + const mainURL = pathToFileURL(mainPath).href; + + runEntryPointWithESMLoader((cascadedLoader) => { + // Note that if the graph contains unfinished TLA, this may never resolve + // even after the event loop stops running. + return cascadedLoader.import(mainURL, undefined, { __proto__: null }, true); + }); } else { // Module._load is the monkey-patchable CJS module loader. const { Module } = require('internal/modules/cjs/loader'); @@ -145,5 +184,6 @@ function executeUserEntryPoint(main = process.argv[1]) { module.exports = { executeUserEntryPoint, - handleMainPromise, + runEntryPointWithESMLoader, + handleProcessExit, }; diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js deleted file mode 100644 index 0865d7ceef66b7..00000000000000 --- a/lib/internal/process/esm_loader.js +++ /dev/null @@ -1,40 +0,0 @@ -'use strict'; - -const { createModuleLoader } = require('internal/modules/esm/loader'); -const { getOptionValue } = require('internal/options'); -const { - hasUncaughtExceptionCaptureCallback, -} = require('internal/process/execution'); -const { kEmptyObject, getCWDURL } = require('internal/util'); - -let esmLoader; - -module.exports = { - get esmLoader() { - return esmLoader ??= createModuleLoader(); - }, - async loadESM(callback) { - esmLoader ??= createModuleLoader(); - try { - const userImports = getOptionValue('--import'); - if (userImports.length > 0) { - const parentURL = getCWDURL().href; - for (let i = 0; i < userImports.length; i++) { - await esmLoader.import(userImports[i], parentURL, kEmptyObject); - } - } else { - esmLoader.forceLoadHooks(); - } - await callback(esmLoader); - } catch (err) { - if (hasUncaughtExceptionCaptureCallback()) { - process._fatalException(err); - return; - } - internalBinding('errors').triggerUncaughtException( - err, - true, /* fromPromise */ - ); - } - }, -}; diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 5de5edfb2d5524..e69add7394e60f 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -15,6 +15,7 @@ const { ERR_EVAL_ESM_CANNOT_PRINT, }, } = require('internal/errors'); +const { pathToFileURL } = require('internal/url'); const { exitCodes: { kGenericUserError } } = internalBinding('errors'); const { @@ -46,19 +47,30 @@ function tryGetCwd() { } } -function evalModule(source, print) { +let evalIndex = 0; +function getEvalModuleUrl() { + return pathToFileURL(`${process.cwd()}/[eval${++evalIndex}]`).href; +} + +/** + * Evaluate an ESM entry point and return the promise that gets fulfilled after + * it finishes evaluation. + * @param {string} source Source code the ESM + * @param {boolean} print Whether the result should be printed. + * @returns {Promise} + */ +function evalModuleEntryPoint(source, print) { if (print) { throw new ERR_EVAL_ESM_CANNOT_PRINT(); } - const { loadESM } = require('internal/process/esm_loader'); - const { handleMainPromise } = require('internal/modules/run_main'); RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. - return handleMainPromise(loadESM((loader) => loader.eval(source))); + return require('internal/modules/run_main').runEntryPointWithESMLoader( + (loader) => loader.eval(source, getEvalModuleUrl(), true), + ); } function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { const CJSModule = require('internal/modules/cjs/loader').Module; - const { pathToFileURL } = require('internal/url'); const cwd = tryGetCwd(); const origModule = globalThis.module; // Set e.g. when called from the REPL. @@ -67,15 +79,12 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { module.filename = path.join(cwd, name); module.paths = CJSModule._nodeModulePaths(cwd); - const { handleMainPromise } = require('internal/modules/run_main'); - const asyncESM = require('internal/process/esm_loader'); const baseUrl = pathToFileURL(module.filename).href; - const { loadESM } = asyncESM; if (getOptionValue('--experimental-detect-module') && getOptionValue('--input-type') === '' && getOptionValue('--experimental-default-type') === '' && containsModuleSyntax(body, name)) { - return evalModule(body, print); + return evalModuleEntryPoint(body, print); } const runScript = () => { @@ -92,8 +101,8 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { const result = module._compile(script, `${name}-wrapper`)(() => { const hostDefinedOptionId = Symbol(name); async function importModuleDynamically(specifier, _, importAttributes) { - const loader = asyncESM.esmLoader; - return loader.import(specifier, baseUrl, importAttributes); + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + return cascadedLoader.import(specifier, baseUrl, importAttributes); } const script = makeContextifyScript( body, // code @@ -118,9 +127,10 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { }; if (shouldLoadESM) { - return handleMainPromise(loadESM(runScript)); + require('internal/modules/run_main').runEntryPointWithESMLoader(runScript); + return; } - return runScript(); + runScript(); } const exceptionHandlerState = { @@ -228,7 +238,7 @@ function readStdin(callback) { module.exports = { readStdin, tryGetCwd, - evalModule, + evalModuleEntryPoint, evalScript, onGlobalUncaughtException: createOnGlobalUncaughtException(), setUncaughtExceptionCaptureCallback, diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index b45f2a61e0ddaf..85777c9e4a3ed5 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -173,9 +173,7 @@ function wrapProcessMethods(binding) { memoryUsage.rss = rss; function exit(code) { - const { - handleProcessExit, - } = require('internal/modules/esm/handle_process_exit'); + const { handleProcessExit } = require('internal/modules/run_main'); process.off('exit', handleProcessExit); if (arguments.length !== 0) { diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 184d44dce6c162..0a3fd9cad83edd 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -154,8 +154,8 @@ async function getReportersMap(reporters, destinations) { parentURL = 'file:///'; } - const { esmLoader } = require('internal/process/esm_loader'); - reporter = await esmLoader.import(name, parentURL, { __proto__: null }); + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + reporter = await cascadedLoader.import(name, parentURL, { __proto__: null }); } if (reporter?.default) { diff --git a/lib/repl.js b/lib/repl.js index 1fbce42888c9a2..d16f8882211a42 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -462,9 +462,8 @@ function REPLServer(prompt, // Continue regardless of error. } async function importModuleDynamically(specifier, _, importAttributes) { - const asyncESM = require('internal/process/esm_loader'); - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + return cascadedLoader.import(specifier, parentURL, importAttributes); } // `experimentalREPLAwait` is set to true by default. // Shall be false in case `--no-experimental-repl-await` flag is used. From 74221f8869bd5a1bc0ad5a08de191120177e3e10 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 5 Mar 2024 16:26:58 +0100 Subject: [PATCH 2/5] src: refactor out FormatErrorMessage for error formatting --- src/node_errors.cc | 14 ++++++++++++-- src/node_internals.h | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/node_errors.cc b/src/node_errors.cc index 7ef6ea7f07998f..ff091fd20d915b 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -303,17 +303,27 @@ std::string FormatCaughtException(Isolate* isolate, Local err, Local message, bool add_source_line = true) { - std::string result; node::Utf8Value reason(isolate, err->ToDetailString(context) .FromMaybe(Local())); + std::string reason_str = reason.ToString(); + return FormatErrorMessage( + isolate, context, reason_str, message, add_source_line); +} + +std::string FormatErrorMessage(Isolate* isolate, + Local context, + const std::string& reason, + Local message, + bool add_source_line) { + std::string result; if (add_source_line) { bool added_exception_line = false; std::string source = GetErrorSource(isolate, context, message, &added_exception_line); result = source + '\n'; } - result += reason.ToString() + '\n'; + result += reason + '\n'; Local stack = message->GetStackTrace(); if (!stack.IsEmpty()) result += FormatStackTrace(isolate, stack); diff --git a/src/node_internals.h b/src/node_internals.h index eeb0fac3fa1aa9..5f0adcf8aaba93 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -98,7 +98,11 @@ void PrintCaughtException(v8::Isolate* isolate, std::string FormatCaughtException(v8::Isolate* isolate, v8::Local context, const v8::TryCatch& try_catch); - +std::string FormatErrorMessage(v8::Isolate* isolate, + v8::Local context, + const std::string& reason, + v8::Local message, + bool add_source_line = true); void ResetStdio(); // Safe to call more than once and from signal handlers. #ifdef __POSIX__ void SignalExit(int signal, siginfo_t* info, void* ucontext); From 178a286ec947718edfc9a88f4f8d73f7dcefab1a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 7 Mar 2024 05:43:44 +0100 Subject: [PATCH 3/5] module: print location of unsettled top-level await in entry points When the entry point is a module and the graph it imports still contains unsettled top-level await when the Node.js instance finishes the event loop, search from the entry point module for unsettled top-level await and print their location. To avoid unnecessary overhead, we register a promise that only gets settled when the entry point graph evaluation returns from await, and only search the module graph if it's still unsettled by the time the instance is exiting. This patch only handles this for entry point modules. Other kinds of modules are more complicated so will be left for the future. Drive-by: update the terminology "unfinished promise" to the more correct one "unsettled promise" in the codebase. --- doc/api/process.md | 4 +-- lib/internal/modules/esm/hooks.js | 4 +-- lib/internal/modules/esm/loader.js | 8 ++--- lib/internal/modules/esm/module_job.js | 12 +++++-- lib/internal/modules/run_main.js | 27 ++++++--------- lib/internal/process/per_thread.js | 3 -- src/api/embed_helpers.cc | 15 +++++++- src/env-inl.h | 4 +++ src/env.cc | 37 ++++++++++++++++++++ src/env.h | 2 ++ src/env_properties.h | 2 ++ src/module_wrap.cc | 38 ++++++++++++++++++++ src/module_wrap.h | 1 + src/node_exit_code.h | 2 +- test/es-module/test-esm-loader-hooks.mjs | 29 ++++++++++++++-- test/es-module/test-esm-tla-unfinished.mjs | 40 ++++++++++++++++++++-- 16 files changed, 192 insertions(+), 36 deletions(-) diff --git a/doc/api/process.md b/doc/api/process.md index 9faf4432f9b9f2..e16659cd74498a 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -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. diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 60942130dd48b9..ba655116a0bb57 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -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'); @@ -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); } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index fd4faa3bd4af62..1b46caf9c6412d 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -182,7 +182,7 @@ class ModuleLoader { } } - async eval(source, url) { + async eval(source, url, isEntryPoint = false) { const evalInstance = (url) => { const { ModuleWrap } = internalBinding('module_wrap'); const { registerModule } = require('internal/modules/esm/utils'); @@ -201,7 +201,7 @@ 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, @@ -311,9 +311,9 @@ class ModuleLoader { * module import. * @returns {Promise} */ - 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(); } diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 05db7f3867efe2..c55398304b480d 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -5,6 +5,7 @@ const { ArrayPrototypePush, ArrayPrototypeSome, FunctionPrototype, + globalThis, ObjectSetPrototypeOf, PromiseResolve, PromisePrototypeThen, @@ -20,7 +21,11 @@ const { } = primordials; const { ModuleWrap } = internalBinding('module_wrap'); - +const { + privateSymbols: { + entry_point_module_private_symbol, + }, +} = internalBinding('util'); const { decorateErrorStack, kEmptyObject } = require('internal/util'); const { getSourceMapsEnabled, @@ -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(); diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 6488bedb21cf6c..a0f43e90b43554 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -1,6 +1,7 @@ 'use strict'; const { + globalThis, StringPrototypeEndsWith, } = primordials; @@ -16,9 +17,12 @@ const { } = require('internal/process/execution'); const { triggerUncaughtException, - exitCodes: { kUnfinishedTopLevelAwait }, } = internalBinding('errors'); - +const { + privateSymbols: { + entry_point_promise_private_symbol, + }, +} = internalBinding('util'); /** * Get the absolute path to the main entry point. * @param {string} main - Entry point path @@ -102,20 +106,10 @@ function shouldUseESMLoader(mainPath) { return type === 'module'; } -/** - * 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() { - process.exitCode ??= kUnfinishedTopLevelAwait; -} - /** * @param {function(ModuleLoader):ModuleWrap|undefined} callback */ async function asyncRunEntryPointWithESMLoader(callback) { - process.on('exit', handleProcessExit); const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); try { const userImports = getOptionValue('--import'); @@ -137,8 +131,6 @@ async function asyncRunEntryPointWithESMLoader(callback) { err, true, /* fromPromise */ ); - } finally { - process.off('exit', handleProcessExit); } } @@ -152,6 +144,10 @@ async function asyncRunEntryPointWithESMLoader(callback) { */ function runEntryPointWithESMLoader(callback) { const promise = asyncRunEntryPointWithESMLoader(callback); + // Register the promise - if by the time the event loop finishes running, this is + // still unsettled, we'll search the graph from the entry point module and print + // the location of any unsettled top-level await found. + globalThis[entry_point_promise_private_symbol] = promise; return promise; } @@ -171,7 +167,7 @@ function executeUserEntryPoint(main = process.argv[1]) { const mainURL = pathToFileURL(mainPath).href; runEntryPointWithESMLoader((cascadedLoader) => { - // Note that if the graph contains unfinished TLA, this may never resolve + // Note that if the graph contains unsettled TLA, this may never resolve // even after the event loop stops running. return cascadedLoader.import(mainURL, undefined, { __proto__: null }, true); }); @@ -185,5 +181,4 @@ function executeUserEntryPoint(main = process.argv[1]) { module.exports = { executeUserEntryPoint, runEntryPointWithESMLoader, - handleProcessExit, }; diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index 85777c9e4a3ed5..891767da529ca0 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -173,9 +173,6 @@ function wrapProcessMethods(binding) { memoryUsage.rss = rss; function exit(code) { - const { handleProcessExit } = require('internal/modules/run_main'); - process.off('exit', handleProcessExit); - if (arguments.length !== 0) { process.exitCode = code; } diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc index 6fac48d1b534d2..1a2cb29993057f 100644 --- a/src/api/embed_helpers.cc +++ b/src/api/embed_helpers.cc @@ -73,7 +73,20 @@ Maybe SpinEventLoopInternal(Environment* env) { env->PrintInfoForSnapshotIfDebug(); env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); }); - return EmitProcessExitInternal(env); + Maybe exit_code = EmitProcessExitInternal(env); + if (exit_code.FromMaybe(ExitCode::kGenericUserError) != + ExitCode::kNoFailure) { + return exit_code; + } + + auto unsettled_tla = env->CheckUnsettledTopLevelAwait(); + if (unsettled_tla.IsNothing()) { + return Nothing(); + } + if (!unsettled_tla.FromJust()) { + return Just(ExitCode::kUnsettledTopLevelAwait); + } + return Just(ExitCode::kNoFailure); } struct CommonEnvironmentSetup::Impl { diff --git a/src/env-inl.h b/src/env-inl.h index 666dad97b021f4..c547fc987ea508 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -371,6 +371,10 @@ inline void Environment::set_exiting(bool value) { exit_info_[kExiting] = value ? 1 : 0; } +inline bool Environment::exiting() const { + return exit_info_[kExiting] == 1; +} + inline ExitCode Environment::exit_code(const ExitCode default_code) const { return exit_info_[kHasExitCode] == 0 ? default_code diff --git a/src/env.cc b/src/env.cc index 9f83720fefc773..5d2ac14b29c168 100644 --- a/src/env.cc +++ b/src/env.cc @@ -4,6 +4,7 @@ #include "debug_utils-inl.h" #include "diagnosticfilename-inl.h" #include "memory_tracker-inl.h" +#include "module_wrap.h" #include "node_buffer.h" #include "node_context_data.h" #include "node_contextify.h" @@ -50,6 +51,7 @@ using v8::HeapSpaceStatistics; using v8::Integer; using v8::Isolate; using v8::Local; +using v8::Maybe; using v8::MaybeLocal; using v8::NewStringType; using v8::Number; @@ -1228,6 +1230,41 @@ void Environment::AtExit(void (*cb)(void* arg), void* arg) { at_exit_functions_.push_front(ExitCallback{cb, arg}); } +Maybe Environment::CheckUnsettledTopLevelAwait() { + HandleScope scope(isolate_); + Local ctx = context(); + Local value; + + Local entry_point_promise; + if (!ctx->Global() + ->GetPrivate(ctx, entry_point_promise_private_symbol()) + .ToLocal(&entry_point_promise)) { + return v8::Nothing(); + } + if (!entry_point_promise->IsPromise()) { + return v8::Just(true); + } + if (entry_point_promise.As()->State() != + Promise::PromiseState::kPending) { + return v8::Just(true); + } + + if (!ctx->Global() + ->GetPrivate(ctx, entry_point_module_private_symbol()) + .ToLocal(&value)) { + return v8::Nothing(); + } + if (!value->IsObject()) { + return v8::Just(true); + } + Local object = value.As(); + CHECK(BaseObject::IsBaseObject(isolate_data_, object)); + CHECK_EQ(object->InternalFieldCount(), + loader::ModuleWrap::kInternalFieldCount); + auto* wrap = BaseObject::FromJSObject(object); + return wrap->CheckUnsettledTopLevelAwait(); +} + void Environment::RunAndClearInterrupts() { while (native_immediates_interrupts_.size() > 0) { NativeImmediateQueue queue; diff --git a/src/env.h b/src/env.h index ff09da28b2cadc..cc1233050f6fb1 100644 --- a/src/env.h +++ b/src/env.h @@ -735,6 +735,7 @@ class Environment : public MemoryRetainer { // a pseudo-boolean to indicate whether the exit code is undefined. inline AliasedInt32Array& exit_info(); inline void set_exiting(bool value); + bool exiting() const; inline ExitCode exit_code(const ExitCode default_code) const; // This stores whether the --abort-on-uncaught-exception flag was passed @@ -840,6 +841,7 @@ class Environment : public MemoryRetainer { void AtExit(void (*cb)(void* arg), void* arg); void RunAtExitCallbacks(); + v8::Maybe CheckUnsettledTopLevelAwait(); void RunWeakRefCleanup(); v8::MaybeLocal RunSnapshotSerializeCallback() const; diff --git a/src/env_properties.h b/src/env_properties.h index 1ecd06e1a3546d..6f99d819ac78a8 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -24,6 +24,8 @@ V(transfer_mode_private_symbol, "node:transfer_mode") \ V(host_defined_option_symbol, "node:host_defined_option_symbol") \ V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper") \ + V(entry_point_module_private_symbol, "node:entry_point_module") \ + V(entry_point_promise_private_symbol, "node:entry_point_promise") \ V(napi_type_tag, "node:napi:type_tag") \ V(napi_wrapper, "node:napi:wrapper") \ V(untransferable_object_private_symbol, "node:untransferableObject") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 58ebe7b837af5f..25eb79d450c807 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -104,6 +104,44 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env, return nullptr; } +v8::Maybe ModuleWrap::CheckUnsettledTopLevelAwait() { + Isolate* isolate = env()->isolate(); + Local context = env()->context(); + + // This must be invoked when the environment is shutting down, and the module + // is kept alive by the module wrap via an internal field. + CHECK(env()->exiting()); + CHECK(!module_.IsEmpty()); + + Local module = module_.Get(isolate); + // It's a synthetic module, likely a facade wrapping CJS. + if (!module->IsSourceTextModule()) { + return v8::Just(true); + } + + if (!module->IsGraphAsync()) { // There is no TLA, no need to check. + return v8::Just(true); + } + auto stalled = module->GetStalledTopLevelAwaitMessage(isolate); + if (stalled.size() == 0) { + return v8::Just(true); + } + + if (env()->options()->warnings) { + for (auto pair : stalled) { + Local message = std::get<1>(pair); + + std::string reason = "Warning: Detected unsettled top-level await at "; + std::string info = + FormatErrorMessage(isolate, context, "", message, true); + reason += info; + FPrintF(stderr, "%s\n", reason); + } + } + + return v8::Just(false); +} + // new ModuleWrap(url, context, source, lineOffset, columnOffset) // new ModuleWrap(url, context, exportNames, syntheticExecutionFunction) void ModuleWrap::New(const FunctionCallbackInfo& args) { diff --git a/src/module_wrap.h b/src/module_wrap.h index e17048357feca2..6f44d722ee0b01 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -58,6 +58,7 @@ class ModuleWrap : public BaseObject { } v8::Local context() const; + v8::Maybe CheckUnsettledTopLevelAwait(); SET_MEMORY_INFO_NAME(ModuleWrap) SET_SELF_SIZE(ModuleWrap) diff --git a/src/node_exit_code.h b/src/node_exit_code.h index 07c5a94e35ee28..0ba0511b5a3de5 100644 --- a/src/node_exit_code.h +++ b/src/node_exit_code.h @@ -27,7 +27,7 @@ namespace node { /* This was intended for invalid inspector arguments but is actually now */ \ /* just a duplicate of InvalidCommandLineArgument */ \ V(InvalidCommandLineArgument2, 12) \ - V(UnfinishedTopLevelAwait, 13) \ + V(UnsettledTopLevelAwait, 13) \ V(StartupSnapshotFailure, 14) \ /* If the process exits from unhandled signals e.g. SIGABRT, SIGTRAP, */ \ /* typically the exit codes are 128 + signal number. We also exit with */ \ diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 6f035bccb87f5c..2085ffaed1049e 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -51,7 +51,7 @@ describe('Loader hooks', { concurrency: true }, () => { }); describe('should handle never-settling hooks in ESM files', { concurrency: true }, () => { - it('top-level await of a never-settling resolve', async () => { + it('top-level await of a never-settling resolve without warning', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ '--no-warnings', '--experimental-loader', @@ -65,7 +65,20 @@ describe('Loader hooks', { concurrency: true }, () => { assert.strictEqual(signal, null); }); - it('top-level await of a never-settling load', async () => { + it('top-level await of a never-settling resolve with warning', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--experimental-loader', + fixtures.fileURL('es-module-loaders/never-settling-resolve-step/loader.mjs'), + fixtures.path('es-module-loaders/never-settling-resolve-step/never-resolve.mjs'), + ]); + + assert.match(stderr, /Warning: Detected unsettled top-level await at.+never-resolve\.mjs:5/); + assert.match(stdout, /^should be output\r?\n$/); + assert.strictEqual(code, 13); + assert.strictEqual(signal, null); + }); + + it('top-level await of a never-settling load without warning', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ '--no-warnings', '--experimental-loader', @@ -79,6 +92,18 @@ describe('Loader hooks', { concurrency: true }, () => { assert.strictEqual(signal, null); }); + it('top-level await of a never-settling load with warning', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--experimental-loader', + fixtures.fileURL('es-module-loaders/never-settling-resolve-step/loader.mjs'), + fixtures.path('es-module-loaders/never-settling-resolve-step/never-load.mjs'), + ]); + + assert.match(stderr, /Warning: Detected unsettled top-level await at.+never-load\.mjs:5/); + assert.match(stdout, /^should be output\r?\n$/); + assert.strictEqual(code, 13); + assert.strictEqual(signal, null); + }); it('top-level await of a race of never-settling hooks', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ diff --git a/test/es-module/test-esm-tla-unfinished.mjs b/test/es-module/test-esm-tla-unfinished.mjs index 48bc4d77f42b4e..24c761042def51 100644 --- a/test/es-module/test-esm-tla-unfinished.mjs +++ b/test/es-module/test-esm-tla-unfinished.mjs @@ -6,18 +6,30 @@ import { describe, it } from 'node:test'; const commonArgs = [ - '--no-warnings', '--input-type=module', '--eval', ]; describe('ESM: unsettled and rejected promises', { concurrency: true }, () => { - it('should exit for an unsettled TLA promise via --eval', async () => { + it('should exit for an unsettled TLA promise via --eval with a warning', async () => { const { code, stderr, stdout } = await spawnPromisified(execPath, [ ...commonArgs, 'await new Promise(() => {})', ]); + assert.match(stderr, /Warning: Detected unsettled top-level await at.+\[eval1\]:1/); + assert.match(stderr, /await new Promise/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 13); + }); + + it('should exit for an unsettled TLA promise via --eval without warning', async () => { + const { code, stderr, stdout } = await spawnPromisified(execPath, [ + '--no-warnings', + ...commonArgs, + 'await new Promise(() => {})', + ]); + assert.strictEqual(stderr, ''); assert.strictEqual(stdout, ''); assert.strictEqual(code, 13); @@ -59,7 +71,18 @@ describe('ESM: unsettled and rejected promises', { concurrency: true }, () => { assert.strictEqual(code, 1); }); - it('should exit for an unsettled TLA promise via stdin', async () => { + it('should exit for an unsettled TLA promise with warning', async () => { + const { code, stderr, stdout } = await spawnPromisified(execPath, [ + fixtures.path('es-modules/tla/unresolved.mjs'), + ]); + + assert.match(stderr, /Warning: Detected unsettled top-level await at.+unresolved\.mjs:1/); + assert.match(stderr, /await new Promise/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 13); + }); + + it('should exit for an unsettled TLA promise without warning', async () => { const { code, stderr, stdout } = await spawnPromisified(execPath, [ '--no-warnings', fixtures.path('es-modules/tla/unresolved.mjs'), @@ -115,6 +138,17 @@ describe('ESM: unsettled and rejected promises', { concurrency: true }, () => { }); it('should be unaffected by `process.exit()` in worker thread', async () => { + const { code, stderr, stdout } = await spawnPromisified(execPath, [ + fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs'), + ]); + + assert.match(stderr, /Warning: Detected unsettled top-level await at.+with-worker-process-exit\.mjs:5/); + assert.match(stderr, /await new Promise/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 13); + }); + + it('should be unaffected by `process.exit()` in worker thread without warning', async () => { const { code, stderr, stdout } = await spawnPromisified(execPath, [ '--no-warnings', fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs'), From a3b96bb2adea7d2b4d956d8fecf8862beec88cfa Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 7 Mar 2024 16:12:06 +0100 Subject: [PATCH 4/5] fixup! module: print location of unsettled top-level await in entry points --- lib/internal/modules/esm/module_job.js | 2 +- lib/internal/modules/run_main.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index c55398304b480d..108a8138443de0 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -5,7 +5,6 @@ const { ArrayPrototypePush, ArrayPrototypeSome, FunctionPrototype, - globalThis, ObjectSetPrototypeOf, PromiseResolve, PromisePrototypeThen, @@ -18,6 +17,7 @@ const { StringPrototypeIncludes, StringPrototypeSplit, StringPrototypeStartsWith, + globalThis, } = primordials; const { ModuleWrap } = internalBinding('module_wrap'); diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index a0f43e90b43554..b134a54cfec92a 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -1,8 +1,8 @@ 'use strict'; const { - globalThis, StringPrototypeEndsWith, + globalThis, } = primordials; const { containsModuleSyntax } = internalBinding('contextify'); From dc1e09df555a10853cf83795f1ddc761030be42b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 8 Mar 2024 00:08:43 +0100 Subject: [PATCH 5/5] fixup! module: refactor ESM loader initialization and entry point handling --- .github/CODEOWNERS | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index e4241de7d42e75..e1fe4d0add3daf 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -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