node:vm: keep a vm SyntaxError's parse location out of the source map of the file its filename names

[robobun]

Problem

• A SyntaxError from compiling node:vm code under a filename that Bun has loaded reports the wrong line: a script that runs new vm.Script(src, { filename: __filename }) with src failing on its line 10 gets err.line === 5 and at <parse> (/tmp/remap_syntax_short.js:5) in err.stack (5 being the line of the script that line 10 of its transpiled output maps back to; it varies with the host file), while the same source compiled under a name Bun never loaded reports 10. err.originalLine is set to 10. The arrow header node:vm prepends (/tmp/remap_syntax_short.js:10 plus the source line) is right, so the error contradicts itself. Same for vm.runInThisContext / runInContext / runInNewContext (all built on Script) and vm.compileFunction. Repro in the details block.
• The vm code's line went through the source map of index.js. formatStackTrace() (src/jsc/bindings/FormatStackTraceForJS.cpp, the <parse> branch) builds a frame out of err->sourceURL() / err->line() and passes it to Bun__remapStackFramePositions, which is keyed by URL, and then overwrites line (which becomes err.line) with the result.
• At that point nothing else about the source is available. ParserError::toErrorObject() calls JSC's addErrorInfo(), which copies the provider's URL and the line onto the ErrorInstance and materializes .stack (running Bun's formatter) before returning the error to node:vm, so the NodeVMScriptFetcher that marks the provider as vm code (the signal node:vm: don't remap vm code through the source map of the file its filename names #38344 uses for runtime frames) is not reachable from the formatter, and node:vm gets the error back only after the string has been built.
• vm.SourceTextModule compiles with no URL today, so its parse errors took the other path through the same branch: nothing remapped, hasSet stayed false, and the frame loop then published the first caller frame as the error's location (err.line === 225, err.sourceURL === "node:vm" for a parse error on line 5 of the module source).

Fix

• NodeVM::createParseError() (src/jsc/bindings/NodeVM.cpp) replaces the three toErrorObject() calls (new Script, compileFunction, SourceTextModule). For a SyntaxError it does what toErrorObject() does (createSyntaxError + addErrorInfo + setParseError), but puts a private vmParseError property on the instance before addErrorInfo() runs; every other ParserError type is still delegated to toErrorObject().
• The formatter's <parse> branch skips the remap for a tagged error and sets hasSet, so the frame shows err->sourceURL() and the parser's line, err.line keeps the parser's line, and no originalLine is written (originalLine means a map was applied; node:vm: don't remap vm code through the source map of the file its filename names #38344 stops writing it for runtime frames in vm code for the same reason). Today an unloaded name gets an identity remap and originalLine === line; that goes away too.
• Why the tag is correct: node:vm compiles the caller's own string under a caller-chosen name, and Bun's source maps describe Bun's transpiled output of the file on disk that happens to have that name; the two share nothing but the name, so a map lookup can never apply to a vm parse location. This is the same argument node:vm: don't remap vm code through the source map of the file its filename names #38344 makes for runtime frames; this PR covers the compile-time error, where the provider is gone and the tag is what carries the fact. Node reports the vm source's own line here (its arrow header; Node has neither err.line nor a <parse> frame), which is now also what Bun's header, <parse> frame and err.line agree on.
• Tagging the instance rather than, say, a flag set around the compile means anything that later re-formats the error (Error.captureStackTrace, or the error printer once it can mark string-parsed frames as vm frames) can still tell. vmParseError is a private symbol like node:vm's existing vmErrorDecorated, so it is not observable from JS.
• createModuleRecord() now clears an exception left by a throwing user Error.prepareStackTrace before throwing the SyntaxError, as constructScript() / constructAnonymousFunction() already do; without it the new test's SourceTextModule case aborts under BUN_JSC_validateExceptionChecks=1 (which CI sets for this file on the ASAN lane). node:vm: apply SourceTextModule lineOffset/columnOffset like Node and name frames after the identifier #38235 makes the same change at this site.
• Test: test/js/node/vm/vm.test.ts, "SyntaxError from compiling vm code under the filename of a file Bun transpiled". A fixture starting with a 20-line comment (so every line its source map can produce is > 20) compiles a source failing on line 5 under its own path and under an unloaded name through all six APIs and checks the header, <parse> frame, err.line, err.originalLine, err.sourceURL and the <parse> frame Bun.inspect prints. Fails on the released build with line 23 for the own-name cases; passes with this change, also under BUN_JSC_validateExceptionChecks=1.
• Also run against the debug build: the rest of vm.test.ts, vm-sourceUrl.test.ts, test/js/bun/test/stack.test.ts, test/js/node/v8/capture-stack-trace.test.js, the test-vm-syntax-error-*, test-vm-module-errors, test-vm-source-map-url and test-repl-{recoverable,syntax-error-*,unexpected-token-recoverable} node tests. inspect-error.test.js has two minified-file snapshot failures that are the known debug-build require frame, unrelated.
• Not changed: the source excerpt the error printer shows above a tagged error still comes from the file the name denotes (before this change it showed that file too, just at the remapped line). Fixing that needs the per-frame vm flag node:vm: don't remap vm code through the source map of the file its filename names #38344 adds to ZigStackFrame; with it, the printer can flag the <parse> frame of a tagged error when it parses the stack string. Open PRs error: don't add a synthetic <parse> frame to SyntaxErrors that aren't parser errors #37386 / error: gate the <parse> stack frame on the parser having recorded a location, not on sourceURL #37432 / error printer: show file/line for JSC parser SyntaxErrors with no stack #36634 / Release the stack frame strings leaked by the C++ source map remap callers #37459 edit the same <parse> branch for other reasons and compose with this; node:vm: report compileFunction body errors as SyntaxErrors and reject bodies that close the function #38239 / node:vm: throw runInContext/runInNewContext compile errors from the context's realm #38317, which touch the toErrorObject() call sites, would switch to createParseError() on rebase.

Background

• Source map remapping: Bun transpiles every file it loads, so positions JSC reports are in the transpiled output. Bun__remapStackFramePositions takes (URL, line, column) and, if Bun holds a source map for that URL, rewrites the position to the original file; err.originalLine is where the formatter stores the pre-remap line when it does this.
• <parse> frame: for a SyntaxError raised by JSC's parser there is no stack frame inside the rejected source, so Bun's formatStackTrace() synthesizes an at <parse> (url:line) line from the location addErrorInfo() recorded on the instance, and treats that location as the error's line / sourceURL.
• addErrorInfo(vm, error, line, SourceCode) (JSC runtime/Error.cpp): stores the source's URL and the line on the ErrorInstance and immediately materializes .stack, which invokes Bun's computeErrorInfo hook with only the frames, the line/column, the URL and the instance.
• node:vm's arrow header: decorateParseErrorStack() / handleException() (NodeVM.cpp) prepend Node's url:line + source line + caret to .stack after the error exists, computed from the ParserError directly, which is why it was already right.
• Private builtin names (src/js/builtins/BunBuiltinNames.h): per-VM private symbols C++ can use as property keys that JS code cannot name or enumerate.

Repro output on the released build

// /tmp/remap_syntax_short.js
const vm = require("node:vm");
const src = "\n".repeat(9) + "let let = 1;"; // syntax error on physical line 10
for (const filename of [__filename, "not-a-loaded-file.js"]) {
  try { new vm.Script(src, { filename }); }
  catch (e) { console.log(e.line, e.originalLine, e.stack.split("\n").find(l => l.includes("<parse>"))); }
}

$ bun /tmp/remap_syntax_short.js            # 1.4.0
5 10     at <parse> (/tmp/remap_syntax_short.js:5)
10 10     at <parse> (not-a-loaded-file.js:10)

$ bun-debug /tmp/remap_syntax_short.js      # this branch
10 undefined     at <parse> (/tmp/remap_syntax_short.js:10)
10 undefined     at <parse> (not-a-loaded-file.js:10)

vm.compileFunction on the released build, same source on line 5 of the body: line: 2, originalLine: 5, at <parse> (/tmp/remap_module.js:2). new vm.SourceTextModule on the released build, parse error on line 5: line: 225, originalLine: 225, sourceURL: "node:vm"; this branch: line: 5, no originalLine / sourceURL.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 19 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: 526f2d82-452d-4310-8235-954a39b39f5c

📥 Commits

Reviewing files that changed from the base of the PR and between 3753c8b and 28a4bfd.

📒 Files selected for processing (7)

• src/js/builtins/BunBuiltinNames.h
• src/jsc/bindings/FormatStackTraceForJS.cpp
• src/jsc/bindings/NodeVM.cpp
• src/jsc/bindings/NodeVM.h
• src/jsc/bindings/NodeVMScript.cpp
• src/jsc/bindings/NodeVMSourceTextModule.cpp
• test/js/node/vm/vm.test.ts

---

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

[robobun]

Status

• Reproduced on the released 1.4.0 build with USE_SYSTEM_BUN=1 bun test test/js/node/vm/vm.test.ts -t "compiling vm code under the filename": the own-filename cases report line 23 (remapped through the fixture's source map) instead of line 5 for Script, runInThisContext, runInContext, runInNewContext and compileFunction.
• With this branch the test passes (bun bd test ...), including under BUN_JSC_validateExceptionChecks=1; the related vm, stack-trace and REPL suites listed in the description pass as well.
• Waiting on CI.

[claude]

I reviewed this PR and didn't find any bugs. Because it touches the shared stack-trace formatter (FormatStackTraceForJS.cpp) and adds new JSC exception-scope handling across three node:vm compile paths, a human look would still be worthwhile — particularly given the several open PRs the description names as editing the same <parse> branch and toErrorObject() sites.

What was reviewed:

• createParseError() replicates ParserError::toErrorObject()'s SyntaxError path (createSyntaxError + addErrorInfo(line()) + setParseError) — the dropped -1 overrideLineNumber arg is equivalent to passing parseError.line().
• The formatter's new getDirect(vmParseErrorPrivateName) check is only reached when errorInstance is non-null, which excludes the GC-finalizer path (computeErrorInfoToString passes nullptr).
• The sb.append(remappedFrame.source_url...) → sb.append(sourceURLForFrame) change: for the tagged branch this correctly uses err->sourceURL(); for the untouched remap branch sourceURLForFrame was already reassigned from the remapped URL, so no behavior change there.
• createModuleRecord()'s new tryClearException() matches the existing pattern at the other two call sites.

Extended reasoning...

Overview

This PR fixes incorrect line numbers on SyntaxErrors thrown when compiling node:vm code under a filename that Bun has already loaded (and thus holds a source map for). It adds a private vmParseError builtin symbol, a NodeVM::createParseError() helper that tags the error before addErrorInfo() materializes the stack, wires the three vm compile paths (new Script, compileFunction, SourceTextModule) through it, and teaches formatStackTrace()'s <parse> branch to skip source-map remapping when the tag is present. It also removes a redundant nested isEmpty() check and switches the <parse> frame's URL append to use sourceURLForFrame (which is now correct for both the tagged and remapped branches). createModuleRecord() gains the same tryClearException() handling for a throwing user Error.prepareStackTrace that the other two sites already have. A comprehensive subprocess test covers all six vm entry points under both a loaded and an unloaded filename.

Security risks

None. The private symbol is not JS-observable; the change only affects which line number is reported in a compile-time SyntaxError's stack string. No user input reaches new parsing/allocation paths.

Level of scrutiny

Moderate-to-high. FormatStackTraceForJS.cpp is invoked for every error's .stack materialization across the runtime, so a mistake there would have broad reach. The new C++ also involves JSC exception-scope discipline (tryClearException / RETURN_IF_EXCEPTION ordering) at three sites. While the change is focused and the mechanism (private-symbol tag, mirroring the existing vmErrorDecorated pattern) is clean, this is not a mechanical fix.

Other factors

• The uncheckedDowncast<ErrorInstance>(createSyntaxError(...)) is safe: JSC's createSyntaxError uses the internal error structure, unaffected by user overrides of globalThis.SyntaxError.
• Setting hasSet = true for the tagged branch leaves the line out-param at the value addErrorInfo() seeded it with (parseError.line()), which is what becomes err.line — matching the test's assertion of the physical line 5.
• The PR description names six other open PRs touching the same <parse> branch or toErrorObject() call sites; a maintainer should confirm merge order / rebase expectations.
• The test is well-constructed (20-line comment padding guarantees any remapped line would exceed 20, so a false pass is not possible) and covers the full API matrix plus Bun.inspect.