error: report default class constructor frames at the class definition

src/jsc/bindings/ErrorStackFrame.cpp

@@ -1,13 +1,71 @@
 #include "root.h"
+#include "ErrorStackFrame.h"
 #include "JavaScriptCore/CodeBlock.h"
-#include "headers-handwritten.h"
-#include "JavaScriptCore/BytecodeIndex.h"
+#include "JavaScriptCore/FunctionExecutable.h"
+#include "JavaScriptCore/SourceProvider.h"
+#include "JavaScriptCore/StackFrame.h"
+#include "JavaScriptCore/UnlinkedFunctionExecutable.h"
 #include "wtf/Assertions.h"
 #include "wtf/text/OrdinalNumber.h"
 
 namespace Bun {
 using namespace JSC;
 
+SourceCode defaultClassConstructorClassSource(ScriptExecutable* executable)
+{
+    auto* function = dynamicDowncast<FunctionExecutable>(executable);
+    if (!function || !function->unlinkedExecutable()->isBuiltinDefaultClassConstructor())
+        return {};
+    return function->classSource();
+}
+
+static bool isLineTerminator(char16_t c)
+{
+    return c == '\n' || c == '\r' || c == 0x2028 || c == 0x2029;
+}
+
+ZigStackFramePosition classSourceStartPosition(const SourceCode& classSource)
+{
+    auto* provider = classSource.provider();
+    int start = classSource.startOffset();
+    WTF::StringView source = provider->source();
+    // node:vm lineOffset/columnOffset: JSC applies them (the column to the first line only) but clamps negative ones away; V8 does not.
+    TextPosition providerStart = provider->startPosition();
+
+    int line = classSource.firstLine().zeroBasedInt() + std::min(providerStart.m_line.zeroBasedInt(), 0);
+
+    // startColumn() is only a fallback: on the first line of a lazily parsed function it counts from the function.
+    int column = classSource.startColumn().zeroBasedInt();
+    if (static_cast<unsigned>(start) <= source.length()) {
+        int lineStart = start;
+        while (lineStart > 0 && !isLineTerminator(source[lineStart - 1]))
+            lineStart--;
+        column = start - lineStart;
+        if (lineStart == 0)
+            column += providerStart.m_column.zeroBasedInt();
+    }
+
+    return ZigStackFramePosition {
+        .line_zero_based = line,
+        .column_zero_based = column,
+        .byte_position = start,
+    };
+}
+
+LineColumn computeLineAndColumn(const StackFrame& frame)
+{
+    auto* codeBlock = frame.codeBlock();
+    auto classSource = codeBlock ? defaultClassConstructorClassSource(codeBlock->ownerExecutable()) : SourceCode();
+    if (!classSource.isNull()) {
+        auto position = classSourceStartPosition(classSource);
+        return LineColumn {
+            .line = static_cast<unsigned>(position.line().oneBasedInt()),
+            .column = static_cast<unsigned>(position.column().oneBasedInt()),
+        };
+    }
+    return frame.computeLineAndColumn();
+}
+
 /// Adjust a `ZigStackFramePosition` by a number of bytes. This accounts for when the adjustment
 /// crosses line boundaries, and thus requires the source code in order to properly compute
 /// the result.
@@ -64,6 +122,10 @@ void adjustPositionBackwards(ZigStackFramePosition& pos, int amount, CodeBlock*
 
 ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC::BytecodeIndex bc)
 {
+    auto classSource = defaultClassConstructorClassSource(code->ownerExecutable());
+    if (!classSource.isNull())
+        return classSourceStartPosition(classSource);
+
     auto expr = code->expressionInfoForBytecodeIndex(bc);
 
     ZigStackFramePosition pos {

src/jsc/bindings/ErrorStackFrame.h

@@ -1,9 +1,28 @@
+#pragma once
+
 #include "root.h"
 #include "headers-handwritten.h"
 #include "JavaScriptCore/BytecodeIndex.h"
+#include "JavaScriptCore/LineColumn.h"
+#include "JavaScriptCore/SourceCode.h"
+
+namespace JSC {
+class CodeBlock;
+class ScriptExecutable;
+class StackFrame;
+}
 
 namespace Bun {
 
+/// The class of a constructor JSC synthesized (its own source() is the URL-less "(function () { })" template), else null.
+JSC::SourceCode defaultClassConstructorClassSource(JSC::ScriptExecutable* executable);
+
+/// Position of the `class` keyword `classSource` starts at; frames of its default constructor go there, as in V8.
+ZigStackFramePosition classSourceStartPosition(const JSC::SourceCode& classSource);
+
 ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC::BytecodeIndex bc);
 
+/// frame.computeLineAndColumn(), with default class constructor frames placed at their class.
+JSC::LineColumn computeLineAndColumn(const JSC::StackFrame& frame);
+
 } // namespace Bun

src/jsc/bindings/ErrorStackTrace.cpp

@@ -304,7 +304,16 @@ JSCStackFrame::JSCStackFrame(JSC::VM& vm, const JSC::StackFrame& frame)
 
 intptr_t JSCStackFrame::sourceID() const
 {
-    return m_codeBlock ? m_codeBlock->ownerExecutable()->sourceID() : JSC::noSourceID;
+    if (!m_codeBlock) {
+        return JSC::noSourceID;
+    }
+
+    auto classSource = Bun::defaultClassConstructorClassSource(m_codeBlock->ownerExecutable());
+    if (!classSource.isNull()) {
+        return classSource.providerID();
+    }
+
+    return m_codeBlock->ownerExecutable()->sourceID();
 }
 
 JSC::JSString* JSCStackFrame::sourceURL()
@@ -371,12 +380,9 @@ ALWAYS_INLINE String JSCStackFrame::retrieveSourceURL()
     // Instead, try to get some identifying information for this frame
 
     // Try to use sourceID if available
-    if (m_codeBlock) {
-        auto sourceID = m_codeBlock->ownerExecutable()->sourceID();
-        if (sourceID != JSC::noSourceID) {
-            // Use a placeholder that includes the sourceID to make frames distinguishable
-            return makeString("[source:"_s, sourceID, "]"_s);
-        }
+    if (auto sourceID = this->sourceID(); sourceID != JSC::noSourceID) {
+        // Use a placeholder that includes the sourceID to make frames distinguishable
+        return makeString("[source:"_s, sourceID, "]"_s);
     }
 
     // Last resort: return a distinguishable placeholder instead of empty string
@@ -470,6 +476,11 @@ String sourceURL(JSC::CodeBlock& codeBlock)
         return String();
     }
 
+    auto classSource = Bun::defaultClassConstructorClassSource(codeBlock.ownerExecutable());
+    if (!classSource.isNull()) {
+        return sourceURL(classSource);
+    }
+
     const auto& source = codeBlock.source();
     return sourceURL(source);
 }

src/jsc/bindings/FormatStackTraceForJS.cpp

@@ -21,6 +21,7 @@
 
 #include "BunClientData.h"
 #include "CallSite.h"
+#include "ErrorStackFrame.h"
 #include "ErrorStackTrace.h"
 #include "headers-handwritten.h"
 
@@ -264,7 +265,7 @@ WTF::String formatStackTrace(
 
         if (!frame.hasLineAndColumnInfo()) continue;
 
-        originalLineColumns[i] = frame.computeLineAndColumn();
+        originalLineColumns[i] = Bun::computeLineAndColumn(frame);
 
         JSC::JSGlobalObject* globalObjectForFrame = lexicalGlobalObject;
         if (auto* callee = frame.callee()) {

src/jsc/bindings/ZigException.cpp

@@ -135,7 +135,9 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
     if (!code)
         return;
 
-    auto* provider = code->source().provider();
+    // Same source getAdjustedPositionForBytecode() positions the frame in.
+    auto classSource = Bun::defaultClassConstructorClassSource(code->ownerExecutable());
+    auto* provider = classSource.isNull() ? code->source().provider() : classSource.provider();
     if (!provider) [[unlikely]]
         return;
     // Make sure the range is valid:
@@ -146,7 +148,7 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
 
     if (!stackFrame.hasBytecodeIndex()) {
         if (stackFrame.hasLineAndColumnInfo()) {
-            auto lineColumn = stackFrame.computeLineAndColumn();
+            auto lineColumn = Bun::computeLineAndColumn(stackFrame);
             position.line_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.line).zeroBasedInt();
             position.column_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.column).zeroBasedInt();
         }

test/js/bun/test/stack.test.ts

@@ -1,7 +1,8 @@
 import { $ } from "bun";
-import { expect, test } from "bun:test";
-import { bunEnv, bunExe, bunRun, normalizeBunSnapshot } from "harness";
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, bunRun, normalizeBunSnapshot, tempDir } from "harness";
 import { join } from "node:path";
+import vm from "node:vm";
 
 test("name property is used for function calls in Error.stack", () => {
   function WRONG() {
@@ -149,3 +150,165 @@ test("Async functions frame should be included in stack trace", async () => {
         at async <anonymous> (file:NN:NN)"
   `);
 });
+
+// JSC compiles the constructor of a class that declares none from a fixed source string, so these
+// frames used to render as "new X (unknown:1:17)". Like V8, they should point at the `class` keyword.
+describe("frames of a class without an explicit constructor", () => {
+  test.concurrent("error.stack, error.sourceURL and error.line point at the class", async () => {
+    using dir = tempDir("default-ctor-stack", {
+      // Every position below is asserted on, so the lines are laid out deliberately. `// @bun` makes
+      // bun load the file as-is: these are JSC's own positions, with no source map involved.
+      "fixture.js": `// @bun
+class Thrower { constructor() { this.err = new Error("T"); } }
+export class Derived extends Thrower {}
+export class Fields { err = new Error("F"); }
+function later() {
+    class Later extends Thrower {}
+    return new Later().err;
+}
+function sameLine() { class SameLine extends Thrower {} return new SameLine().err; }
+const Anon = class extends Thrower {};
+class Nullary extends null {}
+let nullary;
+try { new Nullary(); } catch (e) { nullary = e; }
+const norm = s => String(s).replaceAll(import.meta.path, "<fixture>");
+const frame = (err, name) => norm(err.stack.split("\\n").map(l => l.trim()).find(l => l.startsWith("at new " + name + " ")));
+console.log(JSON.stringify({
+  derived: frame(new Derived().err, "Derived"),
+  fields: frame(new Fields().err, "Fields"),
+  later: frame(later(), "Later"),
+  sameLine: frame(sameLine(), "SameLine"),
+  anon: frame(new Anon().err, "Anon"),
+  nullary: { sourceURL: norm(nullary.sourceURL), line: nullary.line, column: nullary.column },
+}));
+`,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "fixture.js"],
+      env: bunEnv,
+      cwd: String(dir),
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({
+      // The `class` keyword, not `export`, like V8.
+      derived: "at new Derived (<fixture>:3:8)",
+      // A base class: its default constructor is what runs the field initializer.
+      fields: "at new Fields (<fixture>:4:8)",
+      later: "at new Later (<fixture>:6:5)",
+      // On the first line of a function body, JSC's own column for the class counts from the
+      // function rather than from the line (it would give 9:6).
+      sameLine: "at new SameLine (<fixture>:9:23)",
+      anon: "at new Anon (<fixture>:10:14)",
+      // The default constructor of `Nullary` is the top frame: `super()` is what throws.
+      nullary: { sourceURL: "<fixture>", line: 11, column: 1 },
+    });
+    expect(exitCode).toBe(0);
+  });
+
+  test.concurrent("the class position is source mapped back to the original file", async () => {
+    using dir = tempDir("default-ctor-stack-sourcemap", {
+      // The interface and the type alias are removed by the transpiler, so the positions JSC reports
+      // differ from the ones in this file.
+      "fixture.ts": `interface Shape {
+  width: number;
+}
+class Thrower {
+  err: Error;
+  constructor() {
+    this.err = new Error("T");
+  }
+}
+export class Derived extends Thrower {}
+function later(): Error {
+  type Unused = Shape;
+  class Later extends Thrower {}
+  return new Later().err;
+}
+const norm = (s: string | undefined) => String(s).replaceAll(import.meta.path, "<fixture>");
+const frame = (err: Error, name: string) =>
+  norm(err.stack!.split("\\n").map(l => l.trim()).find(l => l.startsWith("at new " + name + " ")));
+console.log(JSON.stringify({ derived: frame(new Derived().err, "Derived"), later: frame(later(), "Later") }));
+`,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "fixture.ts"],
+      env: bunEnv,
+      cwd: String(dir),
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({
+      derived: "at new Derived (<fixture>:10:8)",
+      later: "at new Later (<fixture>:13:3)",
+    });
+    expect(exitCode).toBe(0);
+  });
+
+  test.concurrent("an uncaught error thrown by the default constructor is printed at the class", async () => {
+    using dir = tempDir("default-ctor-stack-uncaught", {
+      "fixture.js": "class Nullary extends null {}\nnew Nullary();\n",
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "fixture.js"],
+      env: bunEnv,
+      cwd: String(dir),
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(stdout).toBe("");
+    expect(stderr).toContain("1 | class Nullary extends null {}\n    ^\n");
+    expect(stderr).toMatch(/^\s+at new Nullary \(.*fixture\.js:1:1\)/m);
+    expect(exitCode).toBe(1);
+  });
+
+  test("node:vm lineOffset and columnOffset are applied to the class position", () => {
+    class Thrower {
+      err: Error;
+      constructor() {
+        this.err = new Error("T");
+      }
+    }
+    const context = vm.createContext({ Thrower });
+    const run = (code: string, options: vm.ScriptOptions) =>
+      new vm.Script(code, options).runInContext(context) as Error;
+    const frame = (err: Error, name: string) =>
+      err
+        .stack!.split("\n")
+        .map(line => line.trim())
+        .find(line => line.startsWith(`at new ${name} `));
+    const offsets = { filename: "offsets.vm.js", lineOffset: 10, columnOffset: 5 };
+
+    // The expected positions are what node prints for the same scripts.
+    expect({
+      firstLine: frame(run("class First extends Thrower {} new First().err;", offsets), "First"),
+      laterLine: frame(run("\n\n    class Later extends Thrower {}\nnew Later().err;", offsets), "Later"),
+      negativeColumn: frame(
+        run("         class NegCol extends Thrower {} new NegCol().err;", { filename: "neg.vm.js", columnOffset: -3 }),
+        "NegCol",
+      ),
+      negativeLine: frame(
+        run("\n\n\n  class NegLine extends Thrower {}\n  new NegLine().err;", {
+          filename: "neg.vm.js",
+          lineOffset: -2,
+        }),
+        "NegLine",
+      ),
+    }).toEqual({
+      // The column offset only applies to the first line.
+      firstLine: "at new First (offsets.vm.js:11:6)",
+      laterLine: "at new Later (offsets.vm.js:13:5)",
+      // JSC clamps negative offsets away for the positions it reports itself.
+      negativeColumn: "at new NegCol (neg.vm.js:1:7)",
+      negativeLine: "at new NegLine (neg.vm.js:2:3)",
+    });
+  });
+});