bake: return bun build --app failures instead of exiting; name the output directory it cannot open

[robobun]

Problem

• When bun build --app cannot open or create its output directory (a regular file named dist in the project, for example), the build ends with error: An internal error occurred (ENOTDIR); when dist is a dangling symlink it ends with ENOENT: Bun could not find a file, and the code that produces this error is missing a better error. Neither line names the directory. The exit code is already 1.
• Reproduced on the 1.4.0 canary (eabb96d) and on main with a react app and dist pre-created as a file, and again with dist as a dangling symlink.
• Cause: build_with_vm in src/runtime/bake/production.rs opens the directory with Dir::cwd().make_open_path(b"dist", ..)?. The ? converts the bun_sys::Error into the crate error, which only keeps the errno, and returns it all the way to Cli::start, whose fallback handler prints the lines above.
• Review of the first version (which printed the error and called Global::crash() like the other failure sites in the file) asked that bake's code not exit the process at all and leave that to the handler, so the PR also converts the existing sites.

Fix

• The output directory failure is printed at the site, for example ENOTDIR: Not a directory: could not open output directory "/home/me/app/dist" (open), and returned as a new Error::BakeBuildFailed. The wording is the one bun build uses for its --outdir in build_command.rs; the path is root_dir_path, the absolute path the prerender step writes into (the user never spelled dist, the production build hardcodes it).
• Error::BakeBuildFailed means "bake already printed why; just exit 1". Every site in production.rs that printed a message and then exited (Global::crash() or the local fail_with_build_error) now returns it instead: entry point count, --watch, getcwd, configure_defines, config file not found, framework imports that do not resolve, the output directory, and the missing prerender / getParams exports. The stopped microtask drain in load_module returns JSError the way the wait_for_promise call above it already does. production.rs no longer imports Global.
• Two handlers, by who has something to clean up:
  • Raised inside build_with_vm (the build VM is up and the config has run): build_command handles it next to its JSError arm. Both set the exit code to 1 unless the build set one and leave through vm.on_exit() + vm.global_exit(), so the config's process.on("exit") handlers run and, under BUN_DESTRUCT_VM_ON_EXIT, the VM is torn down; Global::crash() skipped both. The JSError arm additionally prints the exception, as before; the success path and the Err(e) return for anything else are unchanged.
  • Raised in build_command before that (nothing to exit through yet): it returns to the CLI, and handle_root_error (src/crash_handler/lib.rs) now lists BakeBuildFailed with InstallFailed and the other already-reported names, so it exits 1 without the internal error line. This is the bun install arrangement: the inner function reports, the command returns a marker error, the root exits.
• fail_with_build_error is deleted: the configure_defines errors are in ctx.log, which Cli::start prints for any error a command returns, so the site only returns the marker.
• Verified with the new failures reported by the build block in test/bake/dev/production.test.ts, every case with an exit handler in the config and BUN_DESTRUCT_VM_ON_EXIT=1:
  • dist as a regular file, and on POSIX dist as a dangling symlink (open fails with ENOENT, the mkdir is a no-op on the symlink, the reopen fails again): the errno line with the directory's path on stderr, exit event: 1 on stdout, exit 1. Both fail on the unfixed canary (the two lines quoted above, empty stdout).
  • framework imports that do not resolve (a fixture without the react packages), one of the converted build_with_vm sites: the existing message plus exit event: 1, exit 1. Fails on the canary (empty stdout), and the fixture needs no packages so it runs in under half a second on the debug build.
  • two entry points, one of the converted build_command sites: the existing message, no BakeBuildFailed on stderr, empty stdout, exit 1. This one also passes on the canary; it guards the handle_root_error entry, without which it would print An internal error occurred (BakeBuildFailed).
  • Also exercised by hand on the debug build: --watch, a missing config file with and without an explicit path, and all of the above with and without BUN_DESTRUCT_VM_ON_EXIT. The configure_defines and prerender/getParams sites are the same one line change but have no test; a bad --define or a .env directory did not make configure_defines fail, and the export sites need a custom framework.
  • The whole file passes on the debug build with --timeout 120000 (16/16, including the throwing-page test that goes through the restructured JSError arm); the dist cases get the timeout bake-harness gives its production builds (30_000 * WAIT_MULTIPLIER) because a react bundling takes 2 to 4 s on the debug ASAN build and six of the file's existing tests already hit the 5 s default there. production.test.ts is the only test of bun build --app.
  • cargo clippy -p bun_runtime -p bun_crash_handler and rustfmt are clean.

Background

• bun build --app is bake's static production build (production.rs). build_command validates the arguments, creates a dedicated JS VM and calls build_with_vm, which evaluates the app config in it, bundles the app, opens <cwd>/dist, writes the client output files into it and prerenders every route into it.
• Output::err(err, fmt, args) prints an error line prefixed with the error's name; for a bun_sys::Error it also adds the strerror text and the syscall that failed. Global::crash() is exit(1) after flushing the output buffers.
• VirtualMachine::on_exit emits process's exit event; global_exit exits the process with exit_handler.exit_code (the field behind process.exitCode) and, when BUN_DESTRUCT_VM_ON_EXIT is set (CI's ASAN lane sets it for leak checking), destroys the VM first.
• Cli::start receives whatever error a command returns, prints the accumulated log (ctx.log) and hands the error to handle_root_error, which prints An internal error occurred (<name>) unless the name is one it knows; the known already-reported names (InvalidArgument, InstallFailed, ...) just exit 1.

Earlier versions

c08b2d7 printed the message and called Global::crash() at the site, matching the other failure sites in build_with_vm. 0773af5 replaced that one crash with the BakeBuildFailed return and the build_command arm. e062717 converted the remaining sites, added the handle_root_error entry for the pre-VM ones and the two extra tests.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1799d013-7b91-42c1-8a23-b005744cb7d1

📥 Commits

Reviewing files that changed from the base of the PR and between 2948cb2 and e062717.

📒 Files selected for processing (4)

• src/crash_handler/lib.rs
• src/runtime/bake/production.rs
• src/runtime/error.rs
• test/bake/dev/production.test.ts

Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review.

---

Walkthrough

Changes

Production bake error handling

Layer / File(s)	Summary
Build failure contract and propagation src/runtime/error.rs, src/runtime/bake/production.rs	Production bake failures now return BakeBuildFailed for validation, configuration, framework, export, output, and module-loading errors.
VM result lifecycle src/runtime/bake/production.rs, src/crash_handler/lib.rs	The build path reports JavaScript exceptions after VM reacquisition, preserves reported failures, sets exit status, and performs cleanup.
Failure path validation test/bake/dev/production.test.ts	Tests cover output-path failures, unresolved framework imports, multiple entry points, exit output, exit codes, and VM cleanup.

Possibly related PRs

• oven-sh/bun#38241: Both changes modify VM build-result handling and JavaScript error processing.
• oven-sh/bun#38949: Both changes propagate production-build and module-loading failures instead of terminating the process.

Suggested reviewers: dylan-conway, jarred-sumner

🚥 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 changes: returning build failures and identifying an inaccessible output directory.
Description check	✅ Passed	The description explains the problem, fix, implementation details, and verification results, although it does not use the template headings.

---

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

[robobun]

^{Updated 10:55 PM PT - Aug 15th, 2026}

@robobun, your commit e062717 is building: #99198

[robobun]

Status: reproduced and fixed; reworked twice after review.

• Reproduced on the 1.4.0 canary (eabb96d) with the fixture from the new tests: a react app with a regular file at dist prints error: An internal error occurred (ENOTDIR); a dangling symlink at dist prints the generic ENOENT line. Neither names the directory.
• With this branch both print <errno>: <strerror>: could not open output directory "<cwd>/dist" (open) and exit 1 through the build VM. Per review, every other site in production.rs that printed and exited now returns Error::BakeBuildFailed as well (build_command exits through the VM once it has one, handle_root_error exits 1 before that); nothing in the file exits the process any more.
• Tests: failures reported by the build in test/bake/dev/production.test.ts (the two dist cases and the unresolvable-framework case fail on the canary; the whole file passes with the debug build).

[claude]

