implement more of V8's stack trace API (#3359)

- fix source map positions for getLineNumber / getColumnNumber
- fix return value getting coerced to a string
- implement CallFrame.p.toString
- add tests for getFunction, getThis, isConstructor, isNative, toString,
  getLineNumber, getColumnNumber

still not implemented:
- isPromiseAll/getPromiseIndex
- getEvalOrigin
- getScriptHash
- getPosition
- getEnclosingColumnNumber/getEnclosingLineNumber
- isAsync
- accessing Error.stack should call prepareStackTrace

still broken:
- isEval: often returns false when it should return true
- isToplevel: often returns true when it should return false

Refs: https://v8.dev/docs/stack-trace-api
Refs: v8/src/objects/call-site-info.cc
Fixes: https://github.com/oven-sh/bun/issues/2883

8 files changed, 232 insertions, 39 deletions

diff --git a/src/bun.js/bindings/CallSite.cpp b/src/bun.js/bindings/CallSite.cpp
index 02ac35168..e4b2fc3a7 100644
--- a/src/bun.js/bindings/CallSite.cpp
+++ b/src/bun.js/bindings/CallSite.cpp
@@ -86,6 +86,42 @@ void CallSite::visitChildrenImpl(JSCell* cell, Visitor& visitor)
     visitor.append(thisCallSite->m_sourceURL);
 }
 
+void CallSite::formatAsString(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF::StringBuilder &sb) {
+    JSString* myTypeName = jsTypeStringForValue(globalObject, thisValue());
+    JSString* myFunction = functionName().toString(globalObject);
+    JSString* myFunctionName = functionName().toString(globalObject);
+    JSString* mySourceURL = sourceURL().toString(globalObject);
+
+    JSString* myColumnNumber = columnNumber().toInt32(globalObject) != -1 ? columnNumber().toString(globalObject) : jsEmptyString(vm);
+    JSString* myLineNumber = lineNumber().toInt32(globalObject) != -1 ? lineNumber().toString(globalObject) : jsEmptyString(vm);
+
+    bool myIsConstructor = isConstructor();
+
+    if (myFunctionName->length() > 0) {
+        if (myIsConstructor) {
+            sb.append("new "_s);
+        } else {
+            // TODO: print type or class name if available
+            // sb.append(myTypeName->getString(globalObject));
+            // sb.append(" "_s);
+        }
+        sb.append(myFunctionName->getString(globalObject));
+    } else {
+        sb.append("<anonymous>"_s);
+    }
+    sb.append(" ("_s);
+    if (isNative()) {
+        sb.append("native"_s);
+    } else {
+        sb.append(mySourceURL->getString(globalObject));
+        sb.append(":"_s);
+        sb.append(myLineNumber->getString(globalObject));
+        sb.append(":"_s);
+        sb.append(myColumnNumber->getString(globalObject));
+    }
+    sb.append(")"_s);
+}
+
 DEFINE_VISIT_CHILDREN(CallSite);
 
 }
diff --git a/src/bun.js/bindings/CallSite.h b/src/bun.js/bindings/CallSite.h
index 432725b8a..8780eb8fb 100644
--- a/src/bun.js/bindings/CallSite.h
+++ b/src/bun.js/bindings/CallSite.h
@@ -77,6 +77,11 @@ public:
     bool isStrict() const { return m_flags & static_cast<unsigned int>(Flags::IsStrict); }
     bool isNative() const { return m_flags & static_cast<unsigned int>(Flags::IsNative); }
 
+    void setLineNumber(JSC::JSValue lineNumber) { m_lineNumber = lineNumber; }
+    void setColumnNumber(JSC::JSValue columnNumber) { m_columnNumber = columnNumber; }
+
+    void formatAsString(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF::StringBuilder &sb);
+
 private:
     CallSite(JSC::VM& vm, JSC::Structure* structure)
         : Base(vm, structure)
diff --git a/src/bun.js/bindings/CallSitePrototype.cpp b/src/bun.js/bindings/CallSitePrototype.cpp
index f3e365cb0..1ea1a2c86 100644
--- a/src/bun.js/bindings/CallSitePrototype.cpp
+++ b/src/bun.js/bindings/CallSitePrototype.cpp
@@ -35,6 +35,7 @@ static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsConstructor);
 static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsAsync);
 static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsPromiseAll);
 static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetPromiseIndex);
+static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncToString);
 
 ALWAYS_INLINE static CallSite* getCallSite(JSGlobalObject* globalObject, JSC::JSValue thisValue)
 {
@@ -82,6 +83,7 @@ static const HashTableValue CallSitePrototypeTableValues[]
           { "isAsync"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncIsAsync, 0 } },
           { "isPromiseAll"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncIsPromiseAll, 0 } },
           { "getPromiseIndex"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncGetPromiseIndex, 0 } },
