Skip to content

Report unhandled early rejections from fetch(), Bun.write(), server.fetch() and Bun.resolve() - #41796

Merged
Jarred-Sumner merged 8 commits into
mainfrom
robobun/825bd512/track-early-rejections
Sep 7, 2026
Merged

Jarred-Sumner merged 8 commits into
mainfrom
robobun/825bd512/track-early-rejections

Conversation

@robobun

@robobun robobun commented Sep 7, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • A promise that native code returns already rejected is never reported. bun -e 'fetch()' prints nothing and exits 0: no unhandledRejection event, and bun test passes a test that leaks one. Node reports the same call and exits 1.
  • The cause is JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm (C++ JSC__JSPromise__rejectedPromiseValue). It set the Rejected flag and result slot directly and skipped promiseRejectionTracker. 52 sites used it: fetch() validation (19), Bun.write() and S3 write fast paths (21), server.fetch() (7), Bun.resolve(), subprocess.exited, errored ByteStream reads, JSPromise::wrap_value.
  • Six of those sites rejected with the raw JSC::Exception cell, so .catch() received a value with no message. server.fetch() with a bad body returned with the exception still pending.

Fix

  • Build these promises with JSPromise::rejected_promise(..).to_js(), which goes through JSC::JSPromise::rejectedPromise and the tracker, like Promise.reject().
  • Add JSPromise::rejected_promise_with_caught_exception for the caught-exception sites: create() plus the existing reject(Err(err)), which unwraps the exception and propagates a termination.
  • Delete the helper and the C++ function. Nothing can create an untracked rejected promise anymore.
  • Verified: 44 new cases in fetch-args.test.ts, bun-write.test.js, bun-serve-fetch-invalid-args.test.ts, resolve-error.test.ts fail on 1.4.3 and pass here. Suites listed in Notes. Self-reviewed: 14 concerns probed, 0 survived.

Background

Notes
  • Reproduction on bun 1.4.3 and canary d316760e8:
    let fired = 0; process.on("unhandledRejection", () => fired++);
    fetch(); fetch("http://[bad"); Bun.resolve("no-such-pkg-zz", "/");
    Promise.reject(new Error("control"));
    await Bun.sleep(200);
    console.log(fired); // 1 before, 4 after
    Same for fetch("gopher://x/"), fetch(url, { signal: AbortSignal.abort() }), fetch(url, { body }) with GET, a consumed Request, server.fetch() on a server without a fetch handler, Bun.write(dir, "x"). A late .catch() on any of them emitted rejectionHandled with no unhandledRejection before it; with this change the pair is emitted in order, as for any other promise.
  • test/js/bun/http/fetch-file-upload.test.ts ("missing file throws the expected error") creates 1000 rejected fetch() promises in a loop and asserted each with expect(async () => await resp).toThrow(). toThrow() drains the rejections still pending when it runs, so once the promises are tracked the first assertion reported the other 999. It now uses expect(resp).rejects.toThrow(), and yields one event loop turn before the final Bun.gc(true) because the tracker keeps the promises alive until the end of the tick.
  • pipe_readable_stream_to_blob already marks the stream's own promise handled before it forwards the reason, so only the promise handed to the caller is reported.
  • Native consumers of the converted promises either return the value to JS, resolve another promise with it (Image.rs), or read it with unwrap(MarkHandled) (blob/write_file.rs). None produces a second report. The C++ buffered-read fast path chains a reaction onto the ByteStream promise, so that site is not observable from JS.
  • src/js/internal/debugger.ts calls Bun.write(Bun.stderr, ...) without awaiting. If stderr is closed, that failed write is now reported like any other un-awaited failed write.
  • Also ran with the debug build: fetch-file-upload.test.ts, util/fuzzy-wuzzy.test.ts (calls every Bun.* function with no arguments), web/fetch/body.test.ts, web/fetch/fetch.test.ts, spawn.test.ts -t exited, the rest of bun-write.test.js and resolve-error.test.ts. The only failures are ones the unmodified test files show in this environment too (5 s timeouts under ASAN, root ignoring chmod 000, no outbound network).

…friends