LGTM — replaces a bare ? with the same Output::err + Global::crash() pattern used for every other user-facing failure in build_with_vm, reusing the exact wording from build_command.rs.

What was reviewed:

• root_dir_path is already computed a few lines above and used later for BakeRenderRoutesForProdStatic, so quoting it here is free and correct.
• Checked make_open_path in src/sys/dir.rs: file-at-path yields ENOTDIR directly, dangling symlink yields ENOENT → mkdir no-ops → reopen fails, matching the two test cases.
• Tests follow the file's conventions (tempDirWithBakeDeps, bunEnv, describe.concurrent), assert stderr before exit code, and the symlink case is correctly gated off Windows.

Extended reasoning...

Overview

The PR changes one call site in src/runtime/bake/production.rs: bun_sys::Dir::cwd().make_open_path(b"dist", ..) previously propagated its bun_sys::Error via ?, which reached Cli::start's fallback and printed only An internal error occurred (ENOTDIR) or the generic ENOENT line. Now it matches on the result and, on Err, calls Output::err(err, "could not open output directory {}", quote(&root_dir_path)) followed by Global::crash(). Two tests are added under a describe.concurrent block in test/bake/dev/production.test.ts.

Security risks

None. This is an error-message-only change on a CLI failure path in an experimental feature (bake::print_warning() runs at the top of the command). No new I/O, no user-controlled input reaching new code, no change to the success path.

Level of scrutiny

Low. The Rust change is 11 lines and is a byte-for-byte copy of the pattern used in the same function for the config-resolution error (Output::err(err, "could not resolve...", ...); Global::crash();) and of the wording in src/runtime/cli/build_command.rs:818 for --outdir. root_dir_path is a Box<[u8]> computed via resolve_path::join_abs_string_buf::<platform::Auto>(cwd, ..., &[b"dist"]) a few lines earlier and is already consumed later by BakeRenderRoutesForProdStatic, so it is guaranteed to be in scope and to hold the absolute path the user needs to see. bun_core::fmt::quote is used exactly as the neighboring write_to_disk error paths use it.

Other factors

I read make_open_path in src/sys/dir.rs to confirm the two test scenarios exercise distinct paths through it: a regular file at dist fails the initial open_dir_at with ENOTDIR (returned directly), and a dangling symlink fails with ENOENT, falls through to mkdir_recursive_at (no-op on the existing symlink), then fails the reopen — so both new tests are non-vacuous. The tests assert the exact errno prefix for the file case, the quoted absolute path in both, that the old fallback strings no longer appear, and exitCode === 1. They use tempDirWithBakeDeps/bunEnv/bunExe per the file's conventions, run concurrently, and skip the symlink case on Windows. The PR description confirms both fail on the unfixed canary and pass with bun bd. No prior human reviews or outstanding comments.

[claude]

LGTM — focused error-message fix with a clean exit path, reviewer feedback addressed.

What was reviewed:

• The restructured build_command match: Ok/JSError/Err(e) paths are behavior-preserving; global_exit() -> ! coerces as the tail expression.
• vm.global() returns &'static so the take_exception + print_error_like_object_to_console(&mut self) sequence has no borrow conflict; the fresh &mut *vm_ptr reborrow after build_with_vm matches the SAFETY comment.
• Error wording matches build_command.rs's --outdir message; WAIT_MULTIPLIER and the 30s budget match bake-harness's production-build timeout.

Extended reasoning...

Overview

The PR replaces a bare ? on Dir::cwd().make_open_path(b"dist", ..) in src/runtime/bake/production.rs with an explicit Output::err naming the absolute output-directory path, then returns a new Error::BakeBuildFailed variant. build_command is restructured so JSError and BakeBuildFailed share the same exit tail (exit_code = 1 if unset → vm.on_exit() → vm.global_exit()), which runs the config's process.on('exit') handlers and, under BUN_DESTRUCT_VM_ON_EXIT, tears the VM down. Two subprocess tests assert the errno-prefixed message with the quoted path, the exit event: 1 line on stdout, and exit code 1.

Security risks

None. The change is error-message plumbing in the bake production build's CLI path; no new inputs are parsed, no privilege or trust boundaries are crossed.

Level of scrutiny

