Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/jsc/bindings/BunClientData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "napi_handle_scope.h"
#include "NativePromiseContext.h"
#include "StrongRootBlock.h"
#include "JSDOMException.h"

namespace WebCore {
using namespace JSC;
Expand All @@ -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>())
Expand Down
1 change: 1 addition & 0 deletions src/jsc/bindings/BunClientData.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
27 changes: 20 additions & 7 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "BunClientData.h"
#include "CallSite.h"
#include "ErrorStackTrace.h"
#include "JSDOMException.h"
#include "headers-handwritten.h"

#include <wtf/Scope.h>
Expand Down Expand Up @@ -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 {
Comment thread
robobun marked this conversation as resolved.
name = instance->sanitizedNameString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
message = instance->sanitizedMessageString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
}
}
}

Expand Down Expand Up @@ -673,8 +679,13 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj
}

if (source->stackTrace()) {
destination->stackTrace()->appendVector(*source->stackTrace());
source->stackTrace()->clear();
// Mutate m_stackTrace only through setStackFrames: its cellLock pairs with
// concurrent readers (JSDOMException::visitChildren on the GC marker thread).
Comment thread
robobun marked this conversation as resolved.
Outdated
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());
Expand Down Expand Up @@ -724,7 +735,9 @@ 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, don't move: stealing the live buffer out of m_stackTrace outside the
// cellLock races with a concurrent visitChildren iterating it (JSDOMException).
Comment thread
robobun marked this conversation as resolved.
Outdated
auto ownedStackTrace = makeUnique<WTF::Vector<JSC::StackFrame>>(*stackTrace);
JSC::MarkedArgumentBuffer protectedFrameCells;
protectedFrameCells.ensureCapacity(ownedStackTrace->size() * 2);
for (auto& frame : *ownedStackTrace) {
Expand Down
6 changes: 4 additions & 2 deletions src/jsc/bindings/JSDOMExceptionHandling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/jsc/bindings/JSDOMWrapperCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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)
{
Expand Down
29 changes: 16 additions & 13 deletions src/jsc/bindings/ZigException.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -456,19 +457,18 @@ 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, {});
// Only plain data properties: accessors AND native custom getters
// (e.g. DOMException.prototype.code) are observable and must not run here.
Comment thread
robobun marked this conversation as resolved.
Outdated
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,
Expand Down Expand Up @@ -512,7 +512,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;
}
Expand Down
47 changes: 42 additions & 5 deletions src/jsc/bindings/webcore/JSDOMException.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -232,18 +235,51 @@
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);
// Capture a stack trace like a native Error. Pass a null message/cause so
// they stay as prototype accessors reading from the wrapped DOMException.
Comment thread
robobun marked this conversation as resolved.
Outdated
Base::finishCreation(vm, String(), JSValue(), nullptr, JSC::TypeNothing, true);
ASSERT(inherits(info()));

// static_assert(!std::is_base_of<ActiveDOMObject, DOMException>::value, "Interface is not marked as [ActiveDOMObject] even though implementation class subclasses ActiveDOMObject.");
// ErrorInstance never materializes `stack` from an empty trace, which can
// happen for native entries like AbortSignal.timeout. Give `.stack` the
// `name: message` header so DOMException matches other engines.
Comment thread
robobun marked this conversation as resolved.
Outdated
auto* trace = stackTrace();
if (!trace || trace->isEmpty()) {
auto& impl = wrapped();
auto name = impl.name();
auto message = impl.message();
auto header = message.isEmpty() ? name : makeString(name, ": "_s, message);
putDirect(vm, vm.propertyNames->stack, jsString(vm, WTF::move(header)), static_cast<unsigned>(JSC::PropertyAttribute::DontEnum));
setStackPropertyAlreadyMaterialized();
}

Check warning on line 261 in src/jsc/bindings/webcore/JSDOMException.cpp

View check run for this annotation

Claude / Claude Code Review

