From 148ebd7d147d49c204092f53365ecc2b05d5b146 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 18 Jun 2026 16:20:17 +0200 Subject: [PATCH] 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 --- 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 | 19 +- 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 | 28 +++ ...test-require-module-tla-print-execution.js | 15 +- .../test-require-module-tla-print-nested.js | 30 +++ .../test-require-module-tla-print-preload.js | 31 +++ .../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, 518 insertions(+), 109 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 ef5734822bf877..4d95af1b886cd6 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1356,11 +1356,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 038cf9d1302665..4d502575c9622d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -48,6 +48,7 @@ const { StringPrototypeEndsWith, StringPrototypeIncludes, StringPrototypeIndexOf, + StringPrototypeRepeat, StringPrototypeSlice, StringPrototypeSplit, StringPrototypeStartsWith, @@ -1712,15 +1713,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 219618da8a8a93..bb466d0b68d5cb 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, @@ -1575,17 +1576,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 b274b9c00a0466..1a6622140382ba 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, @@ -293,7 +292,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() }; @@ -321,6 +320,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.`; @@ -371,8 +373,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 a09af9ea990b37..ee055c95d6d5c7 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, @@ -128,6 +131,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'); @@ -186,6 +260,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) @@ -394,6 +526,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) { @@ -403,22 +537,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 @@ -514,9 +644,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); @@ -532,23 +669,19 @@ class ModuleJobSync extends ModuleJobBase { runSync(parent) { debug('ModuleJobSync.runSync()', this.module); assert(this.shouldRunModule(this.phase)); + // 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 0ea07739c17018..37b289fae60dee 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -145,7 +145,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 f977bfaf57498f..7e453171026757 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -362,6 +362,15 @@ function compileSourceTextModule(url, source, type, context = kEmptyObject) { 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 839ce9af4bb678..62b8a743942b7a 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -2,6 +2,7 @@ const { ArrayPrototypeForEach, + ArrayPrototypePush, ObjectDefineProperty, ObjectFreeze, ObjectPrototypeHasOwnProperty, @@ -30,7 +31,11 @@ const { getOptionValue } = require('internal/options'); const { setOwnProperty, getLazy } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const { emitWarningSync } = require('internal/process/warning'); - +const { + privateSymbols: { + module_first_parent_private_symbol: kFirstModuleParent, + }, +} = internalBinding('util'); const lazyTmpdir = getLazy(() => require('os').tmpdir()); const { join } = path; @@ -513,6 +518,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, @@ -544,4 +560,5 @@ module.exports = { _hasStartedUserESMExecution = true; }, urlToFilename, + getRequireStack, }; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 4f47648f15135f..38ac15b337f366 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -940,23 +940,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 ed8e7eed9e3d9b..62cfba88f00d43 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -121,7 +121,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) \ @@ -268,28 +267,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 7301ada71067b9..d061b7d1c3e996 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -954,6 +954,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.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); @@ -1011,6 +1026,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 d98b012c349aa1..5c707269cc2a4f 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 cd9f1b972c63e5..dab795dbdaa406 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 00000000000000..287d2a292453f0 --- /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 583cd7cd0c95db..0b73af5ad35235 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 00000000000000..1a42de21721162 --- /dev/null +++ b/test/es-module/test-require-module-tla-print-arrow.js @@ -0,0 +1,28 @@ +'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) { + common.expectRequiredTLAError(output); + // Indented await: the caret is aligned under the await. + assert.ok(output.includes(' await Promise.resolve(ready);\n ^'), output); + // for-await-of: the caret points at the for keyword. + assert.ok(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 d831856fe676dd..9a6b4277509707 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,14 @@ const fixtures = require('../common/fixtures'); signal: null, status: 1, stderr(output) { - assert.match(output, /I am executed/); + 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.match(output, /tla\/execution\.mjs:3/); + assert.ok(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 00000000000000..c1dedb45d83449 --- /dev/null +++ b/test/es-module/test-require-module-tla-print-nested.js @@ -0,0 +1,30 @@ +'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) { + common.expectRequiredTLAError(output); + // The top-level await lives in a transitive dependency (a.mjs). + assert.match(output, /tla\/a\.mjs:3/); + assert.ok(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 00000000000000..87da00b6375f0f --- /dev/null +++ b/test/es-module/test-require-module-tla-print-preload.js @@ -0,0 +1,31 @@ +'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) { + // 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.match(output, /tla\/execution\.mjs:3/); + assert.ok(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 00000000000000..41b44376fdc3e6 --- /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 0c1f8de2c307f6..07e8da183eca64 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 f35bb68b7dc180..1545ab446819fa 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 35cf12c446129b..2226b2ebba436a 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 00000000000000..30a670d49b0a28 --- /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 00000000000000..91de17919a06f1 --- /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 00000000000000..81279b9e2c017d --- /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 00000000000000..6045e6c0c7a8eb --- /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 00000000000000..ae2a0b7c690217 --- /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 00000000000000..65f312c9d9c606 --- /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 00000000000000..728bba6259a26f --- /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 00000000000000..1bc7bf6e46edfd --- /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 00000000000000..c771d3cdf4851e --- /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 00000000000000..5e820bffa3c2ae --- /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 00000000000000..bf818b8e939838 --- /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 00000000000000..69f3fdcb1c25b5 --- /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 00000000000000..db97c273afd80e --- /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; +}