bake: keep a module bundled when a plugin fall-through resolution failure is inside try/catch

[robobun]

Problem

• Dev server with a [serve.static] plugin whose onResolve matches but returns nothing: a client module that does require("./optional-dep") in try/catch, for a file that does not exist, is silently dropped from the route bundle. Nothing is logged; the browser fails with Error: Failed to load bundled module 'index.ts'. This is not a dynamic import, and therefore is a bug in Bun's bundler.
• Without the plugin the same project works: the require throws and the catch runs.
• Cause: on the resolver path taken after a plugin falls through, a missing module under the dev server emptied the importer before checking whether the import handles its own errors. With try/catch no error is then logged, so the file has neither content nor a recorded failure and is left out of the bundle.
• The no-plugin path only empties the importer when it also logs the error.

Fix

• The fall-through path now empties the importer only in the branch that also logs a Could not resolve error.
• Property to check: an importer is emptied exactly when a failure is recorded against it. A handled failure leaves the file bundled with that import disabled, so the require throws at runtime as on the no-plugin path. Error messages are unchanged.
• The missing specifier is still tracked, so creating the file later still re-bundles the importer.
• Verification: a dev-server test that fails on the current release with the error above and passes with this change, plus one checking a plain import of a missing file still shows the Could not resolve overlay. Two Bun.build cases cover the non-dev-server side and pass before and after.

Background

• Bake is bun's dev server. It keeps an incremental graph of every file it has bundled and re-bundles only what changed on an edit.
• A [serve.static] plugin in bunfig.toml is a bundler plugin loaded into the dev server. An onResolve callback that returns nothing falls through to the builtin resolver on a different code path from the no-plugin case. This PR makes the two agree.
• A require inside try/catch marks its import record as handling its own errors: the bundler leaves it unresolved, logs nothing, and it throws at runtime. ignore_module_resolution_errors does the same for every import.
• Under the dev server, truncating a file's parsed parts to zero marks it invalid: the printer skips it and the graph treats it as failed. That is only safe if a failure is logged for the file at the same time.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_plugin.test.ts

Original description

What does this PR do?

With a [serve.static] plugin whose onResolve matches a specifier and returns nothing (falling through to the builtin resolver), a client module that does

let value = "fallback";
try {
  value = require("./optional-dep").value; // ./optional-dep does not exist
} catch {}
console.log(value);
import.meta.hot.accept();

is silently dropped from the dev server's route bundle. The dev server prints Bundled page ...: index.html with no error, and the browser fails with:

Error: Failed to load bundled module 'index.ts'. This is not a dynamic import, and therefore is a bug in Bun's bundler.

Without the plugin the same project works: the require throws at runtime, the catch runs and the page renders the fallback.

Cause. BundleV2::run_resolver is the resolver path used after a plugin falls through. When the dev server is active and the resolver returns ModuleNotFound, it emptied the importer's parts ("turn this into an invalid AST so incremental mode skips it") before looking at the import record's HANDLES_IMPORT_ERRORS flag. For a require() inside try/catch that flag is set, so no error is logged, but the importer's parts are already gone: finish_from_bake_dev_server filters the file out as failed, while prepare_and_log_resolution_failures has no failure to record for it (the per-file log it obtained is empty). The file is left stale in the incremental graph with neither content nor a failure, and the route's bundle is generated without it. The synchronous path (resolve_import_records) only fails the importer when it actually logs the error, which is why the builtin-resolver-only setup works.

Fix. run_resolver now only invalidates the importer's AST (and only obtains the per-file resolution log, which also registers the file as stale) in the branch that actually logs a "Could not resolve" error, i.e. when the record does not handle import errors and ignore_module_resolution_errors is off. This matches resolve_import_records: an importer whose failed resolution is not reported is a successfully bundled file with a disabled import record, and the unresolved require throws at runtime as it does without plugins. track_resolution_failure is still called unconditionally, as on the synchronous path, so creating the missing file later still re-bundles the importer. Error reporting for unhandled imports is unchanged; the emitted messages are the same, only restructured so the invalidation and the log share one condition.

How did you verify your code works?

• test/bake/dev/plugins.test.ts, "onResolve fall-through keeps a module whose missing require is in try/catch": fails on the current release (USE_SYSTEM_BUN=1, client dies with the Failed to load bundled module 'index.ts' error above) and passes with this change. It also checks that a later edit of the module is a hot update and that creating optional-dep.ts re-bundles the importer (the directory watch is still registered) and the require then succeeds.
• test/bake/dev/plugins.test.ts, "onResolve fall-through still reports an unresolvable import": a plain import of a missing file through the same plugin still shows the Could not resolve error overlay and reloads once the file is created (the branch that still invalidates the importer).
• test/bundler/bundler_plugin.test.ts, plugin/ResolveFallThroughMissingRequireInTryCatch and plugin/ResolveFallThroughMissingImport: the same two cases through Bun.build, covering the non-dev-server side of the restructured code (these pass before and after; the dropped module only happened under the dev server).
• bun bd test test/bake/dev/plugins.test.ts, test/bake/dev/bundle.test.ts and test/bundler/bundler_plugin.test.ts pass.

