Skip to content

Validate app.bundlerOptions values are objects in Bun.serve #30446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

Validate app.bundlerOptions values are objects in Bun.serve #30446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/bake/bake.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,17 @@ pub const UserOptions = struct {
}

if (try config.getOptional(global, "bundlerOptions", JSValue)) |js_options| {
if (!js_options.isObject()) {
return global.throwInvalidArguments("'" ++ api_name ++ ".bundlerOptions' must be an object", .{});
}
if (try js_options.getOptional(global, "server", JSValue)) |server_options| {
bundler_options.server = try BuildConfigSubset.fromJS(global, server_options);
bundler_options.server = try BuildConfigSubset.fromJS(global, "server", server_options);
}
if (try js_options.getOptional(global, "client", JSValue)) |client_options| {
bundler_options.client = try BuildConfigSubset.fromJS(global, client_options);
bundler_options.client = try BuildConfigSubset.fromJS(global, "client", client_options);
}
if (try js_options.getOptional(global, "ssr", JSValue)) |ssr_options| {
bundler_options.ssr = try BuildConfigSubset.fromJS(global, ssr_options);
bundler_options.ssr = try BuildConfigSubset.fromJS(global, "ssr", ssr_options);
}
}

Expand Down Expand Up @@ -202,9 +205,13 @@ const BuildConfigSubset = struct {
minify_identifiers: ?bool = null,
minify_whitespace: ?bool = null,

pub fn fromJS(global: *jsc.JSGlobalObject, js_options: JSValue) bun.JSError!BuildConfigSubset {
pub fn fromJS(global: *jsc.JSGlobalObject, comptime name: []const u8, js_options: JSValue) bun.JSError!BuildConfigSubset {
var options = BuildConfigSubset{};

if (!js_options.isObject()) {
return global.throwInvalidArguments("'bundlerOptions." ++ name ++ "' must be an object", .{});
}

if (try js_options.getOptional(global, "sourcemap", JSValue)) |val| brk: {
if (try bun.schema.api.SourceMapMode.fromJS(global, val)) |sourcemap| {
options.source_map = sourcemap;
Expand All @@ -215,13 +222,17 @@ const BuildConfigSubset = struct {
}

if (try js_options.getOptional(global, "minify", JSValue)) |minify_options| brk: {
if (minify_options.isBoolean() and minify_options.asBoolean()) {
if (minify_options.isBoolean()) {
options.minify_syntax = minify_options.asBoolean();
options.minify_identifiers = minify_options.asBoolean();
options.minify_whitespace = minify_options.asBoolean();
break :brk;
}

if (!minify_options.isObject()) {
return global.throwInvalidArguments("'bundlerOptions." ++ name ++ ".minify' must be a boolean or an object", .{});
}

if (try minify_options.getBooleanLoose(global, "whitespace")) |value| {
options.minify_whitespace = value;
}
Expand Down
36 changes: 36 additions & 0 deletions test/js/bun/http/bun-serve-args.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,3 +670,39 @@
}
});
});

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", () => {

Check failure on line 700 in test/js/bun/http/bun-serve-args.test.ts

View check run for this annotation

Claude / Claude Code Review

New bundlerOptions tests fail on non-canary release builds 

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.serv
Comment on lines +674 to +700

@claude claude Bot May 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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() returns env.is_canary || env.isDebug || BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE.get().
  • src/runtime/api/BunObject.zig:1013 passes allow_bake_config = bun.FeatureFlags.bake() into ServerConfig.fromJS.
  • src/runtime/server/ServerConfig.zig:837-841 — when !opts.allow_bake_config / !bun.FeatureFlags.bake(), the entire if (try args.get(global, "app")) |...| block does break :brk, so bake.UserOptions.fromJS is never called and args.bake stays null.
  • Execution then reaches ServerConfig.zig:908-911, which throws Bun.serve() needs either:\n- A routes object\n- A fetch handler... because no fetch/routes/app was 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:

  1. Test calls serve({ app: { bundlerOptions: 5 } }).
  2. BunObject.zig:1013 → allow_bake_config = bake() = false || false || (env var unset) = false.
  3. ServerConfig.zig:837 reads the "app" property, but ServerConfig.zig:838-840 sees !bun.FeatureFlags.bake() and does break :brk — UserOptions.fromJS (and the new isObject() check) never runs.
  4. args.bake is still null, no fetch/routes were provided → is_fetch_required path at ServerConfig.zig:908 throws "Bun.serve() needs either:...".
  5. expect(...).toThrow("'app.bundlerOptions' must be an object") fails because the actual message is "Bun.serve() needs either:...". The same applies to the bundlerOptions.${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 new describe (the flag is read via getenv at call time, so setting it from JS before serve() works).
  • Wrap the block with describe.skipIf(...) based on the feature being unavailable.
  • Add BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE: "1" to the env in scripts/runner.node.mjs alongside BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING.

for (const value of [true, false]) {
expect(() => {
// @ts-expect-error - Testing runtime validation
serve({ app: { bundlerOptions: { server: { minify: value } } } });
}).toThrow("'app' is missing 'framework'");
}
});
});
Loading