event loop: run the timers phase before the entry point's queued work by robobun · Pull Request #33509 · oven-sh/bun

robobun · 2026-07-06T15:13:27Z

Repro

const L = [];
process.on("exit", () => console.log(JSON.stringify(L)));
setTimeout(() => L.push("timeout"), 1);
setImmediate(() => L.push("immediate"));
const s = Date.now(); while (Date.now() - s < 10);   // the 1ms timer is overdue before the loop is entered

node: ["timeout","immediate"]
bun : ["immediate","timeout"]

Deterministic, 20/20 each. (The same inversion happens against a MessageChannel self-post and a completed fs.readFile, but those are task-queue work and stay unfixed here; see Known exclusion.)

This is not the nondeterministic setTimeout(0) vs setImmediate case Node documents: the busy-wait makes the timer unambiguously expired, and Node is deterministic because its timers phase precedes poll and check.

"Arm a short timer, then do heavy synchronous startup work" is the watchdog / boot-debounce / test-timeout shape. Node guarantees the overdue timer is the first callback to run.

Cause

Two things stack up, both before the loop is entered:

uv_run's iteration is timers, poll, check. EventLoop::autoTick is immediates, poll, timers: the same cycle, rotated, so every iteration after the first lines up with Node. Nothing runs a timers phase before the first one, so the first batch of setImmediate callbacks cuts in front of an already-due timer.
The entry point is evaluated inside EventLoop::tick() (reached from load_entry_point -> wait_for_promise), and that same tick goes on to dispatch the task queue. An fs completion or a port message queued by the entry point's synchronous body therefore runs before autoTick (and its timers phase) is reached at all.

Fix

load_entry_point, and its test-runner twin, now evaluate the entry point's synchronous body with a full tick() and then run one timers phase before the wait loop continues. EventLoop::drain_expired_timers() is that timers phase; it reaches timer::All::drain_timers through the existing link-time extern, the same way run_imminent_gc_timer reaches WTFTimer.

Running the body through tick() (rather than a bare microtask drain) means unhandled-rejection and exception reporting is byte-for-byte what it was before, so rejection semantics are untouched. A body that threw rejects the module evaluation promise, and that case is checked so the timers phase is skipped (Node reports the error and exits without a loop).

This reorders setImmediate relative to an already-due timer: immediates wait in the immediate queue (drained by autoTick), so the timers phase runs ahead of them. Task-queue work is not reordered (see Known exclusion). The worker entry point runs the same timers phase from its own run loop.

Steady state is untouched: autoTick still drains timers at the end of every iteration, so the only iteration whose phase order changes is the first. A top-level-await entry point is unaffected, because by the time its body resumes the loop is already running and autoTick's trailing drain is the timers phase.

Verification

bun bd test test/js/node/timers/node-timers.test.ts, 24 pass. The setImmediate ordering test fails on 1.4.0 and on main with exactly the inversion above.

Ordering, checked against node v26.3.0:

shape	node	bun (this PR)
due timer vs `setImmediate` at loop entry	`["timeout","immediate"]`	`["timeout","immediate"]`
inside a worker	`["timeout","immediate"]`	`["timeout","immediate"]`
same shape armed inside a `setImmediate`	`["timeout","immediate"]`	`["timeout","immediate"]`
same shape armed inside a timer callback	`["immediate","timeout"]`	`["immediate","timeout"]`
same shape armed inside an fs callback	`["immediate","timeout"]`	`["immediate","timeout"]`
same shape after a top-level `await`	`["immediate","timeout"]`	`["immediate","timeout"]`
due timer, then the entry point throws	no timer, exit 1	no timer, exit 1

test/js/bun/test/, node's test-timers* / test-promise* / test-next-tick* / test-event-capture-rejections parallel tests, the http2 client suite, and reject-tostring were run against a build with and without the change: identical failures, no new ones. An earlier revision that evaluated the body with a bare microtask drain instead of tick() regressed test-event-capture-rejections and reject-tostring; routing through tick() fixes both.