Low-to-moderate. The bake production build is experimental (bake::print_warning()), and the change is a diagnostic improvement plus a small control-flow refactor of the caller's match. I traced each arm of the restructured match against the old code: Ok(()) now returns early instead of falling through to a trailing Ok(()) (equivalent, _vm_guard still drops); JSError still takes the exception, prints it, sets exit code 1 and exits through the VM (now via a shared tail instead of inline); Err(e) still returns. global_exit() returns !, so using it as the tail expression of a Result<()> function is sound. The new vm.global()-based JSError arm is equivalent to the old raw-pointer version because global() returns &'static, decoupled from the &mut vm needed by print_error_like_object_to_console.

Other factors

Both prior review comments are resolved: alii's "do something better than crash" led to the BakeBuildFailed return (mirroring InstallFailed's pattern), and the comment-cop note on error.rs was addressed by trimming the doc comment to one line. The error wording is copied from build_command.rs:818 (bun build --outdir's equivalent), and bun_core::fmt::quote is used per REVIEW.md. The tests spread bunEnv, use describe.concurrent for two independent subprocess spawns, assert stderr before exit code, and use bake-harness's own 30_000 * WAIT_MULTIPLIER production-build budget rather than raising a global timeout. The PR description confirms both tests fail on the unfixed canary and that the throwing-page test (which exercises the restructured JSError arm) still passes.

[claude]

I reviewed this PR and didn't find any bugs. Since alii has been actively shaping the design across two rounds and the final commit (e062717) broadens the change to every Global::crash() site plus deletes fail_with_build_error, a human sign-off on the final shape would be worthwhile.

What was reviewed:

• build_command result restructuring — Ok(()) still returns through _vm_guard; JSError/BakeBuildFailed fall through to on_exit+global_exit (-> !), matching the pre-existing JSError exit; Err(e) still propagates.
• configure_defines path — ctx.log is printed by Cli::start before handle_root_error, so the deleted fail_with_build_error print is still covered.
• handle_root_error — BakeBuildFailed slots next to InstallFailed for pre-VM failures; the post-VM ones never reach it.
• Tests cover one site of each handling path (VM-exit and CLI-exit) and assert the exit event fires under BUN_DESTRUCT_VM_ON_EXIT.

Extended reasoning...

Overview

Converts bun build --app (bake production) error handling from in-place Global::crash() to returned errors. Adds Error::BakeBuildFailed (analogous to InstallFailed), wires it into handle_root_error, restructures build_command's post-build_with_vm match so JSError and BakeBuildFailed share the on_exit()/global_exit() tail, adds a proper error message naming the output directory when make_open_path fails, converts every remaining Global::crash() site in production.rs to return BakeBuildFailed, converts the drain_microtasks failure in load_module to JSError, and deletes the now-dead fail_with_build_error. Four new tests in test/bake/dev/production.test.ts cover the file-at-dist, dangling-symlink, unresolved-framework, and two-entry-point cases.

Security risks

None. This is CLI error-path plumbing; no untrusted input parsing, auth, or crypto.

Level of scrutiny

Medium-high. The change is a control-flow refactor across ~10 error sites in code with unsafe VM pointer reborrows and a scopeguard whose interaction with global_exit() (-> !) matters. I traced each path: Ok returns and the guard drops the VM (unchanged); JSError/BakeBuildFailed call global_exit() which never returns, so the guard/api-lock/pt never drop — same as the pre-existing JSError arm; other Err(e) returns and the guard fires (unchanged). The pre-VM BakeBuildFailed returns (entry-point count, --watch, getcwd, configure_defines) reach Cli::start, which prints ctx.log before handle_root_error exits 1 — so the configure_defines diagnostics still surface despite fail_with_build_error being deleted.

Other factors

A human reviewer (alii) drove the design here in two rounds, and the final commit is the largest, directly responding to their second request to convert every crash site. All threads are marked resolved, but there's no explicit approval on the final shape. Given the scope grew from "fix one error message" to "remove every process-exit from the file" and touches unsafe VM-lifetime code, deferring for alii to confirm the final revision matches their intent seems right rather than auto-approving over an in-progress human review.