DOMException: inherit from ErrorInstance for [[ErrorData]] and .stack

src/jsc/bindings/BunClientData.cpp

@@ -27,6 +27,7 @@
 #include "napi_handle_scope.h"
 #include "NativePromiseContext.h"
 #include "StrongRootBlock.h"
+#include "JSDOMException.h"
 
 namespace WebCore {
 using namespace JSC;
@@ -39,6 +40,7 @@ JSHeapData::JSHeapData(Heap& heap)
     , m_heapCellTypeForBakeGlobalObject(JSC::IsoHeapCellType::Args<Bake::GlobalObject>())
     , m_heapCellTypeForNapiHandleScopeImpl(JSC::IsoHeapCellType::Args<Bun::NapiHandleScopeImpl>())
     , m_heapCellTypeForNativePromiseContext(JSC::IsoHeapCellType::Args<Bun::NativePromiseContext>())
+    , m_heapCellTypeForJSDOMException(JSC::IsoHeapCellType::Args<WebCore::JSDOMException>())
     , m_domConstructorSpace ISO_SUBSPACE_INIT(heap, heap.cellHeapCellType, JSDOMConstructorBase)
     , m_domNamespaceObjectSpace ISO_SUBSPACE_INIT(heap, heap.cellHeapCellType, JSDOMObject)
     , m_subspaces(makeUnique<ExtendedDOMIsoSubspaces>())

src/jsc/bindings/BunClientData.h

@@ -97,6 +97,7 @@ class JSHeapData {
     JSC::IsoHeapCellType m_heapCellTypeForNapiHandleScopeImpl;
     JSC::IsoHeapCellType m_heapCellTypeForBakeGlobalObject;
     JSC::IsoHeapCellType m_heapCellTypeForNativePromiseContext;
+    JSC::IsoHeapCellType m_heapCellTypeForJSDOMException;
     // JSC::IsoHeapCellType m_heapCellTypeForGeneratedClass;
 
 private:

src/jsc/bindings/FormatStackTraceForJS.cpp

@@ -22,6 +22,7 @@
 #include "BunClientData.h"
 #include "CallSite.h"
 #include "ErrorStackTrace.h"
+#include "JSDOMException.h"
 #include "headers-handwritten.h"
 
 #include <wtf/Scope.h>
@@ -414,10 +415,15 @@ static String computeErrorInfoWithoutPrepareStackTrace(
             if (!lexicalGlobalObject) {
                 lexicalGlobalObject = errorInstance->globalObject();
             }
-            name = instance->sanitizedNameString(lexicalGlobalObject);
-            RETURN_IF_EXCEPTION(scope, {});
-            message = instance->sanitizedMessageString(lexicalGlobalObject);
-            RETURN_IF_EXCEPTION(scope, {});
+            if (auto* domException = dynamicDowncast<WebCore::JSDOMException>(instance)) {
+                name = domException->wrapped().name();
+                message = domException->wrapped().message();
+            } else {
+                name = instance->sanitizedNameString(lexicalGlobalObject);
+                RETURN_IF_EXCEPTION(scope, {});
+                message = instance->sanitizedMessageString(lexicalGlobalObject);
+                RETURN_IF_EXCEPTION(scope, {});
+            }
         }
     }
 
@@ -673,8 +679,12 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj
     }
 
     if (source->stackTrace()) {
-        destination->stackTrace()->appendVector(*source->stackTrace());
-        source->stackTrace()->clear();
+        // setStackFrames takes the cellLock that JSDOMException::visitChildren reads under.
+        WTF::Vector<JSC::StackFrame> combined;
+        combined.appendVector(*destination->stackTrace());
+        combined.appendVector(*source->stackTrace());
+        destination->setStackFrames(vm, WTF::move(combined));
+        source->setStackFrames(vm, {});
     }
 
     return JSC::JSValue::encode(jsUndefined());