The promises these APIs return when they fail before doing any work were
built with JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm,
which never registers the rejection with the promise rejection tracker.
An unhandled one was therefore silently dropped and the process exited 0.
Build them with JSPromise::rejected_promise instead, like the rest of the
runtime does.

Sites that rejected with a caught exception used to hand out the
JSC::Exception cell itself; they now reject with the thrown value via
JSPromise::rejected_promise_with_caught_exception. server.fetch() also
takes the exception thrown while converting the body off the VM instead
of returning a promise with it still pending.

Bun__resolve, another user of the helper, had no callers and is removed.
Every early exit in fetch() (missing or blank URL, unsupported scheme,
unresolvable blob: URL, GET with a body, pre-aborted signal, unreadable
Bun.file() body, s3 signing errors, exceptions thrown while converting the
arguments, ...) built its rejected promise with
JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm,
which never tells the promise rejection tracker about the rejection. When
the caller attached no handler, nothing was printed, unhandledRejection
listeners never ran, and the process exited 0.

Build those promises with JSPromise::rejected_promise instead, the same
helper Body.rs, streams.rs and the s3 client already use, so an unhandled
early rejection is reported like any other rejected promise.
expect(fn).toThrow() reports every rejection that is still pending when it
is called. Now that fetch() registers its early rejections with the VM, the
1000 promises this test creates before asserting on them were reported as
unhandled. expect(promise).rejects marks the promise handled when the
assertion runs, which is what the test meant.
With the untracked promise, attaching a handler still reached the tracker's
handled path, so process emitted rejectionHandled (and
PromiseRejectionHandledWarning under --unhandled-rejections=warn) for a
rejection that was never reported as unhandled.
The tracker holds the 1000 rejected promises until the end of the tick, so
the trailing Bun.gc(true) was no longer collecting anything. Yield one turn
of the event loop first; a microtask is not enough because the list is
drained after the microtask queue.
…otifying_vm and JSC__JSPromise__rejectedPromiseValue

Every caller now builds its rejected promise through JSPromise::rejected_promise,
which registers the rejection with the promise rejection tracker. Nothing is
left that can create an untracked rejected promise.
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file.

Or wait 4 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: ffbee760-e753-4103-b36d-6851688ecfa3

📥 Commits

Reviewing files that changed from the base of the PR and between d316760 and dc97e44.

📒 Files selected for processing (14)
  • src/jsc/JSPromise.rs
  • src/jsc/bindings/bindings.cpp
  • src/jsc/bindings/headers.h
  • src/runtime/api/BunObject.rs
  • src/runtime/api/bun/subprocess.rs
  • src/runtime/server/server_body.rs
  • src/runtime/webcore/Blob.rs
  • src/runtime/webcore/ByteStream.rs
  • src/runtime/webcore/fetch.rs
  • test/js/bun/http/bun-serve-fetch-invalid-args.test.ts
  • test/js/bun/http/fetch-file-upload.test.ts
  • test/js/bun/io/bun-write.test.js
  • test/js/bun/resolve/resolve-error.test.ts
  • test/js/web/fetch/fetch-args.test.ts

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

@github-actions github-actions Bot added the claude label Sep 7, 2026
@robobun

robobun commented Sep 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on bun 1.4.3 and on main (d316760e8): bun -e 'fetch()', fetch("http://[bad"), fetch("gopher://x/"), Bun.resolve("no-such-pkg-zz", "/"), server.fetch() on a server without a fetch handler and Bun.write(dir, "x") all print nothing and exit 0, and a process.on("unhandledRejection") listener never runs. With this branch each one is delivered to unhandledRejection (or reported with exit code 1), and a handled one stays silent.

The 44 new test cases in test/js/web/fetch/fetch-args.test.ts, test/js/bun/io/bun-write.test.js, test/js/bun/http/bun-serve-fetch-invalid-args.test.ts and test/js/bun/resolve/resolve-error.test.ts fail on 1.4.3 and pass with the debug build of this branch.

CI (build 111974): every lane is green except two files on the x64-asan lane that this diff does not touch. test/js/node/test/parallel/test-crypto-dh-leak.js fails the same way on main. test/js/third_party/grpc-js/test-client.test.ts uses a 100 ms connect deadline and ran in the parallel batch; it fails identically with a debug build of main in a loaded container, and three other grpc-js files are already excluded from that batch for the same reason. Both are reported separately. Ready for review.

