Skip to content

error: don't double source-map rethrown frames after reading .stack - #36726

Closed
robobun wants to merge 3 commits into
mainfrom
claude/61bbdf70/fix-15859-double-sourcemap
Closed

error: don't double source-map rethrown frames after reading .stack#36726
robobun wants to merge 3 commits into
mainfrom
claude/61bbdf70/fix-15859-double-sourcemap

Conversation

@robobun

@robobun robobun commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15859.

Repro

import * as i1 from "util";
import * as i2 from "util";
import * as i3 from "util";
function err() {
    throw new Error()
};
function f1(){
    err()
}
function f2(){

}
try {
    f1();
} catch (error: any) {
    let x = error.stack
    throw error
}

Before:

Error:
      at err (test.ts:5:15)
      at f1 (test.ts:13:5)

After (matches error.stack):

Error:
      at err (test.ts:5:15)
      at f1 (test.ts:8:5)
      at test.ts:14:5

Cause

Reading error.stack causes JSC to clear m_stackTrace after materializing the string. On the rethrow, fromErrorInstance has no live StackFrame vector and falls back to parsing the .stack string with V8StackTraceIterator, which sets remapped = true on every parsed frame (the positions are already source-mapped).

remap_zig_exception checked that flag for the top frame but not in the loop over the remaining frames, so each non-top frame was fed back through resolve_source_mapping and remapped a second time. With three unused imports stripped by the transpiler, source line 8 is generated line 5; remapping source 8 as if it were generated yields source 13.

Separately, V8StackTraceIterator treated a line of the form at /path:line:col (no function name, no parentheses) as end-of-stack, dropping the anonymous top-level frame.

Fix

  • remap_zig_exception: skip frames with remapped == true in the non-top loop (matching the top-frame check and remap_stack_frame_positions).
  • V8StackTraceIterator::parseFrame: when no parentheses are present, parse the whole line as sourceURL:line:col with an empty function name (handles at <url>:<l>:<c> and at async <url>:<l>:<c>).

Verification

test/regression/issue/15859.test.ts has three cases (wrong-line, dropped frame, printed-frames-match-error.stack). All three fail on 1.4.0-canary.1+1498d7b77 and pass with this change.

When user code reads error.stack and then rethrows, JSC clears
m_stackTrace after materializing the string. The uncaught-exception
printer then falls back to parsing the .stack string via
V8StackTraceIterator, which sets remapped=true on every parsed frame
because the positions in the string are already source-mapped.

remap_zig_exception checked that flag for the top frame but not for the
rest, so every non-top frame was fed back through resolve_source_mapping
and remapped a second time, producing wrong line numbers.

Also teach V8StackTraceIterator to parse V8-style frames with no
function name ("at /path:line:col"), which it previously treated as
end-of-stack, so the anonymous top-level frame is no longer dropped.

Fixes #15859
@robobun

robobun commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

Duplicate of #36602, which already includes both changes needed for #15859 (the frames[i].remapped skip in remap_zig_exception and the V8StackTraceIterator no-paren frame handling). Closing this in favor of that PR. The regression test here (test/regression/issue/15859.test.ts) is available on this branch if wanted.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fbec397f-faef-4c4b-b821-4d0b8aecffe7

📥 Commits

Reviewing files that changed from the base of the PR and between f91d5c9 and 79d52a2.

📒 Files selected for processing (3)
  • src/jsc/VirtualMachine.rs
  • src/jsc/bindings/ZigException.cpp
  • test/regression/issue/15859.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 4:13 AM PT - Aug 1st, 2026

@robobun, your commit 79d52a2 is building: #87194

Comment thread src/jsc/bindings/ZigException.cpp Outdated
Comment on lines +311 to +312
// /path/to/file.js:1:2
// async /path/to/file.js:1:2

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. error printer: print the AggregateError header, label [cause]/[errors] blocks, and guard the .errors walk #36602 - Fix Support import assertions #4 in that PR addresses the exact same double source-map remapping bug in remap_zig_exception, skipping frames where remapped is already true

🤖 Generated with Claude Code

@robobun

robobun commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as duplicate of #36602.

@robobun robobun closed this Aug 1, 2026
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.

Incorrect line numbers in error stack for TypeScript code

2 participants