Propagate errors thrown by a runtime plugin's onResolve callback

src/jsc/bindings/BunPlugin.cpp

@@ -866,13 +866,15 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, BunS
         if (auto* promise = dynamicDowncast<JSPromise>(result)) {
             switch (promise->status()) {
             case JSPromise::Status::Pending: {
+                // Discarded here, so a later rejection must not surface as unhandled.
+                promise->markAsHandled();
                 JSC::throwTypeError(globalObject, scope, "onResolve() doesn't support pending promises yet"_s);
                 return {};
             }
             case JSPromise::Status::Rejected: {
-                promise->setFlags(static_cast<uint16_t>(JSC::JSPromise::Status::Fulfilled));
-                result = promise->result();
-                return JSValue::encode(result);
+                promise->markAsHandled();
+                JSC::throwException(globalObject, scope, promise->result());
+                return {};
             }
             case JSPromise::Status::Fulfilled: {
                 result = promise->result();

src/jsc/bindings/ZigGlobalObject.cpp

@@ -3626,7 +3626,7 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* jsGlobalObject
     Zig::GlobalObject* globalObject = static_cast<Zig::GlobalObject*>(jsGlobalObject);
 
     ErrorableString res;
-    res.success = false;
+    memset(&res, 0, sizeof(res));
 
     BunString keyZ;
     if (key.isString()) {
@@ -3682,26 +3682,28 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* jsGlobalObject
     }
 
     BunString queryString = { BunStringTag::Empty, nullptr };
+    auto& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
     Zig__GlobalObject__resolve(&res, globalObject, &keyZ, &referrerZ, &queryString);
     keyZ.deref();
     referrerZ.deref();
 
-    if (res.success) {
-        if (!queryString.isEmpty()) {
-            auto result = JSC::Identifier::fromString(globalObject->vm(), makeString(res.result.value.toWTFString(BunString::ZeroCopy), queryString.toWTFString(BunString::ZeroCopy)));
-            res.result.value.deref();
-            queryString.deref();
-            return result;
-        }
+    // The resolve hook leaves `res` unwritten when it throws (e.g. from an onResolve plugin).
+    if (!res.success && !scope.exception()) [[unlikely]] {
+        throwException(scope, res.result.err, globalObject);
+    }
+    RETURN_IF_EXCEPTION(scope, vm.propertyNames->emptyIdentifier);
 
-        auto result = Identifier::fromString(globalObject->vm(), res.result.value.toWTFString(BunString::ZeroCopy));
+    if (!queryString.isEmpty()) {
+        auto result = JSC::Identifier::fromString(vm, makeString(res.result.value.toWTFString(BunString::ZeroCopy), queryString.toWTFString(BunString::ZeroCopy)));
         res.result.value.deref();
+        queryString.deref();
         return result;
-    } else {
-        auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
-        throwException(scope, res.result.err, globalObject);
-        return globalObject->vm().propertyNames->emptyIdentifier;
     }
+
+    auto result = Identifier::fromString(vm, res.result.value.toWTFString(BunString::ZeroCopy));
+    res.result.value.deref();
+    return result;
 }
 
 JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalObject,

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

@@ -936,3 +936,170 @@ describe.concurrent("Bun.plugin.clearAll()", () => {
     });
   });
 });
