Skip to content

dev server: read a directory again when a file that no build bundled fails to open - #46846

Draft
robobun wants to merge 3 commits into
mainfrom
robobun/761f31d9/dev-server-reread-directory-of-missing-unbundled-file
Draft

robobun wants to merge 3 commits into
mainfrom
robobun/761f31d9/dev-server-reread-directory-of-missing-unbundled-file

Conversation

@robobun

@robobun robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Draft, not ready. The branch has a first design that is worse than main in some orders. The fix below waits for #38212 and #36874.

Problem

  • A file gets another extension after the resolver listed its directory and before any build read it. Its import ships dead (status 200, Failed to load bundled module in the browser), also after a save.
  • The read of the old path fails with ENOENT. on_file_deleted (src/runtime/bake/dev_server/incremental_graph.rs:1620) returns early for a file that no build bundled, and the resolvers stay at generation 0, so the stale listing stays.

Fix

  • Prototype: the build after such a failed open draws a new resolver generation, and reads each directory it resolves in again.
  • Verified on the prototype: 8 of 10 new tests in test/bake/dev/bundle.test.ts fail on main, and all pass. Self-reviewed: 12 concerns raised, none addressed yet (Notes).

Background

Downsides

  • A directory read that fails with an error other than ENOENT breaks imports into it until restart (main recovers). resolver: keep cached DirEntry when a stale-generation refresh fails #36874 addresses it.
  • Later builds pay one failed directory open per bare import of a hoisted package: 3.3 per save of a file with three (main 0).
  • A page built from removed files of a re-read directory stays at 200 (main: 500 when another page asks for them).
Notes

Origin. Found while a fuzz report on #46648 was checked. No user reported this form. It needs no barrel. #46648 makes it reach more pages: a barrel that loads every export reads each re-export in every build, so each page that imports the barrel asks for the moved file. #17576 ("HMR cannot resolve new or renamed files") is the same class for a file that a build has bundled.

Repro without a barrel (Bun.serve HTML routes /a and /b, development: true): a.ts imports ./src/x, b.ts imports ./src/y. Load /a, rename src/y.ts to src/y.tsx, then load /b.

Step main 179664e prototype
first build of /b 200, asks for src/y.ts, no EXPORT_Y the same
second request of /b the same the same
after a save of a.ts the same the same
after a save of b.ts the same asks for src/y.tsx, holds EXPORT_Y

Designs tried, in order. All numbers are from release builds.

  1. Drop the listing (bust_dir_cache) in the early return of on_file_deleted (this branch, with a check that the directory exists). It heals the rename at the next build of the importer. It is worse than main in these orders:
    • A file named by a bare specifier or a tsconfig alias is removed, two builds ask for it, and it returns. It stays Could not resolve until restart. main finds it at a save of the importer.
    • A package is updated in place and one build misses two files, one bundled and one not. The early drop makes the importer walk of on_file_deleted skip an importer that no longer resolves. A page then answers 200 with removed modules in 10 of 15 runs. main answers 500.
    • Each drop leaks the old listing: 898 to 964 KB of RSS for a directory of 5,000 entries.
  2. Drop the listing when the build ends, and only if the directory has the file's name with another extension. It removed the two orders above for most files. A design review ranked it below no change: the guess strands a returned file when a same-name sibling exists (c.css beside c.js), it scans the directory on the JS thread, and it still leaks.
  3. The prototype: raise the resolver generation for the next build.

The prototype (robobun/761f31d9/dev-server-generation-after-failed-open, 10 source files, +46/-9, of which 6 files and +27 are in src/runtime/bake/dev_server).

  • handle_parse_task_failure sets DevServer.reread_directories when a file with no graph record fails to open. A failed resolve (ModuleNotFound) does not set it.
  • start_async_bundle takes the flag, draws bun_resolver::next_generation() and writes it to the server, client and SSR resolvers. The generation stays raised.
  • While the generation is above 0, an unresolved import sets the flag again. This is what finds a file again after it was away.
  • Generation is u32, next_generation() is one counter for the process, and the Bun.build thread draws from it.

What the prototype does in each case.

