Subtle change in Vite runtime stacktrace with `prepareStackTrace` since 5.1.5

[hi-ogawa]

Describe the bug

While upgrading Vite from 5.1.0 to 5.1.6 on my monorepo with vite-node-miniflare hi-ogawa/vite-plugins#198, I found my tests which asserts stacktrace are failing. On further investigation, there seems to be a subtle difference since 5.1.5 as seen in this simpler reproduction.

node run.mjs ./repro-entry.ts
...
Error: crash ssr
    at crash (.../repro-entry.ts:2:9)
    at main (.../repro-entry.ts:6:9)  <---- 6:3 became 6:9 since 5.1.5

function crash(message) {
  throw new Error(message);
}

function main() {
  crash("crash ssr");
//^ 6:3        <- before
//      ^ 6:9  <- after
}

main();

On my actual case, I use a plugin vitePluginSimpleHmr which adds one more transform to ssr code, then stacktrace seems more broken (not only column, but also line and absolute path resolution is not working). For this part, there might be an issue with my plugin, so I'm not sure whether it's related to Vite side change and I haven't tried minimal repro for that yet.

Reproduction

https://github.com/hi-ogawa/reproductions/tree/main/vite-runtime-stacktrace-5.1.5

Steps to reproduce

In the reproduction, I added a comparison with node and also when vite entry is plain javascript, in which case there's no difference in stacktrace.

Stackblitz doesn't seem to have a good support on stacktrace, so it might be necessary to copy the reproduction locally. To skip cloning the entire repo, you can use tools such as https://github.com/tiged/tiged

npx tiged hi-ogawa/reproductions/vite-runtime-stacktrace-5.1.5 repro

System Info

  System:
    OS: Linux 6.7 Arch Linux
    CPU: (12) x64 AMD Ryzen 5 5625U with Radeon Graphics
    Memory: 9.30 GB / 14.98 GB
    Container: Yes
    Shell: 5.2.26 - /bin/bash
  Binaries:
    Node: 20.11.1 - ~/.volta/tools/image/node/20.11.1/bin/node
    Yarn: 1.22.19 - ~/.volta/tools/image/yarn/1.22.19/bin/yarn
    npm: 10.2.4 - ~/.volta/tools/image/node/20.11.1/bin/npm
    pnpm: 8.12.0 - ~/.volta/bin/pnpm
    bun: 1.0.26 - ~/.volta/bin/bun
  Browsers:
    Chromium: 122.0.6261.111
  npmPackages:
    vite: 5.1.5 => 5.1.5

Used Package Manager

npm

Logs

This is the same content as reproduction readme, but just in case:

# node
node ./repro-entry.mjs
...
Error: crash ssr
    at crash (.../repro-entry.mjs:2:9)
    at main (.../repro-entry.mjs:6:3)
    ...


# vite + js
node run.mjs ./repro-entry.mjs
...
Error: crash ssr
    at crash (.../repro-entry.mjs:2:9)
    at main (.../repro-entry.mjs:6:3)
    ...


# vite 5.1.4 + ts
node run.mjs ./repro-entry.ts
...
Error: crash ssr
    at crash (.../repro-entry.ts:2:9)
    at main (.../repro-entry.ts:6:3)
    ...


# vite 5.1.5 + ts
node run.mjs ./repro-entry.ts
...
Error: crash ssr
    at crash (.../repro-entry.ts:2:9)
    at main (.../repro-entry.ts:6:9)  <---- 6:3 became 6:9

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[hi-ogawa]

I did some digging in #16220 and it seems bias === LEAST_UPPER_BOUND => true transform introduced in #15907 doesn't seem to be correct.

For example, when solving originalPositionFor for { line: 7, column: 2 }) and this sourcemap, I see a following combination in traceSegmentInternal

• found: false
• index: 0
• bias: 1 (default from GREATEST_LOWER_BOUND = 1)

Then, due to bias === LEAST_UPPER_BOUND (= -1) replaced to true, index: 0 is pushed to index: 1, so it's picking up a next segment in originalPositionFor (i.e. 6:9 instead of 6:3 as in the reproduction).

---

Also this transform to help tree-shaking might not be necessary anymore since @jridgewell/trace-mapping did some restructuring in jridgewell/trace-mapping@fc2a76a

I tested removing "replace bias" transform in #16220 and the bundle size of dist/runtime.js seems to be mostly same.

@sapphi-red Do you think it's okay to remove this transform now? Also if you have some insight on bias === LEAST_UPPER_BOUND, I would be happy to hear.

[sapphi-red]

I added the replacement so that I can preserve the code before the PR.
The code before:

vite/packages/vite/src/node/ssr/runtime/sourcemap/decoder.ts

Lines 228 to 233 in 42fd11c

	let index = memoizedBinarySearch(segments, column, memo, line)
	if (found) {
	index = lowerBound(segments, column, index)
	}
	if (index === -1 || index === segments.length) return -1
	return index

The code in trace-mapping:
https://github.com/jridgewell/trace-mapping/blob/2c7bf531d923eb304a4b647148baecd4aae1130f/src/trace-mapping.ts#L381-L387
But actually bias === LEAST_UPPER_BOUND should be replaced with false instead of true🤦.

I'm fine with removing the replacement as long as the runtime chunk doesn't get that bigger because I guess that was the intent.