This PR replaces #37434 and #37474, which are closed in its favor.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it changes user-observable behavior across ~30 hot-path early-exit sites (fetch, Bun.write, server.fetch, S3, subprocess.exited) — each now goes through the rejection tracker and can enter JS where the old helper could not — a human sign-off on the behavioral shift is still worthwhile.

What was reviewed:

  • All migrated call sites: plain-reason sites go through rejected_promise(..).to_js(); caught-exception sites use the new rejected_promise_with_caught_exception, which clears the pending exception and unwraps the JSC::Exception cell before rejecting.
  • server.fetch invalid-body path no longer returns with an exception still pending on the VM, and now rejects with the user's thrown value instead of a generic message.
  • The fetch-args.test.ts "Bun.file() body that is a directory" case was examined for Windows uv_fs_open(O_RDONLY) behavior on directories and ruled out.
  • New tests use bunExe/bunEnv, port: 0, tempDir, drain pipes concurrently, and assert stderr before exit code; the bun-write suite branches per-platform on the expected errno.
Extended reasoning...

Overview

This PR deletes the deprecated JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm (and its C++ backing JSC__JSPromise__rejectedPromiseValue), which wrote the Rejected flag and result slot directly and skipped promiseRejectionTracker. About 30 call sites across fetch.rs, Blob.rs (Bun.write / S3 write fast paths), server_body.rs (server.fetch), BunObject.rs (Bun.resolve), subprocess.rs (.exited), and ByteStream.rs are migrated to either JSPromise::rejected_promise(..).to_js() for plain reason values or a new JSPromise::rejected_promise_with_caught_exception(global, err) for sites that hold a JsError proving a pending exception. JSPromise::wrap now uses value.to_error() and the notifying path. server.fetch with a body that throws during conversion now rejects with the actual thrown value and no longer leaves the exception pending on the VM. Roughly 390 lines of new subprocess-spawning tests across four existing test files verify that each early-exit path (a) reports as an unhandled rejection with exit 1 and the expected message, (b) delivers the same promise instance to process.on('unhandledRejection'), and (c) stays silent when .catch() is attached.

Security risks

None identified. The change swaps one promise-construction path for another that goes through JSC's own JSPromise::rejectedPromise, which is what Promise.reject() already uses. No new input parsing, no auth/crypto/permissions surface. The one semantic subtlety is that rejectedPromise invokes globalObject->promiseRejectionTracker(...), which in Bun enqueues the promise for later reporting — it does not synchronously call user JS at the rejection site — so the migrated sites do not gain a new reentrancy hazard mid-argument-processing.

Level of scrutiny

Moderate-to-high. The transformation itself is mechanical and the deleted helper was already documented as deprecated with the correct replacement named, so each individual site is low-risk. What raises the bar is scope and observability: this is a deliberate user-visible behavior change (previously-silent code now prints an error and exits 1) across fetch, Bun.write, server.fetch, S3, and subprocess — all high-traffic paths. The PR description itself flags a knock-on in src/js/internal/debugger.ts (an un-awaited Bun.write(Bun.stderr, …) will now report if stderr is closed), which is exactly the class of latent surprise a maintainer should weigh. The change also replaces two prior PRs (#37434, #37474) that were rebased and folded together, which suggests this has had some churn and would benefit from a maintainer confirming the final shape.

Other factors

Test coverage is thorough and follows repo conventions closely: describe.concurrent for independent subprocess suites (with a Windows carve-out in bun-write.test.js), tempDir from harness, port: 0, Promise.all over stdout/stderr/exited, stderr asserted before exit code, and per-platform errno branching where Windows differs (EINVAL for truncating a directory, UV_EBADF accepted via substring). The fetch-file-upload.test.ts change replaces expect(async () => await resp).toThrow() with expect(resp).rejects.toThrow() and adds a single await Bun.sleep(0) before Bun.gc(true) — this is a tick yield to let the tracker release its references before forcing collection, not a condition wait. No CODEOWNERS entries cover the changed paths, and the timeline shows no third-party reviews or objections. The bug hunt exited on a dry streak with no findings; one candidate (Windows EISDIR on the directory-body fetch test) was examined and ruled out.

@Jarred-Sumner
Jarred-Sumner merged commit 85c9f6e into main Sep 7, 2026
10 of 12 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the robobun/825bd512/track-early-rejections branch September 7, 2026 07:09
Jarred-Sumner pushed a commit that referenced this pull request Sep 14, 2026
)

