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
1 change: 1 addition & 0 deletions src/js/builtins/BunBuiltinNames.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ using namespace JSC;
macro(byobRequest) \
macro(bytes) \
macro(cancel) \
macro(capturedStackTrace) \
macro(checkBufferRead) \
macro(checks) \
macro(cloneArrayBuffer) \
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/CallSite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ void CallSite::formatAsString(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WT
std::optional<OrdinalNumber> column = columnNumber().zeroBasedInt() >= 0 ? std::optional(columnNumber()) : std::nullopt;
std::optional<OrdinalNumber> line = lineNumber().zeroBasedInt() >= 0 ? std::optional(lineNumber()) : std::nullopt;

if (isAsync()) {
sb.append("async "_s);
}

if (functionName.length() > 0) {

if (isConstructor()) {
Expand Down
127 changes: 111 additions & 16 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,36 @@

WTF::StringBuilder sb;

WTF::String name = "Error"_s;
auto errorName = errorObject->getIfPropertyExists(lexicalGlobalObject, vm.propertyNames->name);
RETURN_IF_EXCEPTION(scope, {});
if (errorName && !errorName.isUndefined()) {
auto* str = errorName.toString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
auto value = str->value(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
name = value.data;
}

WTF::String message;
auto errorMessage = errorObject->getIfPropertyExists(lexicalGlobalObject, vm.propertyNames->message);
RETURN_IF_EXCEPTION(scope, {});
if (errorMessage) {
if (errorMessage && !errorMessage.isUndefined()) {
auto* str = errorMessage.toString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
if (str->length() > 0) {
auto value = str->view(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
sb.append("Error: "_s);
sb.append(value.data);
} else {
sb.append("Error"_s);
}
auto value = str->value(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
message = value.data;
}

if (name.isEmpty()) {
sb.append(message);
} else if (message.isEmpty()) {
sb.append(name);
} else {
sb.append("Error"_s);
sb.append(name);
sb.append(": "_s);
sb.append(message);
}

for (size_t i = 0; i < framesCount; i++) {
Expand Down Expand Up @@ -428,7 +443,7 @@
return Bun::formatStackTrace(vm, globalObject, lexicalGlobalObject, name, message, line, column, sourceURL, stackTrace, errorInstance);
}

static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector<StackFrame>& stackFrames, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorObject, JSObject* prepareStackTrace)
static JSArray* buildSourceMappedCallSitesArray(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector<StackFrame>& stackFrames)
{
auto scope = DECLARE_THROW_SCOPE(vm);

Expand Down Expand Up @@ -513,6 +528,19 @@
JSArray* callSitesArray = JSC::constructArray(globalObject, globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), callSites);
RETURN_IF_EXCEPTION(scope, {});

return callSitesArray;
}

static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector<StackFrame>& stackFrames, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorObject, JSObject* prepareStackTrace)
{
UNUSED_PARAM(line);
UNUSED_PARAM(column);
UNUSED_PARAM(sourceURL);
auto scope = DECLARE_THROW_SCOPE(vm);

JSArray* callSitesArray = buildSourceMappedCallSitesArray(vm, globalObject, lexicalGlobalObject, stackFrames);
RETURN_IF_EXCEPTION(scope, {});

RELEASE_AND_RETURN(scope, formatStackTraceToJSValue(vm, globalObject, lexicalGlobalObject, errorObject, callSitesArray, prepareStackTrace));
}

Expand Down Expand Up @@ -756,6 +784,66 @@
return true;
}

JSC_DEFINE_CUSTOM_GETTER(nonErrorInstanceLazyStackCustomGetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, PropertyName))
{
auto& vm = JSC::getVM(lexicalGlobalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

JSObject* receiver = JSValue::decode(thisValue).getObject();
if (!receiver) [[unlikely]]
return JSValue::encode(jsUndefined());

const auto& privateName = WebCore::builtinNames(vm).capturedStackTracePrivateName();
JSObject* errorObject = nullptr;
JSC::JSArray* callSites = nullptr;
for (JSObject* o = receiver; o;) {
JSValue v = o->getDirect(vm, privateName);
if (auto* arr = v ? dynamicDowncast<JSC::JSArray>(v) : nullptr) {
callSites = arr;
errorObject = o;
break;
}
JSValue proto = o->getPrototypeDirect();
o = proto.isObject() ? asObject(proto) : nullptr;
Comment thread
robobun marked this conversation as resolved.
}

Check warning on line 808 in src/jsc/bindings/FormatStackTraceForJS.cpp

View check run for this annotation

Claude / Claude Code Review

Proto-chain walk continues past the jsNull() re-entrancy sentinel to an ancestor's capture

The proto-chain walk and the `jsNull()` re-entrancy sentinel (both added in this PR's follow-up commits) interact incorrectly: when `v` is `jsNull()`, `v ? dynamicDowncast<JSArray>(v) : nullptr` yields `nullptr` and the loop *continues* to the prototype instead of stopping, so a re-entrant `.stack` read on a child whose own slot holds the sentinel walks past it and finds/formats an ancestor's unresolved CallSite array. Fix: break (and return `jsUndefined()`) when `v.isNull()`; only continue on e
Comment thread
claude[bot] marked this conversation as resolved.
if (!callSites) [[unlikely]]
return JSValue::encode(jsUndefined());
Comment thread
claude[bot] marked this conversation as resolved.

JSC::EnsureStillAliveScope keepCallSites(callSites);

// Sentinel so re-entry through a name/message getter hits !callSites above; restored if the user code throws.
errorObject->putDirect(vm, privateName, jsNull(), 0);

auto* globalObject = defaultGlobalObject(lexicalGlobalObject);

JSValue result;
if (globalObject->isInsideErrorPrepareStackTraceCallback) {
result = formatStackTraceToJSValue(vm, globalObject, lexicalGlobalObject, errorObject, callSites);
} else {
globalObject->isInsideErrorPrepareStackTraceCallback = true;
result = formatStackTraceToJSValueWithoutPrepareStackTrace(vm, globalObject, lexicalGlobalObject, errorObject, callSites);
globalObject->isInsideErrorPrepareStackTraceCallback = false;
}
Comment thread
claude[bot] marked this conversation as resolved.
if (scope.exception()) [[unlikely]] {
errorObject->putDirect(vm, privateName, callSites, 0);
return {};
}

Check warning on line 830 in src/jsc/bindings/FormatStackTraceForJS.cpp

View check run for this annotation

Claude / Claude Code Review

Exception-path restore leaves callSites pinned when prepareStackTrace threw

The exception-path restore (`errorObject->putDirect(vm, privateName, callSites, 0)`) is only useful when `.stack` is still the CustomAccessor — but if the throw came from `Error.prepareStackTrace` itself, `formatStackTraceToJSValue(..., prepareStackTrace)` has already replaced `.stack` with a data property (the temp string) *before* invoking the callback, so the restored `JSArray<CallSite>` is never consumed and stays pinned to the target for its lifetime. This is the same bounded-retention shap
Comment thread
claude[bot] marked this conversation as resolved.

errorObject->putDirect(vm, vm.propertyNames->stack, result, JSC::PropertyAttribute::DontEnum | 0);
errorObject->putDirect(vm, privateName, jsUndefined(), 0);
Comment thread
claude[bot] marked this conversation as resolved.
return JSValue::encode(result);
}

JSC_DEFINE_CUSTOM_SETTER(nonErrorInstanceLazyStackCustomSetter, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue value, PropertyName))
{
auto& vm = JSC::getVM(globalObject);
if (auto* object = JSValue::decode(thisValue).getObject()) {
object->putDirect(vm, vm.propertyNames->stack, JSValue::decode(value), JSC::PropertyAttribute::DontEnum | 0);
object->putDirect(vm, WebCore::builtinNames(vm).capturedStackTracePrivateName(), jsUndefined(), 0);
}
return true;
}

JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame))
{
Zig::GlobalObject* globalObject = static_cast<Zig::GlobalObject*>(lexicalGlobalObject);
Expand Down Expand Up @@ -806,12 +894,19 @@
instance->putDirectCustomAccessor(vm, vm.propertyNames->stack, globalObject->m_lazyStackCustomGetterSetter.get(globalObject), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::CustomAccessor | 0);
}
} else {
OrdinalNumber line;
OrdinalNumber column;
String sourceURL;
JSValue result = computeErrorInfoToJSValue(vm, stackTrace, line, column, sourceURL, errorObject, nullptr);
JSArray* callSitesArray = buildSourceMappedCallSitesArray(vm, globalObject, lexicalGlobalObject, stackTrace);
RETURN_IF_EXCEPTION(scope, {});
errorObject->putDirect(vm, vm.propertyNames->stack, result, JSC::PropertyAttribute::DontEnum | 0);

{
const auto& propertyName = vm.propertyNames->stack;
VM::DeletePropertyModeScope deleteScope(vm, VM::DeletePropertyMode::IgnoreConfigurable);
DeletePropertySlot slot;
JSObject::deleteProperty(errorObject, globalObject, propertyName, slot);
}
RETURN_IF_EXCEPTION(scope, {});

errorObject->putDirect(vm, WebCore::builtinNames(vm).capturedStackTracePrivateName(), callSitesArray, 0);
errorObject->putDirectCustomAccessor(vm, vm.propertyNames->stack, globalObject->m_nonErrorLazyStackCustomGetterSetter.get(globalObject), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::CustomAccessor | 0);
Comment thread
claude[bot] marked this conversation as resolved.
}