[coderabbitai]

Walkthrough

Changes

The resolver now suppresses development-server invalidation and diagnostics for handled or ignored ModuleNotFound imports. Tests cover require() fall-through, static import errors, hot updates, dependency creation, and recovery.

Resolver error handling

Layer / File(s)	Summary
Guard resolution diagnostics src/bundler/bundle_v2.rs	run_resolver records whether imports handle errors and restricts AST invalidation and diagnostics to qualifying missing-module failures.
Validate fall-through behavior test/bundler/bundler_plugin.test.ts, test/bake/dev/plugins.test.ts	Tests cover handled missing require() calls, unresolved static imports, development updates, dependency creation, and recovery after module creation.

Possibly related PRs

• oven-sh/bun#37850: Both changes update development-server resolution-error handling in src/bundler/bundle_v2.rs.

Suggested reviewers: jarred-sumner, dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly describes the primary fix for plugin fall-through resolution failures inside try/catch.
Description check	✅ Passed	The description explains the problem, fix, background, verification steps, and includes both required template sections.

---

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

[robobun]

Status: reproduced and fixed, ready for review.

Reproduced on the current release with the new test/bake/dev/plugins.test.ts case "onResolve fall-through keeps a module whose missing require is in try/catch" (USE_SYSTEM_BUN=1): the client fails with Failed to load bundled module 'index.ts'; with this branch it loads, hot-updates, and picks up the dependency once it is created.

Fix is in BundleV2::run_resolver (src/bundler/bundle_v2.rs): the importer's AST is only invalidated when a resolution error is actually logged for it, matching resolve_import_records.

CI note: builds 93550 and 93554 failed only because build agents got HTTP 503 from github.com while downloading vendored dependency tarballs (c-ares, mimalloc, WebKit), before any code in this diff compiled. No test failures. Needs a maintainer to re-run or merge when GitHub recovers.

[robobun]

^{Updated 1:41 PM PT - Aug 12th, 2026}

❌ @robobun, your commit a6a47cc has 3 failures in Build #93554 (All Failures):

• test/cli/install/bun-install.test.ts - code 1 on 🐧 25.04 x64
• scripts/build/ci.ts - build failed on 🐧 aarch64 - build-bun
• scripts/build/ci.ts - build failed on 🐧 x64-asan - build-bun
• scripts/build/ci.ts - build failed on 🍎 aarch64 - build-bun
• /opt/winsysroot/Windows Kits/10/Include/10.0.26100/um/winver.h - clang warning on 🪟 aarch64 - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 37899

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

bun-37899 --bun

[claude]

I reviewed this PR and didn't find any bugs. The reorder is sound and well-tested, but since it touches run_resolver's dev-server error path and moves the log_for_resolution_failures call (with its insert_stale / resolution_failure_entries side effects) inside the !handles_import_errors gate — where the synchronous resolve_import_records path still calls it unconditionally — a human familiar with the incremental graph should confirm that asymmetry is intended.

What was reviewed:

• Verified the moved set_len(0) and log_for_resolution_failures now share the same !handles_import_errors && !ignore_module_resolution_errors gate, and that the three add_error messages are byte-identical to before.
• Confirmed track_resolution_failure is still called unconditionally, so the directory watch that re-bundles the importer when optional-dep.ts appears is preserved (covered by the new dev test).
• Compared against resolve_import_records (~line 6300): that path still obtains the per-file log unconditionally but never truncates parts, so an empty log there is harmless — the two paths now differ only in whether insert_stale runs for a handled failure.

Extended reasoning...

Overview

The PR fixes a bug in BundleV2::run_resolver (src/bundler/bundle_v2.rs), the resolver path used when a bundler plugin's onResolve returns nothing and falls through to the builtin resolver. Previously, on ModuleNotFound under the dev server, the importer's AST parts were truncated to zero and the per-file resolution log obtained before checking HANDLES_IMPORT_ERRORS. For a require() inside try/catch that flag is set, so no error was logged — leaving the importer with no parts and no recorded failure, which finish_from_bake_dev_server then dropped from the route bundle. The fix hoists !handles_import_errors && !ignore_module_resolution_errors up so the parts-truncation, the log_for_resolution_failures call, and the add_error all live under one condition. Two new bake dev tests and two itBundled tests cover the handled and unhandled cases.

Security risks

None. This is bundler error-path bookkeeping; no untrusted input parsing, no new syscalls, no auth or crypto.

Level of scrutiny

