Bun.serve, node:http: run request handlers with the event loop entered

[robobun]

Problem

• In a fetch(), route, or node:http request handler, server.upgrade() (which runs open() before it returns) or ws.close() (which runs close()) drains the microtask queue in the middle of the handler: then(f); ws.close(); g() runs f before g. Node runs f after the handler returns. Async continuations and abort listeners are affected too.
• Cause: the dispatch paths under src/runtime/server/ (list in Notes) call into JS with entered_event_loop_count at 0 and drain explicitly afterwards. The enter()/exit() pair around open() or close() (ServerWebSocket.rs) is then the outermost one, so EventLoop::exit() drains. The explicit drains ran at 0 too.

Fix

• Adds EventLoop::enter_scope_without_checkpoint(): an enter() whose exit only balances the count. Each dispatch path holds it across the handler call and its explicit drains.
• Every other dispatcher runs callbacks with the count above 0 (tick(), timers, sockets). Under the scope the nested pair is not the outermost one and does not drain. The queued callbacks run at the drain the dispatcher performs anyway, so the checkpoints per request are unchanged. Matches Node.
• Verified: 9 new cases in websocket-server.test.ts, node-http-with-ws.test.ts and serve-http3.test.ts, all failing on current bun. Also ran the websocket, http, node/http, ws and bake suites.

Background

• entered_event_loop_count (src/jsc/event_loop.rs) is the depth of native entries into JS. exit() runs the microtask checkpoint (nextTick callbacks, then promise jobs) only for the outermost exit. This gives each callback run-to-completion.
• The request paths drain explicitly: RequestContext::on_response drains before it looks at the returned value, so a promise the drain settled is unwrapped without a tick. Hence a scope that exits without a checkpoint.

Notes

Dispatch paths changed: mod.rs on_request, on_user_route_request, on_node_http_request_with_upgrade_ctx, on_saved_request (bake); server_body.rs on_web_socket_upgrade, upgrade_web_socket_user_route, on_request_for and on_user_route_request_for (HTTP/3); RequestContext::on_abort.

Cost: on a release build, the difference in the disassembly of NewServer::on_request is two loads, an inc of entered_event_loop_count before the handler call and a dec after it, plus a push/pop of the register that holds the pointer. The node:http dispatch gets the same and still contains its two calls to drain_microtasks_with_global. No drain call is added or removed on any path. Details in the comments below.

Tests: 6 cases in test/js/bun/websocket/websocket-server.test.ts (fetch and route, each calling upgrade() and closing a socket; an abort listener; an async continuation), 2 in test/js/node/http/node-http-with-ws.test.ts (a ws connection handler run from the upgrade request, and a request handler closing a socket), 1 in test/js/bun/http/serve-http3.test.ts (fetch and a route over HTTP/3).

Repro on bun 1.4.0 (prints ["microtask","after"], expected ["after"]):

const server = Bun.serve({
  port: 0,
  fetch(req, server) {
    const order = [];
    Promise.resolve().then(() => order.push("microtask"));
    server.upgrade(req);
    order.push("after");
    console.log(JSON.stringify(order));
  },
  websocket: { open() {}, message() {} },
});
new WebSocket(`ws://127.0.0.1:${server.port}`).onopen = () => server.stop(true);

The same code in a setTimeout callback prints the expected order, because timers run under enter()/exit(). The async variant (await null first, then the same statements) also printed the wrong order: the continuation ran from the explicit drain in on_response, with the count at 0. The two node:http cases print ["rest of handler","nextTick","microtask"] on Node 26 and printed ["nextTick","microtask","rest of handler"] on bun.

Out of scope, unchanged:

• The explicit drains are unconditional. server.stop(true) called from inside a handler still runs on_abort, and its drain, inside the handler. Making every explicit drain observe the count would be a change to drain_microtasks_with_global, the hottest function of the request path, and would affect every subsystem that drains explicitly.
• Evaluation of the main module also runs with the count at 0.
• The bake on_saved_request path gets the same one-line change but has no test here, since it needs a framework app. The bake framework suites (react-response, request-cookies, response-to-bake-response) pass.