+          { "toString"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncToString, 0 } },
       };
 
 const JSC::ClassInfo CallSitePrototype::s_info = { "CallSite"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(CallSitePrototype) };
@@ -222,7 +224,15 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetPromiseIndex, (JSGlobalObject * glo
 {
     ENTER_PROTO_FUNC();
 
-    return JSC::JSValue::encode(JSC::jsNumber(0));
+    return JSC::JSValue::encode(JSC::jsNull());
 }
 
-}
\ No newline at end of file
+JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncToString, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
+{
+    ENTER_PROTO_FUNC();
+    WTF::StringBuilder sb;
+    callSite->formatAsString(vm, globalObject, sb);
+    return JSC::JSValue::encode(JSC::JSValue(jsString(vm, sb.toString())));
+}
+
+}
diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp
index ec4976bc4..9b3bfd2a2 100644
--- a/src/bun.js/bindings/ZigGlobalObject.cpp
+++ b/src/bun.js/bindings/ZigGlobalObject.cpp
@@ -2502,7 +2502,7 @@ void GlobalObject::createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalO
     }
 }
 
-JSC::JSValue GlobalObject::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites, ZigStackFrame remappedStackFrames[])
+JSC::JSValue GlobalObject::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites)
 {
     auto scope = DECLARE_THROW_SCOPE(vm);
     JSValue errorValue = this->get(this, JSC::Identifier::fromString(vm, "Error"_s));
@@ -2557,39 +2557,8 @@ JSC::JSValue GlobalObject::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* le
     for (size_t i = 0; i < framesCount; i++) {
         JSC::JSValue callSiteValue = callSites->getIndex(lexicalGlobalObject, i);
         CallSite* callSite = JSC::jsDynamicCast<CallSite*>(callSiteValue);
-
-        JSString* typeName = jsTypeStringForValue(lexicalGlobalObject, callSite->thisValue());
-        JSString* function = callSite->functionName().toString(lexicalGlobalObject);
-        JSString* functionName = callSite->functionName().toString(lexicalGlobalObject);
-        JSString* sourceURL = callSite->sourceURL().toString(lexicalGlobalObject);
-        JSString* columnNumber = remappedStackFrames[i].position.column_start >= 0 ? jsNontrivialString(vm, String::number(remappedStackFrames[i].position.column_start)) : jsEmptyString(vm);
-        JSString* lineNumber = remappedStackFrames[i].position.line >= 0 ? jsNontrivialString(vm, String::number(remappedStackFrames[i].position.line)) : jsEmptyString(vm);
-        bool isConstructor = callSite->isConstructor();
-
         sb.append("    at "_s);
-        if (functionName->length() > 0) {
-            if (isConstructor) {
-                sb.append("new "_s);
-            } else {
-                // TODO: print type or class name if available
-                // sb.append(typeName->getString(lexicalGlobalObject));
-                // sb.append(" "_s);
-            }
-            sb.append(functionName->getString(lexicalGlobalObject));
-        } else {
-            sb.append("<anonymous>"_s);
-        }
-        sb.append(" ("_s);
-        if (callSite->isNative()) {
-            sb.append("native"_s);
-        } else {
-            sb.append(sourceURL->getString(lexicalGlobalObject));
-            sb.append(":"_s);
-            sb.append(lineNumber->getString(lexicalGlobalObject));
-            sb.append(":"_s);
-            sb.append(columnNumber->getString(lexicalGlobalObject));
-        }
-        sb.append(")"_s);
+        callSite->formatAsString(vm, lexicalGlobalObject, sb);
         if (i != framesCount - 1) {
             sb.append("\n"_s);
         }
@@ -2598,7 +2567,6 @@ JSC::JSValue GlobalObject::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* le
     return JSC::JSValue(jsString(vm, sb.toString()));
 }
 
-extern "C" void Bun__remapStackFramePositions(JSC::JSGlobalObject*, ZigStackFrame*, size_t);
 extern "C" EncodedJSValue JSPasswordObject__create(JSC::JSGlobalObject*, bool);
 
 JSC_DECLARE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace);
@@ -2662,9 +2630,26 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb
     }
 
     // remap line and column start to original source
+    // XXX: this function does not fully populate the fields of ZigStackFrame,
+    // be careful reading the fields below.
     Bun__remapStackFramePositions(lexicalGlobalObject, remappedFrames, framesCount);
 
