Skip to content

Commit 0cd443d

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 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 34a537c commit 0cd443d

36 files changed

Lines changed: 522 additions & 109 deletions

‎doc/api/cli.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,11 +1349,14 @@ resolution algorithm.
13491349
added:
13501350
- v22.0.0
13511351
- v20.17.0
1352+
changes:
1353+
- version: REPLACEME
1354+
pr-url: https://github.com/nodejs/node/pull/64154
1355+
description: Print the top-level awaits without evaluating the modules.
13521356
-->
13531357

1354-
If the ES module being `require()`'d contains top-level `await`, this flag
1355-
allows Node.js to evaluate the module, try to locate the
1356-
top-level awaits, and print their location to help users find them.
1358+
If the ES module graph cannot be `require()`'d because it contains any top-level `await`,
1359+
this flag allows Node.js to locate and print their locations.
13571360

13581361
### `--experimental-quic`
13591362

‎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,
@@ -1707,15 +1708,26 @@ E('ERR_QUIC_STREAM_ABORTED', '%s', Error);
17071708
E('ERR_QUIC_STREAM_RESET',
17081709
'The QUIC stream was reset by the peer with error code %d',Error);
17091710
E('ERR_QUIC_VERSION_NEGOTIATION_ERROR','The QUIC session requires version negotiation',Error);
1710-
E('ERR_REQUIRE_ASYNC_MODULE',function(filename,parentFilename){
1711-
letmessage='require() cannot be used on an ESM '+
1712-
'graph with top-level await. Use import() instead. To see where the'+
1713-
' top-level await comes from, use --experimental-print-required-tla.';
1714-
if(parentFilename){
1715-
message+=`\n From ${parentFilename} `;
1711+
E('ERR_REQUIRE_ASYNC_MODULE',function(filename,parent,locations){
1712+
letmessage='require() cannot be used on an ESM graph with top-level await. Use import() instead.';
1713+
const{ getOptionValue }=require('internal/options');
1714+
if(!getOptionValue('--experimental-print-required-tla')){
1715+
message+=' To see where the top-level await comes from, use --experimental-print-required-tla.';
17161716
}
1717-
if(filename){
1718-
message+=`\n Requiring ${filename} `;
1717+
if(parent){
1718+
const{ getRequireStack }=require('internal/modules/helpers');
1719+
constrequireStack=getRequireStack(parent);
1720+
if(requireStack.length>0){
1721+
message+='\nRequire stack:\n- '+
1722+
ArrayPrototypeJoin(requireStack,'\n- ');
1723+
}
1724+
this.requireStack=requireStack;
1725+
}
1726+
if(locations&&locations.length>0){
1727+
const{ urlToFilename }=require('internal/modules/helpers');
1728+
constframes=ArrayPrototypeMap(locations,({ url, line, column, sourceLine })=>
1729+
`${urlToFilename(url)}:${line}\n\n${sourceLine}\n${StringPrototypeRepeat(' ',column)}^\n`);
1730+
setArrowMessage(this,ArrayPrototypeJoin(frames,'\n'));
17191731
}
17201732
returnmessage;
17211733
},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,
@@ -1572,17 +1573,6 @@ Module._resolveFilename = function(request, parent, isMain, options) {
15721573
throwerr;
15731574
};
15741575

1575-
functiongetRequireStack(parent){
1576-
constrequireStack=[];
1577-
for(letcursor=parent;
1578-
cursor;
1579-
// TODO(joyeecheung): it makes more sense to use kLastModuleParent here.
1580-
cursor=cursor[kFirstModuleParent]){
1581-
ArrayPrototypePush(requireStack,cursor.filename||cursor.id);
1582-
}
1583-
returnrequireStack;
1584-
}
1585-
15861576
functiongetRequireStackMessage(request,requireStack){
15871577
letmessage=`Cannot find module '${request}'`;
15881578
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
constassert=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,
@@ -293,7 +292,7 @@ class ModuleLoader {
293292
debug('Module status',job,status);
294293
// hasAsyncGraph is available after module been instantiated.
295294
if(status>=kInstantiated&&job.module.hasAsyncGraph){
296-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parentFilename);
295+
job.throwAsyncGraphError(parent);
297296
}
298297
if(status===kEvaluated){
299298
return{wrap: job.module,namespace: job.module.getNamespace()};
@@ -321,6 +320,9 @@ class ModuleLoader {
321320
}
322321
if(status!==kEvaluating){
323322
assert(status===kUninstantiated,`Unexpected module status ${status}`);
323+
// A previous require() of the same graph may have bailed out before
324+
// instantiation because it contains top-level await.
325+
job.throwIfAsyncGraph(parent);
324326
thrownewERR_REQUIRE_ESM_RACE_CONDITION(filename,parentFilename,false);
325327
}
326328
letmessage=`Cannot require() ES Module ${filename} in a cycle.`;
@@ -371,8 +373,8 @@ class ModuleLoader {
371373

372374
// Otherwise the module could be imported before but the evaluation may be already
373375
// completed (e.g. the require call is lazy) so it's okay. We will return the
374-
// job and check asynchronicity of the entire graph later, after the
375-
// graph is instantiated.
376+
// job and check asynchronicity of the entire graph later, before the
377+
// graph is evaluated.
376378
}
377379

378380
/**

‎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,
@@ -128,6 +131,77 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
128131
}
129132
};
130133

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

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

395527
debug('ModuleJob.runSync()',status,this.module);
396528
if(status===kUninstantiated){
529+
// TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that
530+
// the async graph error supersedes instantiation (mismatch export) errors in the graph.
397531
// FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
398532
// fully synchronous instead.
399533
if(this.module.getModuleRequests().length===0){
@@ -403,22 +537,18 @@ class ModuleJob extends ModuleJobBase {
403537
status=this.module.getStatus();
404538
}
405539
if(status===kInstantiated||status===kErrored){
406-
constfilename=urlToFilename(this.url);
407-
constparentFilename=urlToFilename(parent?.filename);
408-
if(this.module.hasAsyncGraph&&!getOptionValue('--experimental-print-required-tla')){
409-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parentFilename);
540+
if(this.module.hasAsyncGraph){
541+
this.throwAsyncGraphError(parent);
410542
}
411543
if(status===kInstantiated){
412544
setHasStartedUserESMExecution();
413-
constnamespace=this.module.evaluateSync(filename,parentFilename);
545+
constnamespace=this.module.evaluateSync();
414546
return{__proto__: null,module: this.module, namespace };
415547
}
416548
throwthis.module.getError();
417549
}elseif(status===kEvaluating||status===kEvaluated){
418550
if(this.module.hasAsyncGraph){
419-
constfilename=urlToFilename(this.url);
420-
constparentFilename=urlToFilename(parent?.filename);
421-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parentFilename);
551+
this.throwAsyncGraphError(parent);
422552
}
423553
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
424554
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
@@ -514,9 +644,16 @@ class ModuleJobSync extends ModuleJobBase {
514644
awaitthis.evaluationPromise;
515645
}
516646
return{__proto__: null,module: this.module};
517-
}elseif(status===kInstantiated){
518-
// The evaluation may have been canceled because instantiate() detected TLA first.
519-
// But when it is imported again, it's fine to re-evaluate it asynchronously.
647+
}elseif(status===kInstantiated||status===kUninstantiated){
648+
// The require() of this (synchronously linked) module bailed out: either
649+
// it was rejected for containing top-level await after instantiation
650+
// (kInstantiated), or its instantiation failed and left it uninstantiated
651+
// (kUninstantiated, e.g. a missing named export). When it's reached via async
652+
// run() from import, finish the instantiation and evaluate it asynchronously,
653+
// re-throwing any instantiation error.
654+
if(status===kUninstantiated){
655+
this.module.instantiate();
656+
}
520657
consttimeout=-1;
521658
constbreakOnSigint=false;
522659
this.evaluationPromise=this.module.evaluate(timeout,breakOnSigint);
@@ -532,23 +669,19 @@ class ModuleJobSync extends ModuleJobBase {
532669
runSync(parent){
533670
debug('ModuleJobSync.runSync()',this.module);
534671
assert(this.shouldRunModule(this.phase));
672+
// TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that the
673+
// async graph error supersedes instantiation (mismatch export) errors in the graph.
535674
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
536675
this.module.instantiate();
537-
// If --experimental-print-required-tla is true, proceeds to evaluation even
538-
// if it's async because we want to search for the TLA and help users locate
539-
// them.
540-
// TODO(joyeecheung): track the asynchroniticy using v8::Module::HasTopLevelAwait()
541-
// and we'll be able to throw right after compilation of the modules, using acron
542-
// to find and print the TLA. This requires the linking to be synchronous in case
543-
// it runs into cached asynchronous modules that are not yet fetched.
544-
constparentFilename=urlToFilename(parent?.filename);
545-
constfilename=urlToFilename(this.url);
546-
if(this.module.hasAsyncGraph&&!getOptionValue('--experimental-print-required-tla')){
547-
thrownewERR_REQUIRE_ASYNC_MODULE(filename,parentFilename);
676+
// On the deprecated async loader hook worker thread, dependencies linked by an
677+
// earlier import may not be walkable synchronously, so double-check with
678+
// V8 now that the graph is instantiated.
679+
if(this.module.hasAsyncGraph){
680+
this.throwAsyncGraphError(parent);
548681
}
549682
setHasStartedUserESMExecution();
550683
try{
551-
constnamespace=this.module.evaluateSync(filename,parentFilename);
684+
constnamespace=this.module.evaluateSync();
552685
return{__proto__: null,module: this.module, namespace };
553686
}catch(e){
554687
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
@@ -145,7 +145,7 @@ function loadCJSModuleWithSpecialRequire(module, source, url, filename, isMain,
145145
// On the main thread, the authentic require() is used instead (fixed by #60380).
146146
constrequest={ specifier,attributes: importAttributes,phase: kEvaluationPhase,__proto__: null};
147147
constjob=cascadedLoader.getOrCreateModuleJob(url,request,kRequireInImportedCJS);
148-
job.runSync();
148+
job.runSync(module);
149149
letmod=cjsCache.get(job.url);
150150
assert(job.module,`Imported CJS module ${url} failed to load module ${job.url} using require() due to race condition`);
151151

‎lib/internal/modules/esm/utils.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,15 @@ function compileSourceTextModule(url, source, type, context = kEmptyObject) {
362362
wrap.isMain=true;
363363
}
364364

365+
// Add an extra reference to the source of modules containing top-level await so that if the
366+
// module ends up being require()'d, we can parse the location of the top-level awaits to print
367+
// better errors. There will be other references to the same source in the module in V8 so this
368+
// only serves as a shortcut.
369+
if(wrap.hasTopLevelAwait&&
370+
getOptionValue('--experimental-print-required-tla')){
371+
wrap.source=source;
372+
}
373+
365374
// Cache the source map for the module if present.
366375
if(wrap.sourceMapURL){
367376
maybeCacheSourceMap(url,source,wrap,false,wrap.sourceURL,wrap.sourceMapURL);

0 commit comments

Comments
 (0)