error: report default class constructor frames at the class definition

src/jsc/bindings/ErrorStackFrame.cpp

@@ -1,13 +1,70 @@
 #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();
+
+    // classSource.startColumn() is counted from wherever the lexer started, which for a class on
+    // the first line of a lazily parsed function body is that function, not the line. Count from
+    // the line start instead; the column is only taken as-is when the source text is unavailable.
+    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;
+        // JSC's columns on the first line of a source include its start column (node:vm columnOffset).
+        if (lineStart == 0)
+            column += provider->startPosition().m_column.zeroBasedInt();
+    }
+
+    return ZigStackFramePosition {
+        .line_zero_based = classSource.firstLine().zeroBasedInt(),
+        .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 +121,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,33 @@
+#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 {
 
+/// JSC compiles the constructor of a class that declares none from
+/// BuiltinExecutables::defaultConstructorSourceCode(), a fixed one-line string with no URL, so that
+/// executable's source() and bytecode positions only describe that string. For such an executable
+/// this returns the SourceCode of the class itself (its file, starting at the `class` keyword), which
+/// is where V8 reports these frames. Null for every other executable.
+JSC::SourceCode defaultClassConstructorClassSource(JSC::ScriptExecutable* executable);
+
+/// Position of the `class` keyword a class SourceCode starts at, in the same coordinates JSC reports
+/// bytecode positions of that file in.
+ZigStackFramePosition classSourceStartPosition(const JSC::SourceCode& classSource);
+
 ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC::BytecodeIndex bc);
 
+/// frame.computeLineAndColumn(), except that a default class constructor frame is placed at its 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,10 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
     if (!code)
         return;
 
-    auto* provider = code->source().provider();
+    // The position below is in the class's source for a default class constructor, so the
+    // source lines have to come from it as well.
+    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 +149,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,6 +1,6 @@
 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";
 
 test("name property is used for function calls in Error.stack", () => {
@@ -149,3 +149,122 @@
         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 [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+
+    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/js/node/v8/capture-stack-trace.test.js

@@ -535,6 +535,64 @@ test("CallFrame.p.isConstructor", () => {
   Error.prepareStackTrace = prevPrepareStackTrace;
 });
 
+test("CallSites of a class without a constructor point at the class definition", () => {
+  Error.prepareStackTrace = (err, callSites) => callSites;
+
+  let fromBase;
+  class Base {
+    constructor() {
+      fromBase = new Error();
+    }
+  }
+  // JSC compiles the constructor of a class that declares none from an internal one-line source, and
+  // its frames used to report that source: no file, line 1. V8 reports the line of the `class`
+  // keyword, which is also where each `marked(Base)` below is called from.
+  const markers = [];
+  function marked(Super) {
+    markers.push(new Error());
+    return Super;
+  }
+  class Derived extends marked(Base) {}
+  function makeInner() {
+    class Inner extends marked(Base) {}
+    new Inner();
+  }
+
+  new Derived();
+  const derivedError = fromBase;
+  makeInner();
+  const innerError = fromBase;
+  // In the markers, [0] is `marked` and [1] the `extends` clause; in the errors from Base, [0] is
+  // `new Base` and [1] the synthesized constructor of the subclass.
+  const [derivedClassSite, innerClassSite] = markers.map(marker => marker.stack[1]);
+  const derivedSite = derivedError.stack[1];
+  const innerSite = innerError.stack[1];
+  Error.prepareStackTrace = origPrepareStackTrace;
+
+  const describeSite = site => ({
+    functionName: site.getFunctionName(),
+    isConstructor: site.isConstructor(),
+    fileName: site.getFileName(),
+    lineNumber: site.getLineNumber(),
+    scriptId: site.getScriptId(),
+  });
+  const expectedAt = (classSite, functionName) => ({
+    functionName,
+    isConstructor: true,
+    fileName: classSite.getFileName(),
+    lineNumber: classSite.getLineNumber(),
+    scriptId: classSite.getScriptId(),
+  });
+
+  expect(derivedClassSite.getFileName()).toBe(import.meta.path);
+  expect(innerClassSite.getLineNumber()).not.toBe(derivedClassSite.getLineNumber());
+  expect(describeSite(derivedSite)).toEqual(expectedAt(derivedClassSite, "Derived"));
+  expect(describeSite(innerSite)).toEqual(expectedAt(innerClassSite, "Inner"));
+  // The `class` keyword comes before the `extends` clause on each line.
+  expect(derivedSite.getColumnNumber()).toBeLessThan(derivedClassSite.getColumnNumber());
+  expect(innerSite.getColumnNumber()).toBeLessThan(innerClassSite.getColumnNumber());
+});
+
 test("CallFrame.p.isNative", () => {
   let prevPrepareStackTrace = Error.prepareStackTrace;
   Error.prepareStackTrace = (e, s) => {