Repository navigation
Conversation
|
Warning Review limit reached
On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file. Or wait 8 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (7)
Comment |
|
Status: reproduced on 1.4.3-canary (compiled exe serves the dev error page by default, CI: the new tests and every compile lane pass on each run. The remaining red is unrelated to this diff and changes from run to run ( |
A bun build --compile executable never set the production flag on the transpiler, so Bun.serve defaulted to development mode and served the dev error page (message, stack, source) for an uncaught route error, and the dotenv loader picked .env.development. Set production in apply_standalone_runtime_flags unless NODE_ENV or BUN_ENV is development or test. configure_defines still honours an explicit NODE_ENV afterwards, and development: true in Bun.serve still wins.
fbf7137 to
ae117b5
Compare
Problem
bun build --compileexecutable serves the development error page by default.Bun.serve()reportsdevelopment: true, and an uncaught error in a route handler returns a 50 KBtext/htmlpage with the message, the stack (/$bunfs/root/app:7:11), and the source line. Same on 1.3.14, 1.4.0, 1.4.2, and canary.--compilehelp text says "Implies --production", but nothing sets production for a compiled executable.ServerConfig::from_js(src/runtime/server/ServerConfig.rs:639-645) only leaves development mode when the runtimeNODE_ENVisproductionorvm.transpiler.options.productionis set. The standalone boot sets neither.Fix
apply_standalone_runtime_flags(src/bun.js.rs) now callsset_production(true)on the transpiler and resolver options unlessNODE_ENVorBUN_ENVin the process env isdevelopmentortest. This runs beforeconfigure_defines, so an explicitNODE_ENVstill applies through the existing path.development: trueinBun.serve()still wins. Workers in the executable go through the same hook.Bun.serve()defaults todevelopment: false, and the dotenv loader picks.env.productioninstead of.env.development. Both match whatNODE_ENV=production ./appdoes today.set_productiononbun_bundler::BundleOptionsbecomespubso the runtime crate can call it.ServerConfig.rsis unchanged.test/bundler/bundler_compile.test.ts(compile/ServeDefaultsToProduction,compile/ServeExplicitDevelopment) andtest/bundler/bundler_compile_autoload.test.ts(compile/AutoloadDotenvProductionSuffix). All three fail on the stock binary and pass with this change. Also ran all ofbundler_compile,bundler_compile_autoload,compile-argv, andbun-serve-html(153 pass).Background
options.productionis the transpiler flag thatbun runderives fromNODE_ENV=production.ServerConfig::from_jsreads it to pick theBun.servedefault, andrun_env_loaderreads it to pick the.env.<suffix>file.apply_standalone_runtime_flagsis the hook that applies the executable's embedded flags (dotenv, tsconfig, package.json autoload) to the transpiler at boot, beforeconfigure_definesloads the env.process.env.NODE_ENVas the build-time value ("development"when unset), so code that checksprocess.env.NODE_ENV !== "production"inside the executable still sees development. bun build --compile: read NODE_ENV at runtime, set argv[0] to the executable path #32851 makes that a runtime read. compile: do not inline NODE_ENV into runtime-transpiled files when .env autoload is off #38672 edits the same hook for the autoload-off case and will need a small rebase.Notes
Repro on 1.4.3-canary:
bun build --compile app.ts --outfile app && ./appprinted{ dev: true, ct: "text/html;charset=utf-8", leaks: true }.NODE_ENV=production ./appprinted{ dev: false, ct: "text/plain", leaks: false }.A first version set the default in
ServerConfig::from_jsonly. Review moved it to the standalone boot hook so it uses the existingproductionflag, honoursBUN_ENV, and does not duplicate theNODE_ENVparsing inconfigure_defines.NODE_ENV=testkeeps the non-production default so a compiled executable under a test runner behaves likebun run.Related: #26373 (the help text promise), #32851 (runtime
NODE_ENVread), #38672 (same hook, autoload-off inlining).Self-reviewed: 2 concerns raised. The split-brain with the inlined
process.env.NODE_ENVis pre-existing and tracked by #32851, so this change documents it instead of widening scope. The overlap with #38672 is a small merge in one function.no test proof · iteration 5 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_compile.test.ts