WIP/RFC: Include import stacks in resolution errors

.flowconfig

@@ -19,6 +19,7 @@ suppress_type=$FlowFixMe
 suppress_type=$FlowFixMeProps
 suppress_type=$FlowFixMeState
 suppress_type=$FlowFixMeEmpty
+suppress_type=$FlowExpectedError
 
 [lints]
 sketchy-null-number=warn

packages/metro/src/DeltaBundler/Graph.js

@@ -41,12 +41,14 @@ import type {
   TransformInputOptions,
   TransformResultDependency,
 } from './types.flow';
+import type {DependencyStackNode} from './diagnostics';
 
 import CountingSet from '../lib/CountingSet';
 import {
   deriveAbsolutePathFromContext,
   fileMatchesContext,
 } from '../lib/contextModule';
+import {addImportStackToError} from './diagnostics';
 
 import * as path from 'path';
 const invariant = require('invariant');
@@ -186,6 +188,11 @@ export class Graph<T = MixedOutput> {
       }
     }
 
+    const singleEntryPointForDiagnostics =
+      this.entryPoints.size === 1
+        ? this.entryPoints.values().next().value
+        : null;
+
     for (const [path] of originalModules) {
       // Traverse over modules that are part of the dependency graph.
       //
@@ -198,6 +205,13 @@ export class Graph<T = MixedOutput> {
           path,
           delta,
           internalOptions,
+          this.entryPoints.has(path)
+            ? null
+            : {
+                absolutePath: singleEntryPointForDiagnostics ?? null,
+                dependency: null,
+                parent: null,
+              },
         );
       }
     }
