Fix Error.prepareStackTrace custom getter not being invoked

When Error.prepareStackTrace is defined with Object.defineProperty
using a custom getter, the getter was not being invoked when formatting
stack traces. This broke modules like error-callsites and elastic-apm-node.

Root cause: Bun was directly accessing cached values instead of checking
if the property had a custom accessor descriptor.

Fix: Check if the property is an accessor (slot.isAccessor()) before
calling getIfPropertyExists(). This ensures:
- Custom getters (Object.defineProperty) are properly invoked
- Data descriptors continue to use fast cached path
- No performance regression for normal error handling

Added RAII-style cleanup using WTF::makeScopeExit for recursion guard.

Fixes #23493

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

src/bun.js/bindings/ZigGlobalObject.cpp

@@ -430,13 +430,33 @@ static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalO
 static JSValue formatStackTraceToJSValueWithoutPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites)
 {
     JSValue prepareStackTrace = {};
+
+    // For Zig::GlobalObject, first try the cached value for performance
     if (lexicalGlobalObject->inherits<Zig::GlobalObject>()) {
         if (auto prepare = globalObject->m_errorConstructorPrepareStackTraceValue.get()) {
             prepareStackTrace = prepare;
+        } else if (!globalObject->isInsideErrorPrepareStackTraceCallback) {
+            // If no cached value, check if user code replaced the property with Object.defineProperty
+            auto scope = DECLARE_CATCH_SCOPE(vm);
+            auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(globalObject);
+            auto prepareStackTraceIdentifier = Identifier::fromString(vm, "prepareStackTrace"_s);
+
+            // Check if the property has a custom getter (defined via Object.defineProperty)
+            PropertySlot slot(errorConstructor, PropertySlot::InternalMethodType::GetOwnProperty);
+            if (errorConstructor->getOwnPropertySlot(errorConstructor, lexicalGlobalObject, prepareStackTraceIdentifier, slot)) {
+                if (slot.isAccessor()) {
+                    // Property has a custom getter (set via Object.defineProperty), invoke it
+                    globalObject->isInsideErrorPrepareStackTraceCallback = true;
+                    auto resetFlag = WTF::makeScopeExit([&] {
+                        globalObject->isInsideErrorPrepareStackTraceCallback = false;
+                    });
+                    prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, prepareStackTraceIdentifier);
+                }
+            }
+            CLEAR_IF_EXCEPTION(scope);
         }
     } else {
         auto scope = DECLARE_CATCH_SCOPE(vm);
-
         auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(globalObject);
         prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, JSC::Identifier::fromString(vm, "prepareStackTrace"_s));
         CLEAR_IF_EXCEPTION(scope);
@@ -793,28 +813,49 @@ static JSValue computeErrorInfoToJSValueWithoutSkipping(JSC::VM& vm, Vector<Stac
     if (!globalObject) {
         // node:vm will use a different JSGlobalObject
         globalObject = defaultGlobalObject();
-        if (!globalObject->isInsideErrorPrepareStackTraceCallback) {
-            auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(lexicalGlobalObject);
-            auto prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "prepareStackTrace"_s));
-            RETURN_IF_EXCEPTION(scope, {});
-            if (prepareStackTrace) {
-                if (prepareStackTrace.isCell() && prepareStackTrace.isObject() && prepareStackTrace.isCallable()) {
-                    globalObject->isInsideErrorPrepareStackTraceCallback = true;
-                    auto result = computeErrorInfoWithPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance, prepareStackTrace.getObject());
-                    globalObject->isInsideErrorPrepareStackTraceCallback = false;
-                    RELEASE_AND_RETURN(scope, result);
+    }
+
+    if (!globalObject->isInsideErrorPrepareStackTraceCallback) {
+        JSValue prepareStackTrace = {};
+
+        // For Zig::GlobalObject, first try the cached value for performance
+        if (lexicalGlobalObject->inherits<Zig::GlobalObject>()) {
+            if (auto prepare = globalObject->m_errorConstructorPrepareStackTraceValue.get()) {
+                prepareStackTrace = prepare;
+            } else {
+                // If no cached value, check if user code replaced the property with Object.defineProperty
+                auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(globalObject);
+                auto prepareStackTraceIdentifier = Identifier::fromString(vm, "prepareStackTrace"_s);
+
+                // Check if the property has a custom getter (defined via Object.defineProperty)
+                PropertySlot slot(errorConstructor, PropertySlot::InternalMethodType::GetOwnProperty);
+                if (errorConstructor->getOwnPropertySlot(errorConstructor, lexicalGlobalObject, prepareStackTraceIdentifier, slot)) {
+                    if (slot.isAccessor()) {
+                        // Property has a custom getter (set via Object.defineProperty), invoke it
+                        globalObject->isInsideErrorPrepareStackTraceCallback = true;
+                        auto resetFlag = WTF::makeScopeExit([&] {
+                            globalObject->isInsideErrorPrepareStackTraceCallback = false;
+                        });
+                        prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, prepareStackTraceIdentifier);
+                        RETURN_IF_EXCEPTION(scope, {});
+                    }
                 }
             }
+        } else {
+            auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(lexicalGlobalObject);
+            // Non-Zig globals: still read via accessor if present
+            prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "prepareStackTrace"_s));
+            RETURN_IF_EXCEPTION(scope, {});
         }
