Skip to content

bundler: keep a file's parse warnings when one of its imports does not resolve - #47320

Open
robobun wants to merge 2 commits into
mainfrom
robobun/97fccb4f/keep-parse-warnings-on-resolve-failure
Open

robobun wants to merge 2 commits into
mainfrom
robobun/97fccb4f/keep-parse-warnings-on-resolve-failure

Conversation

@robobun

@robobun robobun commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • A file loses its parse warnings when one of its imports does not resolve. bun build ./warn.js prints only error: Could not resolve: "./does-not-exist.js", and Bun.build().logs holds only the ResolveMessage.
  • run_resolution_for_parse_task (src/bundler/bundle_v2.rs:6561) replaces the parse result, and the log in it, with an error that has an empty log.
  • No user reported this. It is a consistency fix: main keeps these warnings next to a parse error.

Fix

  • The function moves the parse log to the build log before it replaces the result. The success path makes the same call. The failure branch is marked cold.
  • Verified: new tests in test/bundler/bun-build-api.test.ts and test/bundler/cli.test.ts. Both fail without the fix. Notes lists the other suites.
  • Self-reviewed: 17 concerns raised, 17 addressed (see Notes).

Background

Downsides

  • On a failed build, logs, AggregateError.errors and onEnd gain warn entries after the resolve errors.
  • Cost, release build: on_parse_task_complete 16007 B -> 15835 B, and 427 -> 425 instructions per parsed file on a build that succeeds. A failing build with one warning makes about 51 more write calls and no more allocations.
  • The dev server still shows no parse warnings (related to Warnings in HMR are not being emitted or displayed anywhere #17205). The other imports of a failed file are still not parsed.
Notes

Repro

printf 'import "./does-not-exist.js";\nclass A { #m() {} f() { this.#m = 1; } }\nexport default A;\n' > warn.js
bun build ./warn.js --outdir=out

Before: only error: Could not resolve: "./does-not-exist.js". After: warn: Writing to read-only method "#m" will throw, then the error. The exit code is 1 in both. A stylesheet with @import "./does-not-exist.css" and an @nest rule loses and keeps its warning in the same way.

Origin. 81e4203 (2023-05-08, "[bundler] Make module resolution single-threaded") moved resolution to the bundle thread. Since then the resolve failure branch builds the error with Log::init(). Of the five places that build a ResultError, it is the only one that does not carry the parse log.

Where main already keeps these warnings. The same file reports them when it has a parse error, when an onResolve plugin handles the import, and when the warning is in another file of the build. esbuild 0.21.5 prints 1 warning and 1 error for warn.js. That comparison holds for one file only: esbuild also parses the other imports of a failed file, and bun does not.

Order. For one file the log holds the resolve errors, then the parse messages. The CLI prints warnings above errors.

Dev server. Not changed. There the parse log goes to the dev server's build log, like the log of a file that builds, and the dev server does not show that log. A variant that moved the log to the failure log of the file did show the warnings in the error overlay. It is not in this PR: a warning with line 0, or with a line and a column of 0 or -1, breaks the wire writer or the overlay (write_log_data in serialized_failure.rs, renderCodeLine in overlay.ts). #39488 has the changes for that.

AOT. The bun build --compile --aot page bundle takes the same path (src/bundler/aot/mod.rs:144 passes no dev server). I read the code. I did not run it.

CSS positions. A kept CSS warning has the position that the CSS parser gives every warning today: a 0-based line and no source line. #45787 changes that.

Fail-before and mutation. Both new tests fail on a build without the statement: the warn entries are missing and nothing else differs.

Measurements. Release builds on linux-x64, built in one directory: main at 3a9ea00, and the same commit with the source diff of this PR.

  • build that succeeds: on_parse_task_complete 427 -> 425 self instructions per parsed file (200-module graph, 202 calls, qemu-x86_64 -one-insn-per-tb). The new call runs 0 times in that build.
  • release binary: on_parse_task_complete 16007 B -> 15835 B, text +0 B, data and bss unchanged (nm -S and size).
  • failing build of warn.js: +0 allocations (3282 in both), about +51 write calls (30 to 32 -> 82 to 83, the text of the printed warning). Build that succeeds: +0 allocations (3517 in both) and +0 write calls (55 in both). Counted with gdb breakpoints on the mimalloc entry points and with qemu-x86_64 -strace, three runs each.
  • Bun.build logs for a file with a warning and an unresolved import: 1 -> 2 entries, for JS and for CSS. 0 of 3 control outputs changed (a file with a warning only, a stylesheet with a warning only, a failing file with no warning).
  • existing test expectations changed: 0 in the 14 files below.
  • bloaty, strace, valgrind and perf are not installed here, so the numbers come from nm, size, qemu and gdb.

Suites run on a debug build. On the current branch: test/bundler/bun-build-api 90 pass, test/bundler/bundler_regressions 9, and the new cli.test.ts test with its neighbour. On this branch before its last rebase (base 3a9ea00): test/bundler bundler_plugin 68 pass, bundler_barrel 70, bundler_browser 23, bundler_files 28, esbuild/css 68. test/bake/dev bundle 54, css 18, plugins 3, html 15, harness 3. test/js/bun/resolve/resolve-error 32, test/js/bun/css/invalid-utf8-column 6. Timeouts on the 1.6 GB debug binary, none in the changed branch: three --compile tests in cli.test.ts (5 s limit), two in bun-build-api.test.ts (an --aot compile and the build that runs thousands of times), and the 20000-module DeepImportDiamondDAG build in bundler_edgecase.test.ts (60 s limit).

Self-review. An earlier form of this change also moved the log to the dev server's failure log of the file, with a one-line guard in the wire writer. The review raised 17 concerns. Ten were about that dev server half: the guard was one of three cases of the wire format, the other two could abort the dev server or break the overlay, and the failure log key would go out of step with #47062. This PR has no dev server half now, so those are gone. The other seven are answered in this text. One suggestion is not taken: to land the wire format changes of #39488 first as a separate PR. That is a different bug, and #39488 has it.

History. An earlier form of this statement was commit 971c939 in #41801. That PR was closed for its CSS decoding change and stays closed.


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/cli.test.ts, test/bundler/bun-build-api.test.ts

@robobun

robobun commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:32 AM PT - Oct 29th, 2026

❌ @robobun, your commit e615d46 has 1 failures in Build #135112 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 47320

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

bun-47320 --bun

@robobun

robobun commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

Reproduced on 1.4.3-canary (367d939) and on a release build of main at 11c3671:

printf 'import "./does-not-exist.js";\nclass A { #m() {} f() { this.#m = 1; } }\nexport default A;\n' > warn.js
bun build ./warn.js --outdir=out

It prints only error: Could not resolve: "./does-not-exist.js". Without the import line, the same file prints warn: Writing to read-only method "#m" will throw. Bun.build({ entrypoints: ["./warn.js"], throw: false }) returns logs with the ResolveMessage alone.

With this branch, both report the warning and the error.

CI. The diff is green. Each of the two runs has one red lane, and neither failure comes from this change:

  • Build 135063: a segmentation fault in test/aot/workers-within-address-space-limit.test.ts on alpine x64. That lane passed in the next run.
  • Build 135112: test/aot/build-scaling/build-scaling-2.test.ts on darwin aarch64 (twice as many functions cost 2.58 times the peakBytes; allowed: 2.5). That lane passed in the first run. The same test also shows up in builds 134948 and 134964 of main.

Every other lane passed in both runs, and no lane reports a failure in the two test files of this PR. The PR is ready for a maintainer.

PR: #47320

…t resolve

`run_resolution_for_parse_task` replaces the parse result, and the log
in it, with an error when an import does not resolve. `bun build` and
`Bun.build` then reported the resolve error alone.

The function now moves the parse log to the build log, as the success
path does. The statement is in the branch that handles a failed import,
and that branch is marked cold.
@robobun
robobun force-pushed the robobun/97fccb4f/keep-parse-warnings-on-resolve-failure branch from a40b4d5 to f810638 Compare October 29, 2026 08:28
@robobun
robobun marked this pull request as ready for review October 29, 2026 08:52
@coderabbitai

coderabbitai Bot commented Oct 29, 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 da867c6 and f810638.

Files selected for processing: 3
  • src/bundler/bundle_v2.rs
  • test/bundler/bun-build-api.test.ts
  • test/bundler/cli.test.ts

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

Walkthrough

When import resolution fails, the bundler marks the path cold and appends resolver log messages to the transpiler log. New build API and CLI tests check that parse warnings remain present alongside unresolved-import errors.

Changes

Build log handling

Layer / File(s) Summary
Append resolver messages to transpiler logs
src/bundler/bundle_v2.rs
On resolver failure, the code marks the path cold and appends ResultError.log messages to the transpiler log before constructing the resolve-step error.
Verify API and CLI build logs
test/bundler/bun-build-api.test.ts, test/bundler/cli.test.ts
Concurrent build API tests check that returned logs and AggregateError.errors include unresolved-import errors and JavaScript and CSS parse warnings. CLI tests check that both errors and warnings appear, stdout is empty, and the process exits with code 1.

Suggested reviewers: jarred-sumner, alii

Priority: Low

Merge Risk: Minimal · up to f8106

The change preserves parse warnings when imports fail to resolve, with regression coverage for JavaScript and CSS. No actionable merge-blocking issue was identified; mergeability remains subject to normal build and test checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving a file's parse warnings when an import fails to resolve.
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 is complete and covers the change, rationale, limitations, verification steps, test results, and performance impact. It does not use the template headings exactly, but it provides the …
Full details: Description check

Explanation

The description is complete and covers the change, rationale, limitations, verification steps, test results, and performance impact. It does not use the template headings exactly, but it provides the required information under equivalent sections.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • 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.

LGTM — a one-statement fix that mirrors the existing success-path call, with tests on both the API and CLI surfaces.

What was reviewed:

  • Confirmed on_parse_task_complete sets process_log = false after a Step::Resolve failure, so ResultError.log is indeed never read on that path and moving result.log into the build log before the overwrite is the only place the messages can survive.
  • Log::append_to moves msgs and the counters out of the parse log, so dropping the discarded Success result afterwards cannot double-free or double-count.
  • Dev-server builds: the success arm already appends parse warnings to the same transpiler.log, so this only makes the resolve-failure case consistent rather than introducing a new destination.
  • core::hint::cold_path is already used in src/sema and bun_core::hint; the cli test sits inside an existing describe.concurrent, drains pipes with Promise.all, and asserts one combined { stdout, diagnostics, exitCode } object.
Extended reasoning...

The change adds one result.log.append_to(this.transpiler.log_mut()) call and a cold_path() hint in the import-resolution failure branch of run_resolution_for_parse_task in src/bundler/bundle_v2.rs, plus two tests (Bun.build API with throw true/false, and the bun build CLI) covering a JS file and a CSS file. It touches no security-sensitive surface: no user-controlled parsing, auth, or path handling changes. The fix is a single statement identical to the one on the success path, the dead-log claim was verified against the caller, no CODEOWNERS entry covers the changed files, and the hunt ran dry with no findings, which decided approve.

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.

1 participant