Case main 179664e prototype
c.js to c.jsx, or to c/index.js (bare specifier, node_modules) dead import after every save found at the first save after the failing build
the same in a directory of the project the same the same
the same in a symlinked package directory dead import after three saves found
server code (minimalFramework) 500 after every save 500 in the failing build, then found
a build that does not ask for the file comes between dead found
c.js removed (bare or alias), two builds ask for it 200 with a dead import, both builds 200 with a dead import, then 500 Could not resolve
then c.js returns found at a save of the importer found at the next request, with no save
package updated in place, both misses in one build /page1 500 /page1 500
a bundled file fails to open, then a page asks for removed files of another package 500 500
package updated in place, an earlier build asked for the renamed or removed file /page1 500 /page1 200 with old modules
the package path is a regular file for one build, after one flagged build 500, then 200 when it is a directory again 500 on every later save, until restart

The last two rows are worse than main. For the first of them, main gives the same 200 when the first missing file is one that a build has bundled. The last row is the cached error of a failed re-read, which #36874 changes.

Tests on the prototype (test/bake/dev/bundle.test.ts, ten devTests). Eight fail on main. Two pass on main and pin what must not change (reports an updated package to a page that imports a file beside its barrel, a bundled file that is gone leaves the removed files of another package to be reported). All ten pass 30 of 30 on a release build, and the file passes (64) on a debug build.

Each clause was removed in turn on a scratch build:

Removed Tests that fail
the flag at the failed open the eight
the flag at an unresolved import the two "away for two builds" tests
the write to the server resolver the server test and the two "away" tests
the write to the client resolver seven
the raised generation after one build finds a module that moved after a build that does not ask for it
the condition "no graph record" a bundled file that is gone ...
the write to the SSR resolver none
the ModuleNotFound exclusion none
the condition "generation above 0" for the second flag none

Measurements on the prototype (main is a release build of the src/ of 179664e).

  • Directory syscalls (ptrace counter), a page with three bare imports whose packages are not in the nearest node_modules and one relative import. 300 saves before any failed open: 0 directory opens and 0 getdents64 on main and on the prototype. The save that heals: 7 directory opens, 3 failed directory opens, 14 getdents64 (15 with 500 entries in the package, 33 with 5,000). 300 saves after it: 987 failed directory opens (3.3 per save), main 0. A resolver above generation 0 opens a missing directory again on every lookup. Bun.build pays the same from its second build.
  • RSS for each rename that heals, 60 rounds, three runs: 5,000 entries -17 / 43 / 6 KB per round (main 7 / 7 / 22, the listing drop 964 / 898 / 929). No listing is dropped.
  • Instructions for each Bun.resolveSync, warm cache, 250,000 resolves: main 9,927 / 9,927 / 9,929, prototype 9,929 / 9,920 / 9,925.
  • Release text: 107,556,528 to 107,557,808 bytes (+1,280).

Why it waits.

Self-review concerns, none addressed yet.

  1. Move the counter to bundler: advance the resolver generation once per build instead of once per queue drain #38212 (above).
  2. The failed re-read that stays until restart (above).
  3. The second flag has no test for its condition. A miss such as Dev server: tsconfig paths alias never resolves a file created after the server starts #43391 then heals or not by what happened earlier in the process (not re-run by me).
  4. The Bun.build draw keeps the race at thread start and the rule of one generation for each drain.
  5. The runtime resolver of the same process stays at generation 0. After a re-read while a file is away, import() of the returned file fails where main finds it. Not re-run by me.
  6. Only the importer that is built again heals. Other importers of the failing build keep the dead import until they are built again.
  7. The SSR write has no test. Without a separate SSR graph that resolver is not used.
  8. "No graph record" looks at the graph of the failing side only.
  9. No test asserts the two rows that are worse than main.
  10. A route with an import that never resolves reads its directories again on every request, once the generation is above 0.
  11. The symlink test makes the link after the server starts, and has no Windows run.
  12. A bare import of a file that returned answers Could not resolve for one build in one order.

Not in this PR. The directory event that is ignored while no import is unresolved (hot_reload_event.rs:97), the failing build that still serves the dead import, and an importer that on_file_deleted skips because it no longer resolves. Each is the same on main.