@@ -724,7 +734,8 @@ JSC_DEFINE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter, (JSGlobalObject * g
         WTF::Vector<JSC::StackFrame> emptyTrace;
         result = computeErrorInfoToJSValue(vm, emptyTrace, line, column, sourceURL, errorObject, nullptr);
     } else {
-        auto ownedStackTrace = makeUnique<WTF::Vector<JSC::StackFrame>>(WTF::move(*stackTrace));
+        // Copy: a move here races JSDOMException::visitChildren reading under the cellLock.
+        auto ownedStackTrace = makeUnique<WTF::Vector<JSC::StackFrame>>(*stackTrace);
         JSC::MarkedArgumentBuffer protectedFrameCells;
         protectedFrameCells.ensureCapacity(ownedStackTrace->size() * 2);
         for (auto& frame : *ownedStackTrace) {

src/jsc/bindings/JSDOMExceptionHandling.cpp

@@ -103,7 +103,10 @@ String retrieveErrorMessage(JSGlobalObject& lexicalGlobalObject, VM& vm, JSValue
     // FIXME: <http://webkit.org/b/115087> Web Inspector: WebCore::reportException should not evaluate JavaScript handling exceptions
     // If this is a custom exception object, call toString on it to try and get a nice string representation for the exception.
     String errorMessage;
-    if (auto* error = dynamicDowncast<ErrorInstance>(exception))
+    if (auto* error = dynamicDowncast<JSDOMException>(exception)) {
+        auto& impl = error->wrapped();
+        errorMessage = impl.message().isEmpty() ? impl.name() : makeString(impl.name(), ": "_s, impl.message());
+    } else if (auto* error = dynamicDowncast<ErrorInstance>(exception))
         errorMessage = error->sanitizedToString(&lexicalGlobalObject);
     else
         errorMessage = exception.toWTFString(&lexicalGlobalObject);
@@ -184,7 +187,6 @@ JSValue createDOMException(JSGlobalObject* lexicalGlobalObject, ExceptionCode ec
         JSValue errorObject = toJS(lexicalGlobalObject, globalObject, DOMException::create(ec, message));
 
         ASSERT(errorObject);
-        addErrorInfo(lexicalGlobalObject, asObject(errorObject), true);
         return errorObject;
     }
     }

src/jsc/bindings/JSDOMWrapperCache.h

@@ -45,15 +45,15 @@ template<typename WrapperClass> JSC::JSObject* getDOMPrototype(JSC::VM&, JSDOMGl
 JSC::WeakHandleOwner* wrapperOwner(DOMWrapperWorld&, JSC::ArrayBuffer*);
 void* wrapperKey(JSC::ArrayBuffer*);
 
-std::optional<JSDOMObject*> getInlineCachedWrapper(DOMWrapperWorld&, void*);
+std::optional<JSC::JSObject*> getInlineCachedWrapper(DOMWrapperWorld&, void*);
 std::optional<JSDOMObject*> getInlineCachedWrapper(DOMWrapperWorld&, ScriptWrappable*);
 std::optional<JSC::JSArrayBuffer*> getInlineCachedWrapper(DOMWrapperWorld&, JSC::ArrayBuffer*);
 
-bool setInlineCachedWrapper(DOMWrapperWorld&, void*, JSDOMObject*, JSC::WeakHandleOwner*);
+bool setInlineCachedWrapper(DOMWrapperWorld&, void*, JSC::JSObject*, JSC::WeakHandleOwner*);
 bool setInlineCachedWrapper(DOMWrapperWorld&, ScriptWrappable*, JSDOMObject* wrapper, JSC::WeakHandleOwner* wrapperOwner);
 bool setInlineCachedWrapper(DOMWrapperWorld&, JSC::ArrayBuffer*, JSC::JSArrayBuffer* wrapper, JSC::WeakHandleOwner* wrapperOwner);
 
-bool clearInlineCachedWrapper(DOMWrapperWorld&, void*, JSDOMObject*);
+bool clearInlineCachedWrapper(DOMWrapperWorld&, void*, JSC::JSObject*);
 bool clearInlineCachedWrapper(DOMWrapperWorld&, ScriptWrappable*, JSDOMObject* wrapper);
 bool clearInlineCachedWrapper(DOMWrapperWorld&, JSC::ArrayBuffer*, JSC::JSArrayBuffer* wrapper);
 
@@ -98,9 +98,9 @@ inline void* wrapperKey(JSC::ArrayBuffer* domObject)
     return domObject;
 }
 
-inline std::optional<JSDOMObject*> getInlineCachedWrapper(DOMWrapperWorld&, void*) { return std::nullopt; }
-inline bool setInlineCachedWrapper(DOMWrapperWorld&, void*, JSDOMObject*, JSC::WeakHandleOwner*) { return false; }
-inline bool clearInlineCachedWrapper(DOMWrapperWorld&, void*, JSDOMObject*) { return false; }
+inline std::optional<JSC::JSObject*> getInlineCachedWrapper(DOMWrapperWorld&, void*) { return std::nullopt; }
+inline bool setInlineCachedWrapper(DOMWrapperWorld&, void*, JSC::JSObject*, JSC::WeakHandleOwner*) { return false; }
+inline bool clearInlineCachedWrapper(DOMWrapperWorld&, void*, JSC::JSObject*) { return false; }
 
 inline std::optional<JSDOMObject*> getInlineCachedWrapper(DOMWrapperWorld& world, ScriptWrappable* domObject)
 {

src/jsc/bindings/ZigException.cpp

@@ -34,6 +34,7 @@
 #include "ZigGlobalObject.h"
 #include "helpers.h"
 #include "JavaScriptCore/JSObjectInlines.h"
+#include "JSDOMException.h"
 
 #include "wtf/Assertions.h"
 #include "wtf/text/OrdinalNumber.h"
@@ -456,19 +457,17 @@ static void populateStackTrace(JSC::VM& vm, const WTF::Vector<JSC::StackFrame>&
 
 static JSC::JSValue getNonObservable(JSC::VM& vm, JSC::JSGlobalObject* global, JSC::JSObject* obj, const JSC::PropertyName& propertyName)
 {
+    auto scope = DECLARE_THROW_SCOPE(vm);
     PropertySlot slot = PropertySlot(obj, PropertySlot::InternalMethodType::VMInquiry, &vm);
-    if (obj->getNonIndexPropertySlot(global, propertyName, slot)) {
-        if (slot.isAccessor()) {
-            return {};
-        }
-
-        JSValue value = slot.getValue(global, propertyName);
-        if (!value || value.isUndefinedOrNull()) {
-            return {};
-        }
-        return value;
-    }
-    return {};
+    bool found = obj->getNonIndexPropertySlot(global, propertyName, slot);
+    RETURN_IF_EXCEPTION(scope, {});
+    // isValue() also rejects custom getters (DOMException.prototype.code), which isAccessor() does not.
+    if (!found || !slot.isValue())
+        return {};
+    JSValue value = slot.getValue(global, propertyName);
+    if (!value || value.isUndefinedOrNull())
+        return {};
+    return value;
 }
 
 static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global,
@@ -512,7 +511,10 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global,
         return;
     }
 
-    except.name = Bun::toStringRef(err->sanitizedNameString(global));
+    if (auto* domException = dynamicDowncast<WebCore::JSDOMException>(err))
+        except.name = Bun::toStringRef(domException->wrapped().name());
+    else
+        except.name = Bun::toStringRef(err->sanitizedNameString(global));
     if (!scope.clearExceptionExceptTermination()) [[unlikely]] {
         return;
     }

src/jsc/bindings/webcore/JSDOMException.cpp

@@ -38,12 +38,15 @@
 #include <JavaScriptCore/FunctionPrototype.h>
 #include <JavaScriptCore/HeapAnalyzer.h>
 
+#include <JavaScriptCore/JSCInlines.h>
 #include <JavaScriptCore/JSDestructibleObjectHeapCellType.h>
 #include <JavaScriptCore/SlotVisitorMacros.h>
+#include <JavaScriptCore/StackFrame.h>
 #include <JavaScriptCore/SubspaceInlines.h>
 #include <wtf/GetPtr.h>
 #include <wtf/PointerPreparations.h>
 #include <wtf/URL.h>
+#include <wtf/text/MakeString.h>
 
 namespace WebCore {
 using namespace JSC;
@@ -152,19 +155,14 @@
         RETURN_IF_EXCEPTION(throwScope, {});
     }
 
-    auto object = DOMException::create(WTF::move(message), WTF::move(name));
-    if constexpr (IsExceptionOr<decltype(object)>)
-        RETURN_IF_EXCEPTION(throwScope, {});
-    static_assert(TypeOrExceptionOrUnderlyingType<decltype(object)>::isRef);
-    auto jsValue = toJSNewlyCreated<IDLInterface<DOMException>>(*lexicalGlobalObject, *castedThis->globalObject(), throwScope, WTF::move(object));
-    if constexpr (IsExceptionOr<decltype(object)>)
-        RETURN_IF_EXCEPTION(throwScope, {});
-    setSubclassStructureIfNeeded<DOMException>(lexicalGlobalObject, callFrame, asObject(jsValue));
+    // Not toJSNewlyCreated: own properties must wait until after the subclass structure swap.
+    auto* wrapper = createWrapper<DOMException>(castedThis->globalObject(), DOMException::create(WTF::move(message), WTF::move(name)));
+    setSubclassStructureIfNeeded<DOMException>(lexicalGlobalObject, callFrame, wrapper);
     RETURN_IF_EXCEPTION(throwScope, {});
-    if (!cause.isEmpty()) {
-        jsValue.getObject()->putDirect(vm, vm.propertyNames->cause, cause, JSC::PropertyAttribute::DontEnum | 0);
-    }
-    return JSValue::encode(jsValue);
+    wrapper->putHeaderStackIfNoFrames(vm);
+    if (!cause.isEmpty())
+        wrapper->putDirect(vm, vm.propertyNames->cause, cause, JSC::PropertyAttribute::DontEnum | 0);
+    return JSValue::encode(wrapper);
 }
 JSC_ANNOTATE_HOST_FUNCTION(JSDOMExceptionDOMConstructorConstruct, JSDOMExceptionDOMConstructor::construct);
 
@@ -232,18 +230,54 @@
 const ClassInfo JSDOMException::s_info = { "DOMException"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDOMException) };
 
 JSDOMException::JSDOMException(Structure* structure, JSDOMGlobalObject& globalObject, Ref<DOMException>&& impl)
-    : JSDOMWrapper<DOMException>(structure, globalObject, WTF::move(impl))
+    : Base(globalObject.vm(), structure, JSC::ErrorType::Error)
+    , m_wrapped(WTF::move(impl))
 {
 }
 
 void JSDOMException::finishCreation(VM& vm)
 {
-    Base::finishCreation(vm);
+    // Null message/cause: those stay prototype accessors over the wrapped impl.
+    Base::finishCreation(vm, String(), JSValue(), nullptr, JSC::TypeNothing, true);
     ASSERT(inherits(info()));
+    // No own properties here: the constructor may still swap in a subclass structure.
+}
 
-    // static_assert(!std::is_base_of<ActiveDOMObject, DOMException>::value, "Interface is not marked as [ActiveDOMObject] even though implementation class subclasses ActiveDOMObject.");
+void JSDOMException::setStackString(VM& vm, String&& stack)
+{
+    putDirect(vm, vm.propertyNames->stack, jsString(vm, WTF::move(stack)), static_cast<unsigned>(JSC::PropertyAttribute::DontEnum));
+    setStackPropertyAlreadyMaterialized();
 }
 
+// ErrorInstance leaves .stack unset on an empty trace; other engines still give the name: message header.
+void JSDOMException::putHeaderStackIfNoFrames(VM& vm)
+{
+    auto* trace = stackTrace();
+    if (trace && !trace->isEmpty())
+        return;
+    auto& impl = wrapped();
+    auto name = impl.name();
+    auto message = impl.message();
+    setStackString(vm, message.isEmpty() ? name : makeString(name, ": "_s, message));
+}
+
+template<typename Visitor>
+void JSDOMException::visitChildrenImpl(JSCell* cell, Visitor& visitor)
+{
+    auto* thisObject = uncheckedDowncast<JSDOMException>(cell);
+    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
+    Base::visitChildren(thisObject, visitor);
+
+    // Heap sweeps dead frames only for vm.errorInstanceSpace(); this subspace must keep its own alive.
+    Locker locker { thisObject->cellLock() };
+    if (auto* stackTrace = thisObject->stackTrace()) {
+        for (auto& frame : *stackTrace)
+            frame.visitAggregate(visitor);
+    }
+}
+
+DEFINE_VISIT_CHILDREN(JSDOMException);
+
 JSObject* JSDOMException::createPrototype(VM& vm, JSDOMGlobalObject& globalObject)
 {
     return JSDOMExceptionPrototype::create(vm, &globalObject, JSDOMExceptionPrototype::createStructure(vm, &globalObject, globalObject.errorPrototype()));
@@ -316,12 +350,13 @@
 
 JSC::GCClient::IsoSubspace* JSDOMException::subspaceForImpl(JSC::VM& vm)
 {
-    return WebCore::subspaceForImpl<JSDOMException, UseCustomHeapCellType::No>(
+    return WebCore::subspaceForImpl<JSDOMException, UseCustomHeapCellType::Yes>(
         vm,
         [](auto& spaces) { return spaces.m_clientSubspaceForDOMException.get(); },
         [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForDOMException = std::forward<decltype(space)>(space); },
         [](auto& spaces) { return spaces.m_subspaceForDOMException.get(); },
-        [](auto& spaces, auto&& space) { spaces.m_subspaceForDOMException = std::forward<decltype(space)>(space); });
+        [](auto& spaces, auto&& space) { spaces.m_subspaceForDOMException = std::forward<decltype(space)>(space); },
+        [](auto& server) -> JSC::HeapCellType& { return server.m_heapCellTypeForJSDOMException; });
 }
 
 void JSDOMException::analyzeHeap(JSCell* cell, HeapAnalyzer& analyzer)
@@ -350,7 +385,9 @@
 
 JSC::JSValue toJSNewlyCreated(JSC::JSGlobalObject*, JSDOMGlobalObject* globalObject, Ref<DOMException>&& impl)
 {
-    return createWrapper<DOMException>(globalObject, WTF::move(impl));
+    auto* wrapper = createWrapper<DOMException>(globalObject, WTF::move(impl));
+    wrapper->putHeaderStackIfNoFrames(globalObject->vm());
+    return wrapper;
 }
 
 JSC::JSValue toJS(JSC::JSGlobalObject* lexicalGlobalObject, JSDOMGlobalObject* globalObject, DOMException& impl)

src/jsc/bindings/webcore/JSDOMException.h

@@ -24,14 +24,21 @@
 
 #include "DOMException.h"
 #include "JSDOMWrapper.h"
+#include <JavaScriptCore/ErrorInstance.h>
 #include <JavaScriptCore/ErrorPrototype.h>
 #include <wtf/NeverDestroyed.h>
 
 namespace WebCore {
 
-class JSDOMException : public JSDOMWrapper<DOMException> {
+// An ErrorInstance so DOMException has [[ErrorData]] and a stack, as WebIDL requires.
+class JSDOMException : public JSC::ErrorInstance {
 public:
-    using Base = JSDOMWrapper<DOMException>;
+    using Base = JSC::ErrorInstance;
+    using DOMWrapped = DOMException;
+
+    static constexpr unsigned StructureFlags = Base::StructureFlags;
+    static constexpr JSC::DestructionMode needsDestruction = JSC::NeedsDestruction;
+
     static JSDOMException* create(JSC::Structure* structure, JSDOMGlobalObject* globalObject, Ref<DOMException>&& impl)
     {
         JSDOMException* ptr = new (NotNull, JSC::allocateCell<JSDOMException>(globalObject->vm())) JSDOMException(structure, *globalObject, WTF::move(impl));
@@ -45,10 +52,11 @@ class JSDOMException : public JSDOMWrapper<DOMException> {
     static void destroy(JSC::JSCell*);
 
     DECLARE_INFO;
+    DECLARE_VISIT_CHILDREN;
 
     static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype)
     {
-        return JSC::Structure::create(vm, globalObject, prototype, JSC::TypeInfo(JSC::ObjectType, StructureFlags), info(), JSC::NonArray);
+        return JSC::Structure::create(vm, globalObject, prototype, JSC::TypeInfo(JSC::ErrorInstanceType, StructureFlags), info(), JSC::NonArray);
     }
 
     static JSC::JSValue getConstructor(JSC::VM&, const JSC::JSGlobalObject*);
@@ -61,10 +69,24 @@ class JSDOMException : public JSDOMWrapper<DOMException> {
     static JSC::GCClient::IsoSubspace* subspaceForImpl(JSC::VM& vm);
     static void analyzeHeap(JSCell*, JSC::HeapAnalyzer&);
 
+    DOMException& wrapped() const { return m_wrapped; }
+    Ref<DOMException> protectedWrapped() const { return m_wrapped; }
+    static constexpr ptrdiff_t offsetOfWrapped() { return OBJECT_OFFSETOF(JSDOMException, m_wrapped); }
+    constexpr static bool hasCustomPtrTraits() { return false; }
+
+    JSDOMGlobalObject* globalObject() const { return uncheckedDowncast<JSDOMGlobalObject>(JSC::JSNonFinalObject::globalObject()); }
+
+    // Installs `stack` as an own property and keeps the lazily captured trace from overwriting it.
+    void setStackString(JSC::VM&, WTF::String&&);
+    void putHeaderStackIfNoFrames(JSC::VM&);
+
 protected:
     JSDOMException(JSC::Structure*, JSDOMGlobalObject&, Ref<DOMException>&&);
 
     void finishCreation(JSC::VM&);
+
+private:
+    Ref<DOMException> m_wrapped;
 };
 
 class JSDOMExceptionOwner final : public JSC::WeakHandleOwner {