bake: remap server-side stack frames against the server chunk prefix - #37430
bake: remap server-side stack frames against the server chunk prefix#37430robobun wants to merge 1 commit into
Conversation
join_vlq always skipped one generated line for HmrChunk, the line count of
the client prefix (self[Symbol.for("bun:hmr")]({ plus a newline), but the
server chunk is emitted behind a bare ({ with no newline. Every server-side
frame was therefore looked up one generated line early and remapped to the
position of the previous generated line, usually the line above the
statement.
ChunkKind::prefix(side) now provides the prefix to both the bundle emitter
and the source map stitcher, and the line count is derived from the
literal.
|
Status: reproduced with the released build by running |
WalkthroughThe change centralizes HMR runtime prefix selection by chunk kind and side. Bundle generation and source-map assembly use the shared selection. Dev-server tests now verify exact source-mapped stack frames. ChangesHMR prefix contract and integration
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
LGTM — centralizing the chunk prefix in ChunkKind::prefix(side) fixes the off-by-one on server maps and makes the emitter and mapper agree by construction.
What was reviewed:
- Verified
ZStr::from_static/as_bytes()exclude the NUL, sofrom_static(b"({\\0")yieldsline_count = 0and the client literal yields 1. - Confirmed client paths are unchanged:
render_json(.., Side::Client)at DevServer.rs:1774 andrender_mappings(client-only/_bun/report_errorpath) still see the same line counts as before. - Checked
render_json(.., ChunkKind::HmrChunk, Side::Server)at DevServer.rs:4291 is the server call site that now getsline_count = 0. - Test assertions are strictly stronger (full
at fn (…file:line:col)regex vs. substringincludes); no existing coverage weakened.
Extended reasoning...
Overview
Fixes server-side stack-frame remapping in the bake dev server. source_map_store::Entry::join_vlq was ignoring its side parameter and always offsetting mappings by the client HMR-chunk prefix's newline count (1), while take_js_bundle_to_list_server emits a prefix (({) with 0 newlines — so every server frame remapped one generated line too high. The fix introduces ChunkKind::prefix(side) -> HmrRuntime as the single source for both the emitted prefix and its line count, plus a const HmrRuntime::from_static that derives line_count from the literal. take_js_bundle_to_list{,_server} and join_vlq all pull from it.
Five files touched: the three bake dev-server Rust files plus bake/mod.rs (the from_static helper) and the test file, whose expectations are corrected and tightened.
Security risks
None. This is dev-server source-map line-count arithmetic; no user input, auth, or serialization boundaries are involved.
Level of scrutiny
Medium. The change is small and mechanical, but it rewrites the assertions in an existing test file — REVIEW.md flags that as something to check carefully. I verified each rewritten expectation against the fixture source (line 7 = throw new Error, line 2 = the call, etc.) and confirmed the old expectations were the buggy output, not the correct behavior. The client path is provably unchanged: for (HmrChunk, Client) the new helper returns the same literal with line_count = 1, and for (InitialResponse, Client) it returns get_hmr_runtime(Side::Client) exactly as before. render_mappings still hardcodes Side::Client, which is fine since its only caller (get_parsed_source_map → error_report_request.rs) handles client-reported errors.
Other factors
- robobun confirms the updated test fails on the released build with the old positions and passes on this branch, satisfying the "test fails for the right reason" bar.
test/bake/dev/sourcemap.test.tsandhtml.test.ts(client maps through the same function) were run and pass unchanged.- The
(InitialResponse, Server)arm now returnsget_hmr_runtime(Side::Server)where the old code used the client runtime's line count regardless; the comment attake_js_bundle_to_list_servernotes serverInitialResponseis unreachable, so this is a consistency-only change. - The PR description flags a known future conflict with #37396 (one column expectation changes from 7:13 to 7:9) and a separate unaddressed issue (server patch maps overwriting each other under
bake://server.patch.js) — both correctly scoped out.
What does this PR do?
Stack traces for server-side code running in the bake dev server (framework mode, where route modules are evaluated on the server) point at the wrong line. With this route module:
a request prints
and with this change it prints
test/bake/dev/server-sourcemap.test.tshad the wrong positions baked into its expectations (6:16,6:1,5:1,1:28,3:38, and the churn test's6 + i), with a comment rationalizing them as "frames remap to the declaration position". This has been the behavior since server source maps were added; the client side is not affected.Cause
The server HMR chunk and its source map disagree about what precedes the module code.
IncrementalGraph::take_js_bundle_to_list_serveremits a serverHmrChunkas({+ modules +}). The prefix contains no newline, so the first module starts on generated line 0.source_map_store::Entry::join_vlqignored itssideargument (let _ = side;) and, for everyHmrChunk, started the mappings after the line count of the client prefix,self[Symbol.for("bun:hmr")]({\n, i.e. on generated line 1.So the map for the server chunk is shifted down by one generated line relative to the code. A frame on generated line
Lis looked up at map lineL, which holds the mappings recorded for generated lineL - 1, so every frame reports the original position of the previous generated line. For ordinary code that is the line above the statement; the column is whichever mapping on that line happens to precede the frame's generated column (hence6:16, the(offunction myFunc(, or3:38, the{of the function declaration). In denser code it can be further off: areact-server-domframe in the same chunk moved from3971:15(atry {line) to3972:20, the actualComponent(props, void 0)call.The client chunks use the same function but with the client prefix, which is why
test/bake/dev/sourcemap.test.ts(exact positions for the initial response and an HMR chunk) passes before and after.Fix
ChunkKind::prefix(side)returns the code a chunk of that kind starts with on that side (the full HMR runtime forInitialResponse, the one-line client wrapper or the bare({forHmrChunk). Bothtake_js_bundle_to_list*andjoin_vlqtake it from there, andHmrRuntime::from_staticderivesline_countfrom the literal, so the emitted code and the map cannot disagree again.join_vlq'ssideparameter is now used;render_jsonwas already being called withSide::Serverfor server chunks.Verification
test/bake/dev/server-sourcemap.test.tsnow asserts the full remapped frames (at fn (.../file:line:col)) for the throw site and for each caller, across the initial bundle, an HMR update, a throw in an imported non-page module, and four successive reloads. Without the change insrc/all four tests fail with the positions shown above (6:16/1:1,6:1/1:16,5:1/1:28/3:38,6 + i:1); with it they pass.Also run:
test/bake/dev/sourcemap.test.ts(client maps, unchanged behavior) andtest/bake/dev/html.test.ts(the/_bun/report_errorremapping path, which renders client maps through the same function);cargo clippy -p bun_runtimeis clean.Note on columns: in the first test the page component is
async, so React readserror.stackbefore the error is printed, and that rendering currently places a construct frame at the callee (Error,7:13) where the printer places it at thenewkeyword (7:9, what the other tests see). #37396 makes the two agree; once it lands, that one expectation becomes7:9(its current version of this file will conflict with this change either way).While looking at this I also noticed that after any hot update, frames from modules loaded by an earlier server patch stop remapping (every patch is registered under
bake://server.patch.js, so the newest patch's map replaces the older ones; visible asat react-stack-bottom-frame (bake://server.patch.js:3242:29)in the HMR test output). That is a separate problem and is not addressed here.[review] gate passed · iteration 0 · 5 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
self-review · no surviving concerns
26 concerns were raised and did not survive verification.