-    JSC::JSValue formattedStackTrace = globalObject->formatStackTrace(vm, lexicalGlobalObject, errorObject, callSites, remappedFrames);
+    // write the remapped lines back to the CallSites
+    for (size_t i = 0; i < framesCount; i++) {
+        JSC::JSValue callSiteValue = callSites->getIndex(lexicalGlobalObject, i);
+        CallSite* callSite = JSC::jsDynamicCast<CallSite*>(callSiteValue);
+        if (remappedFrames[i].remapped) {
+            int32_t remappedColumnStart = remappedFrames[i].position.column_start;
+            JSC::JSValue columnNumber = JSC::jsNumber(remappedColumnStart);
+            callSite->setColumnNumber(columnNumber);
+
+            int32_t remappedLine = remappedFrames[i].position.line;
+            JSC::JSValue lineNumber = JSC::jsNumber(remappedLine);
+            callSite->setLineNumber(lineNumber);
+        }
+    }
+
+    JSC::JSValue formattedStackTrace = globalObject->formatStackTrace(vm, lexicalGlobalObject, errorObject, callSites);
     RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode({}));
 
     if (errorObject->hasProperty(lexicalGlobalObject, vm.propertyNames->stack)) {
@@ -2673,7 +2658,7 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb
     if (formattedStackTrace.isUndefinedOrNull()) {
         errorObject->putDirect(vm, vm.propertyNames->stack, jsUndefined(), JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum);
     } else {
-        errorObject->putDirect(vm, vm.propertyNames->stack, formattedStackTrace.toString(lexicalGlobalObject), JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum);
+        errorObject->putDirect(vm, vm.propertyNames->stack, formattedStackTrace, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum);
     }
 
     RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode(JSValue {}));
diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h
index dda1c8330..2d69e764f 100644
--- a/src/bun.js/bindings/ZigGlobalObject.h
+++ b/src/bun.js/bindings/ZigGlobalObject.h
@@ -167,7 +167,7 @@ public:
     void clearDOMGuardedObjects();
 
     static void createCallSitesFromFrames(JSC::JSGlobalObject* lexicalGlobalObject, JSC::ObjectInitializationScope& objectScope, JSCStackTrace& stackTrace, JSC::JSArray* callSites);
-    JSC::JSValue formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites, ZigStackFrame remappedStackFrames[]);
+    JSC::JSValue formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites);
 
     static void reportUncaughtExceptionAtEventLoop(JSGlobalObject*, JSC::Exception*);
     static JSGlobalObject* deriveShadowRealmGlobalObject(JSGlobalObject* globalObject);
diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h
index b845c4e00..a4287cf2e 100644
--- a/src/bun.js/bindings/headers-handwritten.h
+++ b/src/bun.js/bindings/headers-handwritten.h
@@ -343,6 +343,8 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSC::JSValue v1, JSC::JS
 
 bool Bun__deepMatch(JSC::JSValue object, JSC::JSValue subset, JSC::JSGlobalObject* globalObject, JSC::ThrowScope* throwScope, bool replacePropsWithAsymmetricMatchers);
 
+extern "C" void Bun__remapStackFramePositions(JSC::JSGlobalObject*, ZigStackFrame*, size_t);
+
 namespace Inspector {
 class ScriptArguments;
 }
diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig
index 63d024ad7..d458b6e7e 100644
--- a/src/bun.js/javascript.zig
+++ b/src/bun.js/javascript.zig
@@ -1983,6 +1983,9 @@ pub const VirtualMachine = struct {
             )) |mapping| {
                 frames[i].position.line = mapping.original.lines;
                 frames[i].position.column_start = mapping.original.columns;
+                frames[i].remapped = true;
+            } else {
+                frames[i].remapped = true;
             }
         }
     }
diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js
index 789503960..75947a001 100644
--- a/test/js/node/v8/capture-stack-trace.test.js
+++ b/test/js/node/v8/capture-stack-trace.test.js
@@ -301,3 +301,155 @@ test("prepare stack trace call sites", () => {
 
   f1();
 });