Suites run with the debug build: test/js/bun/websocket/, test/js/bun/http/serve.test.ts, bun-serve-routes.test.ts, bun-server.test.ts, serve-http3.test.ts, the abort and stream related serve-*.test.ts files, test/js/node/http/, test/js/first_party/ws/, the ws regression tests, and the three bake files above. The failures seen were the same with and without this change: the send() (benchmark) test in websocket-server.test.ts starves its concurrent neighbours on a debug build in this container, and a few tests depend on localhost resolution, on not running as root, or on an empty proxy environment. They fail on stock bun here too.

[coderabbitai]

Warning

Review limit reached

Your included review limit has been reached.

You’re in a promotional period — use the checkbox below to run this review for free:

• Run review for free

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

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset (next review available in 1 minute), then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e7e90255-1790-4276-bdc7-7263868d5cc8

📥 Commits

Reviewing files that changed from the base of the PR and between f8d486a and fdc88c6.

📒 Files selected for processing (8)

• src/jsc/VirtualMachine.rs
• src/jsc/event_loop.rs
• src/runtime/server/RequestContext.rs
• src/runtime/server/mod.rs
• src/runtime/server/server_body.rs
• test/js/bun/http/serve-http3.test.ts
• test/js/bun/websocket/websocket-server.test.ts
• test/js/node/http/node-http-with-ws.test.ts

---

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

[robobun]

Status

Reproduced on bun 1.4.0 and on a debug build of main. A fetch() handler that queues Promise.resolve().then(...) and then calls server.upgrade(req) sees the promise callback run inside upgrade(), before its next statement. The same happens with ws.close() from a fetch(), route, or node:http request handler, from the continuation of an async handler, and from a request's abort listener. The same statements in a setTimeout callback run in the expected order. Node prints the expected order for the two node:http cases (checked with Node 26).

The 9 new test cases fail on a debug build without the src/ change and pass with it. The PR body lists the suites that were run.

CI was green (Buildkite build 101895, 179 of 179 jobs). Merged as 0d34bf2.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes microtask-checkpoint semantics on every Bun.serve / node:http request-dispatch path via a new event-loop primitive, a human look at the design (the enter_scope_without_checkpoint variant and its guard placement) would still be worthwhile.

What was reviewed:

• exit_without_checkpoint only decrements the count; the explicit drain_microtasks calls in on_response/on_abort still run under the held count, so no checkpoint is skipped.
• Guard drop order in on_abort: the scopeguard::defer! drain and _ref's release both run before _entered decrements — matches the comment.
• Every changed dispatch path (on_request, route, node:http, H3, upgrade, bake) has a matching test except on_saved_request, which the PR notes was verified against the bake suites.

Extended reasoning...

Overview

Adds a new EventLoop::enter_scope_without_checkpoint() RAII guard (increments entered_event_loop_count on construction, only decrements on drop — no microtask drain) plus a VirtualMachine wrapper, and holds it across nine request-handler dispatch sites in src/runtime/server/{mod.rs, server_body.rs, RequestContext.rs}. This makes nested enter()/exit() pairs inside a request handler (e.g. server.upgrade() running open(), ws.close() running close()) non-outermost, so their exit() no longer drains microtasks in the middle of the user's handler. The dispatchers' existing explicit drains still run, now with the count held above zero. Nine new test cases cover fetch/route/abort/async-continuation over HTTP/1, HTTP/3, and node:http+ws.

Security risks

None identified. This is a task-ordering fix; no auth, crypto, permission, or input-validation surface is touched.

Level of scrutiny

High. entered_event_loop_count gates every microtask checkpoint in the runtime, and the changed sites are the per-request hot path for Bun.serve and node:http. A misplaced guard could either skip a required checkpoint (leaked microtasks / hung promises) or leave the count permanently elevated on an early-return path. I traced each insertion: guards are placed after early-return validation (503 on stopped server, upgrade-context checks) and before the JS call, and Rust's drop-at-scope-end covers every remaining exit including ? / return. In on_abort the guard is declared before _ref and the scopeguard::defer! drain, so it drops last and both drains run under the held count — consistent with the doc comment.

Other factors