### Problem
- A garbage collection can drop the VM's pending exception. A module
whose evaluation throws then hits `ASSERTION FAILED: exception` in
`JSPromise::rejectWithCaughtException` (assert builds) or segfaults at
`0x8` in `JSPromise::reject <- rejectWithCaughtException <-
CyclicModuleRecord::evaluate <- dynamicImportLoadSettled` (release,
Sentry BUN-4R6E). `Bun.resolve()` hits `panic: A JavaScript exception
was thrown, but it was cleared before it could be read.` in
`rejected_promise_with_caught_exception`.
- Cause: `computeErrorInfoWrapperToString`
(`src/jsc/bindings/FormatStackTraceForJS.cpp`), the `onComputeErrorInfo`
hook, runs in `Heap::runEndPhase` for live Errors whose frames died, and
cleared whatever exception was pending. With concurrent GC it lands
while the mutator is parked at an allocation with an exception pending.

### Fix
- Wrap the hook body in JSC's `SuspendExceptionScope` (as
`TypeProfilerLog::processLogEntries` does). The mutator's exception and
trap bit are set aside and restored, so `tryClearException()` only
swallows what the computation itself raised.
- Add `DeferTerminationForAWhile`. A `terminate()` thrown inside the
window survives the clear, and the restore then overwrites it with the
trap bit still set.
- Verified: `error-stack-finalizer-exception.test.ts` is the
deterministic one (`slowPathAllocsBetweenGCs` fixes where the end phase
lands): all three N values abort every run unfixed, all pass fixed, in
about 6s. `dynamic-import-evaluation-error-gc.test.ts`,
`resolve-error.test.ts` and `sourcetextmodule-link-gc.test.ts` cover the
concurrent-GC faces and fail 3/3 unfixed. The terminate test fails 3/3
on a suspend-only build. `test-stream-writable-write-writev-finish.js`
passes under `BUN_JSC_validateExceptionChecks=1`.

### Background
- JSC keeps one pending exception per VM (`VM::m_exception`), mirrored
by a trap bit. A callee that clears it drops the caller's exception.
- `CyclicModuleRecord::evaluate` step 9 sees the module's error as the
pending exception and step 9.d rejects by re-reading the slot.
`Bun.resolve()` allocates its promise before it takes the exception.
- With concurrent GC the end phase runs while the mutator is stopped at
a safepoint, which is any allocation slow path. `ErrorInstance` keeps
weak stack frames. When a frame's code dies, the end phase materializes
the stack string through this hook.

<details><summary>Notes</summary>
Deterministic door (no concurrent collector):
`BUN_JSC_slowPathAllocsBetweenGCs=7 bun -e 'const e = eval("(() =>
Object.assign(new Error(\"c\"), { code: 1 }))")(); throw e'`. Reported
as 5/5 `Segmentation fault at address 0x8` on release 1.4.2, canary and
a release build of main, and 5/5 `ASSERTION FAILED: exception` on an
ASan build. Measured here on a debug+ASAN build of main: N of 3, 4 and 7
abort every run; 1, 2, 5, 6 and >= 8 print the error normally; with the
fix all of 3, 4, 7 print `error: c` and exit 1 (9/9). The eval'd arrow
matters: it makes the Error's frames garbage before the throw
propagates, so the end phase has a stack to materialize.



Reproduction (plain `import()`, no `node:vm`, no plugin), about 4 of 6
runs abort at 200 iterations and 6 of 6 at 300 on the unfixed assert
build:

