[robobun]

Fixes #4292.

What

When a fetch handler returns new Response(Bun.file(path)) and the file can't be opened, Bun now maps the errno to an HTTP status instead of always returning 500:

errno	status
ENOENT, ENOTDIR, EISDIR	404 Not Found
EACCES, EPERM	403 Forbidden
anything else	500 Internal Server Error (unchanged)

The user's error handler is still invoked with the system error, so the response can be customized. If there is no error handler (or it returns undefined), the mapped status is written directly without reporting an unhandled rejection or rendering the development-mode error page. An error handler that itself throws or rejects is still treated as a handler fault and surfaces as 500.

Repro

const server = Bun.serve({
  port: 0,
  development: false,
  fetch: req => new Response(Bun.file("/tmp/r" + new URL(req.url).pathname)),
});
for (const p of ["/ok.txt", "/missing.txt", "/noperm.txt"])
  console.log(p, (await fetch(server.url + p)).status);

Before (canary): /ok.txt 200, /missing.txt 500, /noperm.txt 500 (plus an ENOENT/EACCES unhandled error printed to stderr on each request).

After: /ok.txt 200, /missing.txt 404, /noperm.txt 403, stderr clean.

Cause

RequestContext::do_sendfile forwarded every bun_sys::open failure (and the EISDIR branch after fstat) to run_error_handler, which hardcodes status 500 and routes to the unhandled-rejection reporter and, in development mode, the HTML error page. The errno was never inspected.

Fix

In the open() error branch of do_sendfile, match on err.get_errno() and pass the mapped status to run_error_handler_with_status_code. The post-fstat EISDIR branch passes 404 the same way. render_production_error gains a 403 arm, and finish_running_error_handler short-circuits for 403/404 so neither triggers the unhandled-rejection report nor the dev error page. When the user's error() handler itself throws or rejects, the call site passes 500 so the handler fault is still reported.

Verification

bun-serve-file.test.ts now covers both development: true and development: false:

• ENOENT / ENOTDIR / EISDIR -> 404, EACCES -> 403 (skipped when running as root or on Windows)
• user error handler still receives the system error and can override the response
• a subprocess with no error handler gets 404 with nothing on stderr
• a throwing or rejecting error handler still surfaces as 500 with the handler's error on stderr

Not changed here: HEAD on a missing file still returns 200 (pre-existing; the HEAD path sizes the blob via resolve_size(), which swallows the stat errno, and never reaches do_sendfile).

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/bun-serve-file.test.ts

[robobun]

Status: tests pass locally on the debug build; fail-before verified with USE_SYSTEM_BUN=1. The EACCES -> 403 test is skipped under root/Windows but verified manually as nobody.

Review feedback addressed:

• c314949: a throwing/rejecting error() handler is still reported as 500 (not silently mapped to the 404/403 default). Tests added for sync throw and Promise.reject.
• 3545599: EISDIR (path is a directory) now also maps to 404, matching ENOTDIR.
• 1cffdc9: an error() handler whose own Bun.file body can't be opened (e.g. a missing custom 404.html) is a handler fault and stays 500. The errno->status mapping is gated on !has_called_error_handler, factored into sendfile_errno_status().

Deliberately left alone:

• error() returning Response.error() (unsendable status) falls through to the mapped 404. Escalating it to 500 would just re-report the original ENOENT as an unhandled rejection, which is not a useful diagnostic and fails bun:test when the server runs in-process.
• HEAD on a missing file still returns 200 (pre-existing; do_render_head_response sizes via Blob::resolve_size() which swallows stat errors). Separate mechanism; better as a follow-up.

CI: bun-serve-file.test.ts passes on all lanes. Remaining reds on build 86611 are unrelated pre-existing/flaky tests (36577.test.ts lockfile on Windows, leak-threshold flakes, node-module-module.test.js parallel-batch flake). Waiting on CI for 8e5cca7.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2968bfb7-930a-4a25-bc77-2697e06c62d9

📥 Commits

Reviewing files that changed from the base of the PR and between df49a6e and 08acac0.

📒 Files selected for processing (2)

• src/runtime/server/RequestContext.rs
• test/js/bun/http/bun-serve-file.test.ts

---

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

[robobun]

^{Updated 2:08 AM PT - Aug 1st, 2026}

✅ @robobun, your commit 08acac020fdfd0334ea276015919f9fa1fba71a5 passed in Build #86645! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 36593

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

bun-36593 --bun

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/runtime/server/RequestContext.rs:3619-3623 — The escalation to 500 at 3623/3721 covers throw/reject, but an error() handler that returns Response.error() (or any unsendable-status Response) still falls through at 3634→3644 and 3688→3689 with the original 404/403, so it now hits the new short-circuit silently instead of the pre-PR 500 + dev error page. The non-Response-return case (error: () => ({status:404})) is in the same boat but shares its fallthrough at 3644/3683 with the intended undefined/Promise.resolve(undefined) path, so that one would need explicit "undefined vs garbage" discrimination rather than a blanket escalation. Not blocking — the pre-PR signal here was just the ENOENT being re-logged, not a real "your handler is broken" diagnostic — but the unsendable-Response arm could escalate to 500 for consistency with what 3623/3721 already do.

  Extended reasoning...

  What's left after the follow-up fix

  Commit c314949 escalated two handler-fault sites to 500 in response to the earlier review comment: line 3623 (handler threw / returned an Error) and line 3721 (handler's promise rejected). Two adjacent shapes still pass the original mapped status through to finish_running_error_handler, where the new matches!(status, 403 | 404) short-circuit at line 3560 renders it silently:

  • Unsendable Response, sync: handler returns Response.error() (status 0). At 3630 as_response(result) is Some, but HTTPStatusText::is_sendable(0) at 3634 is false, so control falls out of the if block to line 3644 → finish_running_error_handler(value, status) with status still 404/403.
  • Unsendable Response, async: handler returns Promise.resolve(Response.error()). Line 3688 !is_sendable(0) → line 3689 finish_running_error_handler(value, status) with the original 404/403.
  • Non-Response return, sync: handler returns e.g. "oops", 42, or {status:404, body:"…"}. At 3617 !is_empty_or_undefined_or_null() is true; to_error() is None; as_any_promise() is None; as_response() is None → falls out to 3644 with the original status.
  • Non-Response return, async: Promise.resolve("oops"). Line 3682 as_response(fulfilled_value) is None → line 3683 with the original status.

  Step-by-step trace

  Bun.serve({
    development: true,
    fetch: () => new Response(Bun.file("/missing")),
    error: () => Response.error(),
  });

  1. do_sendfile gets ENOENT, maps to status = 404, calls run_error_handler_with_status_code(js_err, 404).
  2. run_error_handler_with_status_code_dont_check_responded calls the user's error(), which returns a Response with status 0.
  3. Line 3617: not undefined. Line 3618: to_error() → None. Line 3625: not a promise. Line 3630: as_response() → Some. Line 3634: is_sendable(0) → false. No inner block executes.
  4. Falls through to line 3644: self.finish_running_error_handler(value, 404) where value is the original ENOENT.
  5. finish_running_error_handler at 3560: matches!(404, 403 | 404) → render_production_error(404), reset log, return. No dev error page, no on_unhandled_rejection, nothing on stderr.

  Before this PR: do_sendfile passed status = 500, so step 5 fell into the DEBUG_MODE branch (dev error page showing ENOENT) or the on_unhandled_rejection call (production, ENOENT printed to stderr). Client got 500.

  Why this is not quite the same class as 3623/3721

  The refutation objection is fair and worth stating explicitly: at 3623/3721, err is the handler's own exception — swallowing it is a genuine "swallowed failure". At 3644/3683/3689, the value passed through is the original ENOENT, not anything the handler produced. So what the short-circuit suppresses here is exactly the ENOENT noise the PR set out to eliminate; the fact that the handler returned garbage was never surfaced pre-PR either — you just got the ENOENT re-logged. That makes this a debuggability regression (dev mode used to show something, even if it was the wrong thing) rather than a swallowed-exception regression.

  Why the naive fix is wrong

  Line 3644 is also reached for no handler configured and handler returned undefined (the primary tested case); line 3683 is also reached for Promise.resolve(undefined) (explicitly covered by the new "async error handler resolving to undefined falls through to 404" test). Escalating either line to 500 unconditionally would break both tests and defeat the PR's purpose.

  What could reasonably change

  The unsendable-Response arm is the one that can be handled cleanly without new discrimination logic: at 3634 and 3688 we know the handler returned a Response and it wasn't sendable — that's unambiguously a handler attempt that failed, so escalating those two sites to 500 (mirroring 3623/3721) is safe and consistent. The non-Response-return case (3644/3683) would need an explicit is_empty_or_undefined_or_null() check on the fallthrough to distinguish "declined to override" from "returned garbage" — new logic that neither pre- nor post-PR code has, so arguably out of scope here.

  Given the narrow surface (only affects handlers with a type bug in their return), the fact that pre-PR behavior was incidental rather than diagnostic, and that the clean fix touches only two lines, this reads as a consistency polish rather than a blocker.

[claude]

All four rounds of my earlier feedback are addressed (handler-fault escalation to 500 for sync throw / reject / re-entrant do_sendfile; EISDIR→404; test title). No new issues this pass. Deferring the final call to a maintainer since this is a user-facing Bun.serve behavior change — mapped statuses now bypass on_unhandled_rejection and the dev error page — which is a product decision worth a human sign-off.

What was reviewed

• sendfile_errno_status gate on has_called_error_handler — verified the re-entrant error()→Response(Bun.file(missing)) path now passes 500, and the flag is set before on_error.call().
• Removed if status != 404 guard — confirmed unreachable with 403/404 after the new short-circuit; all other finish_running_error_handler callers pass 500 or the mapped status.
• process_on_error_promise Fulfilled-unsendable arm still passes the mapped status → falls to bare 404; author explicitly chose this over re-reporting the original ENOENT.
• Test isolation: shared errorsSeen array is only read by serial it()s inside the describe; subprocess tests drain both pipes concurrently.

Extended reasoning...

Overview

Maps bun_sys::open() / post-fstat EISDIR failures in RequestContext::do_sendfile to 404/403 instead of 500, adds a 403 arm to render_production_error, and short-circuits finish_running_error_handler for 403/404 so a missing/unreadable Bun.file() body no longer fires on_unhandled_rejection or renders the dev error page. The user's error() is still invoked; a faulting handler (throw, reject, or its own unopenable Bun.file) escalates to 500 via the has_called_error_handler gate in the new sendfile_errno_status() helper. ~45 net lines in RequestContext.rs, ~165 lines of tests.

Security risks

None identified. No new input parsing; the errno originates from a kernel open()/fstat() on a path the user's own handler constructed. 403 does leak "file exists but unreadable" vs 404's "not found", but that's the standard static-server contract and the error() hook lets users collapse both if desired.

Level of scrutiny

Medium-high: production-critical (Bun.serve request path) and a user-visible default change. The mechanism itself is small and the four earlier review rounds caught the handler-fault swallowing edge cases; each now has a dedicated test row. The remaining open items (HEAD→200, truly-pending error()→204) are pre-existing and documented as follow-ups.

Other factors

Test coverage is thorough — dev/prod matrix, ENOENT/ENOTDIR/EISDIR/EACCES, handler override, three faulting-handler shapes in subprocesses asserting stderr + exit code, and a no-handler subprocess asserting clean stderr. All prior inline threads are resolved. Not approving because the "suppress unhandled-rejection reporting for file-open errors by default" semantics is a product/API decision a maintainer should confirm.