error printer: don't invent a 1:1 position for frames parsed out of error.stack without one

src/jsc/VirtualMachine.rs

@@ -5360,7 +5360,9 @@ impl VirtualMachine {
                     }
                 };
             }
-            if has_name && !frame.position.is_invalid() {
+            // Frames parsed back out of error.stack can have a file without a
+            // position ("at foo (native)"); the formatter then prints the file alone.
+            if has_name && (!file.is_empty() || !frame.position.is_invalid()) {
                 pretty_write!(
                     "<r>      <d>at <r>{}<d> (<r>{}<d>)<r>\n",
                     frame.name_formatter(allow_ansi_colors),
@@ -5630,13 +5632,11 @@ impl VirtualMachine {
             Some(bun_sourcemap::mapping::Lookup {
                 mapping: bun_sourcemap::mapping::Mapping {
                     generated: bun_sourcemap::LineColumnOffset::default(),
+                    // Direct copy (both are `bun_core::Ordinal`) so that a frame
+                    // without a position stays INVALID instead of becoming 1:1.
                     original: bun_sourcemap::LineColumnOffset {
-                        lines: bun_sourcemap::Ordinal::from_zero_based(
-                            frames[top].position.line.zero_based().max(0),
-                        ),
-                        columns: bun_sourcemap::Ordinal::from_zero_based(
-                            frames[top].position.column.zero_based().max(0),
-                        ),
+                        lines: frames[top].position.line,
+                        columns: frames[top].position.column,
                     },
                     source_index: 0,
                     name_index: -1,
@@ -5690,6 +5690,11 @@ impl VirtualMachine {
                     // Avoid printing "export default 'native'"
                     break 'code bun_core::ZigStringSlice::EMPTY;
                 }
+                if !mapping.original.lines.is_valid() {
+                    // No line to preview (a frame parsed out of error.stack that
+                    // only names a file); line 1 would be an arbitrary choice.
+                    break 'code bun_core::ZigStringSlice::EMPTY;
+                }
                 let mut log = bun_ast::Log::default();
                 let Ok(original_source) = Self::fetch_without_on_load_plugins(
                     self,
@@ -6558,16 +6563,19 @@ impl VirtualMachine {
 
         let mut has_location = false;
         if let Some(frame) = top_frame {
-            if !frame.position.is_invalid() {
+            if frame.position.line.is_valid() {
                 let source_url = frame.source_url.to_utf8();
                 let file = bun_paths::resolve_path::relative(dir, source_url.slice());
                 let _ = write!(
                     writer,
-                    "\n::error file={},line={},col={},title=",
+                    "\n::error file={},line={},",
                     bun_core::fmt::github_action_property(file),
                     frame.position.line.one_based(),
-                    frame.position.column.one_based(),
                 );
+                if frame.position.column.is_valid() {
+                    let _ = write!(writer, "col={},", frame.position.column.one_based());
+                }
+                let _ = writer.write_all(b"title=");
                 has_location = true;
             }
         }

src/jsc/bindings/ZigException.cpp

@@ -245,8 +245,11 @@ class V8StackTraceIterator {
     public:
         StringView functionName {};
         StringView sourceURL {};
-        WTF::OrdinalNumber lineNumber = WTF::OrdinalNumber::fromZeroBasedInt(0);
-        WTF::OrdinalNumber columnNumber = WTF::OrdinalNumber::fromZeroBasedInt(0);
+        // beforeFirst() (-1) is ZigStackFramePosition's "unknown", so a frame
+        // printed without a line or column ("at foo (native)") stays unknown
+        // instead of becoming line 1, column 1.
+        WTF::OrdinalNumber lineNumber = WTF::OrdinalNumber::beforeFirst();
+        WTF::OrdinalNumber columnNumber = WTF::OrdinalNumber::beforeFirst();
 
         bool isConstructor = false;
         bool isGlobalCode = false;
@@ -611,6 +614,9 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global,
                         current.source_url = Bun::toStringRef(sourceURL);
                         current.position.line_zero_based = frame.lineNumber.zeroBasedInt();
                         current.position.column_zero_based = frame.columnNumber.zeroBasedInt();
+                        // `current = {}` zeroes byte_position; -1 keeps a frame without a
+                        // line and column equal to ZigStackFramePosition::INVALID.
+                        current.position.byte_position = -1;
 
                         current.remapped = true;
                         current.is_async = frame.isAsync;

test/js/bun/util/inspect-error.test.js

@@ -1,4 +1,5 @@
 import { describe, expect, jest, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
 
 test("error.cause", () => {
   const err = new Error("error 1");
@@ -9,21 +10,23 @@ test("error.cause", () => {
       .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"),
   ).toMatchInlineSnapshot(`
 "1 | import { describe, expect, jest, test } from "bun:test";
-2 | 
-3 | test("error.cause", () => {
-4 |   const err = new Error("error 1");
-5 |   const err2 = new Error("error 2", { cause: err });
+2 | import { bunEnv, bunExe, tempDir } from "harness";
+3 | 
+4 | test("error.cause", () => {
+5 |   const err = new Error("error 1");
+6 |   const err2 = new Error("error 2", { cause: err });
                        ^
 error: error 2
-      at <anonymous> ([dir]/inspect-error.test.js:5:20)
+      at <anonymous> ([dir]/inspect-error.test.js:6:20)
 
 1 | import { describe, expect, jest, test } from "bun:test";
-2 | 
-3 | test("error.cause", () => {
-4 |   const err = new Error("error 1");
+2 | import { bunEnv, bunExe, tempDir } from "harness";
+3 | 
+4 | test("error.cause", () => {
+5 |   const err = new Error("error 1");
                       ^
 error: error 1
-      at <anonymous> ([dir]/inspect-error.test.js:4:19)
+      at <anonymous> ([dir]/inspect-error.test.js:5:19)
 "
 `);
 });
@@ -35,15 +38,15 @@ test("Error", () => {
       .replaceAll("\\", "/")
       .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"),
   ).toMatchInlineSnapshot(`
-"27 | "
-28 | \`);
-29 | });
-30 | 
-31 | test("Error", () => {
-32 |   const err = new Error("my message");
+"30 | "
+31 | \`);
+32 | });
+33 | 
+34 | test("Error", () => {
+35 |   const err = new Error("my message");
                        ^
 error: my message
-      at <anonymous> ([dir]/inspect-error.test.js:32:19)
+      at <anonymous> ([dir]/inspect-error.test.js:35:19)
 "
 `);
 });
@@ -71,22 +74,13 @@ note: "duplicateConstDecl" was originally declared here
   }
 });
 
-const normalizeError = str => {
-  // remove debug-only stack trace frames
-  // like "at require (:1:21)"
-  if (str.includes(" (:")) {
-    const splits = str.split("\n");
-    for (let i = 0; i < splits.length; i++) {
-      if (splits[i].includes(" (:")) {
-        splits.splice(i, 1);
-        i--;
-      }
-    }
-    return splits.join("\n");
-  }
-
-  return str;
-};
+const normalizeError = str =>
+  // remove debug-only stack trace frames of bun's own builtins, which have a
+  // position but no file, like "at require (51:24)"
+  str
+    .split("\n")
+    .filter(line => !/^\s*at \S+ \(:?\d+:\d+\)$/.test(line))
+    .join("\n");
 
 test("Error inside minified file (no color) ", () => {
   try {
@@ -111,7 +105,7 @@ test("Error inside minified file (no color) ", () => {
       error: error inside long minified file!
             at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2850)
             at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2890)
-            at <anonymous> ([dir]/inspect-error.test.js:92:7)"
+            at <anonymous> ([dir]/inspect-error.test.js:86:7)"
     `);
   }
 });
@@ -140,7 +134,7 @@ test("Error inside minified file (color) ", () => {
       error: error inside long minified file!
             at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2850)
             at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2890)
-            at <anonymous> ([dir]/inspect-error.test.js:120:7)"
+            at <anonymous> ([dir]/inspect-error.test.js:114:7)"
     `);
   }
 });
@@ -154,7 +148,7 @@ test("Inserted originalLine and originalColumn do not appear in node:util.inspec
       .replaceAll(import.meta.path.replaceAll("\\", "/"), "[file]"),
   ).toMatchInlineSnapshot(`
 "Error: my message
-    at <anonymous> ([file]:149:19)"
+    at <anonymous> ([file]:143:19)"
 `);
 });
 
@@ -188,3 +182,95 @@ test("error.stack throwing an error doesn't lead to a crash", () => {
     throw err;
   }).toThrow();
 });
+
+// Once error.stack has been read (or assigned), JSC no longer has the frames of
+// the error and the printer parses the "    at ..." lines of that string instead.
+// A frame printed without a line and column (native code, "unknown") has no
+// position to parse, and must not come back out as line 1, column 1.
+describe("printing the frames parsed back out of error.stack without a position", () => {
+  const printedFrames = text =>
+    text
+      .split("\n")
+      .map(line => line.trim())
+      .filter(line => line.startsWith("at "));
+
+  const stackWithFrames = frames => ["Error: boom", ...frames.map(frame => `    at ${frame}`)].join("\n");
+
+  const inspectStack = frames => {
+    const err = new Error("boom");
+    err.stack = stackWithFrames(frames);
+    return Bun.inspect(err);
+  };
+
+  test("frames without a position, or with only a line, print as error.stack has them", () => {
+    const frames = [
+      "first (native)",
+      "second (node:child_process)",
+      "unknown",
+      "third (/fake/lib.js:3:4)",
+      "fourth (/fake/lib.js:7)",
+    ];
+    expect(printedFrames(inspectStack(frames))).toEqual(frames.map(frame => `at ${frame}`));
+  });
+
+  test("a native frame is the only frame, so the source preview is looked up for it", () => {
+    expect(printedFrames(inspectStack(["first (native)"]))).toEqual(["at first (native)"]);
+  });
+
+  const inspectedLines = frames => inspectStack(frames).trimEnd().split("\n");
+
+  test("no source preview for a frame that names a file but no line in it", () => {
+    using dir = tempDir("inspect-error-no-position", { "lib.js": "// line 1\n// line 2\n" });
+    const file = `${dir}/lib.js`;
+    expect(inspectedLines([`first (${file})`])).toEqual(["error: boom", `      at first (${file})`]);
+  });
+
+  test("the source preview and its caret belong to the first frame that has a position", () => {
+    using dir = tempDir("inspect-error-no-position", { "lib.js": "// line 1\n// line 2\n" });
+    const file = `${dir}/lib.js`;
+    expect(inspectedLines(["first (native)", `second (${file}:2:5)`])).toEqual([
+      "1 | // line 1",
+      "2 | // line 2",
+      "        ^",
+      "error: boom",
+      "      at first (native)",
+      `      at second (${file}:2:5)`,
+    ]);
+  });
+
+  describe.concurrent("uncaught exception printout and GitHub Actions annotation", () => {
+    const throwUncaught = async frames => {
+      await using proc = Bun.spawn({
+        cmd: [
+          bunExe(),
+          "-e",
+          `const err = new Error("boom"); err.stack = ${JSON.stringify(stackWithFrames(frames))}; throw err;`,
+        ],
+        env: { ...bunEnv, GITHUB_ACTIONS: "true", GITHUB_WORKSPACE: "/fake" },
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect(stdout).toBe("");
+      expect(exitCode).toBe(1);
+      return {
+        frames: printedFrames(stderr),
+        annotation: stderr.split("\n").find(line => line.startsWith("::error ")),
+      };
+    };
+
+    test("top frame without a position: the annotation has no file, line or column", async () => {
+      expect(await throwUncaught(["first (native)", "unknown", "second (/fake/lib.js:3:4)"])).toEqual({
+        frames: ["at first (native)", "at unknown", "at second (/fake/lib.js:3:4)"],
+        annotation: expect.stringMatching(/^::error title=error: boom::/),
+      });
+    });
+
+    test("top frame with only a line: the annotation has no column", async () => {
+      expect(await throwUncaught(["first (/fake/lib.js:7)", "second (native)"])).toEqual({
+        frames: ["at first (/fake/lib.js:7)", "at second (native)"],
+        annotation: expect.stringMatching(/^::error file=(.*[\\/])?lib\.js,line=7,title=error: boom::/),
+      });
+    });
+  });
+});

test/regression/issue/23022-stack-trace-iterator.test.ts

@@ -27,7 +27,8 @@ test("V8StackTraceIterator handles frames without parentheses (issue #23022)", a
   const stackFrames = err.stack?.split("\n").filter(line => line.trim().startsWith("at"));
   expect(stackFrames?.length).toBeGreaterThan(3);
 
-  // Ensure both "unknown" frames and regular frames are present
-  expect(inspected).toContain("at unknown");
+  // Ensure both "unknown" frames and regular frames are present. The "unknown"
+  // frame has no position in error.stack, so the printout must not invent one.
+  expect(inspected).toMatch(/^\s+at unknown$/m);
   expect(inspected).toContain("at _write");
 });