Propagate errors thrown by a runtime plugin's onResolve callback

[robobun]

Repro

// plugin.js
Bun.plugin({
  name: "p",
  setup(build) {
    build.onResolve({ filter: /^boom$/, namespace: "ns" }, async () => {
      throw new Error("real reason: config missing");
    });
  },
});

// main.js
const error = await import("ns:boom").catch(e => e);
console.log(error.name, "|", error.message);

$ bun --preload ./plugin.js ./main.js
ResolveMessage | Cannot find package 'ns:boom' from '.../main.js'
error: real reason: config missing        <- leaked as an unhandled rejection
$ echo $?
1

The plugin's actual error is lost, the importer gets a generic "cannot resolve" message, and the rejection escapes to the unhandled-rejection queue, which exits the process under the default policy. Bun.build() with the same plugin reports the plugin's error correctly.

A second repro, which segfaults on current main with no async involved:

// plugin.js
Bun.plugin({
  name: "p",
  setup(build) {
    build.onResolve({ filter: /entry\.js$/ }, () => {
      throw new Error("config missing");
    });
  },
});

$ bun --preload ./plugin.js ./entry.js
panic(main thread): Segmentation fault at address 0x10

Cause

BunPlugin::OnResolve::run handled a rejected promise with
promise->setFlags(static_cast<uint16_t>(JSPromise::Status::Fulfilled)) and then
returned promise->result() as if it were the { path } object the callback is
supposed to return.

JSPromise::setFlags replaces the whole 16-bit flags word, so rather than marking
the promise as handled it cleared isHandled (and isFirstResolvingFunctionCalled,
and the inline-reaction bits). The rejection reason was then handed back to the
resolver, which found no path property on it and fell through to default
resolution. The promise, still unhandled, surfaced later as an unhandled rejection.

Fixing that routed a throwing onResolve into moduleLoaderResolve, the resolve
hook for static imports and the entry point, where:

ErrorableString res;
res.success = false;          // res.result is never initialized
...
Zig__GlobalObject__resolve(&res, globalObject, &keyZ, &referrerZ, &queryString);
...
} else {
    throwException(scope, res.result.err, globalObject);   // reads uninitialized stack
}

resolve_hook (src/runtime/jsc_hooks.rs) has three return false paths and all
three leave a JS exception pending without writing res; a plugin's onResolve
throwing is one of them. So throwException built an Exception around a wild
JSValue, and the first thing that touched it crashed:

#0 WTF::Ref<JSC::JSLock>::operator-> (this=0x3ff0000000000060)
#1 JSC::VM::currentThreadIsHoldingAPILock

This is reachable on stock Bun today with a plain synchronous throw, because the
entry point is resolved through this hook. The sibling hook for dynamic imports,
moduleLoaderImportModule, already zero-initializes its ErrorableString and checks
scope.exception() before reading result.err, which is why await import() and
require() were unaffected.

Fix

• BunPlugin::OnResolve::run: markAsHandled() + throwException(globalObject, scope, promise->result()) for a rejected promise, matching ModuleLoader.cpp's handling of a rejected virtual module. A still-pending promise is also marked handled, since nothing attaches a handler to it once the existing onResolve() doesn't support pending promises yet TypeError is thrown.
• moduleLoaderResolve: zero-initialize res and check for a pending exception before reading res.result.err, the same shape moduleLoaderImportModule uses.

The invariant moduleLoaderResolve now relies on

The refactor makes the success-path deref of res.result.value unconditional, so it is worth
stating the contract it depends on explicitly. resolve_hook (src/runtime/jsc_hooks.rs) has
exactly five return true paths and three return false paths:

line	writes to *res	success	exception pending
4972	err(NameTooLong, js_err)	false	no
4999	whatever on_resolve_jsc returned	either	no (see below)
5020	ok(hardcoded alias path)	true	no
5117	err(err, js_err)	false	no
5138	ok(clone_utf8(result_path))	true	no
4968, 5004, 5113	nothing	false (memset)	yes

The only path that could pair success == true with a pending exception is 4999, and it cannot.
Inside plugin_runner_on_resolve_jsc every call that can throw propagates with ?, which becomes
Err(_) => return false at 5004. The two places that turn a thrown value into an ErrorableString
(VirtualMachine.rs:6704 and :6713) call global.try_take_exception(), which removes the
exception from the global before storing it, and those yield success == false anyway. The single
ok(...) return (:6719) is reached only after every fallible step has succeeded.