Known exclusion

Only setImmediate is reordered. Task-queue work the entry body queues (a MessageChannel self-post, a completed fs.readFile, a --preload module's import) is dispatched by the entry tick() itself, before the timers phase, so it keeps the 1.4.0 ordering. Reordering it would need the body to settle without dispatching the task queue, and a bare microtask drain that did so regressed rejection handling (below), so it is out of scope here. None of these are regressions: the ordering is unchanged from 1.4.0 in every case.

Evaluating the body through tick() keeps rejection reporting exactly where it was. An earlier revision that used a bare microtask drain and reported rejections before the timers phase broke test/js/node/test/parallel/test-event-capture-rejections.js and reject-tostring (both pass on main); routing through tick() fixes both. tick()'s tail also sets unhandled_error_counter at exactly the pre-existing point, so the timers-phase guard reads it afterwards and skips the timers phase for a body that threw or left an unhandled rejection, matching the worker path and is_event_loop_alive(). Whether the counter is set synchronously at entry time varies by build, so only the throw case is asserted.

One behavior does change, deliberately: top-level setTimeout(fn, 0) with no blocking work before the loop is entered now depends on whether the 1ms clamp has elapsed by then, which is the nondeterminism Node documents for that case. Bun used to always run the setImmediate first because it never looked at the timer heap that early.

Related but distinct: #32807 fixes setImmediate vs newly completed async work, with no timers involved.

coderabbitai · 2026-07-06T15:13:37Z

Walkthrough

Adds a timer-draining ABI hook, exposes it on EventLoop, routes entry-point loading through a shared microtask-and-timer phase helper, and expands tests to verify expired-timer ordering and error cases across worker and test-runner flows.

Changes

Expired timer draining at entry point

Layer / File(s)	Summary
Timer drain ABI hook `src/runtime/dispatch.rs`	Adds `__bun_drain_expired_timers`, fetching the per-thread timer heap and invoking `drain_timers`, with a no-op when the timer handle is null.
EventLoop drain_expired_timers method `src/jsc/event_loop.rs`	Declares the hook and adds `EventLoop::drain_expired_timers()` to forward the owning VM pointer.
run_entry_point_body helper and entry-point wiring `src/jsc/VirtualMachine.rs`, `src/jsc/web_worker.rs`	Adds `run_entry_point_body`, calls it from the main and test-runner entry-point paths after `perform_gc()`, and skips it in the web-worker path in favor of termination-aware waiting. `Worker::spin` also drains expired timers only when not terminating and no unhandled error is pending.
Timer ordering tests and fixture `test/js/node/timers/node-timers.test.ts`, `test/js/node/timers/timers-phase-order-worker-fixture.js`	Adds tests for expired-timer ordering across spawn, worker, `bun test`, and rejection/error scenarios, plus a worker fixture that expires a timer before reporting callback order.

Sequence Diagram(s)

sequenceDiagram
  participant VirtualMachine
  participant run_entry_point_body
  participant EventLoop
  participant __bun_drain_expired_timers

  VirtualMachine->>VirtualMachine: perform_gc()
  VirtualMachine->>run_entry_point_body: run_entry_point_body(evaluation promise)
  run_entry_point_body->>run_entry_point_body: drain_microtasks()
  run_entry_point_body->>EventLoop: drain_expired_timers()
  EventLoop->>__bun_drain_expired_timers: __bun_drain_expired_timers(vm)
  __bun_drain_expired_timers->>__bun_drain_expired_timers: drain_timers(vm)

Estimated code review effort: 4/5

Related issues: None referenced in provided context.

Related PRs: None referenced in provided context.

Suggested labels: jsc, event-loop, timers, tests

Suggested reviewers: None determinable from provided context.

_{🐰 An expired timer waits in line,}
_{Microtasks drain before the sign,}
_{The hook descends, the phases turn,}
_{Then tests confirm what runs, and when,}
_{Order stays true through every churn.}

🚥 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 change: running the timers phase before queued entry-point work.
Description check	✅ Passed	The description is detailed and covers the change and verification, though it uses custom sections instead of the template headings.

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

robobun · 2026-07-06T15:16:32Z

^{Updated 2:05 AM PT - Jul 7th, 2026}

❌ @robobun, your commit e29dbe1 has some failures in Build #69636 (All Failures)

🧪 To try this PR locally:

bunx bun-pr 33509

That installs a local version of the PR into your bun-33509 executable, so you can run:

bun-33509 --bun

coderabbitai

Actionable comments posted: 3

🤖 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.rs`:
- Around line 2413-2415: The repeated perform_gc() + run_entry_point_body()
sequence should be consolidated so callers only invoke one helper. Update
VirtualMachine’s run_entry_point_body to perform the GC internally before
executing the body, then remove the standalone
self.event_loop_mut().perform_gc() calls at every parallel call site so the
behavior stays centralized and consistent.

In `@test/js/node/timers/node-timers.test.ts`:
- Around line 286-297: The timer subprocess test is collecting stderr but not
surfacing it on failures, so update the affected test cases in
node-timers.test.ts to follow the existing pattern used elsewhere in the file.
In the test block that uses Bun.spawn and destructures stdout, stderr, and
exitCode, add an assertion that checks stderr is empty when exitCode is
non-zero, placed immediately before expect(exitCode).toBe(0), and apply the same
fix to the related “inside a test file” case so failures include useful stderr
diagnostics.
- Around line 249-363: The tests in the timers phase ordering block are running
sequentially even though each case independently spawns subprocesses and uses
tempDir, so update the surrounding describe/test structure to run them
concurrently. Use describe.concurrent for the main block, switch the
parameterized case in the existing test.each to test.concurrent.each, and keep
the unique fixtures like the worker and throw cases isolated so the shared
tempDir names and Bun.spawn invocations don’t serialize the suite.

🪄 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: 8190669c-5383-4bf1-b13d-c3f9cb152a50

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and e0abe9d.

📒 Files selected for processing (5)

src/jsc/VirtualMachine.rs
src/jsc/event_loop.rs
src/runtime/dispatch.rs
test/js/node/timers/node-timers.test.ts
test/js/node/timers/timers-phase-order-worker-fixture.js

claude

The latest commit addresses both remaining gaps I flagged (snapshot-and-compare on unhandled_error_counter, and the post-handle_rejected_promises() microtask drain), with tests for each — nothing further from me. Deferring to a human since this rewires event-loop entry across bun run/bun test/workers and the b41068c build is showing http2/worker test failures across all platforms that are worth confirming as pre-existing.

Extended reasoning...

Overview

This PR inserts a timers phase between entry-point module evaluation and the first tick()/auto_tick() cycle, so an already-expired setTimeout fires before setImmediate/task-queue work at loop entry (matching Node's uv_run phase order). It touches VirtualMachine::load_entry_point (both watcher/non-watcher branches), load_entry_point_for_web_worker, and load_entry_point_for_test_runner (both branches), adds EventLoop::drain_expired_timers() with a link-time extern into bun_runtime::dispatch, and adds ~200 lines of ordering tests.

Over four review rounds I raised: (1) a throwing entry point firing timers before the error is reported, (2) the test-runner path being excluded, (3) unhandledRejection ordering, (4) --preload exclusion, and (5) the monotonic-counter check + missing post-rejection microtask drain. All were either fixed with tests or (in the --preload case) documented as a known exclusion with a verified reason. Commit 9177333 addresses the last two gaps and adds regression tests for both. The bug-hunting pass on the current head found nothing new.

Security risks

None. No untrusted-input parsing, auth, crypto, or filesystem-path handling is touched. The change is pure event-loop phase reordering at process/worker/test-file entry.

Level of scrutiny

High. load_entry_point* is the spine of every bun run, bun test, and worker startup. The new run_entry_point_body helper now runs drain_microtasks() twice, calls handle_rejected_promises(), and fires timers — all before the wait loop that previously did this work. That's a subtle but real change to when user callbacks (unhandledRejection listeners, timer callbacks) first observe process state. The PR itself acknowledges one deliberate behavior change (setTimeout(fn, 0) vs setImmediate at loop entry becoming nondeterministic, matching Node) and one known exclusion (--preload). This is exactly the kind of change where a maintainer familiar with the event-loop invariants should sign off.

Other factors

CI: The robobun status for b41068c (one commit behind head) shows test-http2-generic-streams, test-http2-generic-streams-sendfile, test-http2-write-finishes-after-stream-destroy, and test-worker-terminate-http2-respond-with-file failing across all platforms (not a single flaky lane), plus an ASAN SIGABRT in fs.watchFile.test.ts. Given this PR changes worker entry-point loading and event-loop timing at startup, these need to be confirmed as pre-existing on main before merge — the PR description's "identical failures, no new ones" claim covers test/js/bun/test/ and test-timers*/test-promise*, not the http2/worker suite.
Test coverage: The new tests are thorough — they cover the three ordering variants (setImmediate, MessagePort, fs I/O), the worker path, the bun test path (single-file and following-a-rejecting-file), the handled-rejection path (including listener-scheduled microtasks), and both failure paths (throw / unreported rejection).
Outstanding threads: My 17:49 inline comment is still open on GitHub but is fully addressed by 9177333; no other unresolved reviewer feedback.

libuv opens every `uv_run` iteration with the timers phase, so a timer that expired while the entry point was still running dispatches before anything the entry point queued alongside it. Bun's `autoTick` runs the timers phase at the *end* of an iteration, which is the same cyclic order for every iteration but the first, and the entry point's own `tick()` dispatches the task queue before `autoTick` ever runs. Evaluate the entry point's synchronous body with a microtask drain, which is where module evaluation runs, then run one timers phase before the wait loop's `tick()` can dispatch tasks. Steady state is unchanged: `autoTick` still drains timers at the end of each iteration.

A throwing entry point rejects the module evaluation promise; Node reports the uncaught exception without entering the loop, so no timer runs. Guard the timers phase on the promise not having rejected, and on the microtask drain not returning `JsTerminated`. Also run it for `bun test` entry points, which had the same shape but were left on the old ordering.

A top-level `Promise.reject()` leaves the module evaluation promise fulfilled, so the rejected-promise guard did not cover it and a due timer ran before `unhandledRejection` was emitted. Notify rejections right after the entry point's body, as Node's `processPromiseRejections()` does before `uv_run`, and skip the timers phase when the rejection goes unreported: `unhandled_error_counter` is what `is_event_loop_alive()` reads to exit without entering the loop.

…ying `unhandled_error_counter` is cumulative and is never reset between test files in a run, so an absolute check skipped the timers phase for every file after one that left an unhandled error. Snapshot it on entry instead, the idiom the test runner already uses. `handle_rejected_promises()` invokes the `unhandledRejection` listener without draining afterwards, so a microtask it scheduled ran after the timers phase. Node's `processTicksAndRejections` drains those before `uv_run`, so drain again before the timers phase.

…it loop Draining microtasks at `entered_event_loop_count == 0` ran every nested callback's own microtask drain mid-body, reordering a constructor (e.g. http2 `connect`) against its continuations. Bracket the drain in enter()/exit() so the body evaluates the way it did inside `tick()`. The worker entry point keeps evaluating inside its termination-aware wait: running its body on the microtask queue let a later `tick()` start with a pending TerminationException and abort on `scope.assertNoException()`. web_worker.rs runs the timers phase before the loop, skipped once terminate() has landed.

Match the `is_event_loop_alive()` check on the next line so the worker entrypoint never fires a timer on a path that otherwise exits without entering the loop.

coderabbitai

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.rs (1)
2411-2444: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider a --watch-mode test to exercise the watcher-enabled call sites.

The new run_entry_point_body calls at Lines 2415 and 4646 are only exercised via the watcher-enabled branches of load_entry_point/load_entry_point_for_test_runner; none of the added tests in node-timers.test.ts spawn with --watch. The core timer-ordering logic is shared with the (well-tested) non-watcher path, so the risk is low, but the watcher loop around it (pending_internal_promise swapping) is untested for this new behavior.

Also applies to: 4636-4676
🤖 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.rs` around lines 2411 - 2444, Add coverage for the
watcher-enabled branches that invoke run_entry_point_body in
VirtualMachine::load_entry_point and load_entry_point_for_test_runner, since the
new behavior is only exercised when is_watcher_enabled() is true. Update or add
a --watch-mode test that triggers these call sites and verifies the
pending_internal_promise loop still behaves correctly around the timer-ordering
changes, rather than relying only on the non-watcher path.

🤖 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.

Outside diff comments:
In `@src/jsc/VirtualMachine.rs`:
- Around line 2411-2444: Add coverage for the watcher-enabled branches that
invoke run_entry_point_body in VirtualMachine::load_entry_point and
load_entry_point_for_test_runner, since the new behavior is only exercised when
is_watcher_enabled() is true. Update or add a --watch-mode test that triggers
these call sites and verifies the pending_internal_promise loop still behaves
correctly around the timer-ordering changes, rather than relying only on the
non-watcher path.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 640359f5-59e3-4e7e-ab92-efe82c774e6f

📥 Commits

Reviewing files that changed from the base of the PR and between c540de3 and 67f5746.

📒 Files selected for processing (6)

src/jsc/VirtualMachine.rs
src/jsc/event_loop.rs
src/jsc/web_worker.rs
src/runtime/dispatch.rs
test/js/node/timers/node-timers.test.ts
test/js/node/timers/timers-phase-order-worker-fixture.js

robobun · 2026-07-06T21:48:13Z

Re the --watch-mode coverage suggestion: skipping it, for the reason the comment itself flags (poor tradeoff).

The watcher branches of load_entry_point / load_entry_point_for_test_runner call run_entry_point_body with the same timer-ordering logic as the non-watcher branches, which the added tests cover. The only watcher-specific code is the pending_internal_promise swap loop, and this PR doesn't touch it: the helper call is inserted before it, unchanged.

A non-flaky --watch ordering test is also awkward to write: watch mode keeps the process alive, so the spawn-then-assert-exit pattern the other cases use doesn't apply, and asserting startup callback order under a live watcher would need a bespoke harness for little added confidence over the shared path. Happy to add one if a maintainer wants it.

claude

All the ordering edge cases I raised have been addressed and covered by tests — nothing further from me. Given this changes event-loop phase ordering at every entry point (with a documented behavioral change to top-level setTimeout(0) vs setImmediate and two known exclusions), it's worth a maintainer sign-off on the design tradeoffs.

Extended reasoning...

Overview

This PR inserts a timers phase between entry-point evaluation and the first tick()/auto_tick() iteration, so an already-expired timer fires before setImmediate/task-queue work the entry point queued — matching Node's uv_run phase order. It touches VirtualMachine.rs (new run_entry_point_body helper wired into load_entry_point, its watcher branch, and the test-runner twin), event_loop.rs (new drain_expired_timers method), dispatch.rs (new __bun_drain_expired_timers link-time hook), web_worker.rs (worker-specific timers phase before the run loop), plus ~200 lines of new tests.

Security risks

None. No untrusted-input parsing, no auth/crypto/permissions. The change is pure control-flow ordering of already-scheduled callbacks.

Level of scrutiny

High. This is core event-loop sequencing that affects every bun run, bun test, and worker at startup. The run_entry_point_body helper went through five review iterations in this PR to get its guards right (throwing entry point → skip timers; unhandled rejection → skip timers; monotonic counter snapshot for multi-file test runs; microtask drain after unhandledRejection listeners; enter()/exit() bracketing so nested callbacks don't drain mid-body; worker termination + error-counter guards). Each of those was a real behavioral regression I flagged that the author reproduced and fixed with a test. The final shape looks correct to me and every concern I raised is now resolved and covered.

Other factors

Deliberate behavioral change: top-level setTimeout(fn, 0) vs setImmediate (no busy-wait) becomes nondeterministic, matching Node's documented nondeterminism. Previously Bun was deterministic-but-wrong. This is called out in the PR description but is a user-visible change a maintainer should acknowledge.
Known exclusions: --preload modules and worker MessagePort/fs completions retain the old ordering (documented in the PR description and code comments) because their evaluation paths dispatch the task queue and can't use the bare-drain_microtasks() shape. These are reasonable scope cuts but represent a design decision.
Test coverage is thorough: parameterized over setImmediate/MessagePort/fs, plus worker, test-runner, multi-file test-runner, throwing entry, unhandled rejection with and without listener.
The iterative review history (5 rounds of edge-case fixes to a ~30-line helper) is itself evidence this code is subtle enough to warrant a maintainer's eyes before merge.

Proactively calling `handle_rejected_promises()` during entry-point evaluation reordered unhandled-rejection processing for every program and broke the chained rejection/capture timing in test-event-capture-rejections. Drop it: the entry body is a pure microtask drain again, and reporting rejections stays with the wait loop's `tick()`, as before. Timer-vs-rejection ordering is no longer claimed; the timers phase still runs after the body and is skipped only when the module promise rejected. Drops the rejection-ordering tests accordingly.

Draining the entry body's microtasks directly settled the module promise, so the wait loop's tick() never ran and its rejection reporting was skipped: an unreported rejection under --unhandled-rejections=throw crashed on the first one instead of routing through the uncaughtException handler (reject-tostring), and adding the reporting back broke the chained capture timing (test-event-capture-rejections). Evaluate the body with a full tick() instead, which reports rejections exactly as before; only the timers phase is added after it. This fixes the setImmediate-at-loop-entry case everywhere; MessagePort and fs completions stay with --preload and workers as known exclusions (the tick dispatches the task queue).

…ments tick()'s tail already reports rejections at exactly the pre-existing point and bumps unhandled_error_counter, so reading it afterwards is a pure read: snapshot it and skip the timers phase when the body left an unhandled rejection, the same as the worker path and is_event_loop_alive() do. Make __bun_drain_expired_timers a one-line delegate to the pre-existing drain_timers_export instead of duplicating its body, and drop the two "see the PR" parentheticals from comments since the sentences are already self-contained.

claude

No outstanding issues from my side — all prior findings are addressed — but this changes event-loop phase ordering at entry (with documented tradeoffs around task-queue work, rejection reporting, and a newly-nondeterministic setTimeout(0)-vs-setImmediate case), so it warrants a maintainer sign-off.

Extended reasoning...

Overview

This PR inserts a timers phase between the entry point's synchronous body and the first auto_tick, so an already-expired setTimeout fires before setImmediate at loop entry — matching Node's uv_run phase order. It touches src/jsc/VirtualMachine.rs (entry-point loading for bun run, bun test, and workers), src/jsc/event_loop.rs (new drain_expired_timers() method + link-time extern), src/jsc/web_worker.rs (worker run loop), and src/runtime/dispatch.rs (the #[no_mangle] shim). Tests and a worker fixture are added in test/js/node/timers/.

Security risks

None identified. No user-controlled input parsing, no auth/crypto/permissions, no new external surface. The FFI extern is an internal Rust→Rust link-time hook following the existing __bun_run_wtf_timer pattern.

Level of scrutiny

High. Event-loop phase ordering is one of the most correctness-sensitive areas of a JS runtime — subtle changes here reorder user callbacks in ways that are hard to reason about locally. This PR went through ~10 review iterations in which I flagged (and the author fixed) several real behavioral regressions: timers firing after a throwing entry point, the test-runner path being excluded, a monotonic-counter guard breaking multi-file bun test, worker/main guard asymmetry, and multiple description-drift passes. That history itself indicates the change is not mechanically obvious.

Other factors

All my prior inline findings are resolved; the bug-hunting pass on the current head (e29dbe1) found nothing new.
The PR carries deliberate design tradeoffs a maintainer should ratify: only setImmediate is reordered (task-queue work — MessagePort, fs completions, --preload imports — keeps 1.4.0 ordering); an earlier attempt to also fix rejection ordering broke test-event-capture-rejections and was backed out; and top-level setTimeout(fn, 0) vs setImmediate with no blocking work becomes nondeterministic (matching Node's documented behavior, but changing Bun's prior deterministic ordering).
Test coverage is good for the claimed scope (bun run, worker, bun test, throwing entry point) and the author verified against Node's test-timers*/test-promise*/test-next-tick* suites, but the change affects every program's first loop iteration, so CI results and a human read of run_entry_point_body / the worker guard are worth having.

robobun · 2026-07-07T04:40:54Z

CI state: the diff is green; the only red lane on the last two builds (69480 and 69636) is darwin-26-aarch64 failing on buildkite-agent artifact download timed out after 120s before any tests run, plus two darwin-14 lanes expiring on 69480. No test-failure annotations touch this diff on either build (only retry-pass warnings on unrelated tests).

All prior review findings are addressed; claude[bot] has signed off on e29dbe16d7 with no outstanding issues. 24 timer tests pass, rejection/timer/http2/worker parallel sweeps show no new failures versus main.

Ready for a maintainer to review and merge; the remaining CI red is infrastructure.

github-actions Bot added the claude label Jul 6, 2026

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs Outdated

Comment thread src/jsc/VirtualMachine.rs

coderabbitai Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

Comment thread test/js/node/timers/node-timers.test.ts Outdated

Comment thread test/js/node/timers/node-timers.test.ts

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

robobun force-pushed the farm/7164b6b7/timers-phase-on-loop-entry branch from c540de3 to b41068c Compare July 6, 2026 17:43

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/web_worker.rs Outdated

robobun added 7 commits July 6, 2026 21:27

Run the phase-order tests concurrently and surface stderr on failure

0dec01f

Guard the worker timers phase on unhandled_error_counter too

67f5746

Match the `is_event_loop_alive()` check on the next line so the worker entrypoint never fires a timer on a path that otherwise exits without entering the loop.

robobun force-pushed the farm/7164b6b7/timers-phase-on-loop-entry branch from ccf7674 to 67f5746 Compare July 6, 2026 21:29

coderabbitai Bot reviewed Jul 6, 2026

View reviewed changes

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

claude Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

ci: retrigger

42998a8

claude Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread src/jsc/VirtualMachine.rs

Comment thread src/runtime/dispatch.rs Outdated

Comment thread src/jsc/VirtualMachine.rs Outdated

claude Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread src/jsc/event_loop.rs Outdated

Comment thread src/jsc/VirtualMachine.rs

robobun added 2 commits July 7, 2026 03:19

Align the drain_expired_timers doc comment with its callers

5212c7e

Drop the platform-dependent unreported-rejection ordering assertion

e29dbe1

claude Bot reviewed Jul 7, 2026

View reviewed changes

Uh oh!

Conversation

robobun commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Repro

Cause

Fix

Verification

Known exclusion

Uh oh!

coderabbitai Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

robobun commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

robobun commented Jul 6, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jul 6, 2026 •

edited

Loading

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

robobun commented Jul 6, 2026 •

edited

Loading