return JSC::JSValue::encode(JSC::jsUndefined());
Expand Down
2 changes: 2 additions & 0 deletions src/jsc/bindings/FormatStackTraceForJS.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionDefaultErrorPrepareStackTrace);
// JSC Custom Accessors - error.stack getter/setter
JSC_DECLARE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter);
JSC_DECLARE_CUSTOM_SETTER(errorInstanceLazyStackCustomSetter);
JSC_DECLARE_CUSTOM_GETTER(nonErrorInstanceLazyStackCustomGetter);
JSC_DECLARE_CUSTOM_SETTER(nonErrorInstanceLazyStackCustomSetter);

// Internal wrapper functions for JSC error info callbacks
WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, WTF::Vector<JSC::StackFrame>& stackTrace, unsigned int& line_in, unsigned int& column_in, WTF::String& sourceURL, void* bunErrorData);
Expand Down
5 changes: 5 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2112,6 +2112,11 @@ void GlobalObject::finishCreation(VM& vm)
init.set(CustomGetterSetter::create(init.vm, errorInstanceLazyStackCustomGetter, errorInstanceLazyStackCustomSetter));
});

m_nonErrorLazyStackCustomGetterSetter.initLater(
[](const Initializer<CustomGetterSetter>& init) {
init.set(CustomGetterSetter::create(init.vm, nonErrorInstanceLazyStackCustomGetter, nonErrorInstanceLazyStackCustomSetter));
});

