From 9256b3d77784c9e4e094dbbf3dc34e7824c8e39d Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 17 Feb 2026 12:34:19 -0800 Subject: [PATCH] fix(Error): captureStackTrace with non-stack function returns proper stack string (#27017) ### What does this PR do? When Error.captureStackTrace(e, fn) is called with a function that isn't in the call stack, all frames are filtered out and e.stack should return just the error name and message (e.g. "Error: test"), matching Node.js behavior. Previously Bun returned undefined because: 1. The empty frame vector replaced the original stack frames via setStackFrames(), but the lazy stack accessor was only installed when hasMaterializedErrorInfo() was true (i.e. stack was previously accessed). When it wasn't, JSC's internal materialization saw the empty/null frames and produced no stack property at all. 2. The custom lazy getter returned undefined when stackTrace was nullptr, instead of computing the error name+message string with zero frames. Fix: always force materialization before replacing frames, always install the custom lazy accessor, and handle nullptr stackTrace in the getter by computing the error string with an empty frame list. ### How did you verify your code works? --------- Co-authored-by: Claude Bot Co-authored-by: Claude Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/bindings/FormatStackTraceForJS.cpp | 39 ++++++++++++------- src/js/internal/assert/assertion_error.ts | 13 +++++-- test/js/node/v8/capture-stack-trace.test.js | 36 +++++++++++++++++ 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/src/bun.js/bindings/FormatStackTraceForJS.cpp b/src/bun.js/bindings/FormatStackTraceForJS.cpp index dee8b43601..52fc508112 100644 --- a/src/bun.js/bindings/FormatStackTraceForJS.cpp +++ b/src/bun.js/bindings/FormatStackTraceForJS.cpp @@ -641,13 +641,16 @@ JSC_DEFINE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter, (JSGlobalObject * g OrdinalNumber column; String sourceURL; auto stackTrace = errorObject->stackTrace(); - if (stackTrace == nullptr) { - return JSValue::encode(jsUndefined()); - } - JSValue result = computeErrorInfoToJSValue(vm, *stackTrace, line, column, sourceURL, errorObject, nullptr); - stackTrace->clear(); - errorObject->setStackFrames(vm, {}); + JSValue result; + if (stackTrace == nullptr) { + WTF::Vector emptyTrace; + result = computeErrorInfoToJSValue(vm, emptyTrace, line, column, sourceURL, errorObject, nullptr); + } else { + result = computeErrorInfoToJSValue(vm, *stackTrace, line, column, sourceURL, errorObject, nullptr); + stackTrace->clear(); + errorObject->setStackFrames(vm, {}); + } RETURN_IF_EXCEPTION(scope, {}); errorObject->putDirect(vm, vm.propertyNames->stack, result, 0); return JSValue::encode(result); @@ -687,17 +690,27 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb JSCStackTrace::getFramesForCaller(vm, callFrame, errorObject, caller, stackTrace, stackTraceLimit); if (auto* instance = jsDynamicCast(errorObject)) { + // Force materialization before replacing the stack frames, so that JSC's + // internal lazy error info mechanism doesn't later see the replaced (possibly empty) + // stack trace and fail to create the stack property. + if (!instance->hasMaterializedErrorInfo()) + instance->materializeErrorInfoIfNeeded(vm); + RETURN_IF_EXCEPTION(scope, {}); + instance->setStackFrames(vm, WTF::move(stackTrace)); - if (instance->hasMaterializedErrorInfo()) { + + { const auto& propertyName = vm.propertyNames->stack; - VM::DeletePropertyModeScope scope(vm, VM::DeletePropertyMode::IgnoreConfigurable); + VM::DeletePropertyModeScope deleteScope(vm, VM::DeletePropertyMode::IgnoreConfigurable); DeletePropertySlot slot; JSObject::deleteProperty(instance, globalObject, propertyName, slot); - if (auto* zigGlobalObject = jsDynamicCast(globalObject)) { - instance->putDirectCustomAccessor(vm, vm.propertyNames->stack, zigGlobalObject->m_lazyStackCustomGetterSetter.get(zigGlobalObject), JSC::PropertyAttribute::CustomAccessor | 0); - } else { - instance->putDirectCustomAccessor(vm, vm.propertyNames->stack, CustomGetterSetter::create(vm, errorInstanceLazyStackCustomGetter, errorInstanceLazyStackCustomSetter), JSC::PropertyAttribute::CustomAccessor | 0); - } + } + RETURN_IF_EXCEPTION(scope, {}); + + if (auto* zigGlobalObject = jsDynamicCast(globalObject)) { + instance->putDirectCustomAccessor(vm, vm.propertyNames->stack, zigGlobalObject->m_lazyStackCustomGetterSetter.get(zigGlobalObject), JSC::PropertyAttribute::CustomAccessor | 0); + } else { + instance->putDirectCustomAccessor(vm, vm.propertyNames->stack, CustomGetterSetter::create(vm, errorInstanceLazyStackCustomGetter, errorInstanceLazyStackCustomSetter), JSC::PropertyAttribute::CustomAccessor | 0); } } else { OrdinalNumber line; diff --git a/src/js/internal/assert/assertion_error.ts b/src/js/internal/assert/assertion_error.ts index c12a3f9cd9..e328f75221 100644 --- a/src/js/internal/assert/assertion_error.ts +++ b/src/js/internal/assert/assertion_error.ts @@ -378,10 +378,15 @@ class AssertionError extends Error { this.operator = operator; } ErrorCaptureStackTrace(this, stackStartFn || stackStartFunction); - // JSC::Interpreter::getStackTrace() sometimes short-circuits without creating a .stack property. - // e.g.: https://github.com/oven-sh/WebKit/blob/e32c6356625cfacebff0c61d182f759abf6f508a/Source/JavaScriptCore/interpreter/Interpreter.cpp#L501 - if ($isUndefinedOrNull(this.stack)) { - ErrorCaptureStackTrace(this, AssertionError); + // When all stack frames are above the stackStartFn (e.g. in async + // contexts), captureStackTrace produces a stack with just the error + // message and no frame lines. Retry with AssertionError as the filter + // so we get at least the frames below the constructor. + { + const s = this.stack; + if ($isUndefinedOrNull(s) || (typeof s === "string" && s.indexOf("\n at ") === -1)) { + ErrorCaptureStackTrace(this, AssertionError); + } } // Create error message including the error code in the name. this.stack; // eslint-disable-line no-unused-expressions diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 6e5d2008f2..aff2891f50 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -754,3 +754,39 @@ test("CallFrame.p.isAsync", async () => { expect(prepare).toHaveBeenCalledTimes(1); }); + +test("captureStackTrace with constructor function not in stack returns error string", () => { + // When the second argument to captureStackTrace is a function that isn't in + // the call stack, all frames are filtered out and .stack should still return + // the error name and message (matching Node.js behavior). + function notInStack() {} + + // Case 1: stack not accessed before captureStackTrace + { + const e = new Error("test"); + Error.captureStackTrace(e, notInStack); + expect(e.stack).toBe("Error: test"); + } + + // Case 2: stack accessed before captureStackTrace + { + const e = new Error("test"); + void e.stack; + Error.captureStackTrace(e, notInStack); + expect(e.stack).toBe("Error: test"); + } + + // Case 3: empty message + { + const e = new Error(); + Error.captureStackTrace(e, notInStack); + expect(e.stack).toBe("Error"); + } + + // Case 4: custom error name + { + const e = new TypeError("bad type"); + Error.captureStackTrace(e, notInStack); + expect(e.stack).toBe("TypeError: bad type"); + } +});