…fails to open

The resolver keeps one listing for each directory. A file that moved after its
directory was listed, and that no build had read, still resolves to its old
path. The read fails with ENOENT, and IncrementalGraph::on_file_deleted returned
for a file with no graph record before it dropped that listing. The import then
stayed a dead import in every later build, until the server restarted.

The early return now drops the listing, so the next build that resolves the
import reads the directory again. A file with a graph record keeps its order:
its importers are enqueued before the listing is dropped.
@robobun

robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:15 AM PT - Oct 25th, 2026

❌ @robobun, your commit 49651bc has 1 failures in Build #132439 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 46846

That installs a local version of the PR into your bun-46846 executable, so you can run:

bun-46846 --bun

@robobun

robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: draft. It waits for #38212 (the resolver generation for each Bun.build, which should own the process-wide counter) and #36874 (a failed re-read of a directory). The branch still has the first design. The body links the prototype of the design that replaces it.

How this was reproduced. A dev server (Bun.serve HTML routes, development: true) on main 179664e. a.ts imports ./src/x, and b.ts imports ./src/y. Load /a, rename src/y.ts to src/y.tsx, then load /b: the script of /b asks for src/y.ts and does not hold the module, with status 200. It stays so after a save of b.ts.

The test finds a module that moved before any build read it in test/bake/dev/bundle.test.ts fails on main.

PR: #46846

@claude claude Bot left a comment

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.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread test/bake/dev/bundle.test.ts Resolved
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 6b88a35 and c96c1e8.

Files selected for processing: 1
  • test/bake/dev/bundle.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

When a deleted path is absent from the incremental graph, the dev server now busts the resolver’s directory cache before returning. Two regression tests cover module resolution after a rename and build errors after package removal.

Changes

Dev server cache invalidation

Layer / File(s) Summary
Deletion cache invalidation and regression coverage
src/runtime/bake/dev_server/incremental_graph.rs, test/bake/dev/bundle.test.ts
When a deleted path has no graph index, on_file_deleted derives its directory, busts the resolver cache, and returns. Tests cover resolution after a package module rename and build errors after package removal.

Suggested reviewers: jarred-sumner, alii

Priority: Normal

Merge Risk: Minimal · up to c96c1

No actionable merge-blocking risk remains from the reviewed changes.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly describes the main change: rereading a directory after an unbundled file fails to open in the dev server.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description clearly explains the problem, proposed fix, limitations, design trade-offs, and verification results. Although it does not use the template headings verbatim, it provides the required …
Full details: Description check

Explanation

The description clearly explains the problem, proposed fix, limitations, design trade-offs, and verification results. Although it does not use the template headings verbatim, it provides the required information in equivalent sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

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.

Still open from earlier reviews (1):

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

When a package is removed, the failed open of each bundled file is what
builds its importers again, and each importer is found through the old
listing. A file of that package that no build had read could fail first and
drop the listing, and then the importers of the bundled files were skipped.
The listing is now dropped only while the directory still exists.

@claude claude Bot left a comment

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.

Beyond the inline findings, I also checked the shape of the new bust at incremental_graph.rs:1622-1624 against the existing one at :1672-1673 in the same function: the dirname(...).unwrap_or(abs_path) fallback and the discarded bust_dir_cache bool match it, and bust_dir_cache (resolver.rs:2480) strips trailing Windows slashes itself, so using bun_paths::dirname rather than dirname_platform is not a problem here. The exists(dirname) check can race a concurrent delete, but the worst case is a skipped bust, which is the base behavior.

Extended reasoning...

The change adds five lines to the no-graph-record early return of IncrementalGraph::on_file_deleted that bust the resolver's directory listing when the directory still exists, plus two dev-server tests. It touches no security-sensitive surface. The two confirmed inline findings (a removed-or-replaced package can answer 200 with stale modules, and the test guarding the exists clause depends on ENOENT arrival order) are the reason a human should weigh in; the remaining style concerns about the new lines were ruled out by comparison with the existing bust in the same function.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread test/bake/dev/bundle.test.ts Resolved

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants