error printer: place the code frame caret by the excerpt, not by the top frame's column

src/jsc/VirtualMachine.rs

@@ -5762,6 +5762,7 @@ impl VirtualMachine {
                     current_line_number -= 1;
                 }
                 exception.stack.source_lines_len = take as u8;
+                exception.stack.source_lines_caret_column = frames[top].position.column;
             }
 
             if !code.slice().is_empty() {
@@ -6168,7 +6169,7 @@ impl VirtualMachine {
                         allow_ansi_color,
                         formatter.error_display_level,
                     )?;
-                } else if let Some(top) = top_frame {
+                } else {
                     did_print_name = true;
                     let display_line = source.line + 1;
                     let int_size = count_digits(display_line);
@@ -6197,7 +6198,7 @@ impl VirtualMachine {
                     } else {
                         pretty_write!(writer, "<r><b>{} |<r> {}\n", display_line, hl)?;
 
-                        let col = top.position.column.zero_based();
+                        let col = exception.stack.source_lines_caret_column.zero_based();
                         if clamped.len() < MAX_LINE_LENGTH_WITH_DIVOT
                             || (col as usize) > MAX_LINE_LENGTH_WITH_DIVOT
                         {

src/jsc/ZigException.rs

@@ -168,6 +168,7 @@ impl Holder {
                     source_lines_numbers: self.source_line_numbers.as_mut_ptr(),
                     source_lines_len: Self::SOURCE_LINES_COUNT as u8,
                     source_lines_to_collect: Self::SOURCE_LINES_COUNT as u8,
+                    source_lines_caret_column: bun_core::Ordinal::INVALID,
                     frames_ptr: self.frames.as_mut_ptr(),
                     frames_len: 0,
                     frames_cap: Self::FRAME_COUNT as u8,

src/jsc/ZigStackTrace.rs

@@ -2,6 +2,7 @@ use core::ptr;
 use core::ptr::NonNull;
 
 use crate::exception_list;
+use bun_core::Ordinal;
 use bun_core::String as BunString;
 use bun_core::ZigStringSlice;
 use bun_url::URL as ZigURL;
@@ -16,6 +17,12 @@ pub struct ZigStackTrace {
     pub(crate) source_lines_numbers: *mut i32,
     pub(crate) source_lines_len: u8,
     pub(crate) source_lines_to_collect: u8,
+    /// Column of `source_lines_ptr[0]` the caret is drawn under; set together
+    /// with that line. Not necessarily the top frame's column: on the first
+    /// line of a source with a start column (node:vm's `columnOffset`) the
+    /// frame's column includes the offset, while the excerpt is the physical
+    /// line.
+    pub(crate) source_lines_caret_column: Ordinal,
 
     pub(crate) frames_ptr: *mut ZigStackFrame,
     pub frames_len: u8,
@@ -36,6 +43,7 @@ impl ZigStackTrace {
             source_lines_numbers: ptr::dangling_mut(),
             source_lines_len: 0,
             source_lines_to_collect: 0,
+            source_lines_caret_column: Ordinal::INVALID,
 
             frames_ptr: frames_slice.as_mut_ptr(),
             frames_len: frames_slice.len().min(usize::from(u8::MAX)) as u8,

src/jsc/bindings/ZigException.cpp

@@ -128,7 +128,7 @@ static void populateStackFrameMetadata(JSC::VM& vm, JSC::JSGlobalObject* globalO
 }
 
 static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunString* source_lines,
-    OrdinalNumber* source_line_numbers, uint8_t source_lines_count,
+    OrdinalNumber* source_line_numbers, int32_t* source_lines_caret_column, uint8_t source_lines_count,
     ZigStackFramePosition& position, JSC::SourceProvider** referenced_source_provider, PopulateStackTraceFlags flags)
 {
     auto code = stackFrame.codeBlock();
@@ -187,6 +187,14 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
         *referenced_source_provider = provider;
         source_lines[0] = Bun::toStringView(sourceString.substring(lineStart, lineEnd - lineStart));
         source_line_numbers[0] = location.line();
+        // The caret is measured from the excerpt's text (the printer trims the previous
+        // line's '\n' that lineStart is still on), not taken from location.column(): on
+        // the first line of a source with a start column (node:vm's columnOffset) the
+        // reported column includes that offset, which the excerpt does not contain.
+        int textStart = static_cast<int>(lineStart);
+        if (lineStart < sourceString.length() && sourceString[lineStart] == '\n')
+            textStart++;
+        *source_lines_caret_column = std::max(location.byte_position - textStart, 0);
 
         if (lineStart > 0) {
             auto byte_offset_in_source_string = lineStart - 1;
@@ -230,11 +238,12 @@ static void populateStackFrame(JSC::VM& vm, ZigStackTrace& trace, const JSC::Sta
     if (flags == PopulateStackTraceFlags::OnlyPosition) {
         populateStackFrameMetadata(vm, globalObject, stackFrame, frame, finalizerSafety);
         populateStackFramePosition(stackFrame, nullptr,
-            nullptr,
+            nullptr, nullptr,
             0, frame.position, referenced_source_provider, flags);
     } else if (flags == PopulateStackTraceFlags::OnlySourceLines) {
         populateStackFramePosition(stackFrame, is_top ? trace.source_lines_ptr : nullptr,
             is_top ? trace.source_lines_numbers : nullptr,
+            is_top ? &trace.source_lines_caret_column_zero_based : nullptr,
             is_top ? trace.source_lines_to_collect : 0, frame.position, referenced_source_provider, flags);
     }
 }
@@ -673,6 +682,7 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global,
                                     auto str = jsStr->value(global);
                                     except.stack.source_lines_ptr[0] = Bun::toStringRef(str);
                                     except.stack.source_lines_numbers[0] = except.stack.frames_ptr[0].position.line();
+                                    except.stack.source_lines_caret_column_zero_based = except.stack.frames_ptr[0].position.column_zero_based;
                                     except.stack.source_lines_len = 1;
                                     except.remapped = true;
                                 }

src/jsc/bindings/headers-handwritten.h

@@ -218,6 +218,12 @@ typedef struct ZigStackTrace {
     OrdinalNumber* source_lines_numbers;
     uint8_t source_lines_len;
     uint8_t source_lines_to_collect;
+    /// Column of `source_lines_ptr[0]` the error printer puts its caret under
+    /// (-1 if unset). Set by whoever fills `source_lines_ptr[0]`. It differs
+    /// from the top frame's column when the reported column does not count
+    /// from the start of the excerpted text: positions on the first line of a
+    /// source with a start column (node:vm's columnOffset) include that offset.
+    int32_t source_lines_caret_column_zero_based;
     ZigStackFrame* frames_ptr;
     uint8_t frames_len;
     uint8_t frames_cap;

test/js/node/vm/vm.test.ts

@@ -847,6 +847,99 @@ resp.text().then((a) => {
   }
 });
 
+describe("code frame of an error thrown from a source compiled with columnOffset", () => {
+  const filename = "/virtual/caret.js";
+
+  // The code frame Bun's error printer puts above an error: the excerpted line,
+  // the column of that line the caret sits under, and the top frame's position.
+  function parseCodeFrame(printed: string) {
+    expect(printed).toMatch(/^\d+ \| .*\n *\^\n/m);
+    expect(printed).toMatch(/\/virtual\/caret\.js:\d+:\d+/);
+    const lines = printed.split("\n");
+    const caretIndex = lines.findIndex(line => line.trim() === "^");
+    const textStart = lines[caretIndex - 1].indexOf("| ") + 2;
+    const [, line, column] = printed.match(/\/virtual\/caret\.js:(\d+):(\d+)/)!;
+    return {
+      text: lines[caretIndex - 1].slice(textStart),
+      caret: lines[caretIndex].indexOf("^") - textStart,
+      line: Number(line),
+      column: Number(column),
+    };
+  }
+
+  function inspectThrown(run: () => void) {
+    let thrown: unknown;
+    try {
+      run();
+    } catch (e) {
+      thrown = e;
+    }
+    return parseCodeFrame(Bun.inspect(thrown, { colors: false }));
+  }
+
+  // displayErrors: false keeps node:vm from replacing err.stack with its own
+  // source line and caret; the code frame under test is the printer's.
+  const script = (code: string, columnOffset: number) => () =>
+    new Script(code, { filename, columnOffset }).runInThisContext({ displayErrors: false });
+
+  test.each(['throw new Error("x")', "null.x;", '"use strict"; missing;'])(
+    "the caret stays under the token on the first line: %s",
+    code => {
+      const plain = inspectThrown(script(code, 0));
+      expect(plain.caret).toBeGreaterThan(0);
+      // columnOffset is added to the reported column of the first line (as in
+      // Node), but the excerpt is the physical line, so the caret must not move.
+      expect(inspectThrown(script(code, 20))).toEqual({ ...plain, column: plain.column + 20 });
+    },
+  );
+
+  test("lines after the first do not get the offset", () => {
+    const code = '"line 1";\nthrow new Error("x")';
+    const plain = inspectThrown(script(code, 0));
+    expect(plain).toMatchObject({ text: 'throw new Error("x")', line: 2 });
+    expect(inspectThrown(script(code, 20))).toEqual(plain);
+  });
+
+  test("compileFunction", () => {
+    const body = 'throw new Error("x")';
+    const fn = (columnOffset: number) => () => compileFunction(body, [], { filename, columnOffset })();
+    const plain = inspectThrown(fn(0));
+    expect(plain).toMatchObject({ text: body });
+    expect(plain.caret).toBeGreaterThan(0);
+    // Only the excerpt is compared: which line and column compileFunction
+    // reports for its body is its own business, the caret has to follow the
+    // text either way.
+    const { text, caret } = inspectThrown(fn(20));
+    expect({ text, caret }).toEqual({ text: plain.text, caret: plain.caret });
+  });
+
+  test.concurrent("uncaught error output", async () => {
+    const code = 'throw new Error("x")';
+    async function uncaught(columnOffset: number) {
+      const options = { filename, columnOffset, displayErrors: false };
+      await using proc = Bun.spawn({
+        cmd: [
+          bunExe(),
+          "-e",
+          `require("node:vm").runInThisContext(${JSON.stringify(code)}, ${JSON.stringify(options)})`,
+        ],
+        env: bunEnv,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      const frame = parseCodeFrame(stderr);
+      expect(stdout).toBe("");
+      expect(exitCode).toBe(1);
+      return frame;
+    }
+    const [plain, shifted] = await Promise.all([uncaught(0), uncaught(20)]);
+    expect(plain).toMatchObject({ text: code, line: 1 });
+    expect(plain.caret).toBeGreaterThan(0);
+    expect(shifted).toEqual({ ...plain, column: plain.column + 20 });
+  });
+});
+
 test("can't use export syntax in vm.Script", () => {
   // vm.Script now parses eagerly (like Node), so the SyntaxError surfaces at
   // construction rather than at runInThisContext()/createCachedData().