From 40eb388fc2f5ec5f511a5c6d0fe68ee55101df0b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 2 Jul 2026 11:42:32 +0200 Subject: [PATCH 1/2] esm: print required top-level await locations without evaluating Previously in order to collect the locations of the TLA, we wait until right before evalutation to ensure instantiation is completed so that we can use v8::Module::GetStalledTopLevelAwaitMessages(). Now we try to add an additioanl shortcut to the source code in the module wraps instead during compilation for modules that contain TLAs and use acron to locate the TLAs when we need to throw ERR_REQUIRE_AYNSC_MODULE, so we can do this as early as before instantiation and do not need to run the module again to collect the locations. In addition, we now collect the require stack for ERR_REQUIRE_ASYNC_MODULE too for better metadata in the errors. Signed-off-by: Joyee Cheung PR-URL: https://github.com/nodejs/node/pull/64154 Reviewed-By: Yagiz Nizipli Reviewed-By: Antoine du Hamel --- doc/api/cli.md | 9 +- lib/internal/errors.js | 28 ++- lib/internal/modules/cjs/loader.js | 12 +- lib/internal/modules/esm/loader.js | 10 +- lib/internal/modules/esm/module_job.js | 179 +++++++++++++++--- lib/internal/modules/esm/translators.js | 2 +- lib/internal/modules/esm/utils.js | 9 + lib/internal/modules/helpers.js | 20 +- src/module_wrap.cc | 20 +- src/node_errors.h | 23 --- test/common/index.js | 16 ++ .../test-require-module-cached-tla.js | 6 +- .../test-require-module-tla-execution.js | 6 +- ...-require-module-tla-instantiation-error.js | 21 ++ .../test-require-module-tla-nested.js | 3 +- .../test-require-module-tla-print-arrow.js | 29 +++ ...test-require-module-tla-print-execution.js | 16 +- .../test-require-module-tla-print-nested.js | 31 +++ .../test-require-module-tla-print-preload.js | 32 ++++ .../test-require-module-tla-print.mjs | 39 ++++ .../test-require-module-tla-rejected.js | 3 +- .../test-require-module-tla-resolved.js | 3 +- .../test-require-module-tla-unresolved.js | 3 +- test/fixtures/es-modules/tla/awaitusing.mjs | 5 + test/fixtures/es-modules/tla/preload-main.js | 1 + .../es-modules/tla/preload-main.snapshot | 15 ++ .../es-modules/tla/require-awaitusing.js | 1 + .../tla/require-awaitusing.snapshot | 17 ++ .../es-modules/tla/require-execution.snapshot | 17 ++ .../es-modules/tla/require-indented.js | 1 + .../es-modules/tla/require-indented.snapshot | 22 +++ .../es-modules/tla/require-nested-dep.js | 1 + .../fixtures/es-modules/tla/require-nested.js | 1 + .../es-modules/tla/require-nested.snapshot | 19 ++ .../es-modules/tla/tla-and-missing-export.mjs | 3 + test/fixtures/es-modules/tla/tla-indented.mjs | 9 + 36 files changed, 522 insertions(+), 110 deletions(-) create mode 100644 test/es-module/test-require-module-tla-instantiation-error.js create mode 100644 test/es-module/test-require-module-tla-print-arrow.js create mode 100644 test/es-module/test-require-module-tla-print-nested.js create mode 100644 test/es-module/test-require-module-tla-print-preload.js create mode 100644 test/es-module/test-require-module-tla-print.mjs create mode 100644 test/fixtures/es-modules/tla/awaitusing.mjs create mode 100644 test/fixtures/es-modules/tla/preload-main.js create mode 100644 test/fixtures/es-modules/tla/preload-main.snapshot create mode 100644 test/fixtures/es-modules/tla/require-awaitusing.js create mode 100644 test/fixtures/es-modules/tla/require-awaitusing.snapshot create mode 100644 test/fixtures/es-modules/tla/require-execution.snapshot create mode 100644 test/fixtures/es-modules/tla/require-indented.js create mode 100644 test/fixtures/es-modules/tla/require-indented.snapshot create mode 100644 test/fixtures/es-modules/tla/require-nested-dep.js create mode 100644 test/fixtures/es-modules/tla/require-nested.js create mode 100644 test/fixtures/es-modules/tla/require-nested.snapshot create mode 100644 test/fixtures/es-modules/tla/tla-and-missing-export.mjs create mode 100644 test/fixtures/es-modules/tla/tla-indented.mjs diff --git a/doc/api/cli.md b/doc/api/cli.md index d452ed02328d..ae0f81b5ec53 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1177,11 +1177,14 @@ resolution algorithm. added: - v22.0.0 - v20.17.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/64154 + description: Print the top-level awaits without evaluating the modules. --> -If the ES module being `require()`'d contains top-level `await`, this flag -allows Node.js to evaluate the module, try to locate the -top-level awaits, and print their location to help users find them. +If the ES module graph cannot be `require()`'d because it contains any top-level `await`, +this flag allows Node.js to locate and print their locations. ### `--experimental-quic` diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 669b5d9bef78..63e9c6a325ff 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -48,6 +48,7 @@ const { StringPrototypeEndsWith, StringPrototypeIncludes, StringPrototypeIndexOf, + StringPrototypeRepeat, StringPrototypeSlice, StringPrototypeSplit, StringPrototypeStartsWith, @@ -1692,15 +1693,26 @@ E('ERR_QUIC_STREAM_ABORTED', '%s', Error); E('ERR_QUIC_STREAM_RESET', 'The QUIC stream was reset by the peer with error code %d', Error); E('ERR_QUIC_VERSION_NEGOTIATION_ERROR', 'The QUIC session requires version negotiation', Error); -E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parentFilename) { - let message = 'require() cannot be used on an ESM ' + - 'graph with top-level await. Use import() instead. To see where the' + - ' top-level await comes from, use --experimental-print-required-tla.'; - if (parentFilename) { - message += `\n From ${parentFilename} `; +E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) { + let message = 'require() cannot be used on an ESM graph with top-level await. Use import() instead.'; + const { getOptionValue } = require('internal/options'); + if (!getOptionValue('--experimental-print-required-tla')) { + message += ' To see where the top-level await comes from, use --experimental-print-required-tla.'; } - if (filename) { - message += `\n Requiring ${filename} `; + if (parent) { + const { getRequireStack } = require('internal/modules/helpers'); + const requireStack = getRequireStack(parent); + if (requireStack.length > 0) { + message += '\nRequire stack:\n- ' + + ArrayPrototypeJoin(requireStack, '\n- '); + } + this.requireStack = requireStack; + } + if (locations && locations.length > 0) { + const { urlToFilename } = require('internal/modules/helpers'); + const frames = ArrayPrototypeMap(locations, ({ url, line, column, sourceLine }) => + `${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ', column)}^\n`); + setArrowMessage(this, ArrayPrototypeJoin(frames, '\n')); } return message; }, Error); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 74f2c0ee90db..4bd752be19b6 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -168,6 +168,7 @@ const { setHasStartedUserCJSExecution, stripBOM, toRealPath, + getRequireStack, } = require('internal/modules/helpers'); const { convertCJSFilenameToURL, @@ -1567,17 +1568,6 @@ Module._resolveFilename = function(request, parent, isMain, options) { throw err; }; -function getRequireStack(parent) { - const requireStack = []; - for (let cursor = parent; - cursor; - // TODO(joyeecheung): it makes more sense to use kLastModuleParent here. - cursor = cursor[kFirstModuleParent]) { - ArrayPrototypePush(requireStack, cursor.filename || cursor.id); - } - return requireStack; -} - function getRequireStackMessage(request, requireStack) { let message = `Cannot find module '${request}'`; if (requireStack.length > 0) { diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index b96999d80b6a..345fef1c76a4 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -25,7 +25,6 @@ const { imported_cjs_symbol } = internalBinding('symbols'); const assert = require('internal/assert'); const { - ERR_REQUIRE_ASYNC_MODULE, ERR_REQUIRE_CYCLE_MODULE, ERR_REQUIRE_ESM, ERR_REQUIRE_ESM_RACE_CONDITION, @@ -290,7 +289,7 @@ class ModuleLoader { debug('Module status', job, status); // hasAsyncGraph is available after module been instantiated. if (status >= kInstantiated && job.module.hasAsyncGraph) { - throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename); + job.throwAsyncGraphError(parent); } if (status === kEvaluated) { return { wrap: job.module, namespace: job.module.getNamespace() }; @@ -318,6 +317,9 @@ class ModuleLoader { } if (status !== kEvaluating) { assert(status === kUninstantiated, `Unexpected module status ${status}`); + // A previous require() of the same graph may have bailed out before + // instantiation because it contains top-level await. + job.throwIfAsyncGraph(parent); throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, false); } let message = `Cannot require() ES Module ${filename} in a cycle.`; @@ -368,8 +370,8 @@ class ModuleLoader { // Otherwise the module could be imported before but the evaluation may be already // completed (e.g. the require call is lazy) so it's okay. We will return the - // job and check asynchronicity of the entire graph later, after the - // graph is instantiated. + // job and check asynchronicity of the entire graph later, before the + // graph is evaluated. } /** diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 11d5b0a3cb66..69f631c363d7 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -4,8 +4,11 @@ const { Array, ArrayPrototypeFind, ArrayPrototypeJoin, + ArrayPrototypePop, ArrayPrototypePush, + ArrayPrototypeSort, FunctionPrototype, + ObjectAssign, ObjectSetPrototypeOf, PromisePrototypeThen, PromiseResolve, @@ -127,6 +130,77 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => { } }; +/** + * @typedef {object} TopLevelAwaitLocation + * @property {string} url URL of the module containing the top-level await. + * @property {number} line 1-based line number of the top-level await. + * @property {number} column 0-based column number of the top-level await. + * @property {string} sourceLine The source line containing the top-level await. + */ + +/** + * Locate the top-level awaits in the given module by parsing the source with acron. + * @param {string} source Module source code. + * @returns {object[]} The acorn AST nodes of the top-level awaits, in source order. + */ +function findTopLevelAwait(source) { + const { Parser } = require('internal/deps/acorn/acorn/dist/acorn'); + const walk = require('internal/deps/acorn/acorn-walk/dist/walk'); + let ast; + try { + ast = Parser.parse(source, { + __proto__: null, ecmaVersion: 'latest', sourceType: 'module', locations: true, + }); + } catch { + return []; // The source is not parsable, skip. + } + // We are looking for _top-level_ await, so we don't traverse into function bodies. + const baseVisitor = ObjectAssign({ __proto__: null }, walk.base, { Function: noop }); + const found = []; + walk.simple(ast, { + __proto__: null, + AwaitExpression(node) { ArrayPrototypePush(found, node); }, + // `for await (...)` is a ForOfStatement with `await: true`, not an AwaitExpression. + ForOfStatement(node) { + if (node.await) { ArrayPrototypePush(found, node); } + }, + // `await using x = ...` is a VariableDeclaration, not an AwaitExpression. + VariableDeclaration(node) { + if (node.kind === 'await using') { ArrayPrototypePush(found, node); } + }, + }, baseVisitor); + ArrayPrototypeSort(found, (a, b) => a.start - b.start); + return found; +} + +/** + * Locate the top-level awaits in the given modules. + * @param {ModuleWrap[]} modules Modules that may contain top-level await. + * @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits. + */ +function getTopLevelAwaitLocations(modules) { + const locations = []; + for (let i = 0; i < modules.length; i++) { + const module = modules[i]; + const source = module.source; + if (typeof source !== 'string') { continue; } // Not retained during compilation. Skip. + const found = findTopLevelAwait(source); + if (found.length === 0) { continue; } + const lines = StringPrototypeSplit(source, '\n'); + for (let j = 0; j < found.length; j++) { + const { start } = found[j].loc; + ArrayPrototypePush(locations, { + __proto__: null, + url: module.url, + line: start.line, + column: start.column, + sourceLine: lines[start.line - 1], + }); + } + } + return locations; +} + class ModuleJobBase { constructor(loader, url, importAttributes, phase, isMain, inspectBrk) { assert(typeof phase === 'number'); @@ -185,6 +259,64 @@ class ModuleJobBase { return evaluationDepJobs; } + /** + * Collect the modules that contain top-level await in the linked graph of + * this job. Whether each module contains top-level await is known at + * compilation, so for a synchronously linked graph this finds asynchronous + * graphs before instantiation. + * On the (deprecated) async loader hook worker thread, linking may be asynchronous, in + * which case the subgraphs that are not synchronously linked are skipped + * and callers should still consult hasAsyncGraph after instantiation. + * @returns {ModuleWrap[]} + */ + findModulesWithTopLevelAwait() { + const found = []; + const seen = new SafeSet(); + const stack = [this]; + while (stack.length > 0) { + const job = ArrayPrototypePop(stack); + if (seen.has(job)) { continue; } + seen.add(job); + if (job.module?.hasTopLevelAwait) { + ArrayPrototypePush(found, job.module); + } + // job.linked is the array of evaluation-phase dependency jobs when the + // linking is synchronous. Skip it if it's still a promise. + if (!isPromise(job.linked)) { + for (let i = 0; i < job.linked.length; i++) { + ArrayPrototypePush(stack, job.linked[i]); + } + } + } + return found; + } + + /** + * Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that + * contains top-level await. + * @param {Module|undefined} parent CommonJS module that require()'d this, if any. + * @param {ModuleWrap[]} [modules] Modules with top-level await, when already + * collected by the caller, to avoid walking the graph again. + */ + throwAsyncGraphError(parent, modules = this.findModulesWithTopLevelAwait()) { + const locations = getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : []; + const filename = urlToFilename(this.url); + throw new ERR_REQUIRE_ASYNC_MODULE(filename, parent, locations); + } + + /** + * If the a require()'d graph contains top-level await, collect the source locations + * of the top-level awaits using source code retained during compilation and throw + * ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete. + * @param {Module|undefined} parent CommonJS module that require()'d this, if any. + */ + throwIfAsyncGraph(parent) { + const modules = this.findModulesWithTopLevelAwait(); + if (modules.length > 0) { + this.throwAsyncGraphError(parent, modules); + } + } + /** * Ensure that this ModuleJob is moving towards the required phase * (does not necessarily mean it is ready at that phase - run does that) @@ -386,6 +518,8 @@ class ModuleJob extends ModuleJobBase { debug('ModuleJob.runSync()', status, this.module); if (status === kUninstantiated) { + // TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that + // the async graph error supersedes instantiation (mismatch export) errors in the graph. // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking // fully synchronous instead. if (this.module.getModuleRequests().length === 0) { @@ -395,22 +529,18 @@ class ModuleJob extends ModuleJobBase { status = this.module.getStatus(); } if (status === kInstantiated || status === kErrored) { - const filename = urlToFilename(this.url); - const parentFilename = urlToFilename(parent?.filename); - if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) { - throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename); + if (this.module.hasAsyncGraph) { + this.throwAsyncGraphError(parent); } if (status === kInstantiated) { setHasStartedUserESMExecution(); - const namespace = this.module.evaluateSync(filename, parentFilename); + const namespace = this.module.evaluateSync(); return { __proto__: null, module: this.module, namespace }; } throw this.module.getError(); } else if (status === kEvaluating || status === kEvaluated) { if (this.module.hasAsyncGraph) { - const filename = urlToFilename(this.url); - const parentFilename = urlToFilename(parent?.filename); - throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename); + this.throwAsyncGraphError(parent); } // kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles. // Allow it for now, since we only need to ban ESM <-> CJS cycles which would be @@ -506,9 +636,16 @@ class ModuleJobSync extends ModuleJobBase { await this.evaluationPromise; } return { __proto__: null, module: this.module }; - } else if (status === kInstantiated) { - // The evaluation may have been canceled because instantiate() detected TLA first. - // But when it is imported again, it's fine to re-evaluate it asynchronously. + } else if (status === kInstantiated || status === kUninstantiated) { + // The require() of this (synchronously linked) module bailed out: either + // it was rejected for containing top-level await after instantiation + // (kInstantiated), or its instantiation failed and left it uninstantiated + // (kUninstantiated, e.g. a missing named export). When it's reached via async + // run() from import, finish the instantiation and evaluate it asynchronously, + // re-throwing any instantiation error. + if (status === kUninstantiated) { + this.module.instantiate(); + } const timeout = -1; const breakOnSigint = false; this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint); @@ -524,23 +661,19 @@ class ModuleJobSync extends ModuleJobBase { runSync(parent) { debug('ModuleJobSync.runSync()', this.module); assert(this.phase === kEvaluationPhase); + // TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that the + // async graph error supersedes instantiation (mismatch export) errors in the graph. // TODO(joyeecheung): add the error decoration logic from the async instantiate. this.module.instantiate(); - // If --experimental-print-required-tla is true, proceeds to evaluation even - // if it's async because we want to search for the TLA and help users locate - // them. - // TODO(joyeecheung): track the asynchroniticy using v8::Module::HasTopLevelAwait() - // and we'll be able to throw right after compilation of the modules, using acron - // to find and print the TLA. This requires the linking to be synchronous in case - // it runs into cached asynchronous modules that are not yet fetched. - const parentFilename = urlToFilename(parent?.filename); - const filename = urlToFilename(this.url); - if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) { - throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename); + // On the deprecated async loader hook worker thread, dependencies linked by an + // earlier import may not be walkable synchronously, so double-check with + // V8 now that the graph is instantiated. + if (this.module.hasAsyncGraph) { + this.throwAsyncGraphError(parent); } setHasStartedUserESMExecution(); try { - const namespace = this.module.evaluateSync(filename, parentFilename); + const namespace = this.module.evaluateSync(); return { __proto__: null, module: this.module, namespace }; } catch (e) { explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index c33314ea7828..54e15417adde 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -144,7 +144,7 @@ function loadCJSModuleWithSpecialRequire(module, source, url, filename, isMain, // On the main thread, the authentic require() is used instead (fixed by #60380). const request = { specifier, attributes: importAttributes, phase: kEvaluationPhase, __proto__: null }; const job = cascadedLoader.getOrCreateModuleJob(url, request, kRequireInImportedCJS); - job.runSync(); + job.runSync(module); let mod = cjsCache.get(job.url); assert(job.module, `Imported CJS module ${url} failed to load module ${job.url} using require() due to race condition`); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 5019477c55e5..e23539ddcc23 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -326,6 +326,15 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb wrap.isMain = true; } + // Add an extra reference to the source of modules containing top-level await so that if the + // module ends up being require()'d, we can parse the location of the top-level awaits to print + // better errors. There will be other references to the same source in the module in V8 so this + // only serves as a shortcut. + if (wrap.hasTopLevelAwait && + getOptionValue('--experimental-print-required-tla')) { + wrap.source = source; + } + // Cache the source map for the module if present. if (wrap.sourceMapURL) { maybeCacheSourceMap(url, source, wrap, false, wrap.sourceURL, wrap.sourceMapURL); diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 87af4da02944..c3bc10fe1aab 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -2,6 +2,7 @@ const { ArrayPrototypeForEach, + ArrayPrototypePush, ObjectDefineProperty, ObjectFreeze, ObjectPrototypeHasOwnProperty, @@ -28,10 +29,13 @@ const assert = require('internal/assert'); const { getOptionValue } = require('internal/options'); const { setOwnProperty, getLazy } = require('internal/util'); const { inspect } = require('internal/util/inspect'); - const lazyTmpdir = getLazy(() => require('os').tmpdir()); const { join } = path; - +const { + privateSymbols: { + module_first_parent_private_symbol: kFirstModuleParent, + }, +} = internalBinding('util'); const { canParse: URLCanParse } = internalBinding('url'); const { enableCompileCache: _enableCompileCache, @@ -475,6 +479,17 @@ function getCompileCacheDir() { return _getCompileCacheDir() || undefined; } +function getRequireStack(parent) { + const requireStack = []; + for (let cursor = parent; + cursor; + // TODO(joyeecheung): it makes more sense to use kLastModuleParent here. + cursor = cursor[kFirstModuleParent]) { + ArrayPrototypePush(requireStack, cursor.filename || cursor.id); + } + return requireStack; +} + module.exports = { addBuiltinLibsToObject, assertBufferSource, @@ -505,4 +520,5 @@ module.exports = { _hasStartedUserESMExecution = true; }, urlToFilename, + getRequireStack, }; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 5703590b5ee2..bc1ce3ad3c2d 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -906,23 +906,9 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo& args) { return; } - if (obj->HasAsyncGraph()) { - CHECK(env->options()->print_required_tla); - auto stalled_messages = - std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate)); - if (stalled_messages.size() != 0) { - for (auto& message : stalled_messages) { - std::string reason = "Error: unexpected top-level await at "; - std::string info = - FormatErrorMessage(isolate, context, "", message, true); - reason += info; - FPrintF(stderr, "%s\n", reason); - } - } - THROW_ERR_REQUIRE_ASYNC_MODULE(env, args[0], args[1]); - return; - } - + // Graphs with top-level await are rejected by the caller before evaluation + // starts, so the promise must have been settled synchronously. + CHECK(!obj->HasAsyncGraph()); CHECK_EQ(promise->State(), Promise::PromiseState::kFulfilled); args.GetReturnValue().Set(module->GetModuleNamespace()); diff --git a/src/node_errors.h b/src/node_errors.h index e4c6154aab77..3041a07995c9 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -118,7 +118,6 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details); V(ERR_OPERATION_FAILED, TypeError) \ V(ERR_OPTIONS_BEFORE_BOOTSTRAPPING, Error) \ V(ERR_OUT_OF_RANGE, RangeError) \ - V(ERR_REQUIRE_ASYNC_MODULE, Error) \ V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \ V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \ V(ERR_SOURCE_PHASE_NOT_DEFINED, SyntaxError) \ @@ -264,28 +263,6 @@ inline void THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(Environment* env, env, "Script execution timed out after %dms", timeout); } -inline void THROW_ERR_REQUIRE_ASYNC_MODULE( - Environment* env, - v8::Local filename, - v8::Local parent_filename) { - static constexpr const char* prefix = - "require() cannot be used on an ESM graph with top-level await. Use " - "import() instead. To see where the top-level await comes from, use " - "--experimental-print-required-tla."; - std::string message = prefix; - if (!parent_filename.IsEmpty() && parent_filename->IsString()) { - Utf8Value utf8(env->isolate(), parent_filename); - message += "\n From "; - message += utf8.ToStringView(); - } - if (!filename.IsEmpty() && filename->IsString()) { - Utf8Value utf8(env->isolate(), filename); - message += "\n Requiring "; - message += utf8.ToStringView(); - } - THROW_ERR_REQUIRE_ASYNC_MODULE(env, message); -} - inline v8::Local ERR_BUFFER_TOO_LARGE(v8::Isolate* isolate) { char message[128]; snprintf(message, diff --git a/test/common/index.js b/test/common/index.js index 6cfebae8f499..d04f0d900965 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -917,6 +917,21 @@ function expectRequiredTLAError(err) { } } +// Extract the entries of the rendered "Require stack:" list (each shown as +// "- ") from an error message or a process output string. +function parseRequireStack(output) { + const lines = output.replace(/\r/g, '').split('\n'); + const start = lines.indexOf('Require stack:'); + if (start === -1) { + return []; + } + const stack = []; + for (let i = start + 1; i < lines.length && lines[i].startsWith('- '); i++) { + stack.push(lines[i].slice(2)); + } + return stack; +} + function sleepSync(ms) { const sab = new SharedArrayBuffer(4); const i32 = new Int32Array(sab); @@ -962,6 +977,7 @@ const common = { mustSucceed, nodeProcessAborted, PIPE, + parseRequireStack, parseTestMetadata, platformTimeout, printSkipMessage, diff --git a/test/es-module/test-require-module-cached-tla.js b/test/es-module/test-require-module-cached-tla.js index d98b012c349a..5c707269cc2a 100644 --- a/test/es-module/test-require-module-cached-tla.js +++ b/test/es-module/test-require-module-cached-tla.js @@ -8,7 +8,9 @@ const assert = require('assert'); await import('../fixtures/es-modules/tla/resolved.mjs'); assert.throws(() => { require('../fixtures/es-modules/tla/resolved.mjs'); - }, { - code: 'ERR_REQUIRE_ASYNC_MODULE', + }, (err) => { + common.expectRequiredTLAError(err); + assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); + return true; }); })().then(common.mustCall()); diff --git a/test/es-module/test-require-module-tla-execution.js b/test/es-module/test-require-module-tla-execution.js index cd9f1b972c63..dab795dbdaa4 100644 --- a/test/es-module/test-require-module-tla-execution.js +++ b/test/es-module/test-require-module-tla-execution.js @@ -17,8 +17,10 @@ const fixtures = require('../common/fixtures'); stderr(output) { assert.doesNotMatch(output, /I am executed/); common.expectRequiredTLAError(output); - assert.match(output, /From .*require-execution\.js/); - assert.match(output, /Requiring .*execution\.mjs/); + assert.deepStrictEqual( + common.parseRequireStack(output), + [fixtures.path('es-modules/tla/require-execution.js')], + ); return true; }, stdout: '', diff --git a/test/es-module/test-require-module-tla-instantiation-error.js b/test/es-module/test-require-module-tla-instantiation-error.js new file mode 100644 index 000000000000..287d2a292453 --- /dev/null +++ b/test/es-module/test-require-module-tla-instantiation-error.js @@ -0,0 +1,21 @@ +'use strict'; + +// Tests that when a require()'d graph contains both top-level await and an +// instantiation error, the instantiation error currently takes precedence. +// TODO(joyeecheung): make ERR_REQUIRE_ASYNC_MODULE take precedence. + +const common = require('../common'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +assert.throws(() => { + require(fixtures.path('es-modules/tla/tla-and-missing-export.mjs')); +}, { + name: 'SyntaxError', + message: /does not provide an export named 'doesNotExist'/, +}); + +assert.rejects(import(fixtures.fileURL('es-modules/tla/tla-and-missing-export.mjs')), { + name: 'SyntaxError', + message: /does not provide an export named 'doesNotExist'/, +}).then(common.mustCall()); diff --git a/test/es-module/test-require-module-tla-nested.js b/test/es-module/test-require-module-tla-nested.js index 583cd7cd0c95..0b73af5ad352 100644 --- a/test/es-module/test-require-module-tla-nested.js +++ b/test/es-module/test-require-module-tla-nested.js @@ -9,7 +9,6 @@ assert.throws(() => { require('../fixtures/es-modules/tla/parent.mjs'); }, (err) => { common.expectRequiredTLAError(err); - assert.match(err.message, /From .*test-require-module-tla-nested\.js/); - assert.match(err.message, /Requiring .*parent\.mjs/); + assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); return true; }); diff --git a/test/es-module/test-require-module-tla-print-arrow.js b/test/es-module/test-require-module-tla-print-arrow.js new file mode 100644 index 000000000000..2ba7c165a2bd --- /dev/null +++ b/test/es-module/test-require-module-tla-print-arrow.js @@ -0,0 +1,29 @@ +'use strict'; + +// Tests that require(esm) with --experimental-print-required-tla points the +// caret at the right column for indented top-level await and for-await-of. + +const common = require('../common'); +const assert = require('assert'); +const { spawnSyncAndExit } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); + +{ + spawnSyncAndExit(process.execPath, [ + '--experimental-print-required-tla', + fixtures.path('es-modules/tla/require-indented.js'), + ], { + signal: null, + status: 1, + stderr(output) { + output = output.replace(/\r/g, ''); + common.expectRequiredTLAError(output); + // Indented await: the caret is aligned under the await. + assert(output.includes(' await Promise.resolve(ready);\n ^'), output); + // for-await-of: the caret points at the for keyword. + assert(output.includes('for await (const x of [Promise.resolve(1)]) {\n^'), output); + return true; + }, + stdout: '', + }); +} diff --git a/test/es-module/test-require-module-tla-print-execution.js b/test/es-module/test-require-module-tla-print-execution.js index d831856fe676..68b4dc5a6557 100644 --- a/test/es-module/test-require-module-tla-print-execution.js +++ b/test/es-module/test-require-module-tla-print-execution.js @@ -1,6 +1,7 @@ 'use strict'; -// Tests that require(esm) with top-level-await throws after execution +// Tests that require(esm) with top-level-await throws before execution starts +// and attaches the location of the top-level await to the error // if --experimental-print-required-tla is enabled. const common = require('../common'); @@ -16,12 +17,15 @@ const fixtures = require('../common/fixtures'); signal: null, status: 1, stderr(output) { - assert.match(output, /I am executed/); + output = output.replace(/\r/g, ''); + assert.doesNotMatch(output, /I am executed/); common.expectRequiredTLAError(output); - assert.match(output, /Error: unexpected top-level await at.*execution\.mjs:3/); - assert.match(output, /await Promise\.resolve\('hi'\)/); - assert.match(output, /From .*require-execution\.js/); - assert.match(output, /Requiring .*execution\.mjs/); + // The location of the top-level await is shown with a caret. + assert(output.includes(`${fixtures.path('es-modules/tla/execution.mjs')}:3`), output); + assert(output.includes("await Promise.resolve('hi');\n^"), output); + // The require() chain is shown as a require stack. + assert.match(output, /Require stack:/); + assert.match(output, /require-execution\.js/); return true; }, stdout: '', diff --git a/test/es-module/test-require-module-tla-print-nested.js b/test/es-module/test-require-module-tla-print-nested.js new file mode 100644 index 000000000000..5f48c9b18e6d --- /dev/null +++ b/test/es-module/test-require-module-tla-print-nested.js @@ -0,0 +1,31 @@ +'use strict'; + +// Tests that require(esm) attaches the location of a top-level await found in +// an inner dependency of the graph if --experimental-print-required-tla is +// enabled. The entry point is a real CommonJS file that require()s the ESM. + +const common = require('../common'); +const assert = require('assert'); +const { spawnSyncAndExit } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); + +{ + spawnSyncAndExit(process.execPath, [ + '--experimental-print-required-tla', + fixtures.path('es-modules/tla/require-nested.js'), + ], { + signal: null, + status: 1, + stderr(output) { + output = output.replace(/\r/g, ''); + common.expectRequiredTLAError(output); + // The top-level await lives in a transitive dependency (a.mjs). + assert(output.includes(`${fixtures.path('es-modules/tla/a.mjs')}:3`), output); + assert(output.includes('await new Promise((resolve) => {\n^'), output); + assert.match(output, /Require stack:/); + assert.match(output, /require-nested\.js/); + return true; + }, + stdout: '', + }); +} diff --git a/test/es-module/test-require-module-tla-print-preload.js b/test/es-module/test-require-module-tla-print-preload.js new file mode 100644 index 000000000000..b53d4e89026c --- /dev/null +++ b/test/es-module/test-require-module-tla-print-preload.js @@ -0,0 +1,32 @@ +'use strict'; + +// Tests that preloading an ESM with top-level await via -r throws before +// execution starts and attaches the location of the top-level await +// if --experimental-print-required-tla is enabled. + +const common = require('../common'); +const assert = require('assert'); +const { spawnSyncAndExit } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); + +{ + spawnSyncAndExit(process.execPath, [ + '--experimental-print-required-tla', + '-r', fixtures.path('es-modules/tla/execution.mjs'), + '-e', 'console.log("main ran")', + ], { + signal: null, + status: 1, + stderr(output) { + output = output.replace(/\r/g, ''); + // The main script should not run because preloading fails first. + assert.doesNotMatch(output, /main ran/); + assert.doesNotMatch(output, /I am executed/); + common.expectRequiredTLAError(output); + assert(output.includes(`${fixtures.path('es-modules/tla/execution.mjs')}:3`), output); + assert(output.includes("await Promise.resolve('hi');\n^"), output); + return true; + }, + stdout: '', + }); +} diff --git a/test/es-module/test-require-module-tla-print.mjs b/test/es-module/test-require-module-tla-print.mjs new file mode 100644 index 000000000000..41b44376fdc3 --- /dev/null +++ b/test/es-module/test-require-module-tla-print.mjs @@ -0,0 +1,39 @@ +// Tests the output of require(esm) on a graph with top-level await when +// --experimental-print-required-tla is enabled: the location of the +// top-level await (with a caret) and the require stack. + +import '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import * as snapshot from '../common/assertSnapshot.js'; +import { describe, it } from 'node:test'; + +const flags = ['--experimental-print-required-tla']; + +describe('require(esm) with top-level await and --experimental-print-required-tla', () => { + const tests = [ + // require()'d directly from a CommonJS entry point. + { name: 'es-modules/tla/require-execution.js' }, + // require()'d through a chain of CommonJS files (multi-entry require stack), + // with the top-level await in a transitive ES module dependency. + { name: 'es-modules/tla/require-nested.js' }, + // Indented top-level await and for-await-of, to check caret alignment. + { name: 'es-modules/tla/require-indented.js' }, + // `await using` declaration, which is top-level await without an + // AwaitExpression or for-await-of node. + { name: 'es-modules/tla/require-awaitusing.js' }, + // Preloaded via -r, so the require stack comes from internal/preload. + { + name: 'es-modules/tla/preload-main.js', + flags: ['-r', fixtures.path('es-modules/tla/execution.mjs')], + }, + ]; + for (const { name, flags: extraFlags = [] } of tests) { + it(name, async () => { + await snapshot.spawnAndAssert( + fixtures.path(name), + snapshot.defaultTransform, + { flags: [...flags, ...extraFlags] }, + ); + }); + } +}); diff --git a/test/es-module/test-require-module-tla-rejected.js b/test/es-module/test-require-module-tla-rejected.js index 0c1f8de2c307..07e8da183eca 100644 --- a/test/es-module/test-require-module-tla-rejected.js +++ b/test/es-module/test-require-module-tla-rejected.js @@ -9,7 +9,6 @@ assert.throws(() => { require('../fixtures/es-modules/tla/rejected.mjs'); }, (err) => { common.expectRequiredTLAError(err); - assert.match(err.message, /From .*test-require-module-tla-rejected\.js/); - assert.match(err.message, /Requiring .*rejected\.mjs/); + assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); return true; }); diff --git a/test/es-module/test-require-module-tla-resolved.js b/test/es-module/test-require-module-tla-resolved.js index f35bb68b7dc1..1545ab446819 100644 --- a/test/es-module/test-require-module-tla-resolved.js +++ b/test/es-module/test-require-module-tla-resolved.js @@ -9,7 +9,6 @@ assert.throws(() => { require('../fixtures/es-modules/tla/resolved.mjs'); }, (err) => { common.expectRequiredTLAError(err); - assert.match(err.message, /From .*test-require-module-tla-resolved\.js/); - assert.match(err.message, /Requiring .*resolved\.mjs/); + assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); return true; }); diff --git a/test/es-module/test-require-module-tla-unresolved.js b/test/es-module/test-require-module-tla-unresolved.js index 35cf12c44612..2226b2ebba43 100644 --- a/test/es-module/test-require-module-tla-unresolved.js +++ b/test/es-module/test-require-module-tla-unresolved.js @@ -9,7 +9,6 @@ assert.throws(() => { require('../fixtures/es-modules/tla/unresolved.mjs'); }, (err) => { common.expectRequiredTLAError(err); - assert.match(err.message, /From .*test-require-module-tla-unresolved\.js/); - assert.match(err.message, /Requiring .*unresolved\.mjs/); + assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); return true; }); diff --git a/test/fixtures/es-modules/tla/awaitusing.mjs b/test/fixtures/es-modules/tla/awaitusing.mjs new file mode 100644 index 000000000000..30a670d49b0a --- /dev/null +++ b/test/fixtures/es-modules/tla/awaitusing.mjs @@ -0,0 +1,5 @@ +export const ready = true; + +await using lock = { + async [Symbol.asyncDispose]() {}, +}; diff --git a/test/fixtures/es-modules/tla/preload-main.js b/test/fixtures/es-modules/tla/preload-main.js new file mode 100644 index 000000000000..91de17919a06 --- /dev/null +++ b/test/fixtures/es-modules/tla/preload-main.js @@ -0,0 +1 @@ +console.log('main should not run'); diff --git a/test/fixtures/es-modules/tla/preload-main.snapshot b/test/fixtures/es-modules/tla/preload-main.snapshot new file mode 100644 index 000000000000..81279b9e2c01 --- /dev/null +++ b/test/fixtures/es-modules/tla/preload-main.snapshot @@ -0,0 +1,15 @@ +/test/fixtures/es-modules/tla/execution.mjs:3 + +await Promise.resolve('hi'); +^ + +Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Require stack: +- internal/preload + at + at { + code: 'ERR_REQUIRE_ASYNC_MODULE', + requireStack: [ 'internal/preload' ] +} + +Node.js diff --git a/test/fixtures/es-modules/tla/require-awaitusing.js b/test/fixtures/es-modules/tla/require-awaitusing.js new file mode 100644 index 000000000000..6045e6c0c7a8 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-awaitusing.js @@ -0,0 +1 @@ +require('./awaitusing.mjs'); diff --git a/test/fixtures/es-modules/tla/require-awaitusing.snapshot b/test/fixtures/es-modules/tla/require-awaitusing.snapshot new file mode 100644 index 000000000000..ae2a0b7c6902 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-awaitusing.snapshot @@ -0,0 +1,17 @@ +/test/fixtures/es-modules/tla/awaitusing.mjs:3 + +await using lock = { +^ + +Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Require stack: +- /test/fixtures/es-modules/tla/require-awaitusing.js + at + at { + code: 'ERR_REQUIRE_ASYNC_MODULE', + requireStack: [ + '/test/fixtures/es-modules/tla/require-awaitusing.js' + ] +} + +Node.js diff --git a/test/fixtures/es-modules/tla/require-execution.snapshot b/test/fixtures/es-modules/tla/require-execution.snapshot new file mode 100644 index 000000000000..65f312c9d9c6 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-execution.snapshot @@ -0,0 +1,17 @@ +/test/fixtures/es-modules/tla/execution.mjs:3 + +await Promise.resolve('hi'); +^ + +Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Require stack: +- /test/fixtures/es-modules/tla/require-execution.js + at + at { + code: 'ERR_REQUIRE_ASYNC_MODULE', + requireStack: [ + '/test/fixtures/es-modules/tla/require-execution.js' + ] +} + +Node.js diff --git a/test/fixtures/es-modules/tla/require-indented.js b/test/fixtures/es-modules/tla/require-indented.js new file mode 100644 index 000000000000..728bba6259a2 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-indented.js @@ -0,0 +1 @@ +require('./tla-indented.mjs'); diff --git a/test/fixtures/es-modules/tla/require-indented.snapshot b/test/fixtures/es-modules/tla/require-indented.snapshot new file mode 100644 index 000000000000..1bc7bf6e46ed --- /dev/null +++ b/test/fixtures/es-modules/tla/require-indented.snapshot @@ -0,0 +1,22 @@ +/test/fixtures/es-modules/tla/tla-indented.mjs:4 + + await Promise.resolve(ready); + ^ + +/test/fixtures/es-modules/tla/tla-indented.mjs:7 + +for await (const x of [Promise.resolve(1)]) { +^ + +Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Require stack: +- /test/fixtures/es-modules/tla/require-indented.js + at + at { + code: 'ERR_REQUIRE_ASYNC_MODULE', + requireStack: [ + '/test/fixtures/es-modules/tla/require-indented.js' + ] +} + +Node.js diff --git a/test/fixtures/es-modules/tla/require-nested-dep.js b/test/fixtures/es-modules/tla/require-nested-dep.js new file mode 100644 index 000000000000..c771d3cdf485 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-nested-dep.js @@ -0,0 +1 @@ +require('./parent.mjs'); diff --git a/test/fixtures/es-modules/tla/require-nested.js b/test/fixtures/es-modules/tla/require-nested.js new file mode 100644 index 000000000000..5e820bffa3c2 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-nested.js @@ -0,0 +1 @@ +require('./require-nested-dep.js'); diff --git a/test/fixtures/es-modules/tla/require-nested.snapshot b/test/fixtures/es-modules/tla/require-nested.snapshot new file mode 100644 index 000000000000..bf818b8e9398 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-nested.snapshot @@ -0,0 +1,19 @@ +/test/fixtures/es-modules/tla/a.mjs:3 + +await new Promise((resolve) => { +^ + +Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Require stack: +- /test/fixtures/es-modules/tla/require-nested-dep.js +- /test/fixtures/es-modules/tla/require-nested.js + at + at { + code: 'ERR_REQUIRE_ASYNC_MODULE', + requireStack: [ + '/test/fixtures/es-modules/tla/require-nested-dep.js', + '/test/fixtures/es-modules/tla/require-nested.js' + ] +} + +Node.js diff --git a/test/fixtures/es-modules/tla/tla-and-missing-export.mjs b/test/fixtures/es-modules/tla/tla-and-missing-export.mjs new file mode 100644 index 000000000000..69f3fdcb1c25 --- /dev/null +++ b/test/fixtures/es-modules/tla/tla-and-missing-export.mjs @@ -0,0 +1,3 @@ +import { doesNotExist } from './order.mjs'; + +await Promise.resolve(doesNotExist); diff --git a/test/fixtures/es-modules/tla/tla-indented.mjs b/test/fixtures/es-modules/tla/tla-indented.mjs new file mode 100644 index 000000000000..db97c273afd8 --- /dev/null +++ b/test/fixtures/es-modules/tla/tla-indented.mjs @@ -0,0 +1,9 @@ +export const ready = true; + +if (ready) { + await Promise.resolve(ready); +} + +for await (const x of [Promise.resolve(1)]) { + void x; +} From 54728e73fdef10a1107bbdfa3e1ece9b9567aee6 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 2 Jul 2026 16:24:19 +0200 Subject: [PATCH 2/2] esm: improve ERR_REQUIRE_ASYNC_MODULE This brings back several improvements that were reverted by mistake when landing https://redirect.github.com/nodejs/node/pull/64154 - Update the documentation about how the removal of side effects of source collection - Add non-enumerable `requireStack` and `topLevelAwaitLocations` properties to `ERR_REQUIRE_ASYNC_MODULE`, the latter is only populated when --experimental-print-required-tla is enabled - Add "Required module: " to the error message to identify the required ESM entry point regardless of whether the flag is enabled - Fix TLA caret column from 0-based to 1-based - Store module source via a private symbol instead of a public property - Use `hasAsyncGraph` (post-instantiation) in `throwIfAsyncGraph` instead of walking the graph before instantiation - Merge the require stack checking into the `common.expectRequiredTLAError` helper. - Removed tests that are made redundant by the snapshot tests Signed-off-by: Joyee Cheung PR-URL: https://github.com/nodejs/node/pull/64260 Reviewed-By: Matteo Collina Reviewed-By: Yagiz Nizipli --- doc/api/errors.md | 26 +++- doc/api/modules.md | 7 +- lib/internal/errors.js | 20 ++- lib/internal/modules/esm/loader.js | 12 +- lib/internal/modules/esm/module_job.js | 133 ++++++++---------- lib/internal/modules/esm/utils.js | 3 +- test/common/index.js | 37 +++-- .../test-require-module-cached-tla.js | 3 +- .../test-require-module-tla-error-metadata.js | 26 ++++ ...est-require-module-tla-error-snapshot.mjs} | 5 +- .../test-require-module-tla-execution.js | 16 +-- .../test-require-module-tla-nested.js | 3 +- .../test-require-module-tla-print-arrow.js | 29 ---- ...test-require-module-tla-print-execution.js | 12 +- .../test-require-module-tla-print-nested.js | 31 ---- .../test-require-module-tla-print-preload.js | 32 ----- .../test-require-module-tla-rejected.js | 3 +- .../test-require-module-tla-resolved.js | 3 +- .../test-require-module-tla-unresolved.js | 3 +- .../es-modules/tla/preload-main.snapshot | 4 +- .../tla/require-awaitusing.snapshot | 6 +- .../es-modules/tla/require-execution.snapshot | 6 +- .../es-modules/tla/require-indented.snapshot | 6 +- .../es-modules/tla/require-nested.snapshot | 7 +- 24 files changed, 190 insertions(+), 243 deletions(-) create mode 100644 test/es-module/test-require-module-tla-error-metadata.js rename test/es-module/{test-require-module-tla-print.mjs => test-require-module-tla-error-snapshot.mjs} (87%) delete mode 100644 test/es-module/test-require-module-tla-print-arrow.js delete mode 100644 test/es-module/test-require-module-tla-print-nested.js delete mode 100644 test/es-module/test-require-module-tla-print-preload.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 9cacb4ab2f00..fb9942d5298d 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2765,12 +2765,30 @@ A QUIC session failed because version negotiation is required. ### `ERR_REQUIRE_ASYNC_MODULE` -When trying to `require()` an [ES Module][], the module turns out to be asynchronous. + + +When trying to `require()` a [ES Module][], the module turns out to be asynchronous. That is, it contains top-level await. -To see where the top-level await is, use -`--experimental-print-required-tla` (this would execute the modules -before looking for the top-level awaits). +When uncaught, the flag `--experimental-print-required-tla` prints +the locations of the top-level awaits in the graph to stderr. + +This error has the following additional non-enumerable properties: + +* `requireStack` {string\[]} The chain of modules that led to the failing + `require()`, starting with the module that required the asynchronous module. +* `topLevelAwaitLocations` {Object\[]} The locations of the top-level awaits in + the graph. Only populated when `--experimental-print-required-tla` is enabled. + Each entry has the following properties: + * `url` {string} The URL of the module containing the top-level await. + * `line` {number} The 1-based line number of the top-level await. + * `column` {number} The 1-based column number of the top-level await. + * `sourceLine` {string} The source line containing the top-level await. diff --git a/doc/api/modules.md b/doc/api/modules.md index 087f0905c202..eaaacc46ae7d 100644 --- a/doc/api/modules.md +++ b/doc/api/modules.md @@ -317,10 +317,9 @@ graph it `import`s contains top-level `await`, [`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should load the asynchronous module using [`import()`][]. -If `--experimental-print-required-tla` is enabled, instead of throwing -`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the -module, try to locate the top-level awaits, and print their location to -help users fix them. +If `--experimental-print-required-tla` is enabled and the error is uncaught, +Node.js will try to locate the top-level `await`s in the `require()`'d module graph +and print the locations in the stderr. If support for loading ES modules using `require()` results in unexpected breakage, it can be disabled using `--no-require-module`. diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 63e9c6a325ff..01f05a55ae8f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1699,6 +1699,9 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) { if (!getOptionValue('--experimental-print-required-tla')) { message += ' To see where the top-level await comes from, use --experimental-print-required-tla.'; } + if (filename) { + message += `\nRequired module: ${filename}`; + } if (parent) { const { getRequireStack } = require('internal/modules/helpers'); const requireStack = getRequireStack(parent); @@ -1706,13 +1709,26 @@ E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) { message += '\nRequire stack:\n- ' + ArrayPrototypeJoin(requireStack, '\n- '); } - this.requireStack = requireStack; + ObjectDefineProperty(this, 'requireStack', { + __proto__: null, + enumerable: false, + configurable: true, + writable: true, + value: requireStack, + }); } if (locations && locations.length > 0) { const { urlToFilename } = require('internal/modules/helpers'); const frames = ArrayPrototypeMap(locations, ({ url, line, column, sourceLine }) => - `${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ', column)}^\n`); + `${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ', column - 1)}^\n`); setArrowMessage(this, ArrayPrototypeJoin(frames, '\n')); + ObjectDefineProperty(this, 'topLevelAwaitLocations', { + __proto__: null, + enumerable: false, + configurable: true, + writable: true, + value: locations, + }); } return message; }, Error); diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 345fef1c76a4..e34e6f58538f 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -288,8 +288,8 @@ class ModuleLoader { const status = job.module.getStatus(); debug('Module status', job, status); // hasAsyncGraph is available after module been instantiated. - if (status >= kInstantiated && job.module.hasAsyncGraph) { - job.throwAsyncGraphError(parent); + if (status >= kInstantiated) { + job.throwIfAsyncGraph(parent); } if (status === kEvaluated) { return { wrap: job.module, namespace: job.module.getNamespace() }; @@ -317,9 +317,11 @@ class ModuleLoader { } if (status !== kEvaluating) { assert(status === kUninstantiated, `Unexpected module status ${status}`); - // A previous require() of the same graph may have bailed out before - // instantiation because it contains top-level await. - job.throwIfAsyncGraph(parent); + // If we get here, either there's a race where the job is still being instantiated + // by an in-flight import(), or the cached module previously encountered an + // instantiation error during a prior load (e.g. due to a mismatched import). + // TODO(joyeecheung): the current check is too broad. We should attempt to + // get the potential instantiation error and throw it. throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, false); } let message = `Cannot require() ES Module ${filename} in a cycle.`; diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 69f631c363d7..812db97a4809 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -2,6 +2,7 @@ const { Array, + ArrayIsArray, ArrayPrototypeFind, ArrayPrototypeJoin, ArrayPrototypePop, @@ -14,6 +15,7 @@ const { PromiseResolve, RegExpPrototypeExec, RegExpPrototypeSymbolReplace, + RegExpPrototypeSymbolSplit, SafePromiseAllReturnArrayLike, SafePromiseAllReturnVoid, SafeSet, @@ -36,8 +38,10 @@ const { kUninstantiated, } = internalBinding('module_wrap'); const { + getPromiseDetails, privateSymbols: { entry_point_module_private_symbol, + module_source_private_symbol: kModuleSource, }, } = internalBinding('util'); /** @@ -134,12 +138,12 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => { * @typedef {object} TopLevelAwaitLocation * @property {string} url URL of the module containing the top-level await. * @property {number} line 1-based line number of the top-level await. - * @property {number} column 0-based column number of the top-level await. + * @property {number} column 1-based column number of the top-level await. * @property {string} sourceLine The source line containing the top-level await. */ /** - * Locate the top-level awaits in the given module by parsing the source with acron. + * Locate the top-level awaits in the given module by parsing the source with acorn. * @param {string} source Module source code. * @returns {object[]} The acorn AST nodes of the top-level awaits, in source order. */ @@ -173,27 +177,62 @@ function findTopLevelAwait(source) { return found; } +/** + * Collect the modules that contain top-level await in the linked graph of a job. + * @param {ModuleJobBase} root The root of the module graph to search. + * @returns {ModuleWrap[]} Modules that contain top-level await. + */ +function findModulesWithTopLevelAwait(root) { + const found = []; + const seen = new SafeSet(); + const stack = [root]; + while (stack.length > 0) { + const job = ArrayPrototypePop(stack); + if (seen.has(job)) { continue; } + seen.add(job); + if (job.module?.hasTopLevelAwait) { + ArrayPrototypePush(found, job.module); + } + let linked = job.linked; + if (isPromise(linked)) { + linked = getPromiseDetails(linked)?.[1]; + } + // If `require(esm)` comes from the deprecated async loader hook worker thread, + // linked may be pending at this point. In that case, this branch would be skipped - + // we just allow lossy reporting of TLA locations in an edge case when a deprecated + // feature is used in combination with another experimental flag. + if (ArrayIsArray(linked)) { + for (let i = 0; i < linked.length; i++) { + ArrayPrototypePush(stack, linked[i]); + } + } + } + return found; +} + /** * Locate the top-level awaits in the given modules. - * @param {ModuleWrap[]} modules Modules that may contain top-level await. + * @param {ModuleJobBase} root The root of the module graph to search. * @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits. */ -function getTopLevelAwaitLocations(modules) { +function getTopLevelAwaitLocations(root) { + const modules = findModulesWithTopLevelAwait(root); const locations = []; for (let i = 0; i < modules.length; i++) { const module = modules[i]; - const source = module.source; + const source = module[kModuleSource]; if (typeof source !== 'string') { continue; } // Not retained during compilation. Skip. const found = findTopLevelAwait(source); if (found.length === 0) { continue; } - const lines = StringPrototypeSplit(source, '\n'); + const lines = RegExpPrototypeSymbolSplit(/\r?\n/, source); for (let j = 0; j < found.length; j++) { const { start } = found[j].loc; ArrayPrototypePush(locations, { __proto__: null, url: module.url, line: start.line, - column: start.column, + // Acorn reports 0-based columns, convert them to 1-based to match `line`. + column: start.column + 1, sourceLine: lines[start.line - 1], }); } @@ -260,61 +299,18 @@ class ModuleJobBase { } /** - * Collect the modules that contain top-level await in the linked graph of - * this job. Whether each module contains top-level await is known at - * compilation, so for a synchronously linked graph this finds asynchronous - * graphs before instantiation. - * On the (deprecated) async loader hook worker thread, linking may be asynchronous, in - * which case the subgraphs that are not synchronously linked are skipped - * and callers should still consult hasAsyncGraph after instantiation. - * @returns {ModuleWrap[]} - */ - findModulesWithTopLevelAwait() { - const found = []; - const seen = new SafeSet(); - const stack = [this]; - while (stack.length > 0) { - const job = ArrayPrototypePop(stack); - if (seen.has(job)) { continue; } - seen.add(job); - if (job.module?.hasTopLevelAwait) { - ArrayPrototypePush(found, job.module); - } - // job.linked is the array of evaluation-phase dependency jobs when the - // linking is synchronous. Skip it if it's still a promise. - if (!isPromise(job.linked)) { - for (let i = 0; i < job.linked.length; i++) { - ArrayPrototypePush(stack, job.linked[i]); - } - } - } - return found; - } - - /** - * Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that - * contains top-level await. - * @param {Module|undefined} parent CommonJS module that require()'d this, if any. - * @param {ModuleWrap[]} [modules] Modules with top-level await, when already - * collected by the caller, to avoid walking the graph again. - */ - throwAsyncGraphError(parent, modules = this.findModulesWithTopLevelAwait()) { - const locations = getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : []; - const filename = urlToFilename(this.url); - throw new ERR_REQUIRE_ASYNC_MODULE(filename, parent, locations); - } - - /** - * If the a require()'d graph contains top-level await, collect the source locations + * If the require()'d graph contains top-level await, collect the source locations * of the top-level awaits using source code retained during compilation and throw - * ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete. + * ERR_REQUIRE_ASYNC_MODULE. The module must be at least instantiated. * @param {Module|undefined} parent CommonJS module that require()'d this, if any. */ throwIfAsyncGraph(parent) { - const modules = this.findModulesWithTopLevelAwait(); - if (modules.length > 0) { - this.throwAsyncGraphError(parent, modules); + if (!this.module.hasAsyncGraph) { + return; } + const locations = getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(this) : []; + const filename = urlToFilename(this.url); + throw new ERR_REQUIRE_ASYNC_MODULE(filename, parent, locations); } /** @@ -529,9 +525,7 @@ class ModuleJob extends ModuleJobBase { status = this.module.getStatus(); } if (status === kInstantiated || status === kErrored) { - if (this.module.hasAsyncGraph) { - this.throwAsyncGraphError(parent); - } + this.throwIfAsyncGraph(parent); if (status === kInstantiated) { setHasStartedUserESMExecution(); const namespace = this.module.evaluateSync(); @@ -539,9 +533,7 @@ class ModuleJob extends ModuleJobBase { } throw this.module.getError(); } else if (status === kEvaluating || status === kEvaluated) { - if (this.module.hasAsyncGraph) { - this.throwAsyncGraphError(parent); - } + this.throwIfAsyncGraph(parent); // kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles. // Allow it for now, since we only need to ban ESM <-> CJS cycles which would be // detected earlier during the linking phase, though the CJS handling in the ESM @@ -637,12 +629,11 @@ class ModuleJobSync extends ModuleJobBase { } return { __proto__: null, module: this.module }; } else if (status === kInstantiated || status === kUninstantiated) { - // The require() of this (synchronously linked) module bailed out: either - // it was rejected for containing top-level await after instantiation - // (kInstantiated), or its instantiation failed and left it uninstantiated - // (kUninstantiated, e.g. a missing named export). When it's reached via async - // run() from import, finish the instantiation and evaluate it asynchronously, - // re-throwing any instantiation error. + // If we get here, the module was initially required and is now being imported. + // The require() module failed either because the graph has TLA (kInstantiated), + // or instantiation failed (kUninstantiated, e.g. missing named export). + // Try finishing the instantiation - if it succeeds, proceed to evaluation, + // otherwise the branch below re-throw any instantiation error. if (status === kUninstantiated) { this.module.instantiate(); } @@ -668,9 +659,7 @@ class ModuleJobSync extends ModuleJobBase { // On the deprecated async loader hook worker thread, dependencies linked by an // earlier import may not be walkable synchronously, so double-check with // V8 now that the graph is instantiated. - if (this.module.hasAsyncGraph) { - this.throwAsyncGraphError(parent); - } + this.throwIfAsyncGraph(parent); setHasStartedUserESMExecution(); try { const namespace = this.module.evaluateSync(); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index e23539ddcc23..87190b719095 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -11,6 +11,7 @@ const { const { privateSymbols: { host_defined_option_symbol, + module_source_private_symbol: kModuleSource, }, } = internalBinding('util'); const { @@ -332,7 +333,7 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb // only serves as a shortcut. if (wrap.hasTopLevelAwait && getOptionValue('--experimental-print-required-tla')) { - wrap.source = source; + wrap[kModuleSource] = source; } // Cache the source map for the module if present. diff --git a/test/common/index.js b/test/common/index.js index d04f0d900965..cbf626044659 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -906,30 +906,37 @@ function expectRequiredModule(mod, expectation, checkESModule = true) { assert.deepStrictEqual(clone, { ...expectation }); } -function expectRequiredTLAError(err) { - const message = /require\(\) cannot be used on an ESM graph with top-level await/; - if (typeof err === 'string') { - assert.match(err, /ERR_REQUIRE_ASYNC_MODULE/); - assert.match(err, message); - } else { - assert.strictEqual(err.code, 'ERR_REQUIRE_ASYNC_MODULE'); - assert.match(err.message, message); - } -} - // Extract the entries of the rendered "Require stack:" list (each shown as // "- ") from an error message or a process output string. -function parseRequireStack(output) { +function expectRequireStack(output, expected) { const lines = output.replace(/\r/g, '').split('\n'); const start = lines.indexOf('Require stack:'); if (start === -1) { - return []; + assert.deepStrictEqual([], expected); + return; } const stack = []; for (let i = start + 1; i < lines.length && lines[i].startsWith('- '); i++) { stack.push(lines[i].slice(2)); } - return stack; + assert.deepStrictEqual(stack, expected); +} + +function expectRequiredTLAError(err, stack) { + const message = /require\(\) cannot be used on an ESM graph with top-level await/; + if (typeof err === 'string') { + assert.match(err, /ERR_REQUIRE_ASYNC_MODULE/); + assert.match(err, message); + if (stack) { + expectRequireStack(err, stack); + } + } else { + assert.strictEqual(err.code, 'ERR_REQUIRE_ASYNC_MODULE'); + assert.match(err.message, message); + if (stack) { + assert.deepStrictEqual(err.requireStack, stack); + } + } } function sleepSync(ms) { @@ -977,7 +984,7 @@ const common = { mustSucceed, nodeProcessAborted, PIPE, - parseRequireStack, + expectRequireStack, parseTestMetadata, platformTimeout, printSkipMessage, diff --git a/test/es-module/test-require-module-cached-tla.js b/test/es-module/test-require-module-cached-tla.js index 5c707269cc2a..ee99bfa22497 100644 --- a/test/es-module/test-require-module-cached-tla.js +++ b/test/es-module/test-require-module-cached-tla.js @@ -9,8 +9,7 @@ const assert = require('assert'); assert.throws(() => { require('../fixtures/es-modules/tla/resolved.mjs'); }, (err) => { - common.expectRequiredTLAError(err); - assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); + common.expectRequiredTLAError(err, [__filename]); return true; }); })().then(common.mustCall()); diff --git a/test/es-module/test-require-module-tla-error-metadata.js b/test/es-module/test-require-module-tla-error-metadata.js new file mode 100644 index 000000000000..fc93c6efa947 --- /dev/null +++ b/test/es-module/test-require-module-tla-error-metadata.js @@ -0,0 +1,26 @@ +// Flags: --experimental-print-required-tla +'use strict'; + +// Tests that ERR_REQUIRE_ASYNC_MODULE carries metadata about the failure +// when --experimental-print-required-tla is enabled. + +require('../common'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +const entrypoint = fixtures.path('es-modules/tla/require-indented.js'); +const url = fixtures.fileURL('es-modules/tla/tla-indented.mjs').href; + +assert.throws(() => { + require(entrypoint); +}, (err) => { + assert.strictEqual(err.code, 'ERR_REQUIRE_ASYNC_MODULE'); + assert.deepStrictEqual(err.requireStack, [entrypoint, __filename]); + assert.deepStrictEqual(err.topLevelAwaitLocations, [ + { __proto__: null, url, line: 4, column: 3, sourceLine: ' await Promise.resolve(ready);' }, + { __proto__: null, url, line: 7, column: 1, sourceLine: 'for await (const x of [Promise.resolve(1)]) {' }, + ]); + // The properties are not enumerable so they do not show up in the inspected error. + assert.deepStrictEqual(Object.keys(err), ['code']); + return true; +}); diff --git a/test/es-module/test-require-module-tla-print.mjs b/test/es-module/test-require-module-tla-error-snapshot.mjs similarity index 87% rename from test/es-module/test-require-module-tla-print.mjs rename to test/es-module/test-require-module-tla-error-snapshot.mjs index 41b44376fdc3..bba882437263 100644 --- a/test/es-module/test-require-module-tla-print.mjs +++ b/test/es-module/test-require-module-tla-error-snapshot.mjs @@ -1,6 +1,5 @@ -// Tests the output of require(esm) on a graph with top-level await when -// --experimental-print-required-tla is enabled: the location of the -// top-level await (with a caret) and the require stack. +// Snapshot tests for the output of require(esm) on a graph with top-level await when +// --experimental-print-required-tla is enabled. import '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; diff --git a/test/es-module/test-require-module-tla-execution.js b/test/es-module/test-require-module-tla-execution.js index dab795dbdaa4..688691103fda 100644 --- a/test/es-module/test-require-module-tla-execution.js +++ b/test/es-module/test-require-module-tla-execution.js @@ -1,7 +1,7 @@ 'use strict'; -// Tests that require(esm) with top-level-await throws before execution starts -// if --experimental-print-required-tla is not enabled. +// Tests the output of require(esm) with top-level-await when --experimental-print-required-tla +// is NOT enabled. const common = require('../common'); const assert = require('assert'); @@ -9,18 +9,14 @@ const { spawnSyncAndExit } = require('../common/child_process'); const fixtures = require('../common/fixtures'); { - spawnSyncAndExit(process.execPath, [ - fixtures.path('es-modules/tla/require-execution.js'), - ], { + const filename = fixtures.path('es-modules/tla/require-execution.js'); + spawnSyncAndExit(process.execPath, [ filename ], { signal: null, status: 1, stderr(output) { + assert.match(output, /To see where the top-level await comes from, use --experimental-print-required-tla/); assert.doesNotMatch(output, /I am executed/); - common.expectRequiredTLAError(output); - assert.deepStrictEqual( - common.parseRequireStack(output), - [fixtures.path('es-modules/tla/require-execution.js')], - ); + common.expectRequiredTLAError(output, [filename]); return true; }, stdout: '', diff --git a/test/es-module/test-require-module-tla-nested.js b/test/es-module/test-require-module-tla-nested.js index 0b73af5ad352..879e1ceab6bf 100644 --- a/test/es-module/test-require-module-tla-nested.js +++ b/test/es-module/test-require-module-tla-nested.js @@ -8,7 +8,6 @@ const assert = require('assert'); assert.throws(() => { require('../fixtures/es-modules/tla/parent.mjs'); }, (err) => { - common.expectRequiredTLAError(err); - assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); + common.expectRequiredTLAError(err, [__filename]); return true; }); diff --git a/test/es-module/test-require-module-tla-print-arrow.js b/test/es-module/test-require-module-tla-print-arrow.js deleted file mode 100644 index 2ba7c165a2bd..000000000000 --- a/test/es-module/test-require-module-tla-print-arrow.js +++ /dev/null @@ -1,29 +0,0 @@ -'use strict'; - -// Tests that require(esm) with --experimental-print-required-tla points the -// caret at the right column for indented top-level await and for-await-of. - -const common = require('../common'); -const assert = require('assert'); -const { spawnSyncAndExit } = require('../common/child_process'); -const fixtures = require('../common/fixtures'); - -{ - spawnSyncAndExit(process.execPath, [ - '--experimental-print-required-tla', - fixtures.path('es-modules/tla/require-indented.js'), - ], { - signal: null, - status: 1, - stderr(output) { - output = output.replace(/\r/g, ''); - common.expectRequiredTLAError(output); - // Indented await: the caret is aligned under the await. - assert(output.includes(' await Promise.resolve(ready);\n ^'), output); - // for-await-of: the caret points at the for keyword. - assert(output.includes('for await (const x of [Promise.resolve(1)]) {\n^'), output); - return true; - }, - stdout: '', - }); -} diff --git a/test/es-module/test-require-module-tla-print-execution.js b/test/es-module/test-require-module-tla-print-execution.js index 68b4dc5a6557..c0191e409590 100644 --- a/test/es-module/test-require-module-tla-print-execution.js +++ b/test/es-module/test-require-module-tla-print-execution.js @@ -10,22 +10,22 @@ const { spawnSyncAndExit } = require('../common/child_process'); const fixtures = require('../common/fixtures'); { + const filename = fixtures.path('es-modules/tla/require-execution.js'); spawnSyncAndExit(process.execPath, [ '--experimental-print-required-tla', - fixtures.path('es-modules/tla/require-execution.js'), + filename, ], { signal: null, status: 1, stderr(output) { output = output.replace(/\r/g, ''); assert.doesNotMatch(output, /I am executed/); - common.expectRequiredTLAError(output); + common.expectRequiredTLAError(output, [filename]); + // The immediately required module is shown regardless of the flag. + assert.match(output, /Required module: .*tla[/\\]execution\.mjs/); // The location of the top-level await is shown with a caret. - assert(output.includes(`${fixtures.path('es-modules/tla/execution.mjs')}:3`), output); + assert.match(output, /tla[/\\]execution\.mjs:3/); assert(output.includes("await Promise.resolve('hi');\n^"), output); - // The require() chain is shown as a require stack. - assert.match(output, /Require stack:/); - assert.match(output, /require-execution\.js/); return true; }, stdout: '', diff --git a/test/es-module/test-require-module-tla-print-nested.js b/test/es-module/test-require-module-tla-print-nested.js deleted file mode 100644 index 5f48c9b18e6d..000000000000 --- a/test/es-module/test-require-module-tla-print-nested.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict'; - -// Tests that require(esm) attaches the location of a top-level await found in -// an inner dependency of the graph if --experimental-print-required-tla is -// enabled. The entry point is a real CommonJS file that require()s the ESM. - -const common = require('../common'); -const assert = require('assert'); -const { spawnSyncAndExit } = require('../common/child_process'); -const fixtures = require('../common/fixtures'); - -{ - spawnSyncAndExit(process.execPath, [ - '--experimental-print-required-tla', - fixtures.path('es-modules/tla/require-nested.js'), - ], { - signal: null, - status: 1, - stderr(output) { - output = output.replace(/\r/g, ''); - common.expectRequiredTLAError(output); - // The top-level await lives in a transitive dependency (a.mjs). - assert(output.includes(`${fixtures.path('es-modules/tla/a.mjs')}:3`), output); - assert(output.includes('await new Promise((resolve) => {\n^'), output); - assert.match(output, /Require stack:/); - assert.match(output, /require-nested\.js/); - return true; - }, - stdout: '', - }); -} diff --git a/test/es-module/test-require-module-tla-print-preload.js b/test/es-module/test-require-module-tla-print-preload.js deleted file mode 100644 index b53d4e89026c..000000000000 --- a/test/es-module/test-require-module-tla-print-preload.js +++ /dev/null @@ -1,32 +0,0 @@ -'use strict'; - -// Tests that preloading an ESM with top-level await via -r throws before -// execution starts and attaches the location of the top-level await -// if --experimental-print-required-tla is enabled. - -const common = require('../common'); -const assert = require('assert'); -const { spawnSyncAndExit } = require('../common/child_process'); -const fixtures = require('../common/fixtures'); - -{ - spawnSyncAndExit(process.execPath, [ - '--experimental-print-required-tla', - '-r', fixtures.path('es-modules/tla/execution.mjs'), - '-e', 'console.log("main ran")', - ], { - signal: null, - status: 1, - stderr(output) { - output = output.replace(/\r/g, ''); - // The main script should not run because preloading fails first. - assert.doesNotMatch(output, /main ran/); - assert.doesNotMatch(output, /I am executed/); - common.expectRequiredTLAError(output); - assert(output.includes(`${fixtures.path('es-modules/tla/execution.mjs')}:3`), output); - assert(output.includes("await Promise.resolve('hi');\n^"), output); - return true; - }, - stdout: '', - }); -} diff --git a/test/es-module/test-require-module-tla-rejected.js b/test/es-module/test-require-module-tla-rejected.js index 07e8da183eca..0922cc74de97 100644 --- a/test/es-module/test-require-module-tla-rejected.js +++ b/test/es-module/test-require-module-tla-rejected.js @@ -8,7 +8,6 @@ const assert = require('assert'); assert.throws(() => { require('../fixtures/es-modules/tla/rejected.mjs'); }, (err) => { - common.expectRequiredTLAError(err); - assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); + common.expectRequiredTLAError(err, [__filename]); return true; }); diff --git a/test/es-module/test-require-module-tla-resolved.js b/test/es-module/test-require-module-tla-resolved.js index 1545ab446819..3e15e97cce30 100644 --- a/test/es-module/test-require-module-tla-resolved.js +++ b/test/es-module/test-require-module-tla-resolved.js @@ -8,7 +8,6 @@ const assert = require('assert'); assert.throws(() => { require('../fixtures/es-modules/tla/resolved.mjs'); }, (err) => { - common.expectRequiredTLAError(err); - assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); + common.expectRequiredTLAError(err, [__filename]); return true; }); diff --git a/test/es-module/test-require-module-tla-unresolved.js b/test/es-module/test-require-module-tla-unresolved.js index 2226b2ebba43..c38f4cb2c02b 100644 --- a/test/es-module/test-require-module-tla-unresolved.js +++ b/test/es-module/test-require-module-tla-unresolved.js @@ -8,7 +8,6 @@ const assert = require('assert'); assert.throws(() => { require('../fixtures/es-modules/tla/unresolved.mjs'); }, (err) => { - common.expectRequiredTLAError(err); - assert.deepStrictEqual(common.parseRequireStack(err.message), [__filename]); + common.expectRequiredTLAError(err, [__filename]); return true; }); diff --git a/test/fixtures/es-modules/tla/preload-main.snapshot b/test/fixtures/es-modules/tla/preload-main.snapshot index 81279b9e2c01..b8f0b17bcec0 100644 --- a/test/fixtures/es-modules/tla/preload-main.snapshot +++ b/test/fixtures/es-modules/tla/preload-main.snapshot @@ -4,12 +4,12 @@ await Promise.resolve('hi'); ^ Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Required module: /test/fixtures/es-modules/tla/execution.mjs Require stack: - internal/preload at at { - code: 'ERR_REQUIRE_ASYNC_MODULE', - requireStack: [ 'internal/preload' ] + code: 'ERR_REQUIRE_ASYNC_MODULE' } Node.js diff --git a/test/fixtures/es-modules/tla/require-awaitusing.snapshot b/test/fixtures/es-modules/tla/require-awaitusing.snapshot index ae2a0b7c6902..fcaa21d8580c 100644 --- a/test/fixtures/es-modules/tla/require-awaitusing.snapshot +++ b/test/fixtures/es-modules/tla/require-awaitusing.snapshot @@ -4,14 +4,12 @@ await using lock = { ^ Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Required module: /test/fixtures/es-modules/tla/awaitusing.mjs Require stack: - /test/fixtures/es-modules/tla/require-awaitusing.js at at { - code: 'ERR_REQUIRE_ASYNC_MODULE', - requireStack: [ - '/test/fixtures/es-modules/tla/require-awaitusing.js' - ] + code: 'ERR_REQUIRE_ASYNC_MODULE' } Node.js diff --git a/test/fixtures/es-modules/tla/require-execution.snapshot b/test/fixtures/es-modules/tla/require-execution.snapshot index 65f312c9d9c6..9b3f813d0152 100644 --- a/test/fixtures/es-modules/tla/require-execution.snapshot +++ b/test/fixtures/es-modules/tla/require-execution.snapshot @@ -4,14 +4,12 @@ await Promise.resolve('hi'); ^ Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Required module: /test/fixtures/es-modules/tla/execution.mjs Require stack: - /test/fixtures/es-modules/tla/require-execution.js at at { - code: 'ERR_REQUIRE_ASYNC_MODULE', - requireStack: [ - '/test/fixtures/es-modules/tla/require-execution.js' - ] + code: 'ERR_REQUIRE_ASYNC_MODULE' } Node.js diff --git a/test/fixtures/es-modules/tla/require-indented.snapshot b/test/fixtures/es-modules/tla/require-indented.snapshot index 1bc7bf6e46ed..d3b5f4fc5515 100644 --- a/test/fixtures/es-modules/tla/require-indented.snapshot +++ b/test/fixtures/es-modules/tla/require-indented.snapshot @@ -9,14 +9,12 @@ for await (const x of [Promise.resolve(1)]) { ^ Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Required module: /test/fixtures/es-modules/tla/tla-indented.mjs Require stack: - /test/fixtures/es-modules/tla/require-indented.js at at { - code: 'ERR_REQUIRE_ASYNC_MODULE', - requireStack: [ - '/test/fixtures/es-modules/tla/require-indented.js' - ] + code: 'ERR_REQUIRE_ASYNC_MODULE' } Node.js diff --git a/test/fixtures/es-modules/tla/require-nested.snapshot b/test/fixtures/es-modules/tla/require-nested.snapshot index bf818b8e9398..fe5ada075fdb 100644 --- a/test/fixtures/es-modules/tla/require-nested.snapshot +++ b/test/fixtures/es-modules/tla/require-nested.snapshot @@ -4,16 +4,13 @@ await new Promise((resolve) => { ^ Error [ERR_REQUIRE_ASYNC_MODULE]: require() cannot be used on an ESM graph with top-level await. Use import() instead. +Required module: /test/fixtures/es-modules/tla/parent.mjs Require stack: - /test/fixtures/es-modules/tla/require-nested-dep.js - /test/fixtures/es-modules/tla/require-nested.js at at { - code: 'ERR_REQUIRE_ASYNC_MODULE', - requireStack: [ - '/test/fixtures/es-modules/tla/require-nested-dep.js', - '/test/fixtures/es-modules/tla/require-nested.js' - ] + code: 'ERR_REQUIRE_ASYNC_MODULE' } Node.js