+
+test("sanity check", () => {
+  function f1() {
+    f2();
+  }
+
+  function f2() {
+    f3();
+  }
+
+  function f3() {
+    let e = new Error("bad error!");
+    let prevPrepareStackTrace = Error.prepareStackTrace;
+    Error.prepareStackTrace = (e, s) => {
+      // getThis returns undefined in strict mode
+      expect(s[0].getThis()).toBe(undefined);
+      expect(s[0].getTypeName()).toBe('undefined');
+      // getFunction returns undefined in strict mode
+      expect(s[0].getFunction()).toBe(undefined);
+      expect(s[0].getFunctionName()).toBe('f3');
+      expect(s[0].getMethodName()).toBe('f3');
+      expect(typeof s[0].getLineNumber()).toBe('number');
+      expect(typeof s[0].getColumnNumber()).toBe('number');
+      expect(s[0].getFileName().includes('capture-stack-trace.test.js')).toBe(true);
+
+      expect(s[0].getEvalOrigin()).toBe(undefined);
+      expect(s[0].isToplevel()).toBe(true);
+      expect(s[0].isEval()).toBe(false);
+      expect(s[0].isNative()).toBe(false);
+      expect(s[0].isConstructor()).toBe(false);
+      expect(s[0].isAsync()).toBe(false);
+      expect(s[0].isPromiseAll()).toBe(false);
+      expect(s[0].getPromiseIndex()).toBe(null);
+
+    };
+    Error.captureStackTrace(e);
+    expect(e.stack === undefined).toBe(true);
+    Error.prepareStackTrace = prevPrepareStackTrace;
+  }
+
+  f1();
+});
+
+test("CallFrame.p.getThis\getFunction: works in sloppy mode", () => {
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+  const sloppyFn = new Function('let e=new Error();Error.captureStackTrace(e);return e.stack');
+  sloppyFn.displayName = 'sloppyFnWow';
+  const that = {};
+
+  Error.prepareStackTrace = (e, s) => {
+    expect(s[0].getThis()).toBe(that);
+    expect(s[0].getFunction()).toBe(sloppyFn);
+    expect(s[0].getFunctionName()).toBe(sloppyFn.displayName);
+    expect(s[0].isToplevel()).toBe(false);
+    // TODO: This should be true.
+    expect(s[0].isEval()).toBe(false);
+
+    // Strict-mode functions shouldn't have getThis or getFunction
+    // available.
+    expect(s[1].getThis()).toBe(undefined);
+    expect(s[1].getFunction()).toBe(undefined);
+  };
+
+  sloppyFn.call(that);
+
+  Error.prepareStackTrace = prevPrepareStackTrace;
+});
+
+test("CallFrame.p.getThis\getFunction: strict/sloppy mode interaction", () => {
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+
+  const strictFn = new Function('"use strict";let e=new Error();Error.captureStackTrace(e);return e.stack');
+  const sloppyFn = new Function('x', 'x()');
+  const that = {};
+
+  Error.prepareStackTrace = (e, s) => {
+    // The first strict mode function encounted during stack unwinding
+    // stops subsequent frames from having getThis\getFunction.
+    for (const t of s) {
+      expect(t.getThis()).toBe(undefined);
+      expect(t.getFunction()).toBe(undefined);
+    }
+  };
+
+  sloppyFn.call(that, strictFn);
+
+  Error.prepareStackTrace = prevPrepareStackTrace;
+});
+
+test("CallFrame.p.isConstructor", () => {
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+
+  class C {
+    constructor() {
+      Error.captureStackTrace(new Error(''));
+    }
+  }
+
+  Error.prepareStackTrace = (e, s) => {
+    expect(s[0].isConstructor()).toBe(true);
+    // TODO: should be false: this is an instance of C
+    expect(s[0].isToplevel()).toBe(true);
+    // TODO: should return the class name
+    // expect(s[0].getTypeName()).toBe('C');
+
+    expect(s[1].isConstructor()).toBe(false);
+    expect(s[1].isToplevel()).toBe(true);
+  };
+  new C();
+  Error.prepareStackTrace = prevPrepareStackTrace;
+});
+
+test("CallFrame.p.isNative", () => {
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+  Error.prepareStackTrace = (e, s) => {
+    expect(s[0].isNative()).toBe(false);
+    expect(s[1].isNative()).toBe(true);
+  };
+  [1, 2].sort(() => {
+    Error.captureStackTrace(new Error(''));
+    return 0;
+  });
+  Error.prepareStackTrace = prevPrepareStackTrace;
+});
+
+test("return non-strings from Error.prepareStackTrace", () => {
+  // This behavior is allowed by V8 and used by the node-depd npm package.
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+  Error.prepareStackTrace = (e, s) => s;
+  const e = new Error();
+  Error.captureStackTrace(e);
+  expect(Array.isArray(e.stack)).toBe(true);
+  Error.prepareStackTrace = prevPrepareStackTrace;
+});
+
+test("CallFrame.p.toString", () => {
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+  Error.prepareStackTrace = (e, s) => s;
+  const e = new Error();
+  Error.captureStackTrace(e);
+  expect(e.stack[0].toString().includes("<anonymous>")).toBe(true);
+});
+
+test.todo("err.stack should invoke prepareStackTrace", () => {
+  // This is V8's behavior.
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+  let wasCalled = false;
+  Error.prepareStackTrace = (e, s) => { wasCalled = true; };
+  const e = new Error();
+  e.stack;
+  expect(wasCalled).toBe(true);
+});