Conversation
…of that type DevServer::init skips a file system router type whose root directory does not exist. The route_lookup entry for the server entry point of a later type used the position in framework.file_system_router_types, not the index of the type in the router. After a skip the entry named the wrong route, or one past the end of the route list. A build error in a bundle that holds that server entry point aborted the dev server with 'index out of bounds: the len is 1 but the index is 1' in index_failures. The first request to a page with a syntax error is enough. A save of the server entry point while a browser listened for hot updates aborted the same way in finalize_bundle. Take the index from the number of types pushed so far, and store Type::root_route_index of it.
|
Warning Review limit reached
On-demand reviews are free for the next 5 days. After that, they cost $0.25 per reviewed file. Or wait 18 seconds 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 (2)
Comment |
StatusReproduction (release 1.4.3-canary.1, Linux x64, and a debug build of main):
With this PR the same request answers 500 "Build Failed", and the server stays up. The original report used Verification
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings (both pre-existing limits, not introduced here), I also checked the fix itself: Type::root_route_index(type_index) matches FrameworkRouter::init_empty, which pushes one root route per accepted type in order (FrameworkRouter.rs:187-202), so route index == type index holds. The u8::try_from(types.len()).expect(..) is bounded by the 256-type cap in bake_body.rs:873 and mirrors the identical cast in init_empty. The other config-position TypeIndex at server_body.rs:880 is never read (both matches in server/mod.rs discard the payload), so it is unaffected.
Extended reasoning...
The diff replaces the enumerate() config-position index with the accepted-type index taken from types.len() before the push, and derives the route via root_route_index, which is RouteIndex::init(type_index as u32); init_empty lays out exactly one root route per type in push order, so the mapping is correct. The u8 cast is provably in range given the 256-entry cap enforced in Framework::from_js and the same cast already in init_empty. The two posted findings concern behavior that predates this PR (shared server entry across mounts and the silent skip of a missing root), which the author acknowledges in the description; they are worth a human's judgment on scope but do not indicate a defect in the change itself. Two new dev-server tests exercise both the hot-reload path and the index_failures path with a skipped first router type.
One verified lower-impact observation (a convention, logging or cleanup point) was not posted.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/runtime/bake/DevServer.rs— A user who mistypes thedirof a{ dir, style }mount or a frameworkrootgets a dev server that starts cleanly and answers 404 for that whole mount, with no message. DevServer.rs:924-928 turns a failed read_dir_info_ignore_error into a barecontinue, dropping the router type without logging. Fix: report the missing root (a warning naming the joined path, or an error at startup) at DevServer.rs:928 and at the twin skip in production.rs:552, so the dropped mount is visible. The PR's own tests rely on this silent skip and lock it in as the supported shape.Extended reasoning...
read_dir_info_ignore_error at DevServer.rs:926 swallows ENOENT and any other error and returns None. The
continueat :928 skips types.push and route_lookup.put, so the type has no root route and scan_initial_routes never visits it. server_body.rs:880 still returns AnyRoute::FrameworkRouter for that mount, but both readers (server/mod.rs:185 and :2562) ignore the payload, so nothing on the server side notices the type is gone. Requests under the mount prefix fall through to the router with no matching route and get the dev server's not-found response. Population: anyBun.serve({ routes: { "/x/*": { dir, style } } })user with a wrong relative dir, at every request. The dismissing finder called it pre-existing and out of scope; the diff adds a comment at :938 that documents the skip as intended and the new tests userouterType("missing")as fixture, so this PR is the point where the silent skip becomes a supported contract. Remedy: emit a diagnostic naming the joined root at both skip sites.Verification: pre-existing; acknowledged in diff: PR description says "The silent skip of a missing root stays.
production.rsskips the same way." and the claim is accurate. Trigger: a{ dir, style }mount or frameworkrootpointing at a directory that does not exist. Mechanism verified:read_dir_info_ignore_error(src/resolver/resolver.rs:4118) is.ok().flatten(), so ENOENT and every other error…
|
Replies to the two pre-existing findings of the review. This PR changes neither of them. Silent skip of a missing root. Confirmed on 1.4.3-canary.1: About the tests: they need a skipped router type, because the wrong index exists only after a skip. If a missing root becomes a startup error, this bug class goes away, and these two tests go with it. Shared server entry point. Answered in the inline thread. #42813 makes |
|
Updated 7:25 PM PT - Sep 15th, 2026
✅ @robobun, your commit 831878510f46fcd287ef88b2e5193feaca1e10d3 passed in 🧪 To try this PR locally: bunx bun-pr 42840That installs a local version of the PR into your bun-42840 --bun |
This branch has not been deployed
1 participant
Problem
panic: index out of bounds: the len is 1 but the index is 1. The trigger is a build error in a bundle that holds a router type's server entry point. Example: the first request to a page with a syntax error. It needs a router type with a missing root directory before one whose root exists. Stable builds reach it: eachroutes: { "/a/*": { dir, style } }mount is one router type.DevServer::init(src/runtime/bake/DevServer.rs:912). It skips a type with a missing root, but stores theenumerate()index inroute_lookup, not the type's router index.Fix
types.len()before the push. StoreType::root_route_index(type_index)inroute_lookup.FrameworkRouter::init_emptymakes one root route per pushed type, in order.test/bake/dev/bundle.test.ts, both panic without the fix (release and debug). Alsoframework-router,hot,esm.Background
framework.fileSystemRouterTypesentry: arootdirectory of route files plus a server entry point.FrameworkRouterkeeps accepted types intypesand all routes inroutes. Routeiis the root route of typei.route_lookupmaps a server graph file to its route.IncrementalGraph::trace_dependenciescopies entries intoframework_routes_affected.index_failuresandfinalize_bundlepass each one torouter.route_ptr().Notes
FrameworkRouter::route_ptr(FrameworkRouter.rs:1270),DevServer::index_failures(DevServer.rs:3403),finalize_bundle(DevServer.rs:4206).production.rsskips the same way. Dev server skips a router type with a missing root directory and prints no message #42842 tracks whether a skipped type should print a message.DevServer.zighad the same line:Route.Index.init(@intCast(i))afterorelse continue.process_chunk_dependenciestraces every file of a bundle, so a bundle that holds the server entry point puts its index inframework_routes_affected. The first bundle of a route always holds it.index_failuresreads the whole list when the same bundle has a failure in any file.router.route_ptr():index_failures, the "List 1" block offinalize_bundle(only with a hot update subscriber and a file change), and the client components block offinalize_bundle. The first test covers "List 1": a browser on/must get a server-side reload, which also proves that the right route is marked. The second test coversindex_failurestwo ways, with no browser: a page that is broken on the first request, then a syntax error saved into the server entry point. Each time the route must answer 500 and recover after the next save.BUN_FEATURE_FLAG_EXPERIMENTAL_BAKE:Bun.serve({ development: true, routes: { "/a/*": { dir: "./does-not-exist", style: "nextjs-pages" }, "/b/*": { dir: "./pages", style: "nextjs-pages" } } })with the React packages installed and onepages/index.tsxthat has a syntax error. The firstGET /exits with SIGABRT. With the fix it answers 500 "Build Failed". The{ dir, style }mount has no feature gate (src/runtime/server/server_body.rs:793). This PR does not change that.[missing, A, B]giveroutes = [rootA, rootB, ...]. The old code mapped the server entry ofAto route 1 (rootB) and the server entry ofBto route 2 (the first scanned route, or out of bounds).route_lookupholds one route per file. When router types share a server entry point (all{ dir, style }mounts do), the last accepted type wins the entry, so a save marks only that type's routes. That is a separate, older defect with no missing root needed. bake: serve routes behind symlinks, reload every route that shares a file #42813 makes the map hold several routes per file. It keeps the old index line, so the two changes compose: whichever lands second storesType::root_route_index(type_index)throughadd_route_of_file.src/runtime/bake/the only other loop that uses anenumerate()index after acontinueisproduction.rs:917. It stores an index into the list it enumerates, which is correct.src/runtime/server/server_body.rs:880also makes aTypeIndexfrom the mount position, but nothing reads that payload (both matches areAnyRoute::FrameworkRouter(_)) and Remove dead code from bun_bundler, bun_install, bun_runtime, bun_parsers, and the JSC private host functions #41088 deletes the variant. It is excluded on purpose.fileSystemRouterTypesis capped at 256 entries inFramework::from_js, and{ dir, style }mounts at 255, so theu8cast cannot fail. A 256th existing type would getTypeIndex255, whichGenericIndex::initrejects with a debug assertion.FrameworkRouter::init_emptyalready does the same for that input, so this PR does not change it.clientEntryPoint, so that the page can loadmeta.modules[0]and run the real HMR client. That client is what subscribes to hot updates. Without a client entry point, the route's client bundle has no main module and the HMR client reportsFailed to load bundled module 'null'. This PR does not change that.claude/bake-consolidation) still has the old line, so it does not cover this.{ dir, style }mounts, the three readers, the excludedserver_body.rssite, and the one-route-per-file limit with bake: serve routes behind symlinks, reload every route that shares a file #42813 (4 concerns, all addressed). It raised one more execution concern that is not identified in my notes, so a second pass re-checked the final diff for execution problems. That pass found one should-fix (a 102-column comment, fixed) and three nits. Two nits are applied (helper names, a more exact test comment). The third is not: an extra test where the bad index is in bounds. Both tests already assert behavior that a bounds-check-only change would fail (the reload in the first, the 500 after the server entry point breaks in the second), and each dev server test costs about 2.5 s on a debug build.test/bake/dev/bundle.test.ts(24 pass),test/bake/framework-router.test.ts,test/bake/app-options.test.ts,test/bake/deinitialization.test.ts,test/bake/dev/esm.test.ts,test/bake/dev/plugins.test.ts,test/bake/dev/vfile.test.ts,test/bake/dev/import-meta-inline.test.ts,test/bake/dev/hot.test.ts.[human-review] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file