So success == true implies nothing is pending and control reaches the deref; success == false
implies either an exception is pending with res never written (harmless, memset left it zeroed)
or res.result.err holds a real JSValue and we throw it. The old code relied on the weaker half
of this same contract, just incorrectly: it assumed success == false always meant res.result.err
was populated, which is exactly what those three return false paths violate.

Empirically, the require-cache file-path leak fixture (10,000 require() round trips, which would
leak one resolved path per import if the deref were ever skipped) reports 68 MB on this branch vs
70 MB on main, i.e. no delta beyond noise.

Verification

Six tests in test/js/bun/plugin/plugins.test.ts covering import(), require(), a
non-Error throw, a synchronous throw, entry-point resolution, and a promise that
rejects after the resolver has already returned. Each spawns a subprocess that
collects unhandledRejection and prints it from process.on("exit"), so
unhandled: [] is an assertion rather than an absence check.

test/regression/issue/22199.test.ts's "onResolve with rejected promise should throw
error" passed for the wrong reason: it asserted only exit code 1 and the message on
stderr, both of which the leaked unhandled rejection satisfied while index.js ran to
completion anyway. It now also asserts stdout is empty.

Before / after

$ USE_SYSTEM_BUN=1 bun test test/js/bun/plugin/plugins.test.ts -t "onResolve failures"
  {
    "exitCode": 0,
    "report": {
-     "message": "config missing",
-     "name": "Error",
-     "unhandled": [],
+     "message": "Cannot find package 'asyncthrow:boom' from '[eval]'",
+     "name": "ResolveMessage",
+     "unhandled": [
+       "config missing",
+     ],
    },
  }
 1 pass
 5 fail

$ bun bd test test/js/bun/plugin/plugins.test.ts test/regression/issue/22199.test.ts
 43 pass
 0 fail

No regressions across test/js/bun/plugin/, test/js/bun/resolve/, test/js/bun/module/,
test/js/node/module/, test/bundler/bundler_plugin.test.ts, and the preload suites.

[robobun]

^{Updated 6:00 AM PT - Jul 6th, 2026}

@robobun, your commit 2dc444e is building: #68982

[coderabbitai]

Walkthrough

Changes update native promise handling in BunPlugin's onResolve callback so pending and rejected promises are marked handled and thrown as exceptions rather than converted to fulfilled state, refactor ZigGlobalObject's moduleLoaderResolve exception flow with a zero-initialized result struct and throw scope, and add/update tests validating these failure paths.

Changes

onResolve rejection and pending promise handling

Layer / File(s)	Summary
Core native promise/resolver behavior src/jsc/bindings/BunPlugin.cpp, src/jsc/bindings/ZigGlobalObject.cpp	Pending promises are marked handled before throwing a TypeError; rejected promises are marked handled and their result thrown as an exception instead of being treated as fulfilled. moduleLoaderResolve now zero-initializes its result struct and uses a VM throw scope to conditionally throw res.result.err only when resolution failed and no exception is already pending, relying on RETURN_IF_EXCEPTION for control flow.
Test coverage for failure modes test/js/bun/plugin/plugins.test.ts, test/regression/issue/22199.test.ts	A new concurrent test suite with a subprocess-based reporting helper covers async/sync throw and rejection paths, entry-point resolution failures, and late-settling pending promises without leaking unhandled rejections. The regression test for issue 22199 now pipes both stdout and stderr, asserting stdout is empty and the process exits with code 1.

Possibly related PRs

• oven-sh/bun#29393: Also modifies Zig::GlobalObject::moduleLoaderResolve resolver/exception-throwing and promise-resolution control flow.
• oven-sh/bun#30223: Also modifies GlobalObject::moduleLoaderResolve handling of the resolver result struct.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main fix: propagating errors from runtime plugin onResolve callbacks.
Description check	✅ Passed	The description includes the PR purpose, root cause, fix, and verification, though it uses different headings than the template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

No issues found — the fix mirrors the established pattern in moduleLoaderImportModule and the markAsHandled() + throwException() handling matches ModuleLoader.cpp — but since this restructures exception-scope handling in moduleLoaderResolve (the hook every static import and entry point goes through) and fixes an uninitialized-stack read, it's worth a human pass.

Extended reasoning...

Overview

Two native changes plus tests:

• BunPlugin.cpp (OnResolve::run): For a rejected promise, replaces promise->setFlags(static_cast<uint16_t>(Status::Fulfilled)) + return-result-as-if-it-were-a-resolve-object with markAsHandled() + throwException(globalObject, scope, promise->result()). For a pending promise, adds markAsHandled() before throwing the existing "doesn't support pending promises yet" TypeError so a later rejection doesn't leak to the unhandled queue.
• ZigGlobalObject.cpp (moduleLoaderResolve): Zero-initializes ErrorableString res with memset, moves DECLARE_THROW_SCOPE above the Zig__GlobalObject__resolve call, and guards the read of res.result.err with !scope.exception() before RETURN_IF_EXCEPTION. This is a line-for-line copy of the shape already in the sibling moduleLoaderImportModule (lines ~3505–3538), which was previously the only reason await import() didn't crash on this path.
• Tests: Six new subprocess tests in plugins.test.ts covering async/sync throw, non-Error throw, import() vs require(), entry-point resolution (the segfault repro), and a late-settling pending promise. Each positively asserts unhandled: [] from a process.on('exit') reporter. Also tightens 22199.test.ts which previously passed for the wrong reason (unhandled rejection produced the same exit code + stderr).

Security risks

None identified. This is error-propagation plumbing; no auth, crypto, path handling, or untrusted-input parsing is touched. The change removes an uninitialized-stack read rather than adding attack surface.

Level of scrutiny

High. moduleLoaderResolve is the JSC hook for static imports and the entry point — every module resolution that isn't a dynamic import() flows through it. The change restructures its control flow and moves the ThrowScope declaration, which is exactly the class of change (JSC exception-scope discipline in a hot path) where subtle mistakes manifest as hard-to-reproduce crashes. The old code demonstrably read uninitialized memory, so the fix is clearly needed, but the reshape deserves human eyes.

Other factors

• The new moduleLoaderResolve shape is verifiably identical to the existing moduleLoaderImportModule pattern in the same file, which reduces risk considerably.
• markAsHandled() is the established idiom across ModuleLoader.cpp, bindings.cpp, and NodeVMModule.cpp for consuming a rejection that's being rethrown.
• Test coverage is unusually thorough (variant matrix, positive assertion of no unhandled rejections, entry-point crash repro) and the PR description includes before/after runs against USE_SYSTEM_BUN=1.
• No prior human reviews or outstanding comments on the PR.

[robobun]

The one red lane is windows 2019 x64-baseline → test/napi/napi.test.ts → napi > napi_wrap > has the right lifetime, failing inside gcUntil with Condition was not met after 100 GC attempts. It is unrelated to this PR and pre-existing.

Why it can't be this diff. BunPlugin::OnResolve::run only executes when a Bun.plugin() onResolve callback is registered, and the napi app registers none. moduleLoaderResolve's success path is unchanged: same Identifier construction, same res.result.value.deref(), same queryString.deref(). The only behavior change is on the path where the resolve hook leaves an exception pending, which a GC-lifetime test never reaches.

Why it's a known flake. The test shows up in 22 of the last 40 Buildkite builds. 21 of those are Buildkite's own flaky annotation (failed, then passed on retry); build 68641 on branch farm/f91b8f07/inline-runtime-helpers hard-failed on it the same way this build did. It is a GC-timing assertion (100 iterations of setTimeout(1) + Bun.gc(true) waiting for a wrapped object to be collected), which is exactly the shape that gives out on the slowest runner in the fleet.

Reproduced against the parent commit. Reverting src/ to HEAD~1 and rebuilding reproduces the same failure locally, and restoring the fix does not change it. With the fix applied, all five wrap-lifetime scenarios pass when run directly, so the GC condition itself is met:

test_wrap_lifetime_without_ref                 OK (1801ms)
test_wrap_lifetime_with_weak_ref               OK (1782ms)
test_wrap_lifetime_with_strong_ref             OK (4401ms)
test_remove_wrap_lifetime_with_weak_ref        OK (1810ms)
test_remove_wrap_lifetime_with_strong_ref      OK (4239ms)

Everything else on the build is green: 281 jobs passed, including debian 13 x64-asan and every lane that runs the new tests in test/js/bun/plugin/plugins.test.ts and test/regression/issue/22199.test.ts.

I have not pushed a retrigger, since that test fails in more than half of recent builds anyway and a re-run would cost a full CI cycle. Happy to push one if you'd prefer a clean build before merging.

[claude]

I didn't find any issues — the fix looks correct and matches the sibling moduleLoaderImportModule shape — but since this reworks exception-scope placement and control flow in moduleLoaderResolve (the entry-point/static-import resolve hook), it's worth a human look before merging.

Extended reasoning...

Overview

Two native changes plus tests:

• src/jsc/bindings/BunPlugin.cpp — OnResolve::run's rejected-promise arm now calls markAsHandled() and throwException(..., promise->result()) instead of the buggy setFlags(Fulfilled) + return-rejection-as-result. The pending arm also marks the promise handled before throwing the existing TypeError.
• src/jsc/bindings/ZigGlobalObject.cpp — moduleLoaderResolve now zero-inits res, declares the throw scope before Zig__GlobalObject__resolve, and only reads res.result.err when no exception is already pending; RETURN_IF_EXCEPTION handles both the plugin-threw and resolve-set-err paths. The success path (identifier construction, res.result.value.deref(), queryString.deref()) is byte-for-byte the same as before, just de-nested. I verified the new shape matches moduleLoaderImportModule at ZigGlobalObject.cpp:3505–3506, which already does memset(&resolved, 0, ...) under a scope declared at the top of the function.
• Six new subprocess tests in plugins.test.ts covering async/sync throw × import()/require()/entry-point, plus a late-settling pending promise; and a tightened assertion in 22199.test.ts (adds stdout === "") so the test can no longer pass via the leaked unhandled rejection.

Security risks

None identified. This is error-propagation plumbing for user-registered runtime plugins; no auth, crypto, permissions, or untrusted-input parsing is touched. The change strictly reduces attack surface by removing an uninitialized-stack read.

Level of scrutiny

High. moduleLoaderResolve is the JSC resolve hook for the entry point and every static import — it runs on essentially every program launch. Moving DECLARE_THROW_SCOPE above Zig__GlobalObject__resolve and restructuring the branches around RETURN_IF_EXCEPTION is the kind of change where a maintainer familiar with the Rust-side resolve_hook contract (which paths write res vs. leave a pending exception) should confirm the invariant that res.success && scope.exception() is unreachable, since that combination would now early-return without deref'ing res.result.value. The PR description argues it isn't reachable and the sibling hook already relies on the same assumption, but it warrants human sign-off.

Other factors

• The bug hunter found nothing; CodeRabbit had no actionable comments.
• CI is green except a known-flaky Windows napi GC-timing test that the PR body convincingly rules out as related.
• Test coverage is thorough and follows repo conventions (tempDir, bunEnv, concurrent subprocess drain, positive assertion of unhandled: [] rather than absence-checking).
• No prior review from me on this PR.

[robobun]

On the res.success && scope.exception() invariant

Good question to pin down, since the deref of res.result.value depends on it. It is unreachable, and here is the mechanical argument from the producer side rather than an appeal to the sibling hook.

resolve_hook (src/runtime/jsc_hooks.rs) has exactly five return true paths and three return false paths:

line	writes to *res	success	exception pending
4972	err(NameTooLong, js_err)	false	no
4999	whatever on_resolve_jsc returned	either	no (see below)
5020	ok(hardcoded alias path)	true	no
5117	err(err, js_err)	false	no
5138	ok(clone_utf8(result_path))	true	no
4968, 5004, 5113	nothing	false (memset)	yes

The only path that could smuggle a pending exception alongside success == true is 4999, and it cannot. Inside plugin_runner_on_resolve_jsc every call that can throw propagates with ?, which becomes Err(_) => return false at line 5004. The two places that do produce an ErrorableString from a thrown value (VirtualMachine.rs:6704 and :6713) call global.try_take_exception(), which removes the exception from the global before stuffing it into the err payload, and those yield success == false anyway. The single ok(...) return (:6719) is reached only after every fallible step has already succeeded.

So:

• success == true implies nothing is pending, and control reaches the deref.
• success == false implies either an exception is pending and res was never written (harmless: memset left res.result zeroed, nothing to deref), or res.result.err holds a real JSValue and we throw it.

For what it's worth, the old code was relying on the weaker half of this same contract, just incorrectly: it assumed success == false always meant res.result.err was populated, which is exactly the assumption the three return false paths violate.

CI

Pushed one ci:retrigger (bdb583c) to re-roll. The napi GC test recovered, and a different, also-unrelated set of lanes went red instead:

• darwin 26 aarch64 - test-bun ×2: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. Pure agent infrastructure, no test ran.
• bun-install > should handle --cwd on 2019 x64-baseline. Package manager test; the same file was Buildkite's flaky annotation on the previous build.
• spawn.test.ts timeout on 2019 x64-baseline, recovered on retry.

A different failure set on each run, on lanes this diff does not touch, is flake rather than regression. Every lane that actually runs the new tests is green, including debian 13 x64-asan. Not going to keep re-rolling; the diff is ready for a human look.

[robobun]

One more datapoint on the res.result.value deref question above, this time empirical rather than by inspection.

test/cli/run/require-cache.test.ts has a "files transpiled and loaded don't leak file paths" fixture that does 10,000 require() round trips and compares RSS. If moduleLoaderResolve were skipping the res.result.value.deref() on any reachable path, that fixture would leak one resolved path per import and blow up. Running it against both trees, same build config, only src/ differing:

src/	leaked
main	70 MB
this branch	68 MB

No delta beyond noise, and if anything slightly lower. (Both "fail" locally for an unrelated reason: cjs-fixture-leak-small.js detects ASAN with process.execPath.includes("bun-asan"), which is false for bun bd's bun-debug binary, so the ASAN-widened 320 MB threshold doesn't apply and ASAN's own quarantine trips the 48 MB one. The file is green on CI, where the ASAN lane's binary is actually named bun-asan. Unrelated to this PR, just flagging it since it makes that file look broken to anyone running it locally with bun bd.)

[robobun]

Rebased onto main (now 0b514f88). The only conflict was in test/js/bun/plugin/plugins.test.ts, where #33409 and this PR both appended a block to the end of the file. Kept both; no logic was changed to resolve it.

Worth calling out since #33409 (Runtime onResolve: return the path unprefixed in the file namespace) lands in the same area: the two changes are orthogonal. #33409 reshapes the success path in plugin_runner_on_resolve_jsc, returning ErrorableString::ok(file_path.into_inner()) for the file namespace. This PR only touches the error paths (a rejected onResolve promise, and moduleLoaderResolve reading res.result.err when a JS exception is already pending). The invariant this PR relies on, that res.success == true implies nothing is pending, is unaffected: #33409's new early return is an ok(...) with no JS call between it and the return.

It also happens to exercise the refcount side of my change for free. #33409's "onResolve can redirect a specifier to a real file in the file namespace" test hands a +1 bun_core::String to ErrorableString::ok via into_inner(), which moduleLoaderResolve then consumes with res.result.value.deref(). That test runs seven resolutions through import(), require(), Bun.resolveSync, and import.meta.resolve, and passes clean under the ASAN debug build.

Both test suites green together on the rebased tree:

bun bd test test/js/bun/plugin/plugins.test.ts test/regression/issue/22199.test.ts
 44 pass
 0 fail

Fail-before still holds after the rebase: 5 of the 6 new tests (all but the synchronous-throw control, which already worked) and the tightened 22199 assertion are red on the released binary.

[claude]

I didn't find any issues, but this restructures the error/success control flow in moduleLoaderResolve — the resolve hook every static import and entry point goes through — and its correctness leans on an FFI invariant established by inspection of the Rust producer, so it's worth a human look.

Extended reasoning...

Overview

Two native changes plus tests:

• src/jsc/bindings/BunPlugin.cpp: In OnResolve::run, a rejected promise from an onResolve callback is now markAsHandled() and its rejection reason thrown as an exception (previously the code clobbered the promise's flags word via setFlags and returned the rejection reason as if it were a { path } result). A pending promise is also markAsHandled() before the existing TypeError is thrown, so a late rejection doesn't reach the unhandled queue.
• src/jsc/bindings/ZigGlobalObject.cpp: moduleLoaderResolve now zero-initializes ErrorableString res via memset, declares the throw scope before the FFI call to Zig__GlobalObject__resolve, and only reads res.result.err when !res.success && !scope.exception(). The success path was flattened from if (res.success) { … } else { … } to early-return-on-exception followed by an unconditional deref of res.result.value.
• Six new subprocess tests in test/js/bun/plugin/plugins.test.ts covering async/sync throws, non-Error throws, require(), entry-point resolution, and a late-settling pending promise; test/regression/issue/22199.test.ts tightened to assert stdout is empty.

Security risks

None identified. The change narrows behavior on the error path (propagates the plugin's error instead of leaking it as an unhandled rejection or reading uninitialized stack). No new user-controlled input reaches native code that wasn't already reaching it.

Level of scrutiny

High. moduleLoaderResolve is the JSC GlobalObjectMethodTable hook for static import and entry-point resolution — every module load goes through it. The refactor changes throw-scope placement across an FFI boundary and makes the success-path deref of res.result.value unconditional, relying on the invariant that res.success == true never coincides with a pending exception. The author established that invariant by enumerating the return paths of resolve_hook in src/runtime/jsc_hooks.rs (see the table in the PR thread) and empirically via the require-cache leak fixture, but it is still an FFI contract argued by inspection rather than enforced by a runtime check. JSC exception-scope semantics and cross-language ownership are exactly the areas Bun reviewers scrutinize most closely.

Other factors

• The bug hunting system found no issues.
• The new shape mirrors the sibling moduleLoaderImportModule (same memset + scope.exception() guard pattern), which is reassuring.
• Test coverage is thorough and each test is designed to fail for the right reason (subprocess prints unhandled: [] from process.on('exit'), so absence of an unhandled rejection is a positive assertion).
• CI is green modulo known flakes on unrelated lanes (napi GC timing, darwin artifact-download timeouts), which the author analyzed in detail.
• The author's own last comment says the diff is ready for a human look, which matches my read.

[robobun]

The red lanes on build 68814 are not from this PR, and the Windows one is not flake either. Writing it down so nobody has to re-derive it.

windows 2019 x64-baseline → cookie-map.test.ts. Three deterministic assertion failures in describe("delete with prefixed cookie names"), identical across all four retries:

- "__Host-id=; Path=/; Expires=Fri, 1 Jan 1970 00:00:00 -0000; Secure; SameSite=Lax"
+ "__Host-id=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; SameSite=Lax"

This reproduces on plain main, and on Linux too, so it is neither mine nor Windows-specific. It is a semantic conflict between two PRs that each merged cleanly:

• Hardening round 11: input validation, bounds checks, lifetimes #33072 added the delete with prefixed cookie names block with assertions baked against the old Expires format.
• Fix Bun.Cookie Expires to emit an IMF-fixdate #32926 switched Expires to IMF-fixdate and updated cookie-map.test.ts, but only by two lines. It was branched before Hardening round 11: input validation, bounds checks, lifetimes #33072 landed, so its sweep for old-format assertions could not see the new block.

The emitted value is the correct one (RFC 7231 IMF-fixdate, and new Date(0).toUTCString() is Thu, 01 Jan 1970 00:00:00 GMT; the old string had the wrong weekday, a one-digit day, and -0000 instead of GMT). Already being fixed in #33425 and #33424, so I am not touching it here. Happy to rebase once one of those lands.

darwin 26 aarch64 x2. buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. Agent infrastructure, no test executed. Same failure as on the previous build.

For the record, this branch has now had three CI runs with three disjoint failure sets (napi GC timing, bun-install --cwd, and now a main breakage plus artifact-download timeouts), none of which touch module resolution or plugins. git diff origin/main HEAD is four files: BunPlugin.cpp, ZigGlobalObject.cpp, and the two test files.

[claude]

No issues found, but this restructures exception-scope placement and control flow in moduleLoaderResolve — the hot path for every static import and entry-point resolution — so it's worth a maintainer's eyes on the FFI contract with resolve_hook.

Extended reasoning...

Overview

Two native changes plus tests:

• BunPlugin.cpp: in OnResolve::run, a rejected promise from a plugin callback is now markAsHandled() + throwException(...) instead of the previous setFlags(Fulfilled) + return-result-as-value (which both leaked an unhandled rejection and handed the error object back to the resolver as if it were a {path} result). A still-pending promise is also marked handled before the existing TypeError is thrown.
• ZigGlobalObject.cpp: moduleLoaderResolve now memsets its ErrorableString, declares the throw scope before the Zig__GlobalObject__resolve FFI call, and only reads res.result.err when !res.success && !scope.exception(), then RETURN_IF_EXCEPTION. The success path (identifier construction, res.result.value.deref(), queryString.deref()) is preserved verbatim, just de-nested.
• Six new subprocess tests covering async/sync throws, non-Error throws, require(), entry-point resolution, and a late-settling pending promise; plus a tightened assertion in 22199.test.ts that was previously satisfied by the leaked unhandled rejection.

I verified the new shape matches the sibling moduleLoaderImportModule (which already does memset(&resolved, 0, ...) with a scope declared before the resolve call).

Security risks

None identified. The change strictly narrows behavior: a previously-uninitialized stack read is eliminated, and plugin errors that were being silently misrouted now propagate as thrown exceptions. No new user-controlled data reaches a trust boundary.

Level of scrutiny

High. moduleLoaderResolve is on the hot path for every static import and the entry point itself; the correctness of the refactored flow depends on a cross-FFI invariant with resolve_hook in src/runtime/jsc_hooks.rs (that res.success == true never coexists with a pending exception, and that the three return false-without-writing-res paths always leave one pending). The PR thread lays out that argument carefully and backs it with an RSS leak comparison and ASAN runs, but this is exactly the kind of contract a maintainer who owns that boundary should confirm.

Other factors

• No CODEOWNERS coverage on the touched files.
• Bug-hunting pass found nothing.
• CI failures across three runs are documented in-thread as unrelated (napi GC flake, artifact-download timeouts, and a pre-existing cookie-map.test.ts breakage on main fixed by #33425).
• Test coverage is thorough and follows the harness conventions (subprocess isolation, unhandled: [] as a positive assertion, concurrent pipe drains).

[robobun]

Build 68982 (the rebased sha) is red, but no test ran. It is agent-pool starvation, not a failure.

jobs: 19 passed, 250 waiting_failed, 17 expired, 0 failed
annotations: none

All 17 expired jobs are build-* compile steps with started: never and exit: null (darwin x64 - build-bun, linux x64 - build-cpp, windows x64 - build-rust, and so on). They sat in the queue until expiry without ever getting an agent, and the 250 downstream test jobs cascaded to waiting_failed. Zero tests executed, which is why there is not a single annotation.

Since this branch has now had four builds and none of them was a clean run, here is the part that actually matters. Checking every annotation across all four:

build	error annotations
68702	test/napi/napi.test.ts (GC-timing flake, 22 of last 40 builds)
68734	test/cli/install/bun-install.test.ts (flake, 13 of last 30 builds)
68814	test/js/bun/cookie/cookie-map.test.ts (main breakage, fixed by #33425), test/js/bun/terminal/terminal.test.ts (PTY timeout)
68982	none, nothing ran

plugins.test.ts, 22199.test.ts, and onResolve failures appear in zero annotations across all four builds. The new tests have passed on every lane that ever executed them: linux x64 and aarch64, musl, baseline, x64-asan, darwin, and windows. The red has never once been this diff.

My one re-roll is spent (the ci: retrigger has since been dropped by the rebases), and re-pushing would not help while the agent pool is saturated. A maintainer can hit Rebuild on 68982 for free if a green board is wanted before merging.

[robobun]

FYI #38273 needs the moduleLoaderResolve half of this (zero-initialised res, keep the pending exception) because it makes the test runner report direct-load resolve failures, which otherwise reads the uninitialised error for a throwing onResolve; it carries those two lines and a test for the bun --preload ./plugin.js ./main.js segfault from this description, but not the BunPlugin.cpp change, which stays this PR's. Whichever lands second just drops the duplicate hunk in ZigGlobalObject.cpp.