m_JSDOMFileConstructor.initLater(
[](const Initializer<JSObject>& init) {
JSObject* fileConstructor = Bun::createJSDOMFileConstructor(init.vm, init.owner);
Expand Down
1 change: 1 addition & 0 deletions src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ class GlobalObject : public Bun::GlobalScope {
V(public, LazyPropertyOfGlobalObject<JSObject>, m_performanceObject) \
V(public, LazyPropertyOfGlobalObject<Bun::Process>, m_processObject) \
V(public, LazyPropertyOfGlobalObject<CustomGetterSetter>, m_lazyStackCustomGetterSetter) \
V(public, LazyPropertyOfGlobalObject<CustomGetterSetter>, m_nonErrorLazyStackCustomGetterSetter) \
V(public, LazyPropertyOfGlobalObject<Structure>, m_ServerRouteListStructure) \
V(public, LazyPropertyOfGlobalObject<Structure>, m_JSBunRequestStructure) \
V(public, LazyPropertyOfGlobalObject<JSObject>, m_JSBunRequestParamsPrototype) \
Expand Down
169 changes: 168 additions & 1 deletion test/js/node/v8/capture-stack-trace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,12 @@ test("Error.captureStackTrace installs .stack as non-enumerable", () => {
try {
const o2 = {};
Error.captureStackTrace(o2);
expect(insidePrepare).toEqual({ keys: [], enumerable: false });
// V8 invokes prepareStackTrace lazily on first .stack access, not at capture time.
expect(insidePrepare).toBeUndefined();
expect(Object.keys(o2)).toEqual([]);
expectNonEnumerableStack(o2);
expect(o2.stack).toBe("from-prepare");
expect(insidePrepare).toEqual({ keys: [], enumerable: false });
} finally {
Error.prepareStackTrace = origPrepareStackTrace;
}
Expand Down Expand Up @@ -1121,3 +1123,168 @@ test("lazy error-info materialization does not store an empty stack value when t
});
expect(exitCode).toBe(0);
});

// https://github.com/oven-sh/bun/issues/13904
test("captureStackTrace on a non-Error object reads name/message lazily for the stack header", () => {
// V8 installs a lazy accessor: the header is derived from .name/.message at
// first access, so setting them after capture is observable. This is the
// jsonwebtoken JsonWebTokenError shape.
function JsonWebTokenError(message) {
Error.call(this, message);
Error.captureStackTrace(this, this.constructor);
this.name = "JsonWebTokenError";
this.message = message;
}
JsonWebTokenError.prototype = Object.create(Error.prototype);
JsonWebTokenError.prototype.constructor = JsonWebTokenError;

function hello() {
return [new JsonWebTokenError("Hello world")];
}
noInline(hello);
const [e] = hello();

expect(e.stack.split("\n")[0]).toBe("JsonWebTokenError: Hello world");
expect(e.stack).toContain("at hello");
expect(e instanceof Error).toBe(true);

// plain object, name/message set after capture
const o = {};
Error.captureStackTrace(o);
o.name = "CustomName";
o.message = "custom msg";
expect(o.stack.split("\n")[0]).toBe("CustomName: custom msg");

// lazy means "first access wins": later mutations are not reflected
o.name = "Changed";
expect(o.stack.split("\n")[0]).toBe("CustomName: custom msg");
});

test("captureStackTrace on a non-Error object installs a lazy accessor", () => {
const o = {};
Error.captureStackTrace(o);
const d = Object.getOwnPropertyDescriptor(o, "stack");
expect({
hasGetter: typeof d.get,
hasSetter: typeof d.set,
value: d.value,
enumerable: d.enumerable,
configurable: d.configurable,
}).toEqual({
hasGetter: "function",
hasSetter: "function",
value: undefined,
enumerable: false,
configurable: true,
});

// setter replaces the lazy accessor with the assigned value
o.stack = "overwritten";
expect(o.stack).toBe("overwritten");

// captureStackTrace again re-installs the lazy accessor
Error.captureStackTrace(o);
expect(typeof Object.getOwnPropertyDescriptor(o, "stack").get).toBe("function");
expect(typeof o.stack).toBe("string");
expect(o.stack).toContain("at ");
});

test("captureStackTrace header on a non-Error object matches V8's Error.prototype.toString algorithm", () => {
const headerOf = obj => {
Error.captureStackTrace(obj);
return obj.stack.split("\n")[0];
};
// exact outputs verified against Node
expect(headerOf({})).toBe("Error");
expect(headerOf({ name: "N" })).toBe("N");
expect(headerOf({ message: "M" })).toBe("Error: M");
expect(headerOf({ name: "N", message: "M" })).toBe("N: M");
expect(headerOf({ name: "", message: "M" })).toBe("M");
expect(headerOf({ name: "N", message: "" })).toBe("N");
expect(headerOf({ name: "", message: "" })).toBe("");
expect(headerOf({ name: undefined, message: "M" })).toBe("Error: M");
expect(headerOf({ name: null, message: "M" })).toBe("null: M");
expect(headerOf({ name: 42, message: "M" })).toBe("42: M");
expect(headerOf(Object.create(Error.prototype))).toBe("Error");
});

test("captureStackTrace on a non-Error object preserves the async prefix on await-chain frames", async () => {
async function inner() {
await 1;
const o = {};
Error.captureStackTrace(o);
return o;
}
noInline(inner);
async function outer() {
return await inner();
}
noInline(outer);
const o = await outer();
expect(o.stack).toContain("at async outer");
});

test("captureStackTrace on a non-Error object terminates when a name/message getter reads .stack", () => {
const o = Object.create(null);
Object.defineProperty(o, "message", {
get() {
return String(this.stack);
},
});
Error.captureStackTrace(o);
expect(() => o.stack).not.toThrow();
expect(typeof o.stack).toBe("string");
});

test("captureStackTrace on a non-Error object keeps captured frames when a name getter throws", () => {
let calls = 0;
const o = {};
Object.defineProperty(o, "name", {
get() {
calls++;
if (calls === 1) throw new Error("boom");
return "Retry";
},
});
Error.captureStackTrace(o);
expect(() => o.stack).toThrow("boom");
expect(calls).toBe(1);
expect(o.stack.split("\n")[0]).toBe("Retry");
expect(o.stack).toContain("at ");
expect(calls).toBe(2);
});

test("captureStackTrace lazy .stack resolves when reached via the prototype chain", () => {
const parent = {};
Error.captureStackTrace(parent);
const child = Object.create(parent);
expect(typeof child.stack).toBe("string");
expect(child.stack.split("\n")[0]).toBe("Error");
expect(child.stack).toContain("at ");
expect(parent.stack).toBe(child.stack);
});

test("captureStackTrace on a non-Error object invokes Error.prepareStackTrace at access time", () => {
let callCount = 0;
let sawName;
const o = {};
Error.captureStackTrace(o);
o.name = "LateName";
Error.prepareStackTrace = (err, sites) => {
callCount++;
sawName = err.name;
return { err, sites };
};
const result = o.stack;
expect(callCount).toBe(1);
expect(sawName).toBe("LateName");
expect(result.err).toBe(o);
expect(Array.isArray(result.sites)).toBe(true);
expect(result.sites.length).toBeGreaterThan(0);
expect(typeof result.sites[0].getFileName).toBe("function");

// cached after first access: prepareStackTrace not re-invoked
Error.prepareStackTrace = origPrepareStackTrace;
expect(o.stack).toBe(result);
expect(callCount).toBe(1);
});
Loading