```js
import { mkdirSync, writeFileSync } from "node:fs";
import { join } from "node:path";
for (let it = 0; it < 300; it++) {
  const d = join(import.meta.dir, "graphs", "g" + it);
  mkdirSync(d, { recursive: true });
  writeFileSync(join(d, "bad.mjs"), `export const x = ${it};\nthrow new Error("boom ${it}");\n`);
  writeFileSync(join(d, "tla.mjs"), `await new Promise(r => setTimeout(r, 0));\nexport const t = ${it};\n`);
  writeFileSync(join(d, "leaf.mjs"), `export const l = ${it};\n`);
  writeFileSync(join(d, "mid.mjs"), `import { l } from "./leaf.mjs";\nimport "./bad.mjs";\nexport const m = l + 1;\n`);
  writeFileSync(join(d, "a.mjs"), `import { m } from "./mid.mjs";\nimport { t } from "./tla.mjs";\nexport const a = m + t;\n`);
  writeFileSync(join(d, "b.mjs"), `import { t } from "./tla.mjs";\nimport "./bad.mjs";\nexport const b = t;\n`);
  writeFileSync(join(d, "c.mjs"), `import { m } from "./mid.mjs";\nexport const c = m;\n`);
  await import(join(d, "a.mjs")).catch(() => {});
  await import(join(d, "b.mjs")).catch(() => {});
  await import(join(d, "c.mjs")).catch(() => {});
  await import(`data:text/javascript,throw new Error('d${it}')`).catch(() => {});
}
```

`import(b)` reaches the already-errored `bad.mjs`, so
`InnerModuleEvaluation` step 2 rethrows the stored error and the step 9
to 9.d window runs again. A data:-URL-only loop does not fire. A single
iteration does not fire.

State at the assert (gdb, conditional break on `exception == 0` at the
assert line): the `Exception*` local captured at step 9 is non-null
while `vm.m_exception` is null. The window between the two is
straight-line C++ (`attachErrorInfo`, `setStatus`, `setEvaluationError`)
with no JS and no microtask checkpoint, so the slot is cleared out of
band. `BUN_JSC_useConcurrentGC=false`: 0/16 aborts (baseline 12/16 on
the `vm.SourceTextModule` reproducer). `collectContinuously` also hides
it (it moves the window).

The `vm.SourceTextModule` reproducer (the first one found) needs
`--smol` to fire reliably and a file-backed fixture (a `-e` script has
no source URL, so the stack materializer takes a different path).

Earlier iteration of this PR compared the pending exception before and
after the computation and cleared only a new one.
`SuspendExceptionScope` replaces that: it is the upstream pattern, it
also restores `m_lastException` and the trap bit, and the computation
runs against a clean slot.

The first reports of this assert named a `Bun.plugin` `onResolve`
recipe. That recipe cannot reach the state (its filter never matched),
and the plugin is not part of the cause.

The first push used `SuspendExceptionScope` alone. CI caught the
termination interaction on the debian x64-asan lane (`worker.test.ts`,
"terminate() while importing every builtin module"): `ASSERTION FAILED:
!!(*scope).exception() ==
vm.traps().needHandling(JSC::VMTraps::NeedExceptionHandling)` in
`TopExceptionScope__exceptionIncludingTraps`. A stress (the vm fixture
inside a Worker terminated mid-run, 20 rounds) reproduced it 6/6 on that
build and 0/6 with the deferral; it is now the third test in the vm
file.