• The mechanism is sound and the PR explains it precisely, but adding a second enter/exit variant to the event loop is a design decision (versus, say, making the existing dispatchers use enter_scope and dropping their explicit drains, or making drain_microtasks_with_global count-aware). The description explicitly rules the latter out as too invasive; a maintainer should confirm that call.
• User-observable ordering change: code that (accidentally) depended on microtasks running inside server.upgrade()/ws.close() will see different behavior. The new behavior matches Node and run-to-completion semantics, so this is a correctness fix, but it's still a runtime-wide behavior change.
• Test coverage is thorough (fetch, route, abort listener, async continuation, H3 twin, node:http twin) and the description enumerates the suites re-run; the one uncovered path (on_saved_request / bake) was verified against existing bake tests.

[robobun]

On the two alternatives named in the review above, for whoever looks at the design:

1. Use enter_event_loop_scope() and drop the explicit drains. The explicit drains cannot go. RequestContext::on_response drains before it looks at the returned value, so a promise that the drain settles is unwrapped in the same call. The node:http tail does the same: it drains while the promise is pending and then reads the status again. A drain at the exit of the scope happens after both of these points. If we keep the explicit drains and also drain at exit, every request runs one more checkpoint on an empty queue (releaseWeakRefs, the JSC drain, the deferred task queue). The new scope avoids that. It holds the count and leaves the drain points where they are.

2. Make drain_microtasks_with_global hold the count itself. That only covers code that runs from inside a drain. The handler in the report runs before any drain, directly from the uSockets callback, so the dispatch sites have to enter the loop in any case. Holding the count inside the drain function as well would change every subsystem that drains explicitly, and that function is the hottest one on the request path. It needs the same pointer laundering as run_callback to keep the increment from being folded away across the FFI call. That is a separate change. This PR is limited to the server dispatch paths.

[Jarred-Sumner]

This will cause a performance hit. We deliberately drain microtasks multiple times inside request handlers. Can we continue to do that?

[robobun]

Yes. This PR does not remove or gate any of those drains. They all still run, at the same points, unconditionally:

• RequestContext::on_response (RequestContext.rs:2597), before it looks at the returned value.
• do_render_with_body (:2998), the streaming path (:2083) and the request body path (:2269).
• on_abort (:1363).
• The node:http tail, both the drain while the promise is pending and the one at the end (mod.rs:1379, :1534).

These call EventLoop::drain_microtasks() directly, and that function does not look at entered_event_loop_count. Only exit() and maybe_drain_microtasks() look at it. So the held count changes nothing for them. That is also why the new scope exits without a checkpoint: reusing enter_event_loop_scope() would have added a drain at exit on top of these, and this variant adds none. The set of drains a request runs is the same before and after this change.

The one drain the count removes is the one a nested exit() ran from inside the handler, for example at the end of open() inside server.upgrade(), or at the end of close() inside ws.close(). That drain ran the handler's own promise callbacks before the handler's next statement, which is the bug. Those callbacks now run at the on_response drain right after the handler returns. So an upgrade request runs one drain fewer than before, and every other request runs the same drains as before.

The cost added per request is the guard: count += 1 before the handler and count -= 1 after it. Both are inlined, there is no branch and no call, and the scoped_log! in them is compiled out of release builds.

If you want numbers, I can run the HTTP benchmarks on a release build.

[robobun]

Codegen delta on a release build (bun-profile, x64, this branch against the build of its base, which has no other changes in these files).

NewServer<false, false>::on_request, the Bun.serve dispatch. This is the whole difference between the two disassemblies, apart from addresses:

+ push  r13
  ...
+ mov   rcx, [r12 + vm]
+ mov   r13, [rcx + event_loop]
+ inc   qword ptr [r13 + entered_event_loop_count]
  ... (the handler call and handle_request, unchanged)
+ dec   qword ptr [r13 + entered_event_loop_count]
+ pop   r13

NewServer<false, false>::on_node_http_request_with_upgrade_ctx, the node:http dispatch:

+ mov   rax, [r12 + vm]
+ mov   rax, [rax + event_loop]
+ mov   [rbp - slot], rax
+ inc   qword ptr [rax + entered_event_loop_count]
  ... (unchanged, including the two calls to drain_microtasks_with_global)
+ mov   rax, [rbp - slot]
+ dec   qword ptr [rax + entered_event_loop_count]

The function contains two calls to EventLoop::drain_microtasks_with_global before and after the change. No drain call is added or removed. The other dispatch paths get the same inc/dec pair.