Medium-high. bundle_v2.rs is core bundler code and the change interacts with the dev server's incremental graph state (insert_stale, resolution_failure_entries). The diff itself is a condition hoist plus reindent — the three error messages are unchanged, the unsafe blocks carry the same SAFETY justifications, and record.path.is_disabled / WAS_UNRESOLVED are still set unconditionally. The one thing I'd want a maintainer to confirm is the intentional asymmetry with the synchronous resolve_import_records path: that path still calls log_for_resolution_failures unconditionally (obtaining an empty log and running insert_stale) even when the import handles its own error, whereas the plugin path now skips it entirely. The PR description states this is deliberate (obtaining the log "registers the file as stale", which contributed to the vanishing importer), and the tests exercise hot-update and file-creation rebuild, so the observable behavior is covered — but someone who owns the incremental graph should nod at it.

Other factors

Test coverage is thorough: the primary dev test asserts the fallback renders, that a subsequent edit is a hot update (module is in the graph), and that creating the missing file re-bundles the importer (track_resolution_failure still fires). A second dev test pins the unhandled-import branch to the same error overlay and reload behavior as before. The two itBundled tests lock the non-dev-server side of the reordered code. The bug hunting system found nothing.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/bake/dev/plugins.test.ts`:
- Around line 151-157: Shorten the comment in the fall-through resolver test to
state only the invariant: a missing require() handled by the importer remains
bundled after plugin fall-through. Remove the detailed failure explanation,
implementation history, and browser error context.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0f1e1235-64e1-48c7-bb22-b1b9903bde1a

📥 Commits

Reviewing files that changed from the base of the PR and between 315136d and 090de55.

📒 Files selected for processing (3)

• src/bundler/bundle_v2.rs
• test/bake/dev/plugins.test.ts
• test/bundler/bundler_plugin.test.ts

[claude]

I reviewed this PR and didn't find any bugs. The fix is well-reasoned — moving the AST invalidation under the same condition that logs the error matches how resolve_import_records handles the same case, and the tests cover both the handled-require and unhandled-import paths plus HMR/file-creation. Because it restructures dev-server incremental-graph invalidation (and narrows when log_for_resolution_failures / insert_stale runs), a maintainer familiar with bake's graph semantics should still take a look.

What was reviewed:

• Compared the restructured run_resolver error path against resolve_import_records — the invalidate-only-when-logging invariant now matches.
• Checked get_log_for_resolution_failures → resolution_failure_entries consumer: entries with empty logs are skipped, so no longer creating one for handled imports is a no-op there.
• Confirmed track_resolution_failure remains unconditional, so the directory watch still fires when the missing file is later created (covered by the new dev test).

Extended reasoning...

Overview

The PR fixes a dev-server-specific bug in BundleV2::run_resolver (src/bundler/bundle_v2.rs): when a plugin's onResolve falls through to the builtin resolver and resolution fails with ModuleNotFound, the importer's AST parts were truncated to zero before checking whether the import record has HANDLES_IMPORT_ERRORS set. For a require() inside try/catch, no error is logged, so the file ended up with neither content nor a recorded failure and was silently dropped from the bundle. The fix moves the set_len(0) truncation and the log_for_resolution_failures call inside the same condition that actually logs a "Could not resolve" error, so an importer is emptied exactly when a failure is recorded against it. Four new tests cover the dev-server path (handled require stays bundled + HMR + file creation; unhandled import still errors) and the Bun.build path.

Security risks

None. This is bundler resolution-error bookkeeping; no untrusted input parsing, auth, or crypto is touched.

Level of scrutiny

Medium-high. The change itself is small and the diff reads as a correctness-preserving reordering, but it touches bake's incremental-graph invalidation logic, where the "file has zero parts iff a failure is logged for it" invariant is what keeps the dev server from silently dropping modules. I compared against the synchronous resolve_import_records path (lines ~6250–6400) and confirmed the new structure mirrors it: add_error is gated on !HANDLES_IMPORT_ERRORS && !ignore_module_resolution_errors, and track_resolution_failure stays unconditional. One subtle difference from the sync path is that log_for_resolution_failures (which calls insert_stale on the graph and creates a resolution_failure_entries slot) is now skipped for handled/ignored imports and for non-ModuleNotFound errors; the consumer at DevServer.rs:3292 only acts on entries where log.has_errors(), so an empty entry was already a no-op there, and the importer is already in the graph from its parse task. The new dev test's HMR and file-creation steps exercise exactly the paths that would break if that reasoning were wrong.

Other factors

The tests are well-constructed per repo conventions (bake-harness devTest with expectMessage/expectReload, itBundled for the non-dev side; the primary test is verified to fail under USE_SYSTEM_BUN=1). CodeRabbit's comment nit was addressed. CI failures on builds 93550/93554 are GitHub 503s during dependency download, unrelated to this diff. Given the subtlety of bake's staleness/invalidation semantics and the side-effect change to when insert_stale runs, I'm deferring so a maintainer who owns the dev-server graph can confirm the invariant.