Fix off-by-one in column numbers when applying source maps

src/__tests__/app.test.tsx

@@ -1,6 +1,7 @@
 import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { type ChangeEvent } from 'react'
+import { SourceMapGenerator } from 'source-map'
 import { describe, expect, test, vi } from 'vitest'
 
 import App from '../app.tsx'
@@ -328,6 +329,94 @@ describe('source maps', () => {
   })
 })
 
+describe('column numbers', () => {
+  // Generated line 1 has mappings at 0-based columns 0, 10, and 11 which lead
+  // to different original lines, so an off-by-one in the column conversion
+  // resolves to a wrong original position.
+  function createCraftedSourceMap() {
+    const generator = new SourceMapGenerator({ file: 'crafted.min.js' })
+
+    generator.addMapping({
+      generated: { column: 0, line: 1 },
+      name: 'start',
+      original: { column: 0, line: 1 },
+      source: 'original.ts',
+    })
+    generator.addMapping({
+      generated: { column: 10, line: 1 },
+      name: 'first',
+      original: { column: 2, line: 5 },
+      source: 'original.ts',
+    })
+    generator.addMapping({
+      generated: { column: 11, line: 1 },
+      name: 'second',
+      original: { column: 4, line: 9 },
+      source: 'original.ts',
+    })
+
+    return generator.toString()
+  }
+
+  test('converts 1-based stack trace columns to the 0-based source map convention and back', async () => {
+    render(<App />)
+    const user = userEvent.setup()
+
+    const stacktraceTextarea = screen.getByRole('textbox', { name: /minified stack trace/i })
+    const resultTextArea = screen.getByRole('textbox', { name: /original stack trace/i })
+
+    // 1-based column 11 is 0-based column 10, so it must resolve to `first`,
+    // not to the mapping at 0-based column 11 (`second`).
+    await user.type(stacktraceTextarea, 'Error: boom\n  at crafted.min.js:1:11')
+
+    const sourcemapTextarea = screen.getByRole('textbox', { name: /source map/i })
+    sourcemapTextarea.focus()
+    await user.paste(createCraftedSourceMap())
+
+    await waitFor(() =>
+      expect(resultTextArea).toHaveValue('Error: boom\n  at first (original.ts:5:3)'),
+    )
+  })
+
+  test('transforms frames with column 0', async () => {
+    render(<App />)
+    const user = userEvent.setup()
+
+    const stacktraceTextarea = screen.getByRole('textbox', { name: /minified stack trace/i })
+    const resultTextArea = screen.getByRole('textbox', { name: /original stack trace/i })
+
+    await user.type(stacktraceTextarea, 'Error: boom\n  at crafted.min.js:1:0')
+
+    const sourcemapTextarea = screen.getByRole('textbox', { name: /source map/i })
+    sourcemapTextarea.focus()
+    await user.paste(createCraftedSourceMap())
+
+    await waitFor(() =>
+      expect(resultTextArea).toHaveValue('Error: boom\n  at start (original.ts:1:1)'),
+    )
+  })
+
+  test('keeps the original frame when the position is not in the source map', async () => {
+    render(<App />)
+    const user = userEvent.setup()
+
+    const stacktraceTextarea = screen.getByRole('textbox', { name: /minified stack trace/i })
+    const resultTextArea = screen.getByRole('textbox', { name: /original stack trace/i })
+
+    // Line 2 has no mappings, so the frame must stay as is instead of
+    // becoming `null:null:null`.
+    await user.type(stacktraceTextarea, 'Error: boom\n  at crafted.min.js:2:1')
+
+    const sourcemapTextarea = screen.getByRole('textbox', { name: /source map/i })
+    sourcemapTextarea.focus()
+    await user.paste(createCraftedSourceMap())
+
+    await waitFor(() =>
+      expect(resultTextArea).toHaveValue('Error: boom\n  at (crafted.min.js:2:1)'),
+    )
+  })
+})
+
 describe('theme', () => {
   test('renders with light theme if prefers light theme', () => {
     vi.restoreAllMocks()

src/__tests__/fixtures/regular/index.ts

@@ -5,12 +5,12 @@ export const regular = {
   afterDeleteIndex: `Uncaught Error: Error!
   at e.throwError (index-F7qoIhl0.js:2:1251)
   at onClick (index-F7qoIhl0.js:3:5483)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12317:12)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12867:4)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:1498:35)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12455:2)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15306:6)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15274:6)`,
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12317:13)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12867:5)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:1498:36)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12455:3)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15306:7)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15274:7)`,
   reconstructed: `Uncaught Error: Error!
   at e.throwError (index-F7qoIhl0.js:2:1251)
   at onClick (index-F7qoIhl0.js:3:5483)
@@ -21,14 +21,14 @@ export const regular = {
   at up (vendor-B_FE3Fnm.js:9:28431)
   at cp (vendor-B_FE3Fnm.js:9:28253)`,
   result: `Uncaught Error: Error!
-  at (../../src/source-map.ts:60:14)
-  at (../../src/app.tsx:164:66)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12317:12)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12867:4)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:1498:35)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12455:2)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15306:6)
-  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15274:6)`,
+  at (../../src/source-map.ts:60:15)
+  at (../../src/app.tsx:164:67)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12317:13)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12867:5)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:1498:36)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:12455:3)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15306:7)
+  at (../../node_modules/react-dom/cjs/react-dom-client.production.js:15274:7)`,
   sourcemaps: ['index-F7qoIhl0.js.map', 'vendor-B_FE3Fnm.js.map'].map(fileName => ({
     content: fs.readFileSync(path.resolve(__dirname, 'sourcemaps', fileName)).toString(),
     fileName,

src/lib.ts

@@ -26,17 +26,19 @@ function tryGetOriginalPosition(
   stackFrame: StackFrame,
   bindings: Record<string, SourceMap>,
 ): null | NullableMappedPosition {
-  let result: null | NullableMappedPosition = null
-
   const { column, file, line } = toUnifiedPosition(stackFrame)
 
-  if (!file || !bindings[file] || !line || !column) {
+  if (!file || !bindings[file] || line == null || line < 1 || column == null) {
     return null
   }
 
-  result = bindings[file].consumer.originalPositionFor({ column, line })
+  // Stack traces use 1-based columns, the source-map library expects 0-based.
+  const result = bindings[file].consumer.originalPositionFor({
+    column: Math.max(column - 1, 0),
+    line,
+  })
 
-  return result
+  return result.source == null ? null : result
 }
 
 function generateStackTraceLine(position: UnifiedPosition) {
@@ -55,7 +57,8 @@ function toUnifiedPosition(position: NullableMappedPosition | StackFrame): Unifi
   }
 
   return {
-    column: position.column,
+    // The source-map library returns 0-based columns, stack traces use 1-based.
+    column: position.column == null ? null : position.column + 1,
     file: position.source,
     line: position.line,
     method: position.name,