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>

Author: joyeecheung

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`
 

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);

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) {

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.
   }
 
   /**

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);

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`);
 

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);