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; +}