Detect unsettled top-level await in entry-point loading instead of hanging - #30551
Detect unsettled top-level await in entry-point loading instead of hanging#30551robobun wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDetects unsettled top-level await across module evaluation, test execution, and script shutdown by adding a VM wait helper that avoids busy-spins, explicit pending-promise handling in the test runner, runtime shutdown detection that warns and sets exit code 13, and regression tests. ChangesUnsettled top-level await detection
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
|
Updated 3:58 AM PT - Aug 15th, 2026
⏳ @robobun, your commit bb28fba is still building in
|
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/jsc/VirtualMachine.zig (1)
2290-2295:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn pending preload promises instead of continuing startup.
After
waitForModulePromise()returns, this only propagates.rejected. If a preload's top-level await is still.pending,loadPreloads()falls through and continues with later preloads or the main entrypoint, even though the helper's contract says callers must handle pending on return.Suggested fix
} else { this.eventLoop().performGC(); this.waitForModulePromise(promise); } - if (promise.status() == .rejected) - return promise; + switch (promise.status()) { + .fulfilled => {}, + .rejected, .pending => return promise, + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/jsc/VirtualMachine.zig` around lines 2290 - 2295, After calling waitForModulePromise() in loadPreloads(), the code only returns when promise.status() == .rejected; change the control flow to return the promise whenever its status is not .fulfilled (i.e., handle both .pending and .rejected). Locate the promise handling after waitForModulePromise() in VirtualMachine.loadPreloads (references: waitForModulePromise, promise.status()) and update the check to return the promise if status != .fulfilled so callers receive pending preload promises instead of continuing startup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/jsc/VirtualMachine.zig`:
- Around line 1054-1065: Summary: waitForModulePromise currently returns early
using isEventLoopAlive(), which can be false due to unrelated
unhandled_error_counter and incorrectly abort module settlement. Fix: In
waitForModulePromise, replace the isEventLoopAlive() bail-out with a direct
"loop has work" check on the event loop internals (i.e., inspect the event
loop's pending work counters such as timers/refs/tasks/microtasks) instead of
calling isEventLoopAlive(); ensure you use this.eventLoop().tick()/autoTick()
loop semantics and only break when there is truly no pending work to drive the
promise (so unhandled_error_counter does not cause an early return).
---
Outside diff comments:
In `@src/jsc/VirtualMachine.zig`:
- Around line 2290-2295: After calling waitForModulePromise() in loadPreloads(),
the code only returns when promise.status() == .rejected; change the control
flow to return the promise whenever its status is not .fulfilled (i.e., handle
both .pending and .rejected). Locate the promise handling after
waitForModulePromise() in VirtualMachine.loadPreloads (references:
waitForModulePromise, promise.status()) and update the check to return the
promise if status != .fulfilled so callers receive pending preload promises
instead of continuing startup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 29e9b59a-305d-4dc3-83ef-d9b4f2980353
📒 Files selected for processing (4)
src/bun.js.zigsrc/cli/test_command.zigsrc/jsc/VirtualMachine.zigtest/regression/issue/19049/19049.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/regression/issue/19049/19049.test.ts`:
- Around line 11-25: Change the test suite to run sequentially instead of
concurrently by replacing describe.concurrent with describe (so the child
processes spawned by run are not started in parallel), and add an explicit outer
test timeout greater than the child timeout (run's timeout = 15_000) — e.g., set
the suite or tests to 20_000 ms via your test framework's timeout API
(jest.setTimeout or test.timeout) so the overall test has a clear upper bound.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02cae021-cb75-4bc4-b693-c92761e8eab4
📒 Files selected for processing (3)
src/bun.js.zigsrc/jsc/VirtualMachine.zigtest/regression/issue/19049/19049.test.ts
Status: waiting on a maintainer decision (see below)Branch is rebased onto main at 0d48eb1 as a single commit; all earlier review findings are addressed (latest round: byte-split source lint, stale loader docs, comment trimming; Blocking question, laid out with evidence in this comment: four tests merged to main after this PR was written (#34478 x3 in Everything else: |
|
Should we close the duplicates? e.g. 29549 |
|
@alii re: closing duplicates — here's how they compare:
This PR is the only one that covers |
|
How does this compare to 29739? What are the best implementations? |
|
@alii comparing this PR to #29739 piece by piece:
tl;dr the best implementation is this PR's scope/liveness + #29739's
Happy to fold #29739's |
|
@robobun can you make this PR be the best of both please |
|
@alii done in 6f3835a — this PR is now the combined best-of-both: From #29739:
Kept from here:
10 regression tests (added one for the stalled-module naming), all pass. The Node parallel tests (test-exception-handler2, test-process-exception-capture*, process.test.js exitCode) still pass. #29739 can be closed once this lands — everything in its diff is now here, plus the |
6f3835a to
2344a0c
Compare
|
This also resolves #31501 (another report of I independently reproduced #31501 and verified the
Confirmed the beforeExit-can-resolve-the-await handling here (the |
093cf1e to
1e62f44
Compare
|
Triage of the open PRs for this behavior; this is the one being kept. Nothing has landed on main. Checked at 165dc9f: Why this PR: it carries everything from #29739 (whose Before it can merge:
|
eda73e3 to
bb28fba
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /// Raw liveness inputs shared by the predicates below: active platform | ||
| /// handles, tasks (including ones yielded to the next iteration), | ||
| /// concurrent tasks, and pending refs. `concurrent_tasks` closes a narrow | ||
| /// race where another thread pushed after `tick()`'s drain; `yield_tasks` | ||
| /// are only promoted by the next `auto_tick()`. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// Whether anything remains that could wake the loop and settle a | ||
| /// pending module promise: [`is_event_loop_alive`](Self::is_event_loop_alive) | ||
| /// minus its `unhandled_error_counter` short-circuit. | ||
| /// | ||
| /// That counter persists across files in `bun test`, so an unhandled | ||
| /// rejection in file A would otherwise make every later file with | ||
| /// ordinary async TLA bail in | ||
| /// [`wait_for_module_promise`](Self::wait_for_module_promise) with a | ||
| /// spurious "never resolved" even though ref'd work would settle it. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// `loadPreloads()` — runs `--preload` scripts. Returns the first | ||
| /// non-fulfilled preload promise (rejected, or still pending with an | ||
| /// idle event loop — unsettled TLA) if any, else null. Errors propagate |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// Wait for a module's top-level-await promise to settle. | ||
| /// | ||
| /// Unlike [`wait_for_promise`](Self::wait_for_promise), this breaks out | ||
| /// when nothing remains that could settle the promise (no active | ||
| /// handles/refs, no pending tasks or immediates). Without this, a module | ||
| /// containing e.g. `await new Promise(() => {})` makes the loader | ||
| /// busy-spin forever in `tick()` + `auto_tick()` once the last timer | ||
| /// fires and `auto_tick` degrades to a non-blocking `tickWithoutIdle`. | ||
| /// | ||
| /// Callers must handle a still-`Pending` status on return: for `bun run` | ||
| /// this matches Node's exit-code-13 behavior; for `bun test` the file is | ||
| /// reported as a load error. `Err` has the same meaning as for | ||
| /// [`wait_for_promise`](Self::wait_for_promise): the VM can no longer run | ||
| /// the script that would settle the promise. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // After draining tasks + microtasks, if nothing could still wake | ||
| // the loop the promise can never settle. Break instead of | ||
| // busy-spinning. See `has_pending_loop_work` for why this is NOT | ||
| // `is_event_loop_alive()`. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// Print a warning naming the module(s) whose body is suspended on its | ||
| /// own top-level await. Walks the JSC module registry for | ||
| /// `CyclicModuleRecord`s in `EvaluatingAsync` with `hasTLA` and no | ||
| /// pending async dependencies; falls back to the entry path if nothing | ||
| /// is found (e.g. eval mode). Matches Node's "Detected unsettled | ||
| /// top-level await at <path>". |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // One warning line per stalled module (the C++ helper joins multiple | ||
| // specifiers with '\n'), matching Node's one-warning-per-module shape. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // Find module specifiers whose body is suspended on its own top-level await | ||
| // (status EvaluatingAsync, syntactically has TLA, and isn't waiting on any | ||
| // async dependency). Used to point the unsettled-TLA warning at the actual | ||
| // stalled module rather than the entry path. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // Skip modules that are EvaluatingAsync only because they're waiting | ||
| // on a dependency — the dependency is the actual culprit. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // Main loop + beforeExit, with handling for a still-pending entry | ||
| // module promise (top-level await that `wait_for_module_promise` | ||
| // bailed on because the loop was idle). A `beforeExit` handler | ||
| // may resolve the stuck await, and the resumed body may schedule | ||
| // more work and then suspend again — `continue` re-enters the | ||
| // whole cycle so Node's "beforeExit fires every time the loop | ||
| // drains" semantics hold for any number of rounds. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // A bare `resolve()` inside the handler queues a JSC | ||
| // microtask that `is_event_loop_alive()` doesn't | ||
| // count. Drain it so the module body resumes. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // When the entry module itself never settled (unsettled top-level | ||
| // await), `entry_point_result.value` holds the module pipeline's | ||
| // internal promise rather than the evaluated expression's value, | ||
| // so `--print` would emit a bogus `Promise { <pending> }` to | ||
| // stdout. Skip printing; the warning + exit code 13 below report | ||
| // it. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // The entry module's top-level await never settled and nothing | ||
| // is keeping the event loop alive. Match Node.js: warn + exit 13. | ||
| // Late `.rejected` (the resumed body threw) is reported here | ||
| // because the module-loader pipeline promise is pre-marked | ||
| // handled and never reaches `handle_rejected_promises()`; gate | ||
| // on `pending_internal_promise_reported_at` so an initial-load | ||
| // rejection already handled above isn't double-reported. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // `vm.global()` is `&'static`, decoupled from `vm`'s | ||
| // borrow so `uncaught_exception(&mut self, ..)` can reborrow. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// `--bail` threshold reached on a file that failed to load (rejected, or | ||
| /// a top-level await that can never settle): print the summary, write the | ||
| /// JUnit/timings reports if requested, release per-file runner state, and | ||
| /// exit. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // Top-level await never settled and the event loop drained. | ||
| // Report it as a load error for this file and move on | ||
| // instead of hanging forever. If the pending promise came | ||
| // from a --preload, `load_preloads` put a "Top-level await | ||
| // in preload ..." entry in `vm.log` — print that first so | ||
| // the user sees which file is actually stuck. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// `loadPreloads()` — runs `--preload` scripts. Returns the first | ||
| /// non-fulfilled preload promise (rejected, or still pending with an idle | ||
| /// event loop — unsettled TLA) if any, else null. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // Propagate both rejection and still-pending (unsettled TLA on an | ||
| // idle loop) so callers report it instead of silently continuing to | ||
| // later preloads / the entry point. For `Pending`, name the preload | ||
| // here — downstream reporting only knows the entry path. A wait cut | ||
| // short by a stop request also returns `Pending`; that is not an | ||
| // unsettled await, so it falls through to the stop check below. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
|
Rebased onto main at bb28fba (previously eda73e3, 943 commits). This one was not mechanical; summary of what changed and why: Conflicts
Drift found while reviewing the auto-merged parts
Verification on the rebased branch
|
| // busy-spinning. See `has_pending_loop_work` for why this is NOT | ||
| // `is_event_loop_alive()`. | ||
| if !self.has_pending_loop_work() { | ||
| break; | ||
| } | ||
| self.auto_tick(); |
There was a problem hiding this comment.
🔴 The three unref() + .exited with nothing else ref'd tests at test/js/bun/spawn/spawn.test.ts:810-844 still assert {stdout: "resolved\n", stderr: "", exitCode: 0}, but with wait_for_module_promise a top-level await p.exited on an unref'd subprocess now becomes exit 13 with the unsettled-TLA warning on stderr. The 2026-08-13 robobun triage on this PR explicitly flagged that #36051/#36052 had to update spawn.test.ts for this behavior change; per REVIEW.md ("grep the suite for assertions on the old behavior and update them in the same PR") those tests need updating here — either to expect exit 13 (matching Node) or to keep something ref'd until .exited resolves.
Extended reasoning...
What the bug is
wait_for_module_promise (this PR) breaks out of the entry-point wait loop when has_pending_loop_work() is false — without ever calling auto_tick() to poll the platform loop. That is the intended Node-compatible behavior (a top-level await on a promise that nothing ref'd can settle → exit 13), but it changes the observable outcome for the three existing tests in test/js/bun/spawn/spawn.test.ts:810-844 (describe("unref() + .exited with nothing else ref'd (Windows)")), and this PR does not touch that file.
The specific code path
Each of those tests spawns bun -e with a body of the form:
const opts = { cmd: [bunExe(), "-e", ""], stdio: ["ignore","ignore","ignore"] };
const p = Bun.spawn(opts); p.unref(); await p.exited;
console.log("resolved");and asserts {stdout: "resolved\n", stderr: "", exitCode: 0, signalCode: null} (spawn.test.ts:837-842).
p.unref() → disable_keeping_event_loop_alive() (src/spawn/process.rs) unrefs the pidfd poll (POSIX) / uv_process_t (Windows) from the platform loop's active count. With stdio all-ignore, nothing else is ref'd.
Pre-PR: load_entry_point called wait_for_promise, which spun tick() + auto_tick(); auto_tick() still polled the platform loop each iteration (spawn.test.ts:811-813 documents this: "us_loop_pump now forces one non-blocking iteration"), so the child's exit packet was eventually dequeued, .exited resolved, and the module completed with exit 0.
Post-PR: load_entry_point calls wait_for_module_promise (VirtualMachine.rs:2931). It runs tick() (drains microtasks — does not poll the platform loop) → status still Pending → has_pending_loop_work(): platform_loop_opt().is_active() is false (only unref'd handles), no tasks/immediates/pending refs → break before ever reaching auto_tick(). Control returns to run_command.rs; the main while vm.is_event_loop_alive() loop's predicate is also false (same inputs minus the error counter), on_before_exit() has no listeners, the post-tick() re-check is still false → falls through to the new .Pending arm → report_unsettled_top_level_await() on stderr + exit_code = 13. The inner bun -e "" child hasn't exited yet under debug/ASAN startup latency, so its exit is never observed.
Why existing code doesn't prevent it
Nothing in this PR updates spawn.test.ts. The PR's changed-files list is 6 files (VirtualMachine.rs, ZigGlobalObject.cpp, run_command.rs, test_command.rs, jsc_hooks.rs, 19049.test.ts); spawn.test.ts is absent. The evidence gate only ran the new regression file. And the tests are not platform-gated — despite "(Windows)" in the describe title, they run on all platforms.
The PR's own timeline explicitly warned about this. The 2026-08-13 robobun triage comment says:
#36051 and #36052 both found that a top-level
awaiton an unref'd subprocess's.exitedbecomes exit 13 with this change (as in Node) and updatedtest/js/bun/spawn/spawn.test.ts; worth checking during the rebase.
That rebase action item was not addressed.
Step-by-step proof
Take the first parametrized case ("unref() then await .exited"):
- Outer test spawns
bun -e '<body>'. - Inner bun evaluates the module:
Bun.spawn(opts)registers a pidfd poll;p.unref()decrements the loop's active count to 0;await p.exitedsuspends the module →load_entry_pointreacheswait_for_module_promise(promise). - First iteration: status
Pending→tick()(no queued tasks) → stillPending→has_pending_loop_work():active=false,active_tasks=0,tasks/yield/concurrent/immediatesempty,has_pending_refs()=false→ returns false →break.auto_tick()is never called. - Back in
Run::start:is_event_loop_alive()is false → main loop skipped →on_before_exit()(no listeners) →vm.tick()(nothing) →is_event_loop_alive()still false →break. entry_module_pending = true; the.Pendingarm printsWarning: Detected unsettled top-level await at [eval]to stderr and sets exit code 13.- Outer test receives
{stdout: "", stderr: "Warning: Detected unsettled top-level await at ...\n", exitCode: 13}and thetoEqual({stdout: "resolved\n", stderr: "", exitCode: 0, ...})at line 837 fails.
All three parametrized cases (unref() then await .exited, .exited then unref() then await, onExit then unref()) hit the same path.
Impact
Three concrete CI test failures on merge. Per REVIEW.md: "When changing output/defaults/messages, grep the suite for assertions on the old behavior and update them in the same PR."
Fix
Update the three tests at spawn.test.ts:810-844 to reflect the new (Node-compatible) behavior — Node also exits 13 when the only thing keeping a top-level await alive is an unref'd child. Either:
- change the assertions to expect
{stdout: "", exitCode: 13}andstderrcontaining"Detected unsettled top-level await"(documenting the Node parity), or - if the intent of these tests was to verify the exit packet is dequeued, add a ref'd handle (e.g.
process.on('beforeExit', () => p.ref())or drop theunref()) so the module completes, and add a separate case documenting exit-13 for the fully-unref'd variant.
The former matches what #36051/#36052 did per the triage note.
There was a problem hiding this comment.
Agreed this is real; it also hits abort.test.ts (#37666) on every lane. Whether to re-home those tests (Node's rule, which this PR implements and #36052 also chose) or instead make the liveness check unref-aware is a behavior decision, since #34478 and #37666 pinned the current leniency; I have laid out both options with the evidence in #30551 (comment) and am waiting for a maintainer call before changing tests that main merged. Leaving this thread open until that lands.
|
@alii, one design question surfaced by the rebase that I would rather not decide unilaterally, since it changes behavior that main currently tests. What CI found (build 97918): four tests added to main after this PR was written now fail with it, all the same shape: a top-level
With this PR those exit 13 with the unsettled-TLA warning. That is exactly what Node does for all four shapes (verified on Node 26: Option A, Node's rule (what the PR currently implements): liveness = ref'd work, same predicate as process exit. Simple, platform-independent, and what #36051/#36052 independently chose. Cost: scripts that await unref'd work at top level go from "works" to exit 13, and the four tests above get re-homed so they keep covering what they were written for (the spawn ones move inside a Option B, keep Bun's leniency: "unsettleable" = nothing registered at all, unref'd included. No change to any tested behavior. Cost: a second liveness predicate. On POSIX it is I lean A: it is the rule the PR's own premise is built on, B is a fragile cross-platform predicate, and the four tests lose no coverage when re-homed. But A is a user-visible behavior change relative to what main tests today, so I want a maintainer's yes before changing those tests. If you would rather keep the leniency, I will build B instead. Until then the branch stays as is (CI red on exactly those four tests); the remaining review items (byte-split lint, stale docs, comment trimming) are going up separately. Addendum: #33283 is the open tracking issue for exactly this half of the feature, and it frames today's "waits for unref'd timers" behavior as the divergence to remove, i.e. option A. #34478 and #37666, the PRs whose tests option A re-homes, were about the Windows IOCP pump and timer re-arming respectively; neither discusses top-level-await policy, and the re-homed tests keep covering both mechanics. I have added |
bb28fba to
0d48eb1
Compare
2 participants
Fixes #19049
Fixes #14951
Fixes #33283
Reproduction
The
mock.module+node:http2setup in the original #19049 report is a red herring — any test file with a top-levelawaiton a promise that never settles hits this, includingawait new Promise(() => {}). Same forbun run(#14951). (#29546, a top-level await onAbortSignal.timeout, is not in scope: it already resolves on main today because the wait polls unref'd work; what this PR does with that case is the open question in the discussion below.)Root cause
EventLoop::wait_for_promiseloops onpromise.status() == Pendingwithtick()+auto_tick(), and never checks whether anything can still make progress. Once the last ref'd handle goes away,auto_tick()falls into the!loop.is_active()branch which calls the non-blockingtick_without_idle(), so the whole wait degenerates into a busy-spin that burns one core forever.Both
load_entry_point(forbun run) andload_entry_point_for_test_runner(forbun test) go through this in their non-watch paths (src/jsc/VirtualMachine.rs), as doesload_preloads(src/runtime/jsc_hooks.rs).Fix
Add
VirtualMachine::wait_for_module_promise, used only by the three entry-point loaders. It's the sametick()+auto_tick()loop but breaks out when nothing remains that could settle the promise after draining tasks and microtasks.The liveness check is
has_pending_loop_work()(active handles, tasks, pending refs, concurrent tasks, immediates), notis_event_loop_alive(): the latter short-circuits onunhandled_error_counter != 0, which inbun testpersists across files, so an unhandled rejection in file A would misreport every later file with ordinary async TLA as "never resolved". Covered by a dedicated test.wait_for_promiseitself is unchanged —expect.rs:862and other callers assertUnwrapped::Pending => unreachable!()after calling it, so changing its contract (as #27215 did) regresses the test runner. For the same reason #14950 (expect(p).resolveson a promise that can never settle) is out of scope here: that is theexpect.rscall site, not entry-point loading.Callers now handle a still-pending return:
bun test(src/runtime/cli/test_command.rs) — report the file as a load error and move on (or bail with the same teardown as a rejected load under--bail):bun run(src/runtime/cli/run_command.rs) — after the main loop andon_before_exit, warn + exit 13 (matching Node.js).beforeExithandlers fire first and can resolve the await to continue normally, for any number ofbeforeExit → resolve → suspend againcycles. The warning names the module(s) actually suspended on their ownawait(viaBun__findStalledTopLevelAwaitinZigGlobalObject.cpp, which walks the module registry forEvaluatingAsyncrecords withhasTLAand no pending async dependencies), one line per stalled module like Node.--printskips printing the internal module promise when the entry never settled.Relationship to other PRs
This PR is the combined best of #29739 and the original approach here (per review): #29739's main-loop structure, rejected-promise dedup sentinel, and stalled-module registry walk, plus the
wait_for_module_promiseraw-work liveness check thatbun testneeds. #29549 (--hot) and #27215 (changedwait_for_promiseitself, breakingexpect) remain independent. #14950 is explicitly out of scope.Rebase notes
Rebased past #32621, which removed all
.zigporting-reference sources fromsrc/. The earlier revisions of this PR kept the.zigmirrors (src/bun.js.zig,src/jsc/VirtualMachine.zig,src/runtime/cli/test_command.zig) in sync as a porting reference; those files no longer exist on main, so this PR is now Rust + C++ only. No behavior change from that resolution — the Rust implementation was already the compiled one.Verification
The suite covers: never-settling TLA in
bun test(incl. after a timer fires,--bail, continuing to the next file, cross-file unhandled-rejection isolation, and the originalmock.module+ preload repro), andbun runexit 13 (entry, sub-import, naming the stalled leaf, one warning per stalled sibling,--preload,--print, and abeforeExit-resolves positive control).[review] gate passed · iteration 28 · 6 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 28
evidence per changed file