`Bun.resolve()` face (the helper from #41796): `Bun.resolve()` with no
arguments from a fresh closure per iteration, rejection reasons kept in
a 1024-entry ring, awaited one at a time,
`BUN_JSC_collectContinuously=1`. Unfixed main 09bb546 (debug+ASAN):
9/9 panic after 2500 to 8500 iterations, trace `take_exception <-
JSPromise::reject <- rejected_promise_with_caught_exception <-
bun_object::resolve`. With the fix: 5/5 complete 80000 iterations.
Batching 64 calls per tick never fires. Reported on a release build of
main as well (panic within 0.2 s under collectContinuously). The same
helper serves the early rejections of `fetch()`, `Bun.write()` and
`server.fetch()`.

The module-evaluation standalone above also segfaults shipped release
builds on demand under `BUN_JSC_collectContinuously=1` (reported: 1.4.2
4/5, canary 4/5, release main 5/5 at N=400, SEGV at 0x8).

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 4 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/node/vm/sourcetextmodule-link-gc.test.ts,
test/js/bun/resolve/resolve-error.test.ts,
test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts

<!-- robobun:evidence:end -->
usrbinkat pushed a commit to usrbinkat/bun that referenced this pull request Sep 15, 2026
…n-sh#33584)

### Problem
- A garbage collection can drop the VM's pending exception. A module
whose evaluation throws then hits `ASSERTION FAILED: exception` in
`JSPromise::rejectWithCaughtException` (assert builds) or segfaults at
`0x8` in `JSPromise::reject <- rejectWithCaughtException <-
CyclicModuleRecord::evaluate <- dynamicImportLoadSettled` (release,
Sentry BUN-4R6E). `Bun.resolve()` hits `panic: A JavaScript exception
was thrown, but it was cleared before it could be read.` in
`rejected_promise_with_caught_exception`.
- Cause: `computeErrorInfoWrapperToString`
(`src/jsc/bindings/FormatStackTraceForJS.cpp`), the `onComputeErrorInfo`
hook, runs in `Heap::runEndPhase` for live Errors whose frames died, and
cleared whatever exception was pending. With concurrent GC it lands
while the mutator is parked at an allocation with an exception pending.

### Fix
- Wrap the hook body in JSC's `SuspendExceptionScope` (as
`TypeProfilerLog::processLogEntries` does). The mutator's exception and
trap bit are set aside and restored, so `tryClearException()` only
swallows what the computation itself raised.
- Add `DeferTerminationForAWhile`. A `terminate()` thrown inside the
window survives the clear, and the restore then overwrites it with the
trap bit still set.
- Verified: `error-stack-finalizer-exception.test.ts` is the
deterministic one (`slowPathAllocsBetweenGCs` fixes where the end phase
lands): all three N values abort every run unfixed, all pass fixed, in
about 6s. `dynamic-import-evaluation-error-gc.test.ts`,
`resolve-error.test.ts` and `sourcetextmodule-link-gc.test.ts` cover the
concurrent-GC faces and fail 3/3 unfixed. The terminate test fails 3/3
on a suspend-only build. `test-stream-writable-write-writev-finish.js`
passes under `BUN_JSC_validateExceptionChecks=1`.

### Background
- JSC keeps one pending exception per VM (`VM::m_exception`), mirrored
by a trap bit. A callee that clears it drops the caller's exception.
- `CyclicModuleRecord::evaluate` step 9 sees the module's error as the
pending exception and step 9.d rejects by re-reading the slot.
`Bun.resolve()` allocates its promise before it takes the exception.
- With concurrent GC the end phase runs while the mutator is stopped at
a safepoint, which is any allocation slow path. `ErrorInstance` keeps
weak stack frames. When a frame's code dies, the end phase materializes
the stack string through this hook.

<details><summary>Notes</summary>
Deterministic door (no concurrent collector):
`BUN_JSC_slowPathAllocsBetweenGCs=7 bun -e 'const e = eval("(() =>
Object.assign(new Error(\"c\"), { code: 1 }))")(); throw e'`. Reported
as 5/5 `Segmentation fault at address 0x8` on release 1.4.2, canary and
a release build of main, and 5/5 `ASSERTION FAILED: exception` on an
ASan build. Measured here on a debug+ASAN build of main: N of 3, 4 and 7
abort every run; 1, 2, 5, 6 and >= 8 print the error normally; with the
fix all of 3, 4, 7 print `error: c` and exit 1 (9/9). The eval'd arrow
matters: it makes the Error's frames garbage before the throw
propagates, so the end phase has a stack to materialize.



Reproduction (plain `import()`, no `node:vm`, no plugin), about 4 of 6
runs abort at 200 iterations and 6 of 6 at 300 on the unfixed assert
build:

