Skip to content

Commit d2b02e4

Browse files
joyeecheungaduh95
authored andcommitted
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 <joyeec9h3@gmail.com> PR-URL: #64154 Backport-PR-URL: #65125 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d937c8c commit d2b02e4

36 files changed

Lines changed: 522 additions & 110 deletions

doc/api/cli.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,11 +1177,14 @@ resolution algorithm.
11771177
added:
11781178
- v22.0.0
11791179
- v20.17.0
1180+
changes:
1181+
- version: REPLACEME
1182+
pr-url: https://github.com/nodejs/node/pull/64154
1183+
description: Print the top-level awaits without evaluating the modules.
11801184
-->
11811185

1182-
If the ES module being `require()`'d contains top-level `await`, this flag
1183-
allows Node.js to evaluate the module, try to locate the
1184-
top-level awaits, and print their location to help users find them.
1186+
If the ES module graph cannot be `require()`'d because it contains any top-level `await`,
1187+
this flag allows Node.js to locate and print their locations.
11851188

11861189
### `--experimental-quic`
11871190

lib/internal/errors.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const {
4848
StringPrototypeEndsWith,
4949
StringPrototypeIncludes,
5050
StringPrototypeIndexOf,
51+
StringPrototypeRepeat,
5152
StringPrototypeSlice,
5253
StringPrototypeSplit,
5354
StringPrototypeStartsWith,
@@ -1692,15 +1693,26 @@ E('ERR_QUIC_STREAM_ABORTED', '%s', Error);
16921693
E('ERR_QUIC_STREAM_RESET',
16931694
'The QUIC stream was reset by the peer with error code %d', Error);
16941695
E('ERR_QUIC_VERSION_NEGOTIATION_ERROR', 'The QUIC session requires version negotiation', Error);
1695-
E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parentFilename) {
1696-
let message = 'require() cannot be used on an ESM ' +
1697-
'graph with top-level await. Use import() instead. To see where the' +
1698-
' top-level await comes from, use --experimental-print-required-tla.';
1699-
if (parentFilename) {
1700-
message += `\n From ${parentFilename} `;
1696+
E('ERR_REQUIRE_ASYNC_MODULE', function(filename, parent, locations) {
1697+
let message = 'require() cannot be used on an ESM graph with top-level await. Use import() instead.';
1698+
const { getOptionValue } = require('internal/options');
1699+
if (!getOptionValue('--experimental-print-required-tla')) {
1700+
message += ' To see where the top-level await comes from, use --experimental-print-required-tla.';
17011701
}
1702-
if (filename) {
1703-
message += `\n Requiring ${filename} `;
1702+
if (parent) {
1703+
const { getRequireStack } = require('internal/modules/helpers');
1704+
const requireStack = getRequireStack(parent);
1705+
if (requireStack.length > 0) {
1706+
message += '\nRequire stack:\n- ' +
1707+
ArrayPrototypeJoin(requireStack, '\n- ');
1708+
}
1709+
this.requireStack = requireStack;
1710+
}
1711+
if (locations && locations.length > 0) {
1712+
const { urlToFilename } = require('internal/modules/helpers');
1713+
const frames = ArrayPrototypeMap(locations, ({ url, line, column, sourceLine }) =>
1714+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ', column)}^\n`);
1715+
setArrowMessage(this, ArrayPrototypeJoin(frames, '\n'));
17041716
}
17051717
return message;
17061718
}, Error);

lib/internal/modules/cjs/loader.js

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ const {
168168
setHasStartedUserCJSExecution,
169169
stripBOM,
170170
toRealPath,
171+
getRequireStack,
171172
} = require('internal/modules/helpers');
172173
const {
173174
convertCJSFilenameToURL,
@@ -1567,17 +1568,6 @@ Module._resolveFilename = function(request, parent, isMain, options) {
15671568
throw err;
15681569
};
15691570

1570-
function getRequireStack(parent) {
1571-
const requireStack = [];
1572-
for (let cursor = parent;
1573-
cursor;
1574-
// TODO(joyeecheung): it makes more sense to use kLastModuleParent here.
1575-
cursor = cursor[kFirstModuleParent]) {
1576-
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
1577-
}
1578-
return requireStack;
1579-
}
1580-
15811571
function getRequireStackMessage(request, requireStack) {
15821572
let message = `Cannot find module '${request}'`;
15831573
if (requireStack.length > 0) {

lib/internal/modules/esm/loader.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const { imported_cjs_symbol } = internalBinding('symbols');
2525

2626
const assert = require('internal/assert');
2727
const {
28-
ERR_REQUIRE_ASYNC_MODULE,
2928
ERR_REQUIRE_CYCLE_MODULE,
3029
ERR_REQUIRE_ESM,
3130
ERR_REQUIRE_ESM_RACE_CONDITION,
@@ -290,7 +289,7 @@ class ModuleLoader {
290289
debug('Module status', job, status);
291290
// hasAsyncGraph is available after module been instantiated.
292291
if (status >= kInstantiated && job.module.hasAsyncGraph) {
293-
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
292+
job.throwAsyncGraphError(parent);
294293
}
295294
if (status === kEvaluated) {
296295
return { wrap: job.module, namespace: job.module.getNamespace() };
@@ -318,6 +317,9 @@ class ModuleLoader {
318317
}
319318
if (status !== kEvaluating) {
320319
assert(status === kUninstantiated, `Unexpected module status ${status}`);
320+
// A previous require() of the same graph may have bailed out before
321+
// instantiation because it contains top-level await.
322+
job.throwIfAsyncGraph(parent);
321323
throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, false);
322324
}
323325
let message = `Cannot require() ES Module ${filename} in a cycle.`;
@@ -368,8 +370,8 @@ class ModuleLoader {
368370

369371
// Otherwise the module could be imported before but the evaluation may be already
370372
// completed (e.g. the require call is lazy) so it's okay. We will return the
371-
// job and check asynchronicity of the entire graph later, after the
372-
// graph is instantiated.
373+
// job and check asynchronicity of the entire graph later, before the
374+
// graph is evaluated.
373375
}
374376

375377
/**

lib/internal/modules/esm/module_job.js

Lines changed: 156 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ const {
44
Array,
55
ArrayPrototypeFind,
66
ArrayPrototypeJoin,
7+
ArrayPrototypePop,
78
ArrayPrototypePush,
9+
ArrayPrototypeSort,
810
FunctionPrototype,
11+
ObjectAssign,
912
ObjectSetPrototypeOf,
1013
PromisePrototypeThen,
1114
PromiseResolve,
@@ -127,6 +130,77 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
127130
}
128131
};
129132

133+
/**
134+
* @typedef {object} TopLevelAwaitLocation
135+
* @property {string} url URL of the module containing the top-level await.
136+
* @property {number} line 1-based line number of the top-level await.
137+
* @property {number} column 0-based column number of the top-level await.
138+
* @property {string} sourceLine The source line containing the top-level await.
139+
*/
140+
141+
/**
142+
* Locate the top-level awaits in the given module by parsing the source with acron.
143+
* @param {string} source Module source code.
144+
* @returns {object[]} The acorn AST nodes of the top-level awaits, in source order.
145+
*/
146+
function findTopLevelAwait(source) {
147+
const { Parser } = require('internal/deps/acorn/acorn/dist/acorn');
148+
const walk = require('internal/deps/acorn/acorn-walk/dist/walk');
149+
let ast;
150+
try {
151+
ast = Parser.parse(source, {
152+
__proto__: null, ecmaVersion: 'latest', sourceType: 'module', locations: true,
153+
});
154+
} catch {
155+
return []; // The source is not parsable, skip.
156+
}
157+
// We are looking for _top-level_ await, so we don't traverse into function bodies.
158+
const baseVisitor = ObjectAssign({ __proto__: null }, walk.base, { Function: noop });
159+
const found = [];
160+
walk.simple(ast, {
161+
__proto__: null,
162+
AwaitExpression(node) { ArrayPrototypePush(found, node); },
163+
// `for await (...)` is a ForOfStatement with `await: true`, not an AwaitExpression.
164+
ForOfStatement(node) {
165+
if (node.await) { ArrayPrototypePush(found, node); }
166+
},
167+
// `await using x = ...` is a VariableDeclaration, not an AwaitExpression.
168+
VariableDeclaration(node) {
169+
if (node.kind === 'await using') { ArrayPrototypePush(found, node); }
170+
},
171+
}, baseVisitor);
172+
ArrayPrototypeSort(found, (a, b) => a.start - b.start);
173+
return found;
174+
}
175+
176+
/**
177+
* Locate the top-level awaits in the given modules.
178+
* @param {ModuleWrap[]} modules Modules that may contain top-level await.
179+
* @returns {TopLevelAwaitLocation[]} The locations of the top-level awaits.
180+
*/
181+
function getTopLevelAwaitLocations(modules) {
182+
const locations = [];
183+
for (let i = 0; i < modules.length; i++) {
184+
const module = modules[i];
185+
const source = module.source;
186+
if (typeof source !== 'string') { continue; } // Not retained during compilation. Skip.
187+
const found = findTopLevelAwait(source);
188+
if (found.length === 0) { continue; }
189+
const lines = StringPrototypeSplit(source, '\n');
190+
for (let j = 0; j < found.length; j++) {
191+
const { start } = found[j].loc;
192+
ArrayPrototypePush(locations, {
193+
__proto__: null,
194+
url: module.url,
195+
line: start.line,
196+
column: start.column,
197+
sourceLine: lines[start.line - 1],
198+
});
199+
}
200+
}
201+
return locations;
202+
}
203+
130204
class ModuleJobBase {
131205
constructor(loader, url, importAttributes, phase, isMain, inspectBrk) {
132206
assert(typeof phase === 'number');
@@ -185,6 +259,64 @@ class ModuleJobBase {
185259
return evaluationDepJobs;
186260
}
187261

262+
/**
263+
* Collect the modules that contain top-level await in the linked graph of
264+
* this job. Whether each module contains top-level await is known at
265+
* compilation, so for a synchronously linked graph this finds asynchronous
266+
* graphs before instantiation.
267+
* On the (deprecated) async loader hook worker thread, linking may be asynchronous, in
268+
* which case the subgraphs that are not synchronously linked are skipped
269+
* and callers should still consult hasAsyncGraph after instantiation.
270+
* @returns {ModuleWrap[]}
271+
*/
272+
findModulesWithTopLevelAwait() {
273+
const found = [];
274+
const seen = new SafeSet();
275+
const stack = [this];
276+
while (stack.length > 0) {
277+
const job = ArrayPrototypePop(stack);
278+
if (seen.has(job)) { continue; }
279+
seen.add(job);
280+
if (job.module?.hasTopLevelAwait) {
281+
ArrayPrototypePush(found, job.module);
282+
}
283+
// job.linked is the array of evaluation-phase dependency jobs when the
284+
// linking is synchronous. Skip it if it's still a promise.
285+
if (!isPromise(job.linked)) {
286+
for (let i = 0; i < job.linked.length; i++) {
287+
ArrayPrototypePush(stack, job.linked[i]);
288+
}
289+
}
290+
}
291+
return found;
292+
}
293+
294+
/**
295+
* Throw the ERR_REQUIRE_ASYNC_MODULE with metadata for a require()'d graph that
296+
* contains top-level await.
297+
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
298+
* @param {ModuleWrap[]} [modules] Modules with top-level await, when already
299+
* collected by the caller, to avoid walking the graph again.
300+
*/
301+
throwAsyncGraphError(parent, modules = this.findModulesWithTopLevelAwait()) {
302+
const locations = getOptionValue('--experimental-print-required-tla') ? getTopLevelAwaitLocations(modules) : [];
303+
const filename = urlToFilename(this.url);
304+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parent, locations);
305+
}
306+
307+
/**
308+
* If the a require()'d graph contains top-level await, collect the source locations
309+
* of the top-level awaits using source code retained during compilation and throw
310+
* ERR_REQUIRE_ASYNC_MODULE. This can be run before instantiation is complete.
311+
* @param {Module|undefined} parent CommonJS module that require()'d this, if any.
312+
*/
313+
throwIfAsyncGraph(parent) {
314+
const modules = this.findModulesWithTopLevelAwait();
315+
if (modules.length > 0) {
316+
this.throwAsyncGraphError(parent, modules);
317+
}
318+
}
319+
188320
/**
189321
* Ensure that this ModuleJob is moving towards the required phase
190322
* (does not necessarily mean it is ready at that phase - run does that)
@@ -386,6 +518,8 @@ class ModuleJob extends ModuleJobBase {
386518

387519
debug('ModuleJob.runSync()', status, this.module);
388520
if (status === kUninstantiated) {
521+
// TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that
522+
// the async graph error supersedes instantiation (mismatch export) errors in the graph.
389523
// FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
390524
// fully synchronous instead.
391525
if (this.module.getModuleRequests().length === 0) {
@@ -395,22 +529,18 @@ class ModuleJob extends ModuleJobBase {
395529
status = this.module.getStatus();
396530
}
397531
if (status === kInstantiated || status === kErrored) {
398-
const filename = urlToFilename(this.url);
399-
const parentFilename = urlToFilename(parent?.filename);
400-
if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
401-
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
532+
if (this.module.hasAsyncGraph) {
533+
this.throwAsyncGraphError(parent);
402534
}
403535
if (status === kInstantiated) {
404536
setHasStartedUserESMExecution();
405-
const namespace = this.module.evaluateSync(filename, parentFilename);
537+
const namespace = this.module.evaluateSync();
406538
return { __proto__: null, module: this.module, namespace };
407539
}
408540
throw this.module.getError();
409541
} else if (status === kEvaluating || status === kEvaluated) {
410542
if (this.module.hasAsyncGraph) {
411-
const filename = urlToFilename(this.url);
412-
const parentFilename = urlToFilename(parent?.filename);
413-
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
543+
this.throwAsyncGraphError(parent);
414544
}
415545
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
416546
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
@@ -506,9 +636,16 @@ class ModuleJobSync extends ModuleJobBase {
506636
await this.evaluationPromise;
507637
}
508638
return { __proto__: null, module: this.module };
509-
} else if (status === kInstantiated) {
510-
// The evaluation may have been canceled because instantiate() detected TLA first.
511-
// But when it is imported again, it's fine to re-evaluate it asynchronously.
639+
} else if (status === kInstantiated || status === kUninstantiated) {
640+
// The require() of this (synchronously linked) module bailed out: either
641+
// it was rejected for containing top-level await after instantiation
642+
// (kInstantiated), or its instantiation failed and left it uninstantiated
643+
// (kUninstantiated, e.g. a missing named export). When it's reached via async
644+
// run() from import, finish the instantiation and evaluate it asynchronously,
645+
// re-throwing any instantiation error.
646+
if (status === kUninstantiated) {
647+
this.module.instantiate();
648+
}
512649
const timeout = -1;
513650
const breakOnSigint = false;
514651
this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint);
@@ -524,23 +661,19 @@ class ModuleJobSync extends ModuleJobBase {
524661
runSync(parent) {
525662
debug('ModuleJobSync.runSync()', this.module);
526663
assert(this.phase === kEvaluationPhase);
664+
// TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that the
665+
// async graph error supersedes instantiation (mismatch export) errors in the graph.
527666
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
528667
this.module.instantiate();
529-
// If --experimental-print-required-tla is true, proceeds to evaluation even
530-
// if it's async because we want to search for the TLA and help users locate
531-
// them.
532-
// TODO(joyeecheung): track the asynchroniticy using v8::Module::HasTopLevelAwait()
533-
// and we'll be able to throw right after compilation of the modules, using acron
534-
// to find and print the TLA. This requires the linking to be synchronous in case
535-
// it runs into cached asynchronous modules that are not yet fetched.
536-
const parentFilename = urlToFilename(parent?.filename);
537-
const filename = urlToFilename(this.url);
538-
if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
539-
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
668+
// On the deprecated async loader hook worker thread, dependencies linked by an
669+
// earlier import may not be walkable synchronously, so double-check with
670+
// V8 now that the graph is instantiated.
671+
if (this.module.hasAsyncGraph) {
672+
this.throwAsyncGraphError(parent);
540673
}
541674
setHasStartedUserESMExecution();
542675
try {
543-
const namespace = this.module.evaluateSync(filename, parentFilename);
676+
const namespace = this.module.evaluateSync();
544677
return { __proto__: null, module: this.module, namespace };
545678
} catch (e) {
546679
explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait);

lib/internal/modules/esm/translators.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ function loadCJSModuleWithSpecialRequire(module, source, url, filename, isMain,
144144
// On the main thread, the authentic require() is used instead (fixed by #60380).
145145
const request = { specifier, attributes: importAttributes, phase: kEvaluationPhase, __proto__: null };
146146
const job = cascadedLoader.getOrCreateModuleJob(url, request, kRequireInImportedCJS);
147-
job.runSync();
147+
job.runSync(module);
148148
let mod = cjsCache.get(job.url);
149149
assert(job.module, `Imported CJS module ${url} failed to load module ${job.url} using require() due to race condition`);
150150

lib/internal/modules/esm/utils.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,15 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
326326
wrap.isMain = true;
327327
}
328328

329+
// Add an extra reference to the source of modules containing top-level await so that if the
330+
// module ends up being require()'d, we can parse the location of the top-level awaits to print
331+
// better errors. There will be other references to the same source in the module in V8 so this
332+
// only serves as a shortcut.
333+
if (wrap.hasTopLevelAwait &&
334+
getOptionValue('--experimental-print-required-tla')) {
335+
wrap.source = source;
336+
}
337+
329338
// Cache the source map for the module if present.
330339
if (wrap.sourceMapURL) {
331340
maybeCacheSourceMap(url, source, wrap, false, wrap.sourceURL, wrap.sourceMapURL);

0 commit comments

Comments
 (0)