Repository navigation
Conversation
Location::init_or_null sends the dev server a ~120 byte window of a long source line as line_text, but column still counts from the start of the whole line. The overlay drew the underline column - 1 characters into the window, so for any error more than 40 bytes into a long line it ended up far to the right of the token, or past the end of the shown text. Serialize Location.line_text_column_offset (the width of the part of the line the window dropped) after the location length, read it back on the client, and start the underline at column - 1 - lineTextColumnOffset. column itself is unchanged so it keeps referring to the whole line, like the logger's "at file:line:col" suffix does.
|
Status Reproduced on bun 1.4.0: for Fix is in this PR (one commit on top of #37848, which it is stacked on). Both new tests fail without the change and pass with it on a debug build; the remaining CI for this PR's commit: https://buildkite.com/bun/bun/builds/93349 (the bot comments below refer to the base branch's build because of the stacked base). |
|
Updated 2:05 PM PT - Aug 12th, 2026
❌ @robobun, your commit 0b0beb8 has some failures in 🧪 To try this PR locally: bunx bun-pr 37872That installs a local version of the PR into your bun-37872 --bun |
There was a problem hiding this comment.
LGTM — small, self-contained fix mirroring #37848's terminal-caret change for the dev server overlay.
What was reviewed:
- Wire format: confirmed
error-serialization.tsis the only decoder ofwrite_log_data's output; the inspector agent passesbuildErrorsPayloadBase64through opaquely, so inserting au32betweenlengthandlineTextis safe. renderCodeLineis the only consumer ofBundlerMessageLocation.columnfor underline padding; the runtime-error code-preview path uses a separate structure and serialization, so it is correctly out of scope.Math.max(0, …)guards.repeat()against a negative count (would otherwise throw and blank the overlay), matchingData::write_format's saturating subtraction.- Tests pin ASCII and two-byte UTF-8 prefixes, note locations, and the invariant
lineText[column - 1 - offset] === name; the bake harness'serrorsmatcher derives its column from.spacelength, soindex.ts:2:41directly exercises the fix.
Extended reasoning...
Overview
This PR fixes the dev server error overlay's underline position when the bundler windows a long source line. It adds one u32 field (line_text_column_offset, introduced on bun_ast::Location by the base PR #37848) to the serialized failure payload in serialized_failure.rs, reads it in error-serialization.ts as lineTextColumnOffset, and subtracts it in overlay.ts's renderCodeLine when computing the spacer width. Two tests are added (a wire-format decode test with ASCII + é-padded lines in BunFrontendDevServer.test.ts, and an end-to-end DOM test in test/bake/dev/bundle.test.ts), plus the existing bundleFailed snapshot gains lineTextColumnOffset: 0.
Security risks
None. This is a purely cosmetic rendering fix in the dev-server error overlay. The new field is a u32 written by trusted native code and read by the bundled HMR client; no user-controlled input reaches new parsing.
Level of scrutiny
Low-to-medium. The change is a three-line runtime edit (one write, one read, one arithmetic adjustment) that mirrors an already-landed pattern from #37848. The wire format is internal (single writer in serialized_failure.rs, single reader in error-serialization.ts, both shipped in the same binary; the inspector agent forwards the payload as an opaque base64 string). I verified there are no other decoders in src/ or packages/, and that renderCodeLine is the only place BundlerMessageLocation.column is used to position an underline — the runtime-error preview in updateRuntimeErrorOverlay uses a different CodePreview struct sourced from /_bun/report_error, so it is correctly untouched.
Other factors
- The
Math.max(0, …)clamp is defensive and correct: a negative argument toString.prototype.repeatthrows, which would take down the whole overlay. - Test coverage is thorough for a UI fix: exact-value assertions on the decoded locations (both error and note, ASCII and multi-byte prefix), the structural invariant
line.indexOf(lineText) === lineTextColumnOffset, and a DOM-level check thatexcerpt[underlineStart] === ']'. The bake harness's error matcher (client-fixture.mjs:551) derives its:colfrom.spacelength, so theerrors: ['index.ts:2:41: …']assertion is exactly the changed padding. - The pre-existing
line == 0writer/reader desync noted in the PR description is unchanged by this PR (it already writes column/length/lineText that the reader skips); adding one moreu32to that already-desynced tail doesn't change its observable behavior, and the author is handling it separately. - No CODEOWNERS on the touched paths; no outstanding reviewer comments.
|
Nothing to change from the review above. One note for anyone reading the CI comments on this PR: because the PR is based on #37848's branch, the status comments above point at that branch's build (#93262, flaky-only failures so far). The build for this PR's own commit (0b0beb8) is https://buildkite.com/bun/bun/builds/93349. |
Problem
Fix
bundleFailedevent passes the same bytes through untouched.]in the excerpt, and an inspector test decodes the payload for a long ASCII line and a longéline and checks the offset is where the excerpt starts in the line. Both fail without the fix. The other overlay position tests were run locally and are unchanged.Background
bun ./index.html("bake" in the tree) does not only print bundle errors to the terminal: it serializes them into a binary payload, embedded in the error page and pushed to the HMR client, which draws the in-browser overlay.BunFrontendDevServer.bundleFailed.line_text_column_offsetto that location, the width of the dropped prefix in the same units as the column, and used it for the terminal caret. This PR carries the value to the overlay.column - 1filler characters, thenlengthunderline characters.Original description
Stacked on #37848 (this PR's base is that branch, so the diff here is only this PR's own commit). #37848 adds
Location.line_text_column_offsetand fixes the terminal caret; it leaves the dev server overlay alone, which is what this PR does.Repro
Open the page. The overlay shows the second line as a 120 character window starting 40 characters before the
](aaaa...a]bbbb...b), but the red underline is drawn 100 characters in, under ab60 characters to the right of the]. With a longer line (anything minified) it is drawn past the end of the shown text altogether. Same for notes. Decoding the payload the page embeds shows why:column: 101next to alineTextthat only has 40 characters in front of the].Cause
Location::init_or_null(src/ast/lib.rs) windows a long line to about 120 bytes around the error, butcolumnstays relative to the whole line.write_log_datainsrc/runtime/bake/dev_server/serialized_failure.rsserializes both as they are, andrenderCodeLineinsrc/runtime/bake/client/overlay.tspads the underline withcolumn - 1characters under the windowedlineText. That is the overlay's copy of the caret bug #37848 fixes inData::write_format.Fix
write_log_datawritesloc.line_text_column_offset(the width of the part of the line the window dropped, 0 wheneverline_textis the whole line) as au32after the length. The format has one writer and one reader, both shipped in the same binary (the HMR client and the embedded error page; the inspector'sBunFrontendDevServer.bundleFailedpayload is the same bytes and is passed through opaquely), so adding a field is safe.readBundlerMessageLocationOrNullreads it intoBundlerMessageLocation.lineTextColumnOffset.renderCodeLinepads withcolumn - 1 - lineTextColumnOffset(saturated at 0, likewrite_formatdoes; a negative count would throw out ofrepeatand take the whole overlay down).columnitself is deliberately left alone rather than being rewritten to a window-relative value on the wire: it keeps meaning the same thing asBuildMessage.position.columnand the logger'sat file:line:col, which is also what #37848 chose, and a consumer of the inspector payload that wants to show a real column can still do so. Both values are in UTF-16 code units (ErrorPositionStatecounts columns that way and the offset is derived from the same scan), which is exactly the unitlineTextis indexed in on the client, so the padding lines up for non-ASCII prefixes too. The window is not otherwise changed, and like the terminal output the overlay does not mark the truncation.Verification
test/cli/inspect/BunFrontendDevServer.test.ts: the existingbundleFailedsnapshot gainslineTextColumnOffset: 0for its two short lines, and a new test rewritesutils.tswith two 220 character lines (ASCII and one padded withé), each with a redeclared binding, and decodes the payload with the client's own decoder. It pins the error and note locations of both lines (column: 115withlineTextColumnOffset: 74and91, the notes with0and an untrimmed window) and checks thatline.indexOf(lineText) === lineTextColumnOffsetand thatlineText[column - 1 - lineTextColumnOffset]is the declared name. Fails without the change (field missing), passes with it.test/bake/dev/bundle.test.ts: renders the repro above in the dev server client harness and checks that the underline starts 40 characters in (index.ts:2:41in the harness's underline-derived format, previously2:101) and that the character under it in the shown excerpt is the].Also run locally with the change: the rest of
BunFrontendDevServer.test.ts, andbundle,css,hotandhtmlintest/bake/dev(the other tests that assert overlay positions; every location they cover has an offset of 0, so they are unchanged).While looking at this I noticed that a message whose
Location.lineis0(a throwing dev server plugin produces one) is written with its column/length/text while the client stops reading after the line, which desyncs the rest of the payload. That predates this change and is being handled separately.