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/doc/api/errors.md b/doc/api/errors.md index 07d439da8e17ed..7a5ddcf47d1057 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2823,12 +2823,30 @@ A QUIC session failed because version negotiation is required. ### `ERR_REQUIRE_ASYNC_MODULE` + + 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 3e7e236dad1484..a31852f0b9a733 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 038cf9d1302665..03d27332c894d8 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,42 @@ 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} `; + message += `\nRequired module: ${filename}`; + } + if (parent) { + const { getRequireStack } = require('internal/modules/helpers'); + const requireStack = getRequireStack(parent); + if (requireStack.length > 0) { + message += '\nRequire stack:\n- ' + + ArrayPrototypeJoin(requireStack, '\n- '); + } + 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 - 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/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..f92e87785ce12c 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, @@ -292,8 +291,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) { - throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename); + if (status >= kInstantiated) { + job.throwIfAsyncGraph(parent); } if (status === kEvaluated) { return { wrap: job.module, namespace: job.module.getNamespace() }; @@ -321,6 +320,11 @@ class ModuleLoader { } if (status !== kEvaluating) { assert(status === kUninstantiated, `Unexpected module status ${status}`); + // 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.`; @@ -371,8 +375,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..e3d1bc360fae96 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -2,10 +2,14 @@ const { Array, + ArrayIsArray, ArrayPrototypeFind, ArrayPrototypeJoin, + ArrayPrototypePop, ArrayPrototypePush, + ArrayPrototypeSort, FunctionPrototype, + ObjectAssign, ObjectSetPrototypeOf, PromisePrototypeThen, PromiseResolve, @@ -34,8 +38,10 @@ const { kUninstantiated, } = internalBinding('module_wrap'); const { + getPromiseDetails, privateSymbols: { entry_point_module_private_symbol, + module_source_private_symbol: kModuleSource, }, } = internalBinding('util'); /** @@ -128,6 +134,112 @@ 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 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 acorn. + * @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; +} + +/** + * 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 {ModuleJobBase} root The root of the module graph to search. + * @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits. + */ +function getTopLevelAwaitLocations(root) { + const modules = findModulesWithTopLevelAwait(root); + const locations = []; + for (let i = 0; i < modules.length; i++) { + const module = modules[i]; + 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'); + for (let j = 0; j < found.length; j++) { + const { start } = found[j].loc; + ArrayPrototypePush(locations, { + __proto__: null, + url: module.url, + line: start.line, + // Acorn reports 0-based columns, convert them to 1-based to match `line`. + column: start.column + 1, + sourceLine: lines[start.line - 1], + }); + } + } + return locations; +} + class ModuleJobBase { constructor(loader, url, importAttributes, phase, isMain, inspectBrk) { assert(typeof phase === 'number'); @@ -186,6 +298,21 @@ class ModuleJobBase { return evaluationDepJobs; } + /** + * 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. The module must be at least instantiated. + * @param {Module|undefined} parent CommonJS module that require()'d this, if any. + */ + throwIfAsyncGraph(parent) { + 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); + } + /** * 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 +521,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,23 +532,15 @@ 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); - } + this.throwIfAsyncGraph(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.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 @@ -514,9 +635,15 @@ 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) { + // 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(); + } const timeout = -1; const breakOnSigint = false; this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint); @@ -532,23 +659,17 @@ 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. + this.throwIfAsyncGraph(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..9e27a2db1ebce8 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 { @@ -362,6 +363,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[kModuleSource] = 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..6886c62ddc7d80 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -943,14 +943,36 @@ function expectRequiredModule(mod, expectation, checkESModule = true) { assert.deepStrictEqual(clone, { ...expectation }); } -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 expectRequireStack(output, expected) { + const lines = output.split('\n'); + const start = lines.indexOf('Require stack:'); + if (start === -1) { + assert.deepStrictEqual([], expected); + return; + } + const stack = []; + for (let i = start + 1; i < lines.length && lines[i].startsWith('- '); i++) { + stack.push(lines[i].slice(2)); + } + 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); + } } } @@ -1011,6 +1033,7 @@ const common = { mustSucceed, nodeProcessAborted, PIPE, + 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 d98b012c349aa1..ee99bfa224977f 100644 --- a/test/es-module/test-require-module-cached-tla.js +++ b/test/es-module/test-require-module-cached-tla.js @@ -8,7 +8,8 @@ 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, [__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 00000000000000..fc93c6efa94716 --- /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-error-snapshot.mjs b/test/es-module/test-require-module-tla-error-snapshot.mjs new file mode 100644 index 00000000000000..bba882437263c2 --- /dev/null +++ b/test/es-module/test-require-module-tla-error-snapshot.mjs @@ -0,0 +1,38 @@ +// 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'; +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-execution.js b/test/es-module/test-require-module-tla-execution.js index cd9f1b972c63e5..688691103fdaa9 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,16 +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.match(output, /From .*require-execution\.js/); - assert.match(output, /Requiring .*execution\.mjs/); + common.expectRequiredTLAError(output, [filename]); 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..879e1ceab6bf1b 100644 --- a/test/es-module/test-require-module-tla-nested.js +++ b/test/es-module/test-require-module-tla-nested.js @@ -8,8 +8,6 @@ const assert = require('assert'); 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/); + common.expectRequiredTLAError(err, [__filename]); return true; }); 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..97e1b6960f9e6e 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'); @@ -9,19 +10,21 @@ 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) { - assert.match(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/); + assert.doesNotMatch(output, /I am executed/); + 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.match(output, /tla\/execution\.mjs:3/); + 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 0c1f8de2c307f6..0922cc74de975b 100644 --- a/test/es-module/test-require-module-tla-rejected.js +++ b/test/es-module/test-require-module-tla-rejected.js @@ -8,8 +8,6 @@ const assert = require('assert'); 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/); + 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 f35bb68b7dc180..3e15e97cce303d 100644 --- a/test/es-module/test-require-module-tla-resolved.js +++ b/test/es-module/test-require-module-tla-resolved.js @@ -8,8 +8,6 @@ const assert = require('assert'); 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/); + 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 35cf12c446129b..c38f4cb2c02b59 100644 --- a/test/es-module/test-require-module-tla-unresolved.js +++ b/test/es-module/test-require-module-tla-unresolved.js @@ -8,8 +8,6 @@ const assert = require('assert'); 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/); + common.expectRequiredTLAError(err, [__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..b8f0b17bcec0a7 --- /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. +Required module: /test/fixtures/es-modules/tla/execution.mjs +Require stack: +- internal/preload + at + at { + code: 'ERR_REQUIRE_ASYNC_MODULE' +} + +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..fcaa21d8580cac --- /dev/null +++ b/test/fixtures/es-modules/tla/require-awaitusing.snapshot @@ -0,0 +1,15 @@ +/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. +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' +} + +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..9b3f813d0152ff --- /dev/null +++ b/test/fixtures/es-modules/tla/require-execution.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. +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' +} + +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..d3b5f4fc551590 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-indented.snapshot @@ -0,0 +1,20 @@ +/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. +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' +} + +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..fe5ada075fdb39 --- /dev/null +++ b/test/fixtures/es-modules/tla/require-nested.snapshot @@ -0,0 +1,16 @@ +/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. +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' +} + +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; +}