Header-only .stack fallback is discarded for DOMException subclasses

The header-only `.stack` fallback doesn't survive subclass construction: `finishCreation` runs `putDirect(stack, ...)` on the cached base structure, then `construct()` calls `setSubclassStructureIfNeeded`, which swaps in a fresh property-less structure derived from that same cached base — dropping the `+stack` transition. With `m_errorInfoMaterialized` already set, the lazy path won't repopulate it, so `class Foo extends DOMException {}; Error.stackTraceLimit = 0; new Foo('m','AbortError').stack
Comment thread
robobun marked this conversation as resolved.
Outdated
}

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);

// ErrorInstance drops dead stack frames via finalizeUnconditionally over
// vm.errorInstanceSpace(); our own subspace isn't swept there, so keep the
// frames alive explicitly until the stack is materialized.
Comment thread
robobun marked this conversation as resolved.
Outdated
Locker locker { thisObject->cellLock() };
if (auto* stackTrace = thisObject->stackTrace()) {
for (auto& frame : *stackTrace)
frame.visitAggregate(visitor);
}
Comment thread
claude[bot] marked this conversation as resolved.
}

DEFINE_VISIT_CHILDREN(JSDOMException);

JSObject* JSDOMException::createPrototype(VM& vm, JSDOMGlobalObject& globalObject)
{
return JSDOMExceptionPrototype::create(vm, &globalObject, JSDOMExceptionPrototype::createStructure(vm, &globalObject, globalObject.errorPrototype()));
Expand Down Expand Up @@ -316,12 +352,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)
Expand Down
25 changes: 22 additions & 3 deletions src/jsc/bindings/webcore/JSDOMException.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,22 @@

#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> {
// JSDOMException inherits from ErrorInstance so that, per WebIDL, DOMException
// objects carry [[ErrorData]] (Error.isError returns true) and a captured stack.
Comment thread
robobun marked this conversation as resolved.
Outdated
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));
Expand All @@ -45,10 +53,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*);
Expand All @@ -61,10 +70,20 @@ 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()); }

protected:
JSDOMException(JSC::Structure*, JSDOMGlobalObject&, Ref<DOMException>&&);

void finishCreation(JSC::VM&);

private:
Ref<DOMException> m_wrapped;
};

class JSDOMExceptionOwner final : public JSC::WeakHandleOwner {
Expand Down
8 changes: 4 additions & 4 deletions src/jsc/bindings/webcore/SerializedScriptValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,10 @@
write(String::fromLatin1(JSC::Yarr::flagsString(regExp->regExp()->flags()).data()));
return true;
}
if (obj->inherits<JSDOMException>()) {
dumpDOMException(obj, code);
return true;
}

Check failure on line 1189 in src/jsc/bindings/webcore/SerializedScriptValue.cpp

View check run for this annotation

Claude / Claude Code Review

structuredClone(DOMException) no longer preserves .stack — regresses Node parallel test

Moving the `JSDOMException` check here keeps `DOMExceptionTag` in use, but `dumpDOMException`/`readDOMException` still only round-trip `message`+`name` — so the deserialized wrapper goes through the new `JSDOMException::finishCreation` and captures a *fresh* stack at the `structuredClone` call site. That regresses `test/js/node/test/parallel/test-structuredClone-domexception.js`, which asserts `strictEqual(clone.stack, e.stack)` (previously both were `undefined` so it passed vacuously); that fil
Comment thread
robobun marked this conversation as resolved.
Outdated
if (auto* errorInstance = dynamicDowncast<ErrorInstance>(obj)) {
if (!startObjectInternal(errorInstance)) // handle duplicates
return true;
Expand Down Expand Up @@ -1391,10 +1395,6 @@
return true;
}
#endif
if (obj->inherits<JSDOMException>()) {
dumpDOMException(obj, code);
return true;
}

// write bun types
auto _cloneable = StructuredCloneableSerialize::fromJS(value);
Expand Down
Loading