-    } else if (!globalObject->isInsideErrorPrepareStackTraceCallback) {
-        if (JSValue prepareStackTrace = globalObject->m_errorConstructorPrepareStackTraceValue.get()) {
-            if (prepareStackTrace) {
-                if (prepareStackTrace.isCallable()) {
-                    globalObject->isInsideErrorPrepareStackTraceCallback = true;
-                    auto result = computeErrorInfoWithPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance, prepareStackTrace.getObject());
+
+        if (prepareStackTrace) {
+            if (prepareStackTrace.isCell() && prepareStackTrace.isObject() && prepareStackTrace.isCallable()) {
+                globalObject->isInsideErrorPrepareStackTraceCallback = true;
+                auto resetFlag = WTF::makeScopeExit([&] {
                     globalObject->isInsideErrorPrepareStackTraceCallback = false;
-                    RELEASE_AND_RETURN(scope, result);
-                }
+                });
+                auto result = computeErrorInfoWithPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance, prepareStackTrace.getObject());
+                RELEASE_AND_RETURN(scope, result);
             }
         }
     }

test/regression/issue/23493-error-preparestacktrace-getter.test.ts

@@ -0,0 +1,120 @@
+// https://github.com/oven-sh/bun/issues/23493
+// When Error.prepareStackTrace is defined with Object.defineProperty using a getter/setter,
+// the getter must be called when formatting stack traces
+
+import { expect, test } from "bun:test";
+
+test("Error.prepareStackTrace getter is called when defined with Object.defineProperty", () => {
+  const originalDescriptor = Object.getOwnPropertyDescriptor(Error, "prepareStackTrace");
+  let getterCallCount = 0;
+  let prepareCallCount = 0;
+
+  const myPrepareStackTrace = function (err: Error, callsites: any[]) {
+    prepareCallCount++;
+    return "Custom stack trace";
+  };
+
+  // This is how error-callsites and similar modules define prepareStackTrace
+  Object.defineProperty(Error, "prepareStackTrace", {
+    configurable: true,
+    enumerable: true,
+    get: function () {
+      getterCallCount++;
+      return myPrepareStackTrace;
+    },
+    set: function (fn: any) {
+      // setter not used in this test
+    },
+  });
+
+  try {
+    const err = new Error("test error");
+    const stack = err.stack;
+
+    // The getter should be called at least once when formatting the stack
+    // The prepare function should also be called
+    expect(getterCallCount).toBeGreaterThan(0);
+    expect(prepareCallCount).toBeGreaterThanOrEqual(1);
+    expect(stack).toBe("Custom stack trace");
+  } finally {
+    // Restore original descriptor
+    if (originalDescriptor) {
+      Object.defineProperty(Error, "prepareStackTrace", originalDescriptor);
+    } else {
+      delete (Error as any).prepareStackTrace;
+    }
+  }
+});
+
+test("error-callsites module compatibility", () => {
+  const originalDescriptor = Object.getOwnPropertyDescriptor(Error, "prepareStackTrace");
+
+  // Simulate what error-callsites does
+  const callsitesSym = Symbol("callsites");
+
+  const fallback =
+    (Error as any).prepareStackTrace ||
+    function (err: Error, callsites: any[]) {
+      return err.stack;
+    };
+
+  let lastPrepareStackTrace = fallback;
+
+  Object.defineProperty(Error, "prepareStackTrace", {
+    configurable: true,
+    enumerable: true,
+    get: function () {
+      return csPrepareStackTrace;
+    },
+    set: function (fn: any) {
+      if (fn === csPrepareStackTrace || fn === undefined) {
+        lastPrepareStackTrace = fallback;
+      } else {
+        lastPrepareStackTrace = fn;
+      }
+    },
+  });
+
+  function csPrepareStackTrace(err: any, callsites: any[]) {
+    if (Object.prototype.hasOwnProperty.call(err, callsitesSym)) {
+      return fallback(err, callsites);
+    }
+
+    Object.defineProperty(err, callsitesSym, {
+      enumerable: false,
+      configurable: true,
+      writable: false,
+      value: callsites,
+    });
+
+    return lastPrepareStackTrace(err, callsites);
+  }
+
+  function errorCallsites(err: any) {
+    err.stack; // eslint-disable-line no-unused-expressions
+    return err[callsitesSym];
+  }
+
+  try {
+    const err = new Error("test error");
+    const callsites = errorCallsites(err);
+
+    // Should return an array of callsites, not undefined
+    expect(callsites).toBeDefined();
+    expect(Array.isArray(callsites)).toBe(true);
+    expect(callsites.length).toBeGreaterThan(0);
+
+    // Verify callsites have expected properties
+    const firstCallsite = callsites[0];
+    expect(firstCallsite).toBeDefined();
+    expect(typeof firstCallsite.getFileName).toBe("function");
+    expect(typeof firstCallsite.getLineNumber).toBe("function");
+  } finally {
+    // Restore original descriptor
+    if (originalDescriptor) {
+      Object.defineProperty(Error, "prepareStackTrace", originalDescriptor);
+    } else {
+      delete (Error as any).prepareStackTrace;
+    }
+  }
+});