From 178906144af8c4bf371fd99cd9040d5ad78f70aa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 05:35:18 +0000 Subject: [PATCH] bake: use the server chunk prefix when stitching server source maps 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. --- .../bake/dev_server/incremental_graph.rs | 17 +-- src/runtime/bake/dev_server/mod.rs | 18 +++- .../bake/dev_server/source_map_store.rs | 17 +-- src/runtime/bake/mod.rs | 14 +++ test/bake/dev/server-sourcemap.test.ts | 102 +++++++++--------- 5 files changed, 85 insertions(+), 83 deletions(-) diff --git a/src/runtime/bake/dev_server/incremental_graph.rs b/src/runtime/bake/dev_server/incremental_graph.rs index 3522f6447791..ac4c7a6ad39e 100644 --- a/src/runtime/bake/dev_server/incremental_graph.rs +++ b/src/runtime/bake/dev_server/incremental_graph.rs @@ -1687,14 +1687,7 @@ impl IncrementalGraph { debug_assert!(matches!(SIDE, Side::Client)); debug_assert!(self.current_chunk_len > 0); let kind = options.kind; - - let runtime: bake::HmrRuntime = match kind { - ChunkKind::InitialResponse => bake::get_hmr_runtime(Side::Client), - ChunkKind::HmrChunk => bake::HmrRuntime { - code: bun_core::ZStr::from_static(b"self[Symbol.for(\"bun:hmr\")]({\n\0"), - line_count: 1, - }, - }; + let runtime = kind.prefix(SIDE); let mut end_list: Vec = Vec::with_capacity(256); // SAFETY: see `owner()`. @@ -1802,13 +1795,7 @@ impl IncrementalGraph { debug_assert!(matches!(SIDE, Side::Server)); debug_assert!(self.current_chunk_len > 0); - let runtime: bake::HmrRuntime = match options.kind { - ChunkKind::InitialResponse => bake::get_hmr_runtime(Side::Server), - ChunkKind::HmrChunk => bake::HmrRuntime { - code: bun_core::ZStr::from_static(b"({\0"), - line_count: 0, - }, - }; + let runtime = options.kind.prefix(SIDE); // Server `.InitialResponse` is unreachable per spec; only HmrChunk hits // the end-builder. let end: &[u8] = b"})"; diff --git a/src/runtime/bake/dev_server/mod.rs b/src/runtime/bake/dev_server/mod.rs index 211753667525..4d7ee1412c69 100644 --- a/src/runtime/bake/dev_server/mod.rs +++ b/src/runtime/bake/dev_server/mod.rs @@ -17,7 +17,7 @@ use bun_collections::{HashMap, StringArrayHashMap, bit_set::DynamicBitSet}; use bun_sys::FdExt as _; use super::jsc; -use super::{Graph, Side}; +use super::{Graph, HmrRuntime, Side, get_hmr_runtime}; // ─── submodules ────────────────────────────────────────────────────────────── pub(crate) mod error_report_request; @@ -62,6 +62,22 @@ pub enum ChunkKind { HmrChunk = 1, } +impl ChunkKind { + /// What `IncrementalGraph::take_js_bundle` emits ahead of the module code. + /// `source_map_store::Entry::join_vlq` offsets the chunk's mappings by its + /// `line_count`, so both sides of that agreement come from here. + pub(crate) fn prefix(self, side: Side) -> HmrRuntime { + const CLIENT_HMR_CHUNK: HmrRuntime = + HmrRuntime::from_static(b"self[Symbol.for(\"bun:hmr\")]({\n\0"); + const SERVER_HMR_CHUNK: HmrRuntime = HmrRuntime::from_static(b"({\0"); + match (self, side) { + (ChunkKind::InitialResponse, _) => get_hmr_runtime(side), + (ChunkKind::HmrChunk, Side::Client) => CLIENT_HMR_CHUNK, + (ChunkKind::HmrChunk, Side::Server) => SERVER_HMR_CHUNK, + } + } +} + #[derive(Copy, Clone, Eq, PartialEq)] pub enum TraceImportGoal { FindCss, diff --git a/src/runtime/bake/dev_server/source_map_store.rs b/src/runtime/bake/dev_server/source_map_store.rs index feb1f2866de6..3139c4cfbb33 100644 --- a/src/runtime/bake/dev_server/source_map_store.rs +++ b/src/runtime/bake/dev_server/source_map_store.rs @@ -11,8 +11,8 @@ use bun_core::string_joiner::StringJoiner; use bun_core::{Timespec, TimespecMockMode}; use bun_sourcemap::{self as source_map, SourceMapState}; +use crate::bake::Side; use crate::bake::dev_server_body::map_log; -use crate::bake::{self, Side}; use crate::timer::EventLoopTimerState; use super::{ChunkKind, DevServer, EventLoopTimer, Magic, TimerTag, packed_map}; @@ -249,17 +249,8 @@ impl Entry { j: &mut StringJoiner<'a>, side: Side, ) -> crate::Result<()> { - let _ = side; let map_files = self.files.as_slice(); - // Only the line count of the prefix matters here; the literal has - // exactly one '\n'. - const HMR_CHUNK_PREFIX: &[u8] = b"self[Symbol.for(\"bun:hmr\")]({\n"; - let runtime_line_count: u32 = match kind { - ChunkKind::InitialResponse => bake::get_hmr_runtime(Side::Client).line_count, - ChunkKind::HmrChunk => bun_core::strings::count_char(HMR_CHUNK_PREFIX, b'\n') as u32, - }; - let mut prev_end_state = SourceMapState { generated_line: 0, generated_column: 0, @@ -268,9 +259,9 @@ impl Entry { original_column: 0, }; - // The runtime.line_count counts newlines (e.g., 2941 for a 2942-line file). - // The runtime ends at line 2942 with })({ so modules start after that. - let mut lines_between: u32 = runtime_line_count; + // `line_count` counts the prefix's newlines, which is the 0-based + // generated line the first module starts on. + let mut lines_between: u32 = kind.prefix(side).line_count; // Join all of the mappings together. for (i, file) in map_files.iter().enumerate() { diff --git a/src/runtime/bake/mod.rs b/src/runtime/bake/mod.rs index 12136842c27d..b70cddba61fc 100644 --- a/src/runtime/bake/mod.rs +++ b/src/runtime/bake/mod.rs @@ -598,6 +598,20 @@ pub(crate) struct HmrRuntime { pub(crate) code: &'static bun_core::ZStr, pub(crate) line_count: u32, } +impl HmrRuntime { + /// `code` is a literal including its trailing NUL (`b"({\0"`). + pub(crate) const fn from_static(code: &'static [u8]) -> Self { + let code = bun_core::ZStr::from_static(code); + let bytes = code.as_bytes(); + let mut line_count = 0; + let mut i = 0; + while i < bytes.len() { + line_count += (bytes[i] == b'\n') as u32; + i += 1; + } + Self { code, line_count } + } +} pub(crate) use bake_body::get_hmr_runtime; // (Former `__bun_bake_get_hmr_runtime` link-time bridge deleted — // `bun_bundler::bake_types::get_hmr_runtime` now loads the codegen bytes diff --git a/test/bake/dev/server-sourcemap.test.ts b/test/bake/dev/server-sourcemap.test.ts index 2d217df091ea..94abbde61c83 100644 --- a/test/bake/dev/server-sourcemap.test.ts +++ b/test/bake/dev/server-sourcemap.test.ts @@ -1,6 +1,25 @@ import { expect } from "bun:test"; import { isASAN } from "harness"; -import { devTest } from "../bake-harness"; +import { Dev, devTest } from "../bake-harness"; + +/** + * Matches a remapped stack frame such as `at myFunc (/abs/pages/a.tsx:7:13)`, + * with `line`/`column` being 1-based positions in the fixture source. `\w*` + * tolerates the bundler renaming a symbol (`doSomething` prints as + * `doSomething2`). + */ +function frame(fn: string, file: string, line: number, column: number) { + const path = file + .split("/") + .map(RegExp.escape) + .join(String.raw`[/\\]`); + return new RegExp(String.raw`\bat ${fn}\w* \(.*${path}:${line}:${column}\)`); +} + +/** Dev server output so far, without the ANSI codes interleaved in stack frames. */ +function output(dev: Dev) { + return dev.output.lines.join("\n").replace(/\x1b\[[0-9;]*m/g, ""); +} devTest("server-side source maps show correct error lines", { files: { @@ -27,30 +46,19 @@ export async function getStaticPaths() { }, framework: "react", async test(dev) { - // Make a request that will trigger the error - await dev.fetch("/test-error").catch(() => {}); - - // The output we saw shows the stack trace with correct source mapping - // We need to check that the error shows the right file:line:column - const lines = dev.output.lines.join("\n"); - - // Check that we got the error - expect(lines).toContain("Test error for source maps!"); - - // Check that the stack trace shows correct file and line numbers - // The source maps are working if we see the correct patterns - // We need to check for the patterns because ANSI codes might be embedded - // Strip ANSI codes for cleaner checking - const cleanLines = lines.replace(/\x1b\[[0-9;]*m/g, ""); - - const hasCorrectThrowLine = cleanLines.includes("myFunc") && cleanLines.includes("6:16"); - // const hasCorrectCallLine = cleanLines.includes("MyPage") && cleanLines.includes("2") && cleanLines.includes("3"); - const hasCorrectFileName = cleanLines.includes("pages/[...slug].tsx"); - - expect(hasCorrectThrowLine).toBe(true); - // TODO: renable this when async stacktraces are enabled? - // expect(hasCorrectCallLine).toBe(true); - expect(hasCorrectFileName).toBe(true); + await Promise.all([ + dev.fetch("/test-error").catch(() => {}), + dev.output.waitForLine(/Test error for source maps!/), + ]); + + const out = output(dev); + // Line 7 is ` throw new Error(...)`. The async component rejects and React + // reads `error.stack` before the error is printed; that rendering puts + // construct frames at the callee (`Error`), whereas the sync pages below + // are printed at the `new` keyword. + expect(out).toMatch(frame("myFunc", "pages/[...slug].tsx", 7, 13)); + // Line 2 is ` myFunc();`. + expect(out).toMatch(frame("MyPage", "pages/[...slug].tsx", 2, 3)); }, timeoutMultiplier: 2, // Give more time for the test }); @@ -95,16 +103,11 @@ export async function getStaticPaths() { await Promise.all([dev.fetch("/error-page").catch(() => {}), dev.output.waitForLine(/HMR error test/)]); - // Check source map points to correct lines after HMR - const lines = dev.output.lines.join("\n"); - // Strip ANSI codes for cleaner checking - const cleanLines = lines.replace(/\x1b\[[0-9;]*m/g, ""); - - const hasCorrectThrowLine = cleanLines.includes("throwError") && cleanLines.includes("6:1"); - const hasCorrectCallLine = cleanLines.includes("ErrorPage") && cleanLines.includes("1:16"); - - expect(hasCorrectThrowLine).toBe(true); - expect(hasCorrectCallLine).toBe(true); + const out = output(dev); + // Line 7 is ` throw new Error(...)`, reported at the `new` keyword. + expect(out).toMatch(frame("throwError", "pages/error-page.tsx", 7, 9)); + // Line 2 is ` throwError();`. + expect(out).toMatch(frame("ErrorPage", "pages/error-page.tsx", 2, 3)); }, }); @@ -134,18 +137,13 @@ function helperFunction() { async test(dev) { await Promise.all([dev.fetch("/nested").catch(() => {}), dev.output.waitForLine(/Nested error/)]); - // Check that stack trace shows both files with correct lines - const lines = dev.output.lines.join("\n"); - // Strip ANSI codes for cleaner checking - const cleanLines = lines.replace(/\x1b\[[0-9;]*m/g, ""); - - const hasUtilsThrowLine = cleanLines.includes("helperFunction") && cleanLines.includes("5:1"); - const hasUtilsCallLine = cleanLines.includes("doSomething2") && cleanLines.includes("1:28"); - const hasPageCallLine = cleanLines.includes("NestedPage") && cleanLines.includes("3:38"); - - expect(hasUtilsThrowLine).toBe(true); - expect(hasUtilsCallLine).toBe(true); - expect(hasPageCallLine).toBe(true); + const out = output(dev); + // lib/utils.ts line 6 is ` throw new Error(...)`, reported at the `new` keyword. + expect(out).toMatch(frame("helperFunction", "lib/utils.ts", 6, 9)); + // lib/utils.ts line 2 is ` return helperFunction();`. + expect(out).toMatch(frame("doSomething", "lib/utils.ts", 2, 10)); + // pages/nested.tsx line 4 is ` const result = doSomething();`. + expect(out).toMatch(frame("NestedPage", "pages/nested.tsx", 4, 18)); }, }); @@ -190,13 +188,9 @@ devTest("server-side source maps stay correct across repeated reloads", { dev.output.waitForLine(new RegExp(`Churn error ${name}`)), ]); - // Strip ANSI codes; they interleave within stack-frame lines. - const cleanLines = dev.output.lines.join("\n").replace(/\x1b\[[0-9;]*m/g, ""); - // The throwing function is declared on line 6 + i of round i's version - // of the source file; frames remap to the declaration position (see the - // `helperFunction`/`5:1` expectation above). `\w*` tolerates bundler - // symbol renaming (see `doSomething2` above). - expect(cleanLines).toMatch(new RegExp(`at churn${name}\\w* \\(.*pages[/\\\\]churn\\.tsx:${6 + i}:1\\)`)); + // Round i's version of the file has its ` throw new Error(...)` on + // line 7 + i. + expect(output(dev)).toMatch(frame(`churn${name}`, "pages/churn.tsx", 7 + i, 9)); } }, timeoutMultiplier: 2,