From fb2528fc58c09a80a7c10a57535637eaee18c44b Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Sun, 26 Apr 2026 04:47:49 +0000 Subject: [PATCH] hot: defer reload while a rejected module is unreported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `vm.source_mappings` is a path-hash → blob table overwritten in place on every transpile. The hot-reload Task loop drains microtasks between tasks, so a watcher event that arrives after a module's eval rejects but before `reportExceptionInHotReloadedModuleIfNeeded()` prints it can run another `reload()` — which re-reads the file (possibly mid-rewrite) and overwrites the table entry. The still-unreported error is then either remapped against the wrong map (transpiled coords leak through, e.g. `:1:12`) or dropped entirely when the new pending promise replaces the old one. Two changes: - `VirtualMachine.reload()` now also defers when `pending_internal_promise` is `.rejected` and `pending_internal_promise_reported_at != hot_reload_counter`, not just when `.pending`. The deferred reload runs from `reportExceptionInHotReloadedModuleIfNeeded()` after the error is printed (and remapped against its own sourcemap). - `SavedSourceMap.putMappings()` keeps the existing entry when the incoming blob has zero mappings. A truncate-then-write rewrite can be observed as a comment-only prefix that transpiles to nothing; a 0-mapping map can never answer a lookup, so dropping it is never worse than installing it. The new test makes the window deterministic by having the hot file truncate itself to a comment-only stub immediately before throwing, guaranteeing a fresh watcher event lands between reject and report. --- src/bun.js/SavedSourceMap.zig | 15 +++++++++++ src/bun.js/VirtualMachine.zig | 19 +++++++++++--- test/cli/hot/hot.test.ts | 47 +++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/bun.js/SavedSourceMap.zig b/src/bun.js/SavedSourceMap.zig index cf20d0917e9..382af7d7589 100644 --- a/src/bun.js/SavedSourceMap.zig +++ b/src/bun.js/SavedSourceMap.zig @@ -142,6 +142,21 @@ pub fn deinit(this: *SavedSourceMap) void { } pub fn putMappings(this: *SavedSourceMap, source: *const logger.Source, mappings: MutableString) !void { + // --hot can re-read a file mid-rewrite (truncate + write) and transpile + // a comment-only prefix into a 0-mapping map. Overwriting a real map + // with that would make any still-unreported error from the previous + // transpile remap against nothing and leak transpiled coords. A map + // with no mappings can never answer a lookup, so dropping it is never + // worse than installing it. + if (mappings.list.items.len >= InternalSourceMap.header_size) { + const incoming: InternalSourceMap = .{ .data = mappings.list.items.ptr }; + if (incoming.mappingCount() == 0) { + this.lock(); + defer this.unlock(); + if (this.map.contains(bun.hash(source.path.text))) return; + } + } + const blob = try bun.default_allocator.dupe(u8, mappings.list.items); errdefer bun.default_allocator.free(blob); try this.putValue(source.path.text, Value.init(bun.cast(*InternalSourceMap, blob.ptr))); diff --git a/src/bun.js/VirtualMachine.zig b/src/bun.js/VirtualMachine.zig index f42461f72e0..0979a4b03e2 100644 --- a/src/bun.js/VirtualMachine.zig +++ b/src/bun.js/VirtualMachine.zig @@ -773,10 +773,23 @@ pub fn reload(this: *VirtualMachine, _: ?*HotReloader.Task) void { // race through one registry. Defer instead; the main loop reschedules // via reportExceptionInHotReloadedModuleIfNeeded once the in-flight // promise resolves or rejects. + // + // Also defer when the previous promise has rejected but its error + // hasn't been printed yet: reloadEntryPoint() re-transpiles and + // overwrites source_mappings[path] in place, so a watcher event that + // slips in between the rejection microtask and the report would remap + // that error against the wrong sourcemap. if (this.pending_internal_promise) |p| { - if (p.status() == .pending) { - this.hot_reload_deferred = true; - return; + switch (p.status()) { + .pending => { + this.hot_reload_deferred = true; + return; + }, + .rejected => if (this.pending_internal_promise_reported_at != this.hot_reload_counter) { + this.hot_reload_deferred = true; + return; + }, + .fulfilled => {}, } } this.hot_reload_deferred = false; diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 3e3f4b32789..d0f1bc145a3 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -535,6 +535,53 @@ ${Buffer.alloc(counter * 2, " ").toString()}throw new Error(${counter});`, timeout, ); +it( + "should not remap against a stale sourcemap after a partial-file reload", + async () => { + // Regression: the watcher can deliver a second reload Task between the + // moment a module's eval rejects and the moment that rejection is + // printed. The second reload re-transpiles and overwrites + // source_mappings[path] in place, so the still-unreported error gets + // remapped against the wrong map and transpiled coordinates leak + // through — or, since the new pending promise replaces the old one, + // the error is dropped entirely. + // + // To make the window deterministic the hot file truncates itself to a + // comment-only stub immediately before throwing, guaranteeing a fresh + // watcher event lands between reject and report. + const writeFull = (counter: number) => + writeFileSync( + hotRunnerRoot, + `// source content +${comment_spam}require("fs").writeFileSync(__filename, "// stub ${counter}\\n"); +${Buffer.alloc(counter * 2, " ").toString()}throw new Error('${counter}');`, + ); + writeFull(0); + await using runner = spawn({ + cmd: [bunExe(), "--smol", "--hot", "run", hotRunnerRoot], + env: bunEnv, + cwd, + stdout: "ignore", + stderr: "pipe", + stdin: "ignore", + }); + const reloadCounter = await driveErrorReloadCycle(runner, { + targetCount: 20, + onReload: writeFull, + verifyLine: (errorLine, nextLine, counter) => { + if (!nextLine) throw new Error(errorLine); + const match = nextLine.match(/\s*at.*?:(\d+):(\d+)\)?$/); + if (!match) throw new Error("no :line:col in: " + JSON.stringify(nextLine)); + if (match[1] !== "1003") throw new Error("expected :1003: but got: " + JSON.stringify(nextLine)); + expect(Number(match[2])).toBe(1 + "throw new ".length + counter * 2); + }, + }); + await runner.exited; + expect(reloadCounter).toBe(20); + }, + longTimeout, +); + it( "should work with sourcemap loading", async () => {