Validate app.bundlerOptions values are objects in Bun.serve#30446
Validate app.bundlerOptions values are objects in Bun.serve#30446robobun wants to merge 1 commit into
app.bundlerOptions values are objects in Bun.serve#30446Conversation
Passing a non-object value for app.bundlerOptions, or for its server / client / ssr / minify sub-options, hit a debug assertion in JSValue.get instead of throwing a proper error.
|
Updated 8:54 PM PT - May 9th, 2026
❌ @robobun, your commit b87f822 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30446That installs a local version of the PR into your bun-30446 --bun |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR strengthens ChangesBundler Options Validation
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| describe("app.bundlerOptions validation", () => { | ||
| test.each([5, "hello", true, 1n])("non-object bundlerOptions throws (%p)", value => { | ||
| expect(() => { | ||
| // @ts-expect-error - Testing runtime validation | ||
| serve({ app: { bundlerOptions: value } }); | ||
| }).toThrow("'app.bundlerOptions' must be an object"); | ||
| }); | ||
|
|
||
| test.each(["server", "client", "ssr"])("non-object bundlerOptions.%s throws", key => { | ||
| for (const value of [5, "hello", true, 1n]) { | ||
| expect(() => { | ||
| // @ts-expect-error - Testing runtime validation | ||
| serve({ app: { bundlerOptions: { [key]: value } } }); | ||
| }).toThrow(`'bundlerOptions.${key}' must be an object`); | ||
| } | ||
| }); | ||
|
|
||
| test("non-object bundlerOptions.server.minify throws", () => { | ||
| for (const value of [5, "hello", 1n]) { | ||
| expect(() => { | ||
| // @ts-expect-error - Testing runtime validation | ||
| serve({ app: { bundlerOptions: { server: { minify: value } } } }); | ||
| }).toThrow("'bundlerOptions.server.minify' must be a boolean or an object"); | ||
| } | ||
| }); | ||
|
|
||
| test("boolean bundlerOptions.server.minify does not crash", () => { |
There was a problem hiding this comment.
🔴 These tests call serve({ app: ... }) in-process, but the app option is gated behind bun.FeatureFlags.bake() (is_canary || isDebug || BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE). The CI runner only sets BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING for the test process — the bake flag is only in harness.ts bunEnv for spawned children — so on a non-canary release build (.buildkite/ci.mjs with canary=0 via [release]) the app block is skipped entirely and serve() throws the generic "Bun.serve() needs either:…" error instead, failing all four new test blocks. Consider setting process.env.BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE = "1" at the top of this describe (the flag is read at runtime), or gating it with describe.skipIf.
Extended reasoning...
What the bug is
The new describe("app.bundlerOptions validation", ...) block in test/js/bun/http/bun-serve-args.test.ts invokes serve({ app: ... }) directly in the test process and asserts on specific error messages such as 'app.bundlerOptions' must be an object and 'app' is missing 'framework'. However, the app option in Bun.serve is feature-flagged: when the bake flag is off, the app key is silently ignored and serve() falls through to the generic missing-handler error. On a non-canary, non-debug release build, none of the asserted messages are ever produced and all four test blocks fail.
Code path
src/bun_core/feature_flags.zig:135-138—bake()returnsenv.is_canary || env.isDebug || BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE.get().src/runtime/api/BunObject.zig:1013passesallow_bake_config = bun.FeatureFlags.bake()intoServerConfig.fromJS.src/runtime/server/ServerConfig.zig:837-841— when!opts.allow_bake_config/!bun.FeatureFlags.bake(), the entireif (try args.get(global, "app")) |...|block doesbreak :brk, sobake.UserOptions.fromJSis never called andargs.bakestaysnull.- Execution then reaches
ServerConfig.zig:908-911, which throwsBun.serve() needs either:\n- A routes object\n- A fetch handler...because nofetch/routes/appwas registered.
Why existing infra doesn't cover this
The flag is set in test/harness.ts:64 (BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE: "1"), but that object (bunEnv) is only used as the env for spawned child processes. Every existing bake test in test/bake/ spawns a child via the bake harness and inherits bunEnv, so the flag is always present there. These new tests are the first in the repo to call serve({ app: ... }) in-process, so they inherit the test runner's environment instead.
scripts/runner.node.mjs:1182-1202 builds the env for the bun test process by spreading ...process.env and adding BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING: "1", BUN_DEBUG_QUIET_LOGS, etc. — but not BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE. These two flags are independent (env_var.zig), so the internal-testing flag does not enable bake.
When it manifests
.buildkite/ci.mjs:1293-1297 sets canary: 0 when the commit message contains [release] or the RELEASE env var is set, and line ~522 passes --canary=off to the build. The resulting binary has env.is_canary = false and env.isDebug = false, so bake() evaluates solely to the env var — which the runner doesn't set. skipTests is an independent option, so the test suite still runs against this binary.
Standard PR CI is unaffected because getCanaryRevision() returns 1 for non-main branches (so is_canary is true and bake() short-circuits to true), which is why this PR's own build will pass. The failure surfaces on the next release-build test job, or for anyone running the suite against a stable release binary locally.
Step-by-step proof
On a release (non-canary, non-debug) binary with the runner's env:
- Test calls
serve({ app: { bundlerOptions: 5 } }). BunObject.zig:1013→allow_bake_config = bake() = false || false || (env var unset) = false.ServerConfig.zig:837reads the"app"property, butServerConfig.zig:838-840sees!bun.FeatureFlags.bake()and doesbreak :brk—UserOptions.fromJS(and the newisObject()check) never runs.args.bakeis stillnull, nofetch/routeswere provided →is_fetch_requiredpath atServerConfig.zig:908throws"Bun.serve() needs either:...".expect(...).toThrow("'app.bundlerOptions' must be an object")fails because the actual message is"Bun.serve() needs either:...". The same applies to thebundlerOptions.${key},.minify, and'app' is missing 'framework'assertions.
Fix
Any of:
- Add
process.env.BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE = "1";at the top of the newdescribe(the flag is read viagetenvat call time, so setting it from JS beforeserve()works). - Wrap the block with
describe.skipIf(...)based on the feature being unavailable. - Add
BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE: "1"to the env inscripts/runner.node.mjsalongsideBUN_FEATURE_FLAG_INTERNAL_FOR_TESTING.
1 participant
Passing a non-object value for
app.bundlerOptions, or for itsserver/client/ssr/minifysub-options, hit a debug assertion inJSValue.get(target.isObject()) instead of throwing a proper error.All three now throw a
TypeErrorwith a descriptive message rather than panicking.Also fixes
minify: falsewhich previously fell through the boolean check and attempted a property lookup on the boolean value.Found by Fuzzilli (fingerprint
86855f8bedcf582b).