+
+describe.concurrent("onResolve failures", () => {
+  // Prints `{ ...result, unhandled }` once the event loop is drained, so an
+  // unhandled rejection queued by the plugin has already been reported.
+  const reporter = `
+    const unhandled = [];
+    let result = null;
+    process.on("unhandledRejection", error => unhandled.push(error?.message ?? String(error)));
+    process.on("exit", () => console.log(JSON.stringify({ ...result, unhandled })));
+  `;
+
+  async function report(source: string) {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", reporter + source],
+      stdout: "pipe",
+      stderr: "pipe",
+      env: bunEnv,
+    });
+
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    try {
+      return { report: JSON.parse(stdout), exitCode };
+    } catch {
+      throw new Error(`expected JSON on stdout, got:\n${stdout}\n--- stderr ---\n${stderr}`);
+    }
+  }
+
+  it("rejects the import with the error an async onResolve threw", async () => {
+    expect(
+      await report(`
+        Bun.plugin({
+          name: "rejecting resolver",
+          setup(builder) {
+            builder.onResolve({ filter: /^boom$/, namespace: "asyncthrow" }, async () => {
+              throw new Error("config missing");
+            });
+          },
+        });
+        const error = await import("asyncthrow:boom").catch(error => error);
+        result = { name: error.name, message: error.message };
+      `),
+    ).toEqual({
+      report: { name: "Error", message: "config missing", unhandled: [] },
+      exitCode: 0,
+    });
+  });
+
+  it("rejects the import with a non-Error thrown by an async onResolve", async () => {
+    expect(
+      await report(`
+        Bun.plugin({
+          name: "rejecting resolver",
+          setup(builder) {
+            builder.onResolve({ filter: /^boom$/, namespace: "asyncthrowstring" }, async () => {
+              throw "config missing";
+            });
+          },
+        });
+        const error = await import("asyncthrowstring:boom").catch(error => error);
+        result = { thrown: error };
+      `),
+    ).toEqual({
+      report: { thrown: "config missing", unhandled: [] },
+      exitCode: 0,
+    });
+  });
+
+  it("throws the error an async onResolve threw out of require()", async () => {
+    expect(
+      await report(`
+        Bun.plugin({
+          name: "rejecting resolver",
+          setup(builder) {
+            builder.onResolve({ filter: /^boom$/, namespace: "asyncthrowrequire" }, async () => {
+              throw new Error("config missing");
+            });
+          },
+        });
+        try {
+          require("asyncthrowrequire:boom");
+          result = { name: "(nothing was thrown)", message: "(nothing was thrown)" };
+        } catch (error) {
+          result = { name: error.name, message: error.message };
+        }
+      `),
+    ).toEqual({
+      report: { name: "Error", message: "config missing", unhandled: [] },
+      exitCode: 0,
+    });
+  });
+
+  it("throws the error a sync onResolve threw", async () => {
+    expect(
+      await report(`
+        Bun.plugin({
+          name: "throwing resolver",
+          setup(builder) {
+            builder.onResolve({ filter: /^boom$/, namespace: "syncthrow" }, () => {
+              throw new Error("config missing");
+            });
+          },
+        });
+        const error = await import("syncthrow:boom").catch(error => error);
+        result = { name: error.name, message: error.message };
+      `),
+    ).toEqual({
+      report: { name: "Error", message: "config missing", unhandled: [] },
+      exitCode: 0,
+    });
+  });
+
+  it("fails the entry point when onResolve throws while resolving it", async () => {
+    using dir = tempDir("plugin-entry-throw", {
+      "plugin.js": `
+        Bun.plugin({
+          name: "throwing resolver",
+          setup(builder) {
+            builder.onResolve({ filter: /entry\\.js$/ }, () => {
+              throw new Error("config missing");
+            });
+          },
+        });
+      `,
+      "entry.js": `console.log("the entry point ran");`,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "--preload", "./plugin.js", "./entry.js"],
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+      env: bunEnv,
+    });
+
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(stderr).toContain("config missing");
+    expect(stdout).toBe("");
+    expect(exitCode).toBe(1);
+  });
+
+  it("does not leak the rejection of an onResolve promise that settles too late", async () => {
+    expect(
+      await report(`
+        Bun.plugin({
+          name: "pending resolver",
+          setup(builder) {
+            builder.onResolve({ filter: /^boom$/, namespace: "pendingthrow" }, async () => {
+              await Bun.sleep(1);
+              throw new Error("config missing");
+            });
+          },
+        });
+        const error = await import("pendingthrow:boom").catch(error => error);
+        result = { name: error.name, message: error.message };
+      `),
+    ).toEqual({
+      report: {
+        name: "TypeError",
+        message: "onResolve() doesn't support pending promises yet",
+        unhandled: [],
+      },
+      exitCode: 0,
+    });
+  });
+});

test/regression/issue/22199.test.ts

@@ -101,9 +101,13 @@ test("plugin onResolve with rejected promise should throw error", () => {
     cmd: [bunExe(), "--preload", "./plugin.js", "./index.js"],
     env: bunEnv,
     cwd: String(dir),
+    stdout: "pipe",
     stderr: "pipe",
   });
 
-  expect(result.exitCode).toBe(1);
   expect(result.stderr.toString()).toContain("Custom plugin error");
+  // The error has to fail the resolution, not arrive later as an unhandled
+  // rejection while index.js runs anyway.
+  expect(result.stdout.toString()).toBe("");
+  expect(result.exitCode).toBe(1);
 });