@@ -282,7 +296,12 @@ export class Graph<T = MixedOutput> {
 
     await Promise.all(
       [...this.entryPoints].map((path: string) =>
-        this._traverseDependenciesForSingleFile(path, delta, internalOptions),
+        this._traverseDependenciesForSingleFile(
+          path,
+          delta,
+          internalOptions,
+          null,
+        ),
       ),
     );
 
@@ -301,10 +320,11 @@ export class Graph<T = MixedOutput> {
     path: string,
     delta: Delta,
     options: InternalOptions<T>,
+    dependencyStack: DependencyStackNode | null,
   ): Promise<void> {
     options.onDependencyAdd();
 
-    await this._processModule(path, delta, options);
+    await this._processModule(path, delta, options, dependencyStack);
 
     options.onDependencyAdded();
   }
@@ -313,6 +333,7 @@ export class Graph<T = MixedOutput> {
     path: string,
     delta: Delta,
     options: InternalOptions<T>,
+    dependencyStack: DependencyStackNode | null,
   ): Promise<Module<T>> {
     const resolvedContext = this.#resolvedContexts.get(path);
 
@@ -325,7 +346,9 @@ export class Graph<T = MixedOutput> {
     const currentDependencies = this._resolveDependencies(
       path,
       result.dependencies,
+      delta,
       options,
+      dependencyStack,
     );
 
     const previousModule = this.dependencies.get(path);
@@ -369,7 +392,14 @@ export class Graph<T = MixedOutput> {
         !dependenciesEqual(prevDependency, curDependency, options)
       ) {
         addDependencyPromises.push(
-          this._addDependency(nextModule, key, curDependency, delta, options),
+          this._addDependency(
+            nextModule,
+            key,
+            curDependency,
+            delta,
+            options,
+            dependencyStack,
+          ),
         );
       }
     }
@@ -394,7 +424,7 @@ export class Graph<T = MixedOutput> {
     // the above promises have resolved, this will be the same map but without
     // the added nondeterminism of promise resolution order. Because this
     // assignment does not add or remove edges, it does NOT invalidate any of the
-    // garbage collection state.
+    // garbage collection delta.
 
     // Catch obvious errors with a cheap assertion.
     invariant(
@@ -413,6 +443,7 @@ export class Graph<T = MixedOutput> {
     dependency: Dependency,
     delta: Delta,
     options: InternalOptions<T>,
+    dependencyStack: DependencyStackNode | null,
   ): Promise<void> {
     const path = dependency.absolutePath;
 
@@ -449,7 +480,11 @@ export class Graph<T = MixedOutput> {
           delta.earlyInverseDependencies.set(path, new CountingSet());
 
           options.onDependencyAdd();
-          module = await this._processModule(path, delta, options);
+          module = await this._processModule(path, delta, options, {
+            absolutePath: parentModule.path,
+            dependency: dependency.data,
+            parent: dependencyStack,
+          });
           options.onDependencyAdded();
 
           this.dependencies.set(module.path, module);
@@ -538,7 +573,9 @@ export class Graph<T = MixedOutput> {
   _resolveDependencies(
     parentPath: string,
     dependencies: $ReadOnlyArray<TransformResultDependency>,
+    delta: Delta,
     options: InternalOptions<T>,
+    dependencyStack: DependencyStackNode | null,
   ): Map<string, Dependency> {
     const maybeResolvedDeps = new Map<
       string,
@@ -582,6 +619,17 @@ export class Graph<T = MixedOutput> {
           // clean it up.
           this.#resolvedContexts.delete(resolvedDep.absolutePath);
         } catch (error) {
+          if (error instanceof Error) {
+            addImportStackToError(
+              {
+                absolutePath: parentPath,
+                dependency: dep,
+                parent: dependencyStack,
+              },
+              error,
+            );
+          }
+
           // Ignore unavailable optional dependencies. They are guarded
           // with a try-catch block and will be handled during runtime.
           if (dep.data.isOptional !== true) {

packages/metro/src/DeltaBundler/__tests__/Graph-test.js

@@ -3070,3 +3070,57 @@ describe('parallel edges', () => {
     });
   });
 });
+
+describe('diagnostics', () => {
+  test('appends an import stack to resolution error on initial traversal', async () => {
+    Actions.deleteFile('/bar', graph);
+
+    await expect(
+      graph.initialTraverseDependencies(options),
+    ).rejects.toMatchSnapshot();
+  });
+
+  test('appends a partial import stack to resolution error on incremental traversal', async () => {
+    await graph.initialTraverseDependencies(options);
+
+    Actions.deleteFile('/bar', graph);
+
+    await expect(
+      graph.traverseDependencies([...files], options),
+    ).rejects.toMatchSnapshot();
+  });
+
+  test('import stack is available on the error object', async () => {
+    Actions.deleteFile('/bar', graph);
+
+    const error = await flip(graph.initialTraverseDependencies(options));
+
+    // $FlowIgnore[prop-missing]
+    expect(error.metroImportStack).toMatchSnapshot();
+  });
+
+  test('entry point is not named in the import stack when there are multiple entry points', async () => {
+    Actions.createFile('/bundle-2');
+    Actions.addDependency('/bundle-2', '/foo');
+    graph = new TestGraph({
+      entryPoints: new Set(['/bundle', '/bundle-2']),
+      transformOptions: options.transformOptions,
+    });
+    await graph.initialTraverseDependencies(options);
+
+    Actions.deleteFile('/bar', graph);
+
+    await expect(
+      graph.traverseDependencies([...files], options),
+    ).rejects.toMatchSnapshot();
+  });
+});
+
+function flip<T>(promise: Promise<T>): Promise<Error> {
+  return promise.then(
+    value => {
+      throw value;
+    },
+    error => error,
+  );
+}

packages/metro/src/DeltaBundler/__tests__/__snapshots__/Graph-test.js.snap

@@ -1,5 +1,32 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`diagnostics appends a partial import stack to resolution error on incremental traversal 1`] = `
+[Error: Dependency not found: /foo -> /bar
+Import stack:
+    at /foo
+    (...)
+    at /bundle]
+`;
+
+exports[`diagnostics appends an import stack to resolution error on initial traversal 1`] = `
+[Error: Dependency not found: /foo -> /bar
+Import stack:
+    at /foo
+    at /bundle]
+`;
+
+exports[`diagnostics entry point is not named in the import stack when there are multiple entry points 1`] = `
+[Error: Dependency not found: /foo -> /bar
+Import stack:
+    at /foo
+    (...)]
+`;
+
+exports[`diagnostics import stack is available on the error object 1`] = `
+"    at /foo
+    at /bundle"
+`;
+
 exports[`should do the initial traversal correctly 1`] = `
 TestGraph {
   "dependencies": Map {

packages/metro/src/DeltaBundler/diagnostics.js

@@ -0,0 +1,70 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ * @oncall react_native
+ */
+
+import type {TransformResultDependency} from './types.flow';
+
+export type DependencyStackNode = $ReadOnly<{
+  // If `dependency` is null, this is a dependency whose origin isn't known
+  // at the time we produced this stack. This happens in most incremental
+  // traversals: since we can throw before we've finished processing
+  // updates, all dependencies outside the immediate call stack are
+  // potentially out of date, so we don't try to return them.
+  dependency: null | TransformResultDependency,
+  // Absolute path of the origin module if this is a concrete dependency,
+  // or of the next known module in the chain if dependency is `null`.
+  absolutePath: null | string,
+  // The parent node in the dependency tree.
+  parent: null | DependencyStackNode,
+}>;
+
+export function addImportStackToError(
+  dependencyStack: DependencyStackNode,
+  error: Error,
+) {
+  let importStackString = '';
+  let node: ?DependencyStackNode = dependencyStack;
+  let printedEllipsis = false;
+  while (node) {
+    if (node.dependency == null || node.absolutePath == null) {
+      // Make sure we don't print multiple consecutive ellipses.
+      if (!printedEllipsis) {
+        if (importStackString !== '') {
+          importStackString += '\n';
+        }
+        importStackString += '    (...)';
+        printedEllipsis = true;
+      }
+    }
+
+    if (node.absolutePath != null) {
+      if (importStackString !== '') {
+        importStackString += '\n';
+      }
+      const loc = node.dependency?.data.locs[0];
+
+      importStackString +=
+        // TODO: @nocommit Absolute paths are too verbose - relativize them (here / outside of Graph?)
+        '    at ' +
+        node.absolutePath +
+        (loc ? `:${loc.start.line}:${loc.start.column + 1}` : '');
+      printedEllipsis = false;
+    }
+    node = node.parent;
+  }
+  try {
+    error.message += '\nImport stack:\n' + importStackString;
+
+    // $FlowIgnore
+    error.metroImportStack = importStackString;
+  } catch {
+    // Mutating the error object might fail, so swallow any inner errors.
+  }
+}