```js
import { mkdirSync, writeFileSync } from "node:fs";
import { join } from "node:path";
for (let it = 0; it < 300; it++) {
  const d = join(import.meta.dir, "graphs", "g" + it);
  mkdirSync(d, { recursive: true });
  writeFileSync(join(d, "bad.mjs"), `export const x = ${it};\nthrow new Error("boom ${it}");\n`);
  writeFileSync(join(d, "tla.mjs"), `await new Promise(r => setTimeout(r, 0));\nexport const t = ${it};\n`);
  writeFileSync(join(d, "leaf.mjs"), `export const l = ${it};\n`);
  writeFileSync(join(d, "mid.mjs"), `import { l } from "./leaf.mjs";\nimport "./bad.mjs";\nexport const m = l + 1;\n`);
  writeFileSync(join(d, "a.mjs"), `import { m } from "./mid.mjs";\nimport { t } from "./tla.mjs";\nexport const a = m + t;\n`);
  writeFileSync(join(d, "b.mjs"), `import { t } from "./tla.mjs";\nimport "./bad.mjs";\nexport const b = t;\n`);
  writeFileSync(join(d, "c.mjs"), `import { m } from "./mid.mjs";\nexport const c = m;\n`);
  await import(join(d, "a.mjs")).catch(() => {});
  await import(join(d, "b.mjs")).catch(() => {});
  await import(join(d, "c.mjs")).catch(() => {});
  await import(`data:text/javascript,throw new Error('d${it}')`).catch(() => {});
}
```

`import(b)` reaches the already-errored `bad.mjs`, so
`InnerModuleEvaluation` step 2 rethrows the stored error and the step 9
to 9.d window runs again. A data:-URL-only loop does not fire. A single
iteration does not fire.

State at the assert (gdb, conditional break on `exception == 0` at the
assert line): the `Exception*` local captured at step 9 is non-null
while `vm.m_exception` is null. The window between the two is
straight-line C++ (`attachErrorInfo`, `setStatus`, `setEvaluationError`)
with no JS and no microtask checkpoint, so the slot is cleared out of
band. `BUN_JSC_useConcurrentGC=false`: 0/16 aborts (baseline 12/16 on
the `vm.SourceTextModule` reproducer). `collectContinuously` also hides
it (it moves the window).

The `vm.SourceTextModule` reproducer (the first one found) needs
`--smol` to fire reliably and a file-backed fixture (a `-e` script has
no source URL, so the stack materializer takes a different path).

Earlier iteration of this PR compared the pending exception before and
after the computation and cleared only a new one.
`SuspendExceptionScope` replaces that: it is the upstream pattern, it
also restores `m_lastException` and the trap bit, and the computation
runs against a clean slot.

The first reports of this assert named a `Bun.plugin` `onResolve`
recipe. That recipe cannot reach the state (its filter never matched),
and the plugin is not part of the cause.

The first push used `SuspendExceptionScope` alone. CI caught the
termination interaction on the debian x64-asan lane (`worker.test.ts`,
"terminate() while importing every builtin module"): `ASSERTION FAILED:
!!(*scope).exception() ==
vm.traps().needHandling(JSC::VMTraps::NeedExceptionHandling)` in
`TopExceptionScope__exceptionIncludingTraps`. A stress (the vm fixture
inside a Worker terminated mid-run, 20 rounds) reproduced it 6/6 on that
build and 0/6 with the deferral; it is now the third test in the vm
file.

`Bun.resolve()` face (the helper from oven-sh#41796): `Bun.resolve()` with no
arguments from a fresh closure per iteration, rejection reasons kept in
a 1024-entry ring, awaited one at a time,
`BUN_JSC_collectContinuously=1`. Unfixed main 09bb546 (debug+ASAN):
9/9 panic after 2500 to 8500 iterations, trace `take_exception <-
JSPromise::reject <- rejected_promise_with_caught_exception <-
bun_object::resolve`. With the fix: 5/5 complete 80000 iterations.
Batching 64 calls per tick never fires. Reported on a release build of
main as well (panic within 0.2 s under collectContinuously). The same
helper serves the early rejections of `fetch()`, `Bun.write()` and
`server.fetch()`.

The module-evaluation standalone above also segfaults shipped release
builds on demand under `BUN_JSC_collectContinuously=1` (reported: 1.4.2
4/5, canary 4/5, release main 5/5 at N=400, SEGV at 0x8).

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 4 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/node/vm/sourcetextmodule-link-gc.test.ts,
test/js/bun/resolve/resolve-error.test.ts,
test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts

<!-- robobun:evidence:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants