Module loader: a failed load of a replaced module must not leave its error on the replacement (tests for oven-sh/WebKit#754)

test/cli/hot/hot.test.ts

@@ -829,6 +829,67 @@ it(
   timeout,
 );
 
+it(
+  "should keep the module a hot reload loaded when the import() from before the reload rejects",
+  async () => {
+    using dir = tempDir("hot-reload-import-in-flight-throws", {
+      "a.mjs": `
+        import "./dependency.mjs";
+        export const evaluation = (globalThis.evaluations = (globalThis.evaluations ?? 0) + 1);
+        if (globalThis.throwWhileEvaluating) throw new Error("the module from before the reload threw");
+      `,
+      "dependency.mjs": `export {};`,
+      "entry.mjs": `
+        import { readFileSync, writeFileSync } from "node:fs";
+        globalThis.runs = (globalThis.runs ?? 0) + 1;
+        if (globalThis.runs === 1) {
+          Bun.plugin({
+            name: "hold the dependency's load open until the reload",
+            setup(build) {
+              build.onLoad({ filter: /dependency\\.mjs$/ }, () => {
+                const loaded = { contents: "export {}", loader: "js" };
+                if (globalThis.dependencyMayLoad) return loaded;
+                const { promise, resolve } = Promise.withResolvers();
+                globalThis.dependencyMayLoad = () => resolve(loaded);
+                writeFileSync(import.meta.path, readFileSync(import.meta.path));
+                return promise;
+              });
+            },
+          });
+          globalThis.inFlight = import("./a.mjs");
+        } else {
+          const settled = promise => promise.then(module => "evaluation " + module.evaluation, error => "rejected: " + error.message);
+          try {
+            console.log("reloaded:", await settled(import("./a.mjs")));
+            globalThis.throwWhileEvaluating = true;
+            globalThis.dependencyMayLoad();
+            console.log("in flight:", await settled(globalThis.inFlight));
+            console.log("next:", await settled(import("./a.mjs")));
+            process.exit(0);
+          } catch (error) {
+            // --hot would keep the process alive after an uncaught error.
+            console.log("threw:", error);
+            process.exit(1);
+          }
+        }
+      `,
+    });
+    await using proc = spawn({
+      cmd: [bunExe(), "--hot", "entry.mjs"],
+      cwd: String(dir),
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "inherit",
+    });
+    const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
+    expect(stdout).toBe(
+      "reloaded: evaluation 1\nin flight: rejected: the module from before the reload threw\nnext: evaluation 1\n",
+    );
+    expect(exitCode).toBe(0);
+  },
+  timeout,
+);
+
 it("holds the promise of the entry point itself, which it looks at on every tick", async () => {
   const source = (comment: string) => `
     globalThis.loads = (globalThis.loads ?? 0) + 1;

test/js/bun/plugin/plugins.test.ts

@@ -1114,6 +1114,109 @@ it.concurrent("build.module() of a module whose import() is still loading its de
   });
 });
 
+describe.each(["import", "require"])(
+  "a module that build.module() replaced while it was evaluating does not leave its error on the replacement",
+  how => {
+    it.concurrent(how, async () => {
+      const load = how === "import" ? "(await import(import.meta.path))" : "require(import.meta.path)";
+      using dir = tempDir("plugin-module-replaced-then-threw", {
+        "replaces-itself.mjs": `
+          Bun.plugin({
+            name: "replace this module",
+            setup(build) {
+              build.module(import.meta.path, () => ({ exports: { from: "build.module()" }, loader: "object" }));
+            },
+          });
+          console.log("while evaluating:", ${load}.from);
+          throw new Error("the replaced module threw");
+        `,
+        "import.ts": `
+          console.log("first:", await import("./replaces-itself.mjs").then(module => module.from, error => error.message));
+          console.log("next:", await import("./replaces-itself.mjs").then(module => module.from, error => error.message));
+        `,
+        "require.cjs": `
+          try {
+            console.log("first:", require("./replaces-itself.mjs").from);
+          } catch (error) {
+            console.log("first:", error.message);
+          }
+          import("./replaces-itself.mjs").then(module => console.log("next:", module.from), error => console.log("next:", error.message));
+        `,
+      });
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), how === "import" ? "import.ts" : "require.cjs"],
+        cwd: String(dir),
+        env: bunEnv,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect({ stdout, stderr, exitCode }).toEqual({
+        stdout: "while evaluating: build.module()\nfirst: the replaced module threw\nnext: build.module()\n",
+        stderr: "",
+        exitCode: 0,
+      });
+    });
+  },
+);
+
+// target.ts has no registry entry yet when build.module() replaces it: its onLoad is still running. That onLoad then
+// fails with the error of another module's import(). The loader has tagged that error, and a fetch that fails with a
+// tagged error stores it under its key.
+it.concurrent(
+  "an onLoad that rejects with another module's error does not leave it on the module build.module() put in its place",
+  async () => {
+    using dir = tempDir("plugin-onload-rejects-with-tagged-error", {
+      "target.ts": `export const from = "file";`,
+      "throws.ts": `throw new Error("another module threw");`,
+      "entry.ts": `
+      import { join } from "node:path";
+      const onLoadStarted = Promise.withResolvers<void>();
+      const onLoadMayFail = Promise.withResolvers<void>();
+      const target = join(import.meta.dir, "target.ts");
+      Bun.plugin({
+        name: "the onLoad of target.ts imports a module that throws",
+        setup(build) {
+          build.onLoad({ filter: /target\\.ts$/ }, async () => {
+            onLoadStarted.resolve();
+            await onLoadMayFail.promise;
+            await import("./throws.ts");
+            return { contents: "export const from = 'onLoad';", loader: "ts" };
+          });
+        },
+      });
+      const settled = (promise: Promise<{ from: string }>) => promise.then(module => module.from, error => "rejected: " + error.message);
+
+      const first = settled(import(target));
+      await onLoadStarted.promise;
+      Bun.plugin({
+        name: "replace target.ts",
+        setup(build) {
+          build.module(target, () => ({ exports: { from: "build.module()" }, loader: "object" }));
+        },
+      });
+      console.log("replacement:", await settled(import(target)));
+      onLoadMayFail.resolve();
+      console.log("first:", await first);
+      console.log("next:", await settled(import(target)));
+    `,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "entry.ts"],
+      cwd: String(dir),
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, stderr, exitCode }).toEqual({
+      stdout: "replacement: build.module()\nfirst: rejected: another module threw\nnext: build.module()\n",
+      stderr: "",
+      exitCode: 0,
+    });
+  },
+);
+
 it.concurrent(
   "import() after delete require.cache of a module that onResolve redirected a resolved path to",
   async () => {

test/js/bun/resolve/concurrent-dynamic-import.test.ts

@@ -30,3 +30,39 @@ test("concurrent dynamic imports of the same module both resolve", async () => {
   expect(stdout.trim()).toBe("ok");
   expect(exitCode).toBe(0);
 });
+
+// The first import() fails at its fetch: the file does not parse. The file is fixed and imported again before that
+// failure has settled. The failure belongs to the first import() alone, so a later import() gets the module. JSON is
+// read on the calling thread, which fixes the order of the two fetches.
+test("an import() that failed to parse does not leave its error on the module a later import() loaded", async () => {
+  using dir = tempDir("concurrent-dyn-import-first-fails", {
+    "data.json": `{ "value": `,
+    "entry.mjs": `
+      import { writeFileSync } from "node:fs";
+      const path = import.meta.dir + "/data.json";
+      const settled = promise => promise.then(module => module.default.value, error => "rejected: " + error.name);
+
+      const first = settled(import(path));
+      writeFileSync(path, JSON.stringify({ value: 42 }));
+      const second = import(path);
+
+      console.log("first:", await first);
+      // Whether the second import() shares the first one's load is not what this tests.
+      await second.catch(() => {});
+      console.log("later:", await settled(import(path)));
+    `,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "entry.mjs"],
+    cwd: String(dir),
+    env: bunEnv,
+    stdio: ["ignore", "pipe", "pipe"],
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({ stdout, stderr, exitCode }).toEqual({
+    stdout: "first: rejected: SyntaxError\nlater: 42\n",
+    stderr: "",
+    exitCode: 0,
+  });
+});

test/js/bun/test/mock/mock-module.test.ts

@@ -540,3 +540,79 @@ test.concurrent("mock.module() of a module whose import() is still loading its d
   expect(stderr).toContain(" 1 pass");
   expect(exitCode).toBe(0);
 });
+
+test.concurrent("mock.module() of a module whose import() is in flight and then throws", async () => {
+  using dir = tempDir("mock-module-import-in-flight-throws", {
+    "a.ts": `import "./dependency"; export const a = "real-a"; throw new Error("the replaced module threw");`,
+    "dependency.ts": `export {};`,
+    "in-flight.test.ts": `
+      import { expect, mock, test } from "bun:test";
+
+      test("the mock stays the module after the import in flight rejects", async () => {
+        const dependencyRequested = Promise.withResolvers<void>();
+        const dependencyMayLoad = Promise.withResolvers<void>();
+        Bun.plugin({
+          name: "hold the dependency's load open",
+          setup(build) {
+            build.onLoad({ filter: /dependency\\.ts$/ }, async () => {
+              dependencyRequested.resolve();
+              await dependencyMayLoad.promise;
+              return { contents: "export {}", loader: "ts" };
+            });
+          },
+        });
+
+        const inFlight = import("./a");
+        await dependencyRequested.promise;
+        mock.module("./a", () => ({ a: "mocked-a" }));
+        expect((await import("./a")).a).toBe("mocked-a");
+        dependencyMayLoad.resolve();
+
+        await expect(inFlight).rejects.toThrow("the replaced module threw");
+        expect((await import("./a")).a).toBe("mocked-a");
+      });
+    `,
+  });
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "test", "./in-flight.test.ts"],
+    cwd: String(dir),
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toContain(" 1 pass");
+  expect(exitCode).toBe(0);
+});
+
+// No plugin and nothing to wait for: the failed import() of the real file settles after the mock is registered.
+test.concurrent("mock.module() of a file whose import() before it failed", async () => {
+  using dir = tempDir("mock-module-after-failed-import", {
+    "app.ts": `export const optional = import("./data.json").catch(() => null);`,
+    "data.json": `{ "ok": false, `,
+    "mocked.test.ts": `
+      import { expect, mock, test } from "bun:test";
+      import "./app";
+
+      mock.module("./data.json", () => ({ default: { ok: true } }));
+      const first = await import("./data.json");
+
+      test("the mock is what the import() after mock.module() got", () => {
+        expect(first.default).toEqual({ ok: true });
+      });
+      test("the mock is what a later import() gets", async () => {
+        expect((await import("./data.json")).default).toEqual({ ok: true });
+      });
+    `,
+  });
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "test", "./mocked.test.ts"],
+    cwd: String(dir),
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toContain(" 2 pass");
+  expect(exitCode).toBe(0);
+});