bundler: fail the build when an entry point has no module to bundle - #38778
bundler: fail the build when an entry point has no module to bundle#38778robobun wants to merge 5 commits into
Conversation
An entry point that the resolver disabled (mapped to false by a package.json "browser" field, or a Node.js builtin that browser builds stub out) was dropped without a log entry, as was an entry point an onResolve plugin marked external. With a single entry point the linker then ran with none and aborted on chunks[0]; with several, the build succeeded with the entry point missing. resolve_entry_point now logs an error for a disabled result and returns Err like any other resolution failure, and on_resolve logs esbuild's "cannot be marked as external" error for external entry points. The CLI drivers report entry point errors after wait_for_parse() instead of tearing the bundle down immediately: the runtime parse is already scheduled at that point, and deinit_without_freeing_arena could observe a Worker that get_worker_slow had published but not yet initialized.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
Updated 7:05 AM PT - Aug 15th, 2026
❌ @robobun, your commit 98f0e34 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 38778That installs a local version of the PR into your bun-38778 --bun |
|
Status: ready for review at 98f0e34. Every lane that ran is green for this diff; two items in CI are unrelated to it. Reproduced on 1.4.0-canary.1 (b7a0431) and on a debug build of main: with Current shape: CI (builds 97059 for the previous revision and 97641 for this one): 176 to 177 jobs passed in each, including every lane that runs the new tests. Not green in either: the |
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/bundler/bundle_v2.rs:3912-3918— The new comment at lines 3914-3917 says entry-point errors are now reported after the pool drains to avoid tearing down half-initialized workers, but the?onenqueue_entry_points_normal(...)?one line above (and onenqueue_entry_points_bake_production(...)?at line 4086) still short-circuits pastwait_for_parse()if a post-schedule allocation inside it fails — the runtime parse task is dispatched at line 3292 before those?sites.scan_module_graph_from_cli(lines 4045-4052) already has theif let Err(err) = ... { this.wait_for_parse(); return Err(err); }shape with a comment naming this exact hazard; consider mirroring it here so the invariant the new comment describes actually holds. (Trigger is OOM-only, so nit.)Extended reasoning...
What the bug is
This PR removes the
has_errors()early return betweenenqueue_entry_points_*andwait_for_parse()ingenerate_from_cli(bundle_v2.rs:3912-3918) andgenerate_from_bake_production_cli(bundle_v2.rs:4086-4089), and adds a comment stating that entry-point errors are now reported after the pool drains because "tearing the workers down while one is still setting itself up reads a half-initializedWorkerout ofworkers_assignments".However, the
?onthis.enqueue_entry_points_normal(unsafe { &*entry_points })?(line 3912) andthis.enqueue_entry_points_bake_production(entry_points)?(line 4086) is left unchanged and still short-circuits pastwait_for_parse()when a post-schedule allocation inside those functions fails. So the invariant the new comment describes ("no return before the drain") is not actually enforced by the code one line above it.Code path that triggers it
enqueue_entry_points_common()(called first at bundle_v2.rs:3045) schedules the runtime parse task on the worker pool at line 3292 and returnsOk(()):self.increment_scan_counter(); self.graph.pool().schedule(runtime_parse_task); Ok(())
Every subsequent
?insideenqueue_entry_points_normalruns after that task is already dispatched:self.reserve_source_indexes_for_bake()?— line 3047self.graph.input_files.ensure_unused_capacity(num_entry_points)?— line 3054self.enqueue_entry_item(...)?— lines 3069/3073 and 3098 (which itself hasinput_files.append(...)?at line 2761)
If any of these return
Err, it propagates throughenqueue_entry_points_normal, out through the?at line 3912, out of the closure at line 4005, and intodeinit_without_freeing_arena()at line 4014 — the exact teardown path the removedhas_errors()return took, and per the PR description the one that produced 26/30 ASAN SEGVs. The same shape applies at line 4086.Why existing code does not prevent it
The sibling driver
scan_module_graph_from_cli(bundle_v2.rs:4045-4052) already guards this precise case, with a comment naming the same hazard:// enqueueEntryPoints schedules the runtime task before any fallible // allocation. If a later allocation fails we must still drain the // pool so workers aren't left holding pointers into the caller's // stack-allocated Transpiler. if let Err(err) = this.enqueue_entry_points_normal(entry_points) { this.wait_for_parse(); return Err(err); }
generate_from_cliandgenerate_from_bake_production_clido not have this shape — they use bare?.Step-by-step proof
generate_from_clicallsthis.enqueue_entry_points_normal(...)?at line 3912.enqueue_entry_points_normalcallsenqueue_entry_points_common()at line 3045, which schedules the runtime parse task at line 3292 and returnsOk(()). A worker thread picks it up and beginsget_worker_slow.enqueue_entry_points_normalthen callsreserve_source_indexes_for_bake()?at line 3047. Suppose that (orensure_unused_capacityat 3054, orinput_files.appendat 2761) returnsErr(AllocError).- The
?at line 3912 propagatesErrout of the closure.wait_for_parse()at line 3918 is never reached. - Control jumps to line 4005 and then to
deinit_without_freeing_arena()at line 4014, which tears down the bundle while the runtime parse task is still starting on a worker. - Per the PR description, that teardown is what "reads a half-initialized
Workerout ofworkers_assignments" — the exact race the new comment says is now closed.
Impact
The only trigger is allocation failure on those specific
?sites — all of them are OOM paths. So the PR description's claim that "this change removes the only user-reachable way to hit it" is arguably still true in practice, and the underlyingget_worker_slowpublication-order bug is being fixed separately per the PR. This is why it's a nit rather than blocking.But: (1) the PR adds a comment at lines 3914-3917 describing an invariant the
?one line above violates, so the comment misdescribes what the code enforces; (2) the sibling function 130 lines below already has the correct shape; (3) per REVIEW.md "Fix the whole class in the same PR (same-class sites are ONE concern)", and given the PR's stated goal is closing paths to this race, mirroring the pattern is cheap and makes the comment accurate.Fix
Replace the
?at lines 3912 and 4086 with the same patternscan_module_graph_from_cliuses:if let Err(err) = this.enqueue_entry_points_normal(unsafe { &*entry_points }) { this.wait_for_parse(); return Err(err); }
and likewise for
enqueue_entry_points_bake_productionat line 4086.
|
On the additional finding (the Everything that can fail after the runtime task is scheduled is an |
…-no-bundle entries resolve_entry_point is back to what main has: #38391 already carries the same change with tests that also cover Windows and Bun.build. The browser field message gets the same "(entry point)" suffix #38778 uses, and the new test pins that --no-bundle now resolves "entry" to entry.ts the way a bundling build does.
…ld when no entry point survives The onResolve external arm duplicated the error that #35053 already adds, so it is dropped here. In its place the CLI and Bun.build drivers fail with "None of the entry points could be bundled" when parsing ends with an empty entry point list, so any remaining way of dropping every entry point without logging becomes a build error instead of indexing an empty chunk list. The builtin entry point message no longer suggests another target, since a builtin cannot be an entry point for any target.
There was a problem hiding this comment.
Thanks — the doc-comment concern I raised earlier is resolved (the sentence is gone), and the new fail_if_no_entry_points backstop in 98f0e34 means the over-long-specifier arm I flagged now lands on "None of the entry points could be bundled" instead of the chunks[0] panic even before #38391 lands, so that thread is fully closed from my side.
I reviewed the current revision and didn't find any bugs. Given the coordination with four open PRs (#35053, #37480, #38391, #38752), the deliberate divergence from esbuild's empty-output behavior for browser-disabled entry points, and the removal of the pre-wait_for_parse() early return in the two CLI drivers to work around the get_worker_slow publication race, a human look would still be worthwhile.
What was reviewed
reject_disabled_entry_point— checked thatpath_const().is_none()is the right predicate (it skips disabled paths) and that thenodenamespace branch matches how the resolver tags stubbed builtins.- The three driver call sites for
fail_if_no_entry_points()— placed afterhas_errors()so no double error; bake production intentionally excluded per description. - The removed early returns — confirmed
run_from_js_in_new_threadalready had this shape, so the CLI drivers now match; the remaining?onenqueue_entry_points_*is AllocError-only (author's 01:35 note). - Tests follow harness conventions (
tempDir,test.concurrent, subprocess drain viaPromise.all, exit code asserted last,backend: "cli"to isolate the abort).
Extended reasoning...
Overview
This PR fixes a process abort (index out of bounds: the len is 0 but the index is 0 / assertion failed: chunks.len() > 0) that occurs when every entry point resolves to a module the browser resolver has disabled (via a package.json "browser": {"./x": false} map or a stubbed Node builtin like fs). It touches two Rust files in the bundler core (src/bundler/transpiler.rs, src/bundler/bundle_v2.rs) and three test files. The fix has three parts: (1) resolve_entry_point now routes Ok results through a new reject_disabled_entry_point helper that logs an error and returns Err when path_const() is None; (2) a fail_if_no_entry_points() backstop in generate_from_cli and run_from_js_in_new_thread that catches any remaining path that drops every entry point without logging; (3) the two CLI drivers now defer their entry-point-error check until after wait_for_parse(), matching the existing JS driver, to avoid tearing down the pool while a worker is still being published in get_worker_slow (26/30 ASAN SEGVs otherwise).
Security risks
None. This is an error-reporting path in the bundler; no auth, crypto, network, or untrusted-input parsing is touched. The new error messages echo the user-supplied entry-point specifier via bstr::BStr::new(entry_point), which is the same pattern the existing ModuleNotFound arm uses.
Level of scrutiny
Moderate-to-high. The bundler's build-driver control flow is a critical path, and this PR both adds new failure branches and removes two existing early returns. The removal is justified as a workaround for a threading race that a separate PR (#37480) fixes properly, and the author has verified 30/30 clean runs with the change vs 26/30 SEGVs without — but it does mean more parse work runs before a resolution error surfaces, and the shape depends on #37480 not later changing these same lines. The PR also makes a user-facing API choice (error out instead of esbuild's empty-output behavior for a browser-disabled entry point) that the description argues for but a maintainer should sign off on.
Other factors
- Prior review loop: I previously flagged that a doc comment overstated the "every Err is logged" contract given the pre-existing
MAX_PATH_BYTESguard arm. The author removed the doc sentence, pointed to #38391 which restructures that arm, and — in the latest commit — added thefail_if_no_entry_pointsbackstop, which independently prevents that arm from reaching thechunks[0]panic. That concern is fully addressed. - PR coordination: The description explicitly coordinates with four open PRs (#35053 external-entry-point message, #37480 pool teardown join, #38391 over-long specifier, #38752
--no-bundleswitch). Each interaction is called out and the rebase is described as trivial, but a maintainer should confirm the landing order is what they want. - Test coverage: Five new
itBundledCLI tests inbundler_browser.test.ts(single/multi entry,buntarget negative, package-main variant, node-builtin variant), one subprocess test each inbun-build-api.test.ts(JS API, boththrow: falseand thrown paths) andbundler_plugin.test.ts(declined-then-disabled and plugin-external → backstop). All spawn subprocesses so the pre-fix abort stays contained; all follow the repo's harness conventions. The author reportsUSE_SYSTEM_BUN=1failures andbun bd testpasses across the affected files. - comment-cop bot: Flagged six long comments; the author cut each to one line in 6c215c1 and the current diff's comments are appropriately terse.
Problem
bun build --target=browser ./a.tswith{"browser": {"./a.ts": false}}in the enclosing package.json aborts:panic: index out of bounds: the len is 0 but the index is 0(release),assertion failed: chunks.len() > 0insrc/bundler/linker_context/generateChunksInParallel.rs(debug).Bun.build()with the same entry point aborts the whole process,throw: falseor not.bun build --target=browser fs/node:fs, and for a package or directory entry point whosemain/index the browser field disables.Result::path()isNone.Transpiler::resolve_entry_point(src/bundler/transpiler.rs) returned that asOk, andBundleV2::enqueue_entry_item(src/bundler/bundle_v2.rs) returnsOk(None)for it without logging. The build drivers only checklog.has_errors(), so the bundle went on to link with zero entry points, andgenerate_chunks_in_parallelindexeschunks[0].chunks[0](an onResolve plugin returningexternal: truefor the entry point, the over-long specifier in bundler: log the resolve error for an entry point too long for a path buffer #38391); each producer fix only removes its own cause.Fix
resolve_entry_pointlogs an error for a disabled result and returnsErr, the contract it already has for every other entry point failure (callers skip the entry point onErr; the drivers fail the build from the log). Messages:"./a.ts" is disabled due to "browser" field in package.json (entry point), the wordingbun build --no-bundlealready uses for this case, andCannot use Node.js builtin "fs" as an entry pointfor builtins the browser resolver stubs out (no target hint: a builtin is not a bundleable entry point under--target bun/nodeeither, those fail withFile not found).bun build --no-bundle, the resolver represents "disabled" as "no path" on purpose (imports of the module become{}), and emitting fewer outputs than entry points with exit 0 was the silent failure mode here, so the multi-entry case needs the error anyway. esbuild errors for the builtin case too.resolve_entry_pointcovers every entry point path with one check: the CLI,Bun.build(), the plugin-declined fallback inon_resolve, and the bake callers (which pass absolute file paths and cannot hit it today).generate_from_cliandrun_from_js_in_new_threadnow fail withNone of the entry points could be bundledwhen parsing ends withgraph.entry_pointsempty. Both require at least one entry point, so an empty list there always means every entry point was dropped on a path that did not log; that state now ends as a build error instead of atchunks[0], whatever the cause. The bake production driver is not changed: it already tolerates an empty chunk list on purpose. The plugin-external entry point lands on this error today; bundler: use onResolve-returned path for external imports #35053 (open) adds esbuild's specificcannot be marked as externalmessage for it, so this PR does not carry a second copy.wait_for_parse()instead of returning before it, as theBun.build()driver always has. The immediate return tore the bundle down while the runtime parse task was still starting on a worker; 26/30 ASAN runs of the CLI fix died inWorker::deinit_soonreading aWorkerthatget_worker_slowhad published toworkers_assignmentsbefore initializing it. bundler: join in-flight pool tasks before tearing the bundle down #37480 (open) makesdeinit_without_freeing_arenaitself join in-flight work for every driver and does not touch these lines; these two deletions keep this PR's CLI tests deterministic on main today and stay valid after it (the publication order inget_worker_slowis reported separately).--no-bundletoresolve_entry_pointand keeps apath_const().is_none()arm with the same message; after this PR that arm is unreachable and can go in whichever of the two lands second. bundler: log the resolve error for an entry point too long for a path buffer #38391 (over-long specifier) touches the adjacent match arm and its own test at the same spot inbun-build-api.test.ts; both rebases are trivial.test/bundler/bundler_browser.test.ts:browser/EntryPointDisabledByBrowserField,...NextToLiveEntryPoint(the silent drop),...OnlyAppliesToBrowserTarget,EntryPointDisabledByPackageMainBrowserField,EntryPointIsNodeBuiltinStubbedForBrowser(CLI,backend: "cli"so the unfixed abort stays in the child).test/bundler/bundler_plugin.test.ts: a declined entry point that the browser field disables (theon_resolvefallback) gets the specific error; an entry point a plugin marks external gets the backstop error. The CLI driver calls the same helper; the only CLI-reachable zero-entry-point routes left (bun build --target=bun bun:wrap, builtins under--target bun) tripassert_file_path_is_absolutefirst in debug/canary builds, so the backstop is pinned throughBun.build().test/bundler/bun-build-api.test.ts:Bun.build()returnssuccess: falsewith theBuildMessageand rejects with anAggregateErrorcarrying it.USE_SYSTEM_BUN=1: three browser tests and both child-process tests abort with the panic above, the multi-entry test fails with "Errors were expected while bundling".bun bd test: all pass, as do the fullbundler_browser,bundler_plugin,bundler_files,bundler_html,bundler_html_server,bun-build-api,bundler_edgecase,bundler_naming,cliandesbuild/packagejsonfiles.bun-debug build --target=browser ./a.ts: 30/30 runs exit 1 with the message (26/30 ASAN SEGVs with the early return still in place).Background
"browser"field: a package.json map that browser builds apply during resolution; mapping a file or package tofalsemeans "this module is empty in the browser". The resolver expresses that by marking the paths of the resultis_disabled, andResult::path()/path_const()skip disabled paths, so a fully disabled module resolves to a result with no path. The same representation is used forfsandnode:*builtins that have no browser polyfill (those carry thenodenamespace). It only applies to--target browser; absolute file paths are not looked up in it.enqueue_entry_points_*resolve each entry point withresolve_entry_pointand hand the result toenqueue_entry_item, which schedules a parse task and appends tograph.entry_points. The drivers (generate_from_clifor the CLI,run_from_js_in_new_threadforBun.build()) then wait for parsing and fail the build if the log has errors; the linker requires at least one entry point, so every way of losing one has to leave an error in the log, and the new check is the last line of defense for that invariant.on_resolve: when an onResolve plugin matches an entry point, its answer arrives here.NoMatchfalls back toresolve_entry_point(hence the declined-plugin test);Successwithexternal: truedoes nothing for entry points (the case bundler: use onResolve-returned path for external imports #35053 gives a message to).wait_for_parse()spins the bundler's event loop until every scheduled parse task has completed;enqueue_entry_points_commonschedules the runtime's parse task before any entry point is resolved, so on the CLI error path there was always a task in flight at teardown.Release repro (1.4.0-canary.1)
With this branch each of these prints one error and exits 1 / returns
success: false;--target=bunwith the same package.json still bundlesa.ts.Earlier revision of this PR
The first revision also added esbuild's
The entry point "x" cannot be marked as externalerror inon_resolveand had no backstop. Review turned up #35053, which already carries that error (the two conflicted on the same lines), so the arm was dropped in favor of the generic check above; the builtin message used to end with "set target to 'node' or 'bun'", which does not work for an entry point either.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bun-build-api.test.ts test/bundler/bundler_plugin.test.ts