From 29d287261bc34f8163c9ddcefffef3be72345415 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 14 Oct 2024 13:43:06 -0700 Subject: [PATCH] Fix several bugs when printing exceptions from Error.captureStackTrace (#14548) --- cmake/tools/SetupWebKit.cmake | 2 +- src/bun.js/bindings/CallSitePrototype.cpp | 82 +++-- src/bun.js/bindings/ErrorStackFrame.cpp | 12 +- src/bun.js/bindings/ErrorStackTrace.cpp | 313 +++++++++++++++-- src/bun.js/bindings/ErrorStackTrace.h | 48 ++- src/bun.js/bindings/ZigGlobalObject.cpp | 315 +++++++++++------- src/bun.js/bindings/ZigGlobalObject.h | 5 +- .../bindings/v8-capture-stack-fixture.cjs | 15 + src/bun.js/javascript.zig | 9 + .../parallel/util-format.test.js | 3 + test/js/node/v8/capture-stack-trace.test.js | 122 ++++++- .../v8/error-prepare-stack-default-fixture.js | 36 +- test/regression/issue/013880-fixture.cjs | 15 + test/regression/issue/013880.test.ts | 5 + 14 files changed, 766 insertions(+), 216 deletions(-) create mode 100644 src/bun.js/bindings/v8-capture-stack-fixture.cjs create mode 100644 test/regression/issue/013880-fixture.cjs create mode 100644 test/regression/issue/013880.test.ts diff --git a/cmake/tools/SetupWebKit.cmake b/cmake/tools/SetupWebKit.cmake index 5b58cbb5d6..7c189262f5 100644 --- a/cmake/tools/SetupWebKit.cmake +++ b/cmake/tools/SetupWebKit.cmake @@ -2,7 +2,7 @@ option(WEBKIT_VERSION "The version of WebKit to use") option(WEBKIT_LOCAL "If a local version of WebKit should be used instead of downloading") if(NOT WEBKIT_VERSION) - set(WEBKIT_VERSION 01ac6a63449713c5b7cf38fb03628283041f63be) + set(WEBKIT_VERSION 12e2f46fb01f7c5cf5a992b9414ddfaab32b7110) endif() if(WEBKIT_LOCAL) diff --git a/src/bun.js/bindings/CallSitePrototype.cpp b/src/bun.js/bindings/CallSitePrototype.cpp index 0e9eb93ffd..ba7c8bdf07 100644 --- a/src/bun.js/bindings/CallSitePrototype.cpp +++ b/src/bun.js/bindings/CallSitePrototype.cpp @@ -13,42 +13,39 @@ #include #include #include - +#include +#include using namespace JSC; namespace Zig { -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetThis); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetTypeName); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetFunction); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetFunctionName); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetMethodName); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetFileName); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetLineNumber); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetColumnNumber); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetEvalOrigin); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetScriptNameOrSourceURL); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsToplevel); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsEval); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsNative); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsConstructor); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsAsync); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsPromiseAll); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetPromiseIndex); -static JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncToString); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetThis); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetTypeName); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetFunction); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetFunctionName); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetMethodName); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetFileName); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetLineNumber); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetColumnNumber); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetEvalOrigin); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetScriptNameOrSourceURL); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsToplevel); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsEval); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsNative); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsConstructor); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsAsync); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncIsPromiseAll); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncGetPromiseIndex); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncToString); +JSC_DECLARE_HOST_FUNCTION(callSiteProtoFuncToJSON); ALWAYS_INLINE static CallSite* getCallSite(JSGlobalObject* globalObject, JSC::JSValue thisValue) { JSC::VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - if (UNLIKELY(!thisValue.isCell())) { - JSC::throwVMError(globalObject, scope, createNotAnObjectError(globalObject, thisValue)); - return nullptr; - } - - if (LIKELY(thisValue.asCell()->inherits(CallSite::info()))) { - return JSC::jsCast(thisValue); + if (auto* callSite = JSC::jsDynamicCast(thisValue)) { + return callSite; } throwTypeError(globalObject, scope, "CallSite operation called on non-CallSite object"_s); @@ -84,6 +81,7 @@ static const HashTableValue CallSitePrototypeTableValues[] { "isPromiseAll"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncIsPromiseAll, 0 } }, { "getPromiseIndex"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncGetPromiseIndex, 0 } }, { "toString"_s, JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::Function, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncToString, 0 } }, + { "toJSON"_s, JSC::PropertyAttribute::Function | 0, NoIntrinsic, { HashTableValue::NativeFunctionType, callSiteProtoFuncToJSON, 0 } }, }; const JSC::ClassInfo CallSitePrototype::s_info = { "CallSite"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(CallSitePrototype) }; @@ -165,10 +163,29 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncIsToplevel, (JSGlobalObject * globalOb { ENTER_PROTO_FUNC(); + if (JSValue functionValue = callSite->function()) { + if (JSObject* fn = functionValue.getObject()) { + if (JSFunction* function = jsDynamicCast(fn)) { + if (function->inherits()) { + return JSC::JSValue::encode(JSC::jsBoolean(false)); + } + + if (function->isHostFunction()) { + return JSC::JSValue::encode(JSC::jsBoolean(true)); + } + + if (auto* executable = function->jsExecutable()) { + return JSValue::encode(jsBoolean(executable->isProgramExecutable() || executable->isModuleProgramExecutable())); + } + } else if (auto* function = jsDynamicCast(functionValue)) { + return JSC::JSValue::encode(JSC::jsBoolean(true)); + } + } + } + JSC::JSValue thisValue = callSite->thisValue(); // This is what v8 does (JSStackFrame::IsToplevel in messages.cc): - if (thisValue.isUndefinedOrNull()) { return JSC::JSValue::encode(JSC::jsBoolean(true)); } @@ -237,4 +254,15 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncToString, (JSGlobalObject * globalObje return JSC::JSValue::encode(JSC::JSValue(jsString(vm, sb.toString()))); } +JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncToJSON, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + ENTER_PROTO_FUNC(); + JSObject* obj = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 4); + obj->putDirect(vm, JSC::Identifier::fromString(vm, "sourceURL"_s), callSite->sourceURL()); + obj->putDirect(vm, JSC::Identifier::fromString(vm, "lineNumber"_s), jsNumber(callSite->lineNumber().oneBasedInt())); + obj->putDirect(vm, JSC::Identifier::fromString(vm, "columnNumber"_s), jsNumber(callSite->columnNumber().zeroBasedInt())); + obj->putDirect(vm, JSC::Identifier::fromString(vm, "functionName"_s), callSite->functionName()); + return JSC::JSValue::encode(obj); +} + } diff --git a/src/bun.js/bindings/ErrorStackFrame.cpp b/src/bun.js/bindings/ErrorStackFrame.cpp index cb8c553e28..806a340be2 100644 --- a/src/bun.js/bindings/ErrorStackFrame.cpp +++ b/src/bun.js/bindings/ErrorStackFrame.cpp @@ -22,7 +22,15 @@ void adjustPositionBackwards(ZigStackFramePosition& pos, int amount, CodeBlock* pos.column_zero_based = pos.column_zero_based - amount; if (pos.column_zero_based < 0) { - auto source = code->source().provider()->source(); + auto* provider = code->source().provider(); + if (!provider) { + pos.line_zero_based = 0; + pos.column_zero_based = 0; + pos.byte_position = 0; + return; + } + + auto source = provider->source(); if (!source.is8Bit()) { // Debug-only assertion // Bun does not yet use 16-bit sources anywhere. The transpiler ensures everything @@ -75,6 +83,8 @@ ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC:: switch (inst->opcodeID()) { case op_construct: case op_construct_varargs: + case op_super_construct: + case op_super_construct_varargs: // The divot by default is pointing at the `(` or the end of the class name. // We want to point at the `new` keyword, which is conveniently at the // expression start. diff --git a/src/bun.js/bindings/ErrorStackTrace.cpp b/src/bun.js/bindings/ErrorStackTrace.cpp index ae2e282c1d..6928399151 100644 --- a/src/bun.js/bindings/ErrorStackTrace.cpp +++ b/src/bun.js/bindings/ErrorStackTrace.cpp @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include "ErrorStackFrame.h" @@ -24,6 +26,69 @@ using namespace WebCore; namespace Zig { +static ImplementationVisibility getImplementationVisibility(JSC::CodeBlock* codeBlock) +{ + + if (auto* executable = codeBlock->ownerExecutable()) { + return executable->implementationVisibility(); + } + + return ImplementationVisibility::Public; +} + +static bool isImplementationVisibilityPrivate(JSC::StackVisitor& visitor) +{ + ImplementationVisibility implementationVisibility = [&]() -> ImplementationVisibility { + if (visitor->callee().isCell()) { + if (auto* callee = visitor->callee().asCell()) { + if (auto* jsFunction = jsDynamicCast(callee)) { + if (auto* executable = jsFunction->executable()) + return executable->implementationVisibility(); + } + } + } + + if (auto* codeBlock = visitor->codeBlock()) { + return getImplementationVisibility(codeBlock); + } + +#if ENABLE(WEBASSEMBLY) + if (visitor->isNativeCalleeFrame()) + return visitor->callee().asNativeCallee()->implementationVisibility(); +#endif + + return ImplementationVisibility::Public; + }(); + + return implementationVisibility != ImplementationVisibility::Public; +} + +static bool isImplementationVisibilityPrivate(const JSC::StackFrame& frame) +{ + ImplementationVisibility implementationVisibility = [&]() -> ImplementationVisibility { + +#if ENABLE(WEBASSEMBLY) + if (frame.isWasmFrame()) + return ImplementationVisibility::Public; +#endif + + if (auto* callee = frame.callee()) { + if (auto* jsFunction = jsDynamicCast(callee)) { + if (auto* executable = jsFunction->executable()) + return executable->implementationVisibility(); + } + } + + if (auto* codeBlock = frame.codeBlock()) { + return getImplementationVisibility(codeBlock); + } + + return ImplementationVisibility::Public; + }(); + + return implementationVisibility != ImplementationVisibility::Public; +} + JSCStackTrace JSCStackTrace::fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames) { WTF::Vector newFrames; @@ -35,41 +100,155 @@ JSCStackTrace JSCStackTrace::fromExisting(JSC::VM& vm, const WTF::Vector& stackTrace, size_t stackTraceLimit) { - ImplementationVisibility implementationVisibility = [&]() -> ImplementationVisibility { - if (auto* codeBlock = visitor->codeBlock()) { - if (auto* executable = codeBlock->ownerExecutable()) { - return executable->implementationVisibility(); - } - return ImplementationVisibility::Public; + size_t framesCount = 0; + + bool belowCaller = false; + int32_t skipFrames = 0; + + WTF::String callerName {}; + if (JSC::JSFunction* callerFunction = JSC::jsDynamicCast(caller)) { + callerName = callerFunction->name(vm); + if (callerName.isEmpty() && callerFunction->jsExecutable()) { + callerName = callerFunction->jsExecutable()->name().string(); } + } + if (JSC::InternalFunction* callerFunctionInternal = JSC::jsDynamicCast(caller)) { + callerName = callerFunctionInternal->name(); + } -#if ENABLE(WEBASSEMBLY) - if (visitor->isNativeCalleeFrame()) - return visitor->callee().asNativeCallee()->implementationVisibility(); -#endif + size_t totalFrames = 0; - if (visitor->callee().isCell()) { - if (auto* callee = visitor->callee().asCell()) { - if (auto* jsFunction = jsDynamicCast(callee)) { - if (auto* executable = jsFunction->executable()) - return executable->implementationVisibility(); - return ImplementationVisibility::Public; + if (!callerName.isEmpty()) { + JSC::StackVisitor::visit(callFrame, vm, [&](JSC::StackVisitor& visitor) -> WTF::IterationStatus { + if (isImplementationVisibilityPrivate(visitor)) { + return WTF::IterationStatus::Continue; + } + + framesCount += 1; + + // skip caller frame and all frames above it + if (!belowCaller) { + skipFrames += 1; + + if (visitor->functionName() == callerName) { + belowCaller = true; + return WTF::IterationStatus::Continue; } } + + totalFrames += 1; + + if (totalFrames > stackTraceLimit) { + return WTF::IterationStatus::Done; + } + + return WTF::IterationStatus::Continue; + }); + } else if (caller && caller.isCell()) { + JSC::StackVisitor::visit(callFrame, vm, [&](JSC::StackVisitor& visitor) -> WTF::IterationStatus { + if (isImplementationVisibilityPrivate(visitor)) { + return WTF::IterationStatus::Continue; + } + + framesCount += 1; + + // skip caller frame and all frames above it + if (!belowCaller) { + auto callee = visitor->callee(); + skipFrames += 1; + if (callee.isCell() && callee.asCell() == caller) { + belowCaller = true; + return WTF::IterationStatus::Continue; + } + } + + totalFrames += 1; + + if (totalFrames > stackTraceLimit) { + return WTF::IterationStatus::Done; + } + + return WTF::IterationStatus::Continue; + }); + } else if (caller.isEmpty() || caller.isUndefined()) { + // Skip the first frame. + JSC::StackVisitor::visit(callFrame, vm, [&](JSC::StackVisitor& visitor) -> WTF::IterationStatus { + if (isImplementationVisibilityPrivate(visitor)) { + return WTF::IterationStatus::Continue; + } + + framesCount += 1; + + if (!belowCaller) { + skipFrames += 1; + belowCaller = true; + } + + totalFrames += 1; + + if (totalFrames > stackTraceLimit) { + return WTF::IterationStatus::Done; + } + + return WTF::IterationStatus::Continue; + }); + } + size_t i = 0; + totalFrames = 0; + stackTrace.reserveInitialCapacity(framesCount); + JSC::StackVisitor::visit(callFrame, vm, [&](JSC::StackVisitor& visitor) -> WTF::IterationStatus { + // Skip native frames + if (isImplementationVisibilityPrivate(visitor)) { + return WTF::IterationStatus::Continue; } - return ImplementationVisibility::Public; - }(); + // Skip frames if needed + if (skipFrames > 0) { + skipFrames--; + return WTF::IterationStatus::Continue; + } - return implementationVisibility != ImplementationVisibility::Public; + totalFrames += 1; + + if (totalFrames > stackTraceLimit) { + return WTF::IterationStatus::Done; + } + + if (visitor->isNativeCalleeFrame()) { + + auto* nativeCallee = visitor->callee().asNativeCallee(); + switch (nativeCallee->category()) { + case NativeCallee::Category::Wasm: { + stackTrace.append(StackFrame(visitor->wasmFunctionIndexOrName())); + break; + } + case NativeCallee::Category::InlineCache: { + break; + } + } +#if USE(ALLOW_LINE_AND_COLUMN_NUMBER_IN_BUILTINS) + } else if (!!visitor->codeBlock()) +#else + } else if (!!visitor->codeBlock() && !visitor->codeBlock()->unlinkedCodeBlock()->isBuiltinFunction()) +#endif + stackTrace.append(StackFrame(vm, owner, visitor->callee().asCell(), visitor->codeBlock(), visitor->bytecodeIndex())); + else + stackTrace.append(StackFrame(vm, owner, visitor->callee().asCell())); + + i++; + + return (i == framesCount) ? WTF::IterationStatus::Done : WTF::IterationStatus::Continue; + }); } JSCStackTrace JSCStackTrace::captureCurrentJSStackTrace(Zig::GlobalObject* globalObject, JSC::CallFrame* callFrame, size_t frameLimit, JSC::JSValue caller) @@ -203,6 +382,22 @@ JSCStackTrace JSCStackTrace::getStackTraceForThrownValue(JSC::VM& vm, JSC::JSVal return fromExisting(vm, *jscStackTrace); } +static bool isVisibleBuiltinFunction(JSC::CodeBlock* codeBlock) +{ + if (!codeBlock->ownerExecutable()) { + return false; + } + + const JSC::SourceCode& source = codeBlock->source(); + if (auto* provider = source.provider()) { + const auto& url = provider->sourceURL(); + if (!url.isEmpty()) { + return true; + } + } + return false; +} + JSCStackFrame::JSCStackFrame(JSC::VM& vm, JSC::StackVisitor& visitor) : m_vm(vm) , m_codeBlock(nullptr) @@ -228,9 +423,18 @@ JSCStackFrame::JSCStackFrame(JSC::VM& vm, JSC::StackVisitor& visitor) break; } } - } else if (!!visitor->codeBlock() && !visitor->codeBlock()->unlinkedCodeBlock()->isBuiltinFunction()) { - m_codeBlock = visitor->codeBlock(); - m_bytecodeIndex = visitor->bytecodeIndex(); + } else if (auto* codeBlock = visitor->codeBlock()) { + auto* unlinkedCodeBlock = codeBlock->unlinkedCodeBlock(); + if (!unlinkedCodeBlock->isBuiltinFunction() || isVisibleBuiltinFunction(codeBlock)) { + m_codeBlock = codeBlock; + m_bytecodeIndex = visitor->bytecodeIndex(); + } + } + + if (!m_bytecodeIndex && visitor->hasLineAndColumnInfo()) { + auto lineColumn = visitor->computeLineAndColumn(); + m_sourcePositions = { OrdinalNumber::fromOneBasedInt(lineColumn.line), OrdinalNumber::fromOneBasedInt(lineColumn.column) }; + m_sourcePositionsState = SourcePositionsState::Calculated; } } @@ -250,12 +454,19 @@ JSCStackFrame::JSCStackFrame(JSC::VM& vm, const JSC::StackFrame& frame) if (frame.isWasmFrame()) { m_wasmFunctionIndexOrName = frame.wasmFunctionIndexOrName(); m_isWasmFrame = true; - } else { - m_codeBlock = frame.codeBlock(); - if (frame.hasBytecodeIndex()) { + } else if (auto* codeBlock = frame.codeBlock()) { + auto* unlinkedCodeBlock = codeBlock->unlinkedCodeBlock(); + if (!unlinkedCodeBlock->isBuiltinFunction() || isVisibleBuiltinFunction(codeBlock)) { + m_codeBlock = codeBlock; m_bytecodeIndex = frame.bytecodeIndex(); } } + + if (!m_codeBlock && frame.hasLineAndColumnInfo()) { + auto lineColumn = frame.computeLineAndColumn(); + m_sourcePositions = { OrdinalNumber::fromOneBasedInt(lineColumn.line), OrdinalNumber::fromOneBasedInt(lineColumn.column) }; + m_sourcePositionsState = SourcePositionsState::Calculated; + } } intptr_t JSCStackFrame::sourceID() const @@ -308,16 +519,36 @@ ALWAYS_INLINE String JSCStackFrame::retrieveSourceURL() return String(sourceURLWasmString); } + if (m_callee && m_callee->isObject()) { + if (auto* jsFunction = jsDynamicCast(m_callee)) { + if (auto* executable = jsFunction->executable()) { + if (!executable->isHostFunction()) { + auto* jsExectuable = jsFunction->jsExecutable(); + if (jsExectuable) { + const auto* sourceProvider = jsExectuable->source().provider(); + if (sourceProvider) { + return sourceProvider->sourceURL(); + } + } + } + } + } + } + if (!m_codeBlock) { return String(sourceURLNativeString); } - return m_codeBlock->ownerExecutable()->sourceURL(); + auto* provider = m_codeBlock->source().provider(); + if (provider) { + return provider->sourceURL(); + } + + return String(); } ALWAYS_INLINE String JSCStackFrame::retrieveFunctionName() { - static auto functionNameEvalCodeString = MAKE_STATIC_STRING_IMPL("eval code"); static auto functionNameModuleCodeString = MAKE_STATIC_STRING_IMPL("module code"); static auto functionNameGlobalCodeString = MAKE_STATIC_STRING_IMPL("global code"); @@ -328,7 +559,8 @@ ALWAYS_INLINE String JSCStackFrame::retrieveFunctionName() if (m_codeBlock) { switch (m_codeBlock->codeType()) { case JSC::EvalCode: - return String(functionNameEvalCodeString); + // Node returns null here. + return String(); case JSC::ModuleCode: return String(functionNameModuleCodeString); case JSC::FunctionCode: @@ -340,13 +572,26 @@ ALWAYS_INLINE String JSCStackFrame::retrieveFunctionName() } } - String name; if (m_callee) { - if (m_callee->isObject()) - name = getCalculatedDisplayName(m_vm, jsCast(m_callee)).impl(); + if (auto* callee = m_callee->getObject()) { + // Does the code block have a user-defined name property? + JSC::JSValue name = callee->getDirect(m_vm, m_vm.propertyNames->name); + if (name && name.isString()) { + auto scope = DECLARE_CATCH_SCOPE(m_vm); + auto nameString = name.toWTFString(callee->globalObject()); + if (scope.exception()) { + scope.clearException(); + } + if (!nameString.isEmpty()) { + return nameString; + } + } + + return JSC::getCalculatedDisplayName(m_vm, callee); + } } - return name.isNull() ? emptyString() : name; + return emptyString(); } ALWAYS_INLINE String JSCStackFrame::retrieveTypeName() diff --git a/src/bun.js/bindings/ErrorStackTrace.h b/src/bun.js/bindings/ErrorStackTrace.h index e33cc18a6e..34a8fe0f74 100644 --- a/src/bun.js/bindings/ErrorStackTrace.h +++ b/src/bun.js/bindings/ErrorStackTrace.h @@ -46,13 +46,13 @@ public: private: JSC::VM& m_vm; - JSC::JSCell* m_callee; + JSC::JSCell* m_callee { nullptr }; // May be null JSC::CallFrame* m_callFrame; // May be null - JSC::CodeBlock* m_codeBlock; + JSC::CodeBlock* m_codeBlock { nullptr }; JSC::BytecodeIndex m_bytecodeIndex; // Lazy-initialized @@ -96,8 +96,40 @@ public: SourcePositions* getSourcePositions(); bool isWasmFrame() const { return m_isWasmFrame; } - bool isEval() const { return m_codeBlock && (JSC::EvalCode == m_codeBlock->codeType()); } - bool isConstructor() const { return m_codeBlock && (JSC::CodeForConstruct == m_codeBlock->specializationKind()); } + bool isEval() + { + if (m_codeBlock) { + if (m_codeBlock->codeType() == JSC::EvalCode) { + return true; + } + auto* executable = m_codeBlock->ownerExecutable(); + if (!executable) { + return false; + } + + switch (executable->evalContextType()) { + case JSC::EvalContextType::None: { + return false; + } + case JSC::EvalContextType::FunctionEvalContext: + case JSC::EvalContextType::InstanceFieldEvalContext: + return true; + } + } + + if (m_callee && m_callee->inherits()) { + auto* function = jsCast(m_callee); + if (function->isHostFunction()) { + return false; + } + } + + return false; + } + bool isConstructor() const + { + return m_codeBlock && (JSC::CodeForConstruct == m_codeBlock->specializationKind()); + } private: ALWAYS_INLINE String retrieveSourceURL(); @@ -130,10 +162,17 @@ public: { } + JSCStackTrace(WTF::Vector&& frames) + : m_frames(WTFMove(frames)) + { + } + size_t size() const { return m_frames.size(); } bool isEmpty() const { return m_frames.isEmpty(); } JSCStackFrame& at(size_t i) { return m_frames.at(i); } + WTF::Vector&& frames() { return WTFMove(m_frames); } + static JSCStackTrace fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames); /* This is based on JSC::Interpreter::getStackTrace, but skips native (non js and not wasm) @@ -145,6 +184,7 @@ public: * * Return value must remain stack allocated. */ static JSCStackTrace captureCurrentJSStackTrace(Zig::GlobalObject* globalObject, JSC::CallFrame* callFrame, size_t frameLimit, JSC::JSValue caller); + static void getFramesForCaller(JSC::VM& vm, JSC::CallFrame* callFrame, JSC::JSCell* owner, JSC::JSValue caller, WTF::Vector& stackTrace, size_t stackTraceLimit); /* In JSC, JSC::Exception points to the actual value that was thrown, usually * a JSC::ErrorInstance (but could be any JSValue). In v8, on the other hand, diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 61da2ab06f..2b262f832b 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -276,27 +276,10 @@ extern "C" void* Bun__getVM(); extern "C" void Bun__setDefaultGlobalObject(Zig::GlobalObject* globalObject); -// Error.captureStackTrace may cause computeErrorInfo to be called twice -// Rather than figure out the plumbing in JSC, we just skip the next call -// TODO: thread_local for workers -static bool skipNextComputeErrorInfo = false; - -static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites, JSValue prepareStackTrace) +static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites) { auto scope = DECLARE_THROW_SCOPE(vm); - auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(globalObject); - - if (!prepareStackTrace) { - if (lexicalGlobalObject->inherits()) { - if (auto prepare = globalObject->m_errorConstructorPrepareStackTraceValue.get()) { - prepareStackTrace = prepare; - } - } else { - prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, JSC::Identifier::fromString(vm, "prepareStackTrace"_s)); - } - } - // default formatting size_t framesCount = callSites->length(); @@ -322,21 +305,20 @@ static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalO CallSite* callSite = JSC::jsDynamicCast(callSiteValue); sb.append(" at "_s); callSite->formatAsString(vm, lexicalGlobalObject, sb); + RETURN_IF_EXCEPTION(scope, {}); if (i != framesCount - 1) { sb.append("\n"_s); } } - bool originalSkipNextComputeErrorInfo = skipNextComputeErrorInfo; - skipNextComputeErrorInfo = true; - if (errorObject->hasProperty(lexicalGlobalObject, vm.propertyNames->stack)) { - skipNextComputeErrorInfo = true; - errorObject->deleteProperty(lexicalGlobalObject, vm.propertyNames->stack); - } + return jsString(vm, sb.toString()); +} - skipNextComputeErrorInfo = originalSkipNextComputeErrorInfo; - - JSValue stackStringValue = jsString(vm, sb.toString()); +static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites, JSValue prepareStackTrace) +{ + auto scope = DECLARE_THROW_SCOPE(vm); + auto stackStringValue = formatStackTraceToJSValue(vm, globalObject, lexicalGlobalObject, errorObject, callSites); + RETURN_IF_EXCEPTION(scope, {}); if (prepareStackTrace && prepareStackTrace.isObject()) { JSC::CallData prepareStackTraceCallData = JSC::getCallData(prepareStackTrace); @@ -355,10 +337,10 @@ static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalO JSC::ProfilingReason::Other, prepareStackTrace, prepareStackTraceCallData, - errorConstructor, + lexicalGlobalObject->m_errorStructure.constructor(globalObject), arguments); - RETURN_IF_EXCEPTION(scope, {}); + RETURN_IF_EXCEPTION(scope, stackStringValue); if (result.isUndefinedOrNull()) { result = jsUndefined(); @@ -371,6 +353,26 @@ static JSValue formatStackTraceToJSValue(JSC::VM& vm, Zig::GlobalObject* globalO return stackStringValue; } +static JSValue formatStackTraceToJSValueWithoutPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites) +{ + JSValue prepareStackTrace = {}; + if (lexicalGlobalObject->inherits()) { + if (auto prepare = globalObject->m_errorConstructorPrepareStackTraceValue.get()) { + prepareStackTrace = prepare; + } + } else { + auto scope = DECLARE_CATCH_SCOPE(vm); + + auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(globalObject); + prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, JSC::Identifier::fromString(vm, "prepareStackTrace"_s)); + if (scope.exception()) { + scope.clearException(); + } + } + + return formatStackTraceToJSValue(vm, globalObject, lexicalGlobalObject, errorObject, callSites, prepareStackTrace); +} + WTF::String Bun::formatStackTrace( JSC::VM& vm, Zig::GlobalObject* globalObject, @@ -467,12 +469,13 @@ WTF::String Bun::formatStackTrace( for (size_t i = 0; i < framesCount; i++) { StackFrame& frame = stackTrace.at(i); + WTF::String functionName; + bool isBuiltinFunction = false; sb.append(" at "_s); - WTF::String functionName; - if (auto codeblock = frame.codeBlock()) { + if (codeblock->isConstructor()) { sb.append("new "_s); } @@ -484,11 +487,25 @@ WTF::String Bun::formatStackTrace( case JSC::CodeType::FunctionCode: case JSC::CodeType::EvalCode: { if (auto* callee = frame.callee()) { - if (callee->isObject()) { - JSValue functionNameValue = callee->getObject()->getDirect(vm, vm.propertyNames->name); + if (auto* object = callee->getObject()) { + JSValue functionNameValue = object->getDirect(vm, vm.propertyNames->name); if (functionNameValue && functionNameValue.isString()) { functionName = functionNameValue.toWTFString(lexicalGlobalObject); } + + if (functionName.isEmpty()) { + auto catchScope = DECLARE_CATCH_SCOPE(vm); + functionName = JSC::getCalculatedDisplayName(vm, object); + if (catchScope.exception()) { + catchScope.clearException(); + } + } + + if (auto* unlinkedCodeBlock = codeblock->unlinkedCodeBlock()) { + if (unlinkedCodeBlock->isBuiltinFunction()) { + isBuiltinFunction = true; + } + } } } break; @@ -544,8 +561,10 @@ WTF::String Bun::formatStackTrace( } } - // If it's not a Zig::GlobalObject, don't bother source-mapping it. - if (globalObject == lexicalGlobalObject && globalObject) { + bool isDefinitelyNotRunninginNodeVMGlobalObject = (globalObject == lexicalGlobalObject && globalObject); + + bool isDefaultGlobalObjectInAFinalizer = (globalObject && !lexicalGlobalObject && !errorInstance); + if (isDefinitelyNotRunninginNodeVMGlobalObject || isDefaultGlobalObjectInAFinalizer) { // https://github.com/oven-sh/bun/issues/3595 if (!sourceURLForFrame.isEmpty()) { remappedFrame.source_url = Bun::toStringRef(sourceURLForFrame); @@ -572,7 +591,15 @@ WTF::String Bun::formatStackTrace( } sb.append(" ("_s); - sb.append(sourceURLForFrame); + if (sourceURLForFrame.isEmpty()) { + if (isBuiltinFunction) { + sb.append("native"_s); + } else { + sb.append("unknown"_s); + } + } else { + sb.append(sourceURLForFrame); + } sb.append(":"_s); sb.append(remappedFrame.position.line().oneBasedInt()); sb.append(":"_s); @@ -623,16 +650,14 @@ static String computeErrorInfoWithoutPrepareStackTrace( return Bun::formatStackTrace(vm, globalObject, lexicalGlobalObject, name, message, line, column, sourceURL, stackTrace, errorInstance); } -static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector& stackFrames, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorObject, JSObject* prepareStackTrace) +static JSValue computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, Vector& stackFrames, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorObject, JSObject* prepareStackTrace) { auto scope = DECLARE_THROW_SCOPE(vm); JSCStackTrace stackTrace = JSCStackTrace::fromExisting(vm, stackFrames); // Note: we cannot use tryCreateUninitializedRestricted here because we cannot allocate memory inside initializeIndex() - JSC::JSArray* callSites = JSC::JSArray::create(vm, - globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), - stackTrace.size()); + MarkedArgumentBuffer callSites; // Create the call sites (one per frame) GlobalObject::createCallSitesFromFrames(globalObject, lexicalGlobalObject, stackTrace, callSites); @@ -657,7 +682,7 @@ static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObje Bun__remapStackFramePositions(globalObject, remappedFrames, framesCount); for (size_t i = 0; i < framesCount; i++) { - JSC::JSValue callSiteValue = callSites->getIndex(lexicalGlobalObject, i); + JSC::JSValue callSiteValue = callSites.at(i); CallSite* callSite = JSC::jsDynamicCast(callSiteValue); if (remappedFrames[i].remapped) { callSite->setColumnNumber(remappedFrames[i].position.column()); @@ -666,64 +691,85 @@ static String computeErrorInfoWithPrepareStackTrace(JSC::VM& vm, Zig::GlobalObje } } - JSValue value = formatStackTraceToJSValue(vm, jsDynamicCast(lexicalGlobalObject), lexicalGlobalObject, errorObject, callSites, prepareStackTrace); + JSArray* callSitesArray = JSC::constructArray(globalObject, globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), callSites); - RETURN_IF_EXCEPTION(scope, String()); - - if (errorObject && !value.isEmpty()) { - errorObject->putDirect(vm, vm.propertyNames->stack, value, 0); - } - - if (value.isString()) { - return value.toWTFString(lexicalGlobalObject); - } - - return String(); + return formatStackTraceToJSValue(vm, globalObject, lexicalGlobalObject, errorObject, callSitesArray, prepareStackTrace); } -static String computeErrorInfo(JSC::VM& vm, Vector& stackTrace, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorInstance) +static String computeErrorInfoToString(JSC::VM& vm, Vector& stackTrace, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL) { - if (skipNextComputeErrorInfo) { - return String(); - } Zig::GlobalObject* globalObject = nullptr; JSC::JSGlobalObject* lexicalGlobalObject = nullptr; - if (errorInstance) { - lexicalGlobalObject = errorInstance->globalObject(); - globalObject = jsDynamicCast(lexicalGlobalObject); + return computeErrorInfoWithoutPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, nullptr); +} - // Error.prepareStackTrace - https://v8.dev/docs/stack-trace-api#customizing-stack-traces - if (!globalObject) { - // node:vm will use a different JSGlobalObject - globalObject = defaultGlobalObject(); +static JSValue computeErrorInfoToJSValueWithoutSkipping(JSC::VM& vm, Vector& stackTrace, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorInstance) +{ + Zig::GlobalObject* globalObject = nullptr; + JSC::JSGlobalObject* lexicalGlobalObject = nullptr; + lexicalGlobalObject = errorInstance->globalObject(); + globalObject = jsDynamicCast(lexicalGlobalObject); + + // Error.prepareStackTrace - https://v8.dev/docs/stack-trace-api#customizing-stack-traces + if (!globalObject) { + // node:vm will use a different JSGlobalObject + globalObject = defaultGlobalObject(); + if (!globalObject->isInsideErrorPrepareStackTraceCallback) { auto* errorConstructor = lexicalGlobalObject->m_errorStructure.constructor(lexicalGlobalObject); if (JSValue prepareStackTrace = errorConstructor->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "prepareStackTrace"_s))) { if (prepareStackTrace.isCell() && prepareStackTrace.isObject() && prepareStackTrace.isCallable()) { - return computeErrorInfoWithPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance, prepareStackTrace.getObject()); + globalObject->isInsideErrorPrepareStackTraceCallback = true; + auto result = computeErrorInfoWithPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance, prepareStackTrace.getObject()); + globalObject->isInsideErrorPrepareStackTraceCallback = false; + return result; } } - } else { - if (JSValue prepareStackTrace = globalObject->m_errorConstructorPrepareStackTraceValue.get()) { - if (prepareStackTrace.isCell() && prepareStackTrace.isObject() && prepareStackTrace.isCallable()) { - return computeErrorInfoWithPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance, prepareStackTrace.getObject()); + } + } 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()); + globalObject->isInsideErrorPrepareStackTraceCallback = false; + return result; } } } } - return computeErrorInfoWithoutPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance); + String result = computeErrorInfoWithoutPrepareStackTrace(vm, globalObject, lexicalGlobalObject, stackTrace, line, column, sourceURL, errorInstance); + return jsString(vm, result); +} + +static JSValue computeErrorInfoToJSValue(JSC::VM& vm, Vector& stackTrace, OrdinalNumber& line, OrdinalNumber& column, String& sourceURL, JSObject* errorInstance) +{ + return computeErrorInfoToJSValueWithoutSkipping(vm, stackTrace, line, column, sourceURL, errorInstance); } // TODO: @paperdave: remove this wrapper and make the WTF::Function from JavaScriptCore expect OrdinalNumber instead of unsigned. -static String computeErrorInfoWrapper(JSC::VM& vm, Vector& stackTrace, unsigned int& line_in, unsigned int& column_in, String& sourceURL, JSObject* errorInstance) +static String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& stackTrace, unsigned int& line_in, unsigned int& column_in, String& sourceURL) { OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); - WTF::String result = computeErrorInfo(vm, stackTrace, line, column, sourceURL, errorInstance); + WTF::String result = computeErrorInfoToString(vm, stackTrace, line, column, sourceURL); + + line_in = line.oneBasedInt(); + column_in = column.oneBasedInt(); + + return result; +} + +static JSValue computeErrorInfoWrapperToJSValue(JSC::VM& vm, Vector& stackTrace, unsigned int& line_in, unsigned int& column_in, String& sourceURL, JSObject* errorInstance) +{ + OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); + OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); + + JSValue result = computeErrorInfoToJSValue(vm, stackTrace, line, column, sourceURL, errorInstance); line_in = line.oneBasedInt(); column_in = column.oneBasedInt(); @@ -820,7 +866,8 @@ extern "C" JSC__JSGlobalObject* Zig__GlobalObject__create(void* console_client, Bun__setDefaultGlobalObject(globalObject); JSC::gcProtect(globalObject); - vm.setOnComputeErrorInfo(computeErrorInfoWrapper); + vm.setOnComputeErrorInfo(computeErrorInfoWrapperToString); + vm.setOnComputeErrorInfoJSValue(computeErrorInfoWrapperToJSValue); vm.setOnEachMicrotaskTick([](JSC::VM& vm) -> void { auto* globalObject = defaultGlobalObject(); if (auto nextTickQueue = globalObject->m_nextTickQueue.get()) { @@ -2528,7 +2575,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionPerformMicrotaskVariadic, (JSGlobalObject * g return JSValue::encode(jsUndefined()); } -void GlobalObject::createCallSitesFromFrames(Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSCStackTrace& stackTrace, JSC::JSArray* callSites) +void GlobalObject::createCallSitesFromFrames(Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSCStackTrace& stackTrace, MarkedArgumentBuffer& callSites) { /* From v8's "Stack Trace API" (https://github.com/v8/v8/wiki/Stack-Trace-API): * "To maintain restrictions imposed on strict mode functions, frames that have a @@ -2543,20 +2590,12 @@ void GlobalObject::createCallSitesFromFrames(Zig::GlobalObject* globalObject, JS for (size_t i = 0; i < framesCount; i++) { CallSite* callSite = CallSite::create(lexicalGlobalObject, callSiteStructure, stackTrace.at(i), encounteredStrictFrame); - callSites->putDirectIndex(lexicalGlobalObject, i, callSite); if (!encounteredStrictFrame) { encounteredStrictFrame = callSite->isStrict(); } - } -} -void GlobalObject::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites, JSValue prepareStackTrace) -{ - JSValue stackTraceValue = formatStackTraceToJSValue(vm, this, lexicalGlobalObject, errorObject, callSites, prepareStackTrace); - - if (!stackTraceValue.isEmpty()) { - errorObject->putDirect(vm, vm.propertyNames->stack, stackTraceValue, 0); + callSites.append(callSite); } } @@ -2606,6 +2645,44 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionDefaultErrorPrepareStackTrace, (JSGlobalObjec return JSC::JSValue::encode(result); } +JSC_DEFINE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, PropertyName)) +{ + auto& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + auto* errorObject = jsDynamicCast(JSValue::decode(thisValue)); + + // This shouldn't be possible. + if (!errorObject) { + return JSValue::encode(jsUndefined()); + } + + OrdinalNumber line; + OrdinalNumber column; + String sourceURL; + auto stackTrace = errorObject->stackTrace(); + if (stackTrace == nullptr) { + return JSValue::encode(jsUndefined()); + } + + JSValue result = computeErrorInfoToJSValue(vm, *stackTrace, line, column, sourceURL, errorObject); + stackTrace->clear(); + errorObject->setStackFrames(vm, {}); + RETURN_IF_EXCEPTION(scope, {}); + errorObject->putDirect(vm, vm.propertyNames->stack, result, 0); + return JSValue::encode(result); +} + +JSC_DEFINE_CUSTOM_SETTER(errorInstanceLazyStackCustomSetter, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue value, PropertyName)) +{ + auto& vm = globalObject->vm(); + JSValue decodedValue = JSValue::decode(thisValue); + if (auto* object = decodedValue.getObject()) { + object->putDirect(vm, vm.propertyNames->stack, JSValue::decode(value), 0); + } + + return true; +} + JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) { GlobalObject* globalObject = reinterpret_cast(lexicalGlobalObject); @@ -2625,56 +2702,30 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb stackTraceLimit = DEFAULT_ERROR_STACK_TRACE_LIMIT; } - JSCStackTrace stackTrace = JSCStackTrace::captureCurrentJSStackTrace(globalObject, callFrame, stackTraceLimit, caller); + WTF::Vector stackTrace; + JSCStackTrace::getFramesForCaller(vm, callFrame, errorObject, caller, stackTrace, stackTraceLimit); - // Note: we cannot use tryCreateUninitializedRestricted here because we cannot allocate memory inside initializeIndex() - JSC::JSArray* callSites = JSC::JSArray::create(vm, - globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), - stackTrace.size()); - - // Create the call sites (one per frame) - GlobalObject::createCallSitesFromFrames(globalObject, lexicalGlobalObject, stackTrace, callSites); - - /* Format the stack trace. - * Note that v8 won't actually format the stack trace here, but will create a "stack" accessor - * on the error object, which will format the stack trace on the first access. For now, since - * we're not being used internally by JSC, we can assume callers of Error.captureStackTrace in - * node are interested in the (formatted) stack. */ - - size_t framesCount = stackTrace.size(); - ZigStackFrame remappedFrames[64]; - framesCount = framesCount > 64 ? 64 : framesCount; - - for (int i = 0; i < framesCount; i++) { - memset(remappedFrames + i, 0, sizeof(ZigStackFrame)); - remappedFrames[i].source_url = Bun::toStringRef(lexicalGlobalObject, stackTrace.at(i).sourceURL()); - if (JSCStackFrame::SourcePositions* sourcePositions = stackTrace.at(i).getSourcePositions()) { - remappedFrames[i].position.line_zero_based = sourcePositions->line.zeroBasedInt(); - remappedFrames[i].position.column_zero_based = sourcePositions->column.zeroBasedInt(); - } else { - remappedFrames[i].position.line_zero_based = -1; - remappedFrames[i].position.column_zero_based = -1; + if (auto* instance = jsDynamicCast(errorObject)) { + instance->setStackFrames(vm, WTFMove(stackTrace)); + if (instance->hasMaterializedErrorInfo()) { + const auto& propertyName = vm.propertyNames->stack; + VM::DeletePropertyModeScope scope(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); + } } + } else { + OrdinalNumber line; + OrdinalNumber column; + String sourceURL; + JSValue result = computeErrorInfoToJSValue(vm, stackTrace, line, column, sourceURL, errorObject); + errorObject->putDirect(vm, vm.propertyNames->stack, result, 0); } - // remap line and column start to original source - // XXX: this function does not fully populate the fields of ZigStackFrame, - // be careful reading the fields below. - Bun__remapStackFramePositions(lexicalGlobalObject, remappedFrames, framesCount); - - // write the remapped lines back to the CallSites - for (size_t i = 0; i < framesCount; i++) { - JSC::JSValue callSiteValue = callSites->getIndex(lexicalGlobalObject, i); - CallSite* callSite = JSC::jsDynamicCast(callSiteValue); - if (remappedFrames[i].remapped) { - callSite->setColumnNumber(remappedFrames[i].position.column()); - callSite->setLineNumber(remappedFrames[i].position.line()); - } - } - - globalObject->formatStackTrace(vm, lexicalGlobalObject, errorObject, callSites, JSC::JSValue()); - RETURN_IF_EXCEPTION(scope, {}); - return JSC::JSValue::encode(JSC::jsUndefined()); } @@ -2689,6 +2740,11 @@ void GlobalObject::finishCreation(VM& vm) Bun::addNodeModuleConstructorProperties(vm, this); + m_lazyStackCustomGetterSetter.initLater( + [](const Initializer& init) { + init.set(CustomGetterSetter::create(init.vm, errorInstanceLazyStackCustomGetter, errorInstanceLazyStackCustomSetter)); + }); + m_JSDOMFileConstructor.initLater( [](const Initializer& init) { JSObject* fileConstructor = Bun::createJSDOMFileConstructor(init.vm, init.owner); @@ -3634,6 +3690,7 @@ void GlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) thisObject->m_JSBufferListClassStructure.visit(visitor); thisObject->m_JSBufferSubclassStructure.visit(visitor); thisObject->m_JSCryptoKey.visit(visitor); + thisObject->m_lazyStackCustomGetterSetter.visit(visitor); thisObject->m_JSDOMFileConstructor.visit(visitor); thisObject->m_JSFFIFunctionStructure.visit(visitor); thisObject->m_JSFileSinkClassStructure.visit(visitor); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 52226012de..323bdb96e5 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -191,8 +191,7 @@ public: void clearDOMGuardedObjects(); - static void createCallSitesFromFrames(Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSCStackTrace& stackTrace, JSC::JSArray* callSites); - void formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject, JSC::JSArray* callSites, JSValue prepareStack = JSC::jsUndefined()); + static void createCallSitesFromFrames(Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSCStackTrace& stackTrace, MarkedArgumentBuffer& callSites); static void reportUncaughtExceptionAtEventLoop(JSGlobalObject*, JSC::Exception*); static JSGlobalObject* deriveShadowRealmGlobalObject(JSGlobalObject* globalObject); @@ -374,6 +373,7 @@ public: } bool asyncHooksNeedsCleanup = false; + bool isInsideErrorPrepareStackTraceCallback = false; /** * WARNING: You must update visitChildrenImpl() if you add a new field. @@ -584,6 +584,7 @@ public: LazyProperty m_navigatorObject; LazyProperty m_performanceObject; LazyProperty m_processObject; + LazyProperty m_lazyStackCustomGetterSetter; bool hasOverridenModuleResolveFilenameFunction = false; diff --git a/src/bun.js/bindings/v8-capture-stack-fixture.cjs b/src/bun.js/bindings/v8-capture-stack-fixture.cjs new file mode 100644 index 0000000000..c8d21c775d --- /dev/null +++ b/src/bun.js/bindings/v8-capture-stack-fixture.cjs @@ -0,0 +1,15 @@ +let e = new Error(); + +const { noInline } = require("bun:jsc"); + +function sloppyWrapperFn() { + sloppyFn(); +} +noInline(sloppyWrapperFn); + +function sloppyFn() { + Error.captureStackTrace(e); + module.exports = e.stack; +} +noInline(sloppyFn); +sloppyWrapperFn(); diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index dad8da4bee..9756726628 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -3358,6 +3358,7 @@ pub const VirtualMachine = struct { if (frames.len == 0) return; var top = &frames[0]; + var top_frame_is_builtin = false; if (this.hide_bun_stackframes) { for (frames) |*frame| { if (frame.source_url.hasPrefixComptime("bun:") or @@ -3365,10 +3366,12 @@ pub const VirtualMachine = struct { frame.source_url.isEmpty() or frame.source_url.eqlComptime("native")) { + top_frame_is_builtin = true; continue; } top = frame; + top_frame_is_builtin = false; break; } } @@ -3417,8 +3420,14 @@ pub const VirtualMachine = struct { } } + if (top_frame_is_builtin) { + // Avoid printing "export default 'native'" + break :code ZigString.Slice.empty; + } + var log = logger.Log.init(bun.default_allocator); defer log.deinit(); + var original_source = fetchWithoutOnLoadPlugins(this, this.global, top.source_url, bun.String.empty, &log, .print_source) catch return; must_reset_parser_arena_later.* = true; break :code original_source.source_code.toUTF8(bun.default_allocator); diff --git a/test/js/node/util/node-inspect-tests/parallel/util-format.test.js b/test/js/node/util/node-inspect-tests/parallel/util-format.test.js index 1671f192f3..76d485bae8 100644 --- a/test/js/node/util/node-inspect-tests/parallel/util-format.test.js +++ b/test/js/node/util/node-inspect-tests/parallel/util-format.test.js @@ -430,6 +430,9 @@ test("no assertion failures", () => { } } const customError = new CustomError("bar"); + customError.stack; + delete customError.originalLine; + delete customError.originalColumn; assert.strictEqual(util.format(customError), customError.stack.replace(/^Error/, "Custom$&")); //! temp bug workaround // Doesn't capture stack trace function BadCustomError(msg) { diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index a61aa32133..9feaa8d12a 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -1,6 +1,6 @@ import { nativeFrameForTesting } from "bun:internal-for-testing"; import { afterEach, expect, test } from "bun:test"; - +import { noInline } from "bun:jsc"; const origPrepareStackTrace = Error.prepareStackTrace; afterEach(() => { Error.prepareStackTrace = origPrepareStackTrace; @@ -376,18 +376,38 @@ test("sanity check", () => { f1(); }); -test("CallFrame.p.getThisgetFunction: works in sloppy mode", () => { +test("CallFrame isEval works as expected", () => { + let prevPrepareStackTrace = Error.prepareStackTrace; + + let name, fn; + + Error.prepareStackTrace = (e, s) => { + return s; + }; + + name = "f1"; + const stack = eval(`(function ${name}() { + return new Error().stack; + })()`); + + Error.prepareStackTrace = prevPrepareStackTrace; + // TODO: 0 and 1 should both return true here. + expect(stack[1].isEval()).toBe(true); + expect(stack[0].getFunctionName()).toBe(name); +}); + +test("CallFrame isTopLevel returns false for Function constructor", () => { let prevPrepareStackTrace = Error.prepareStackTrace; const sloppyFn = new Function("let e=new Error();Error.captureStackTrace(e);return e.stack"); sloppyFn.displayName = "sloppyFnWow"; + noInline(sloppyFn); const that = {}; Error.prepareStackTrace = (e, s) => { - expect(s[0].getThis()).toBe(that); - expect(s[0].getFunction()).toBe(sloppyFn); expect(s[0].getFunctionName()).toBe(sloppyFn.displayName); + expect(s[0].getFunction()).toBe(sloppyFn); + expect(s[0].isToplevel()).toBe(false); - // TODO: This should be true. expect(s[0].isEval()).toBe(false); // Strict-mode functions shouldn't have getThis or getFunction @@ -480,7 +500,7 @@ test("CallFrame.p.toString", () => { }); // TODO: line numbers are wrong in a release build -test.todo("err.stack should invoke prepareStackTrace", () => { +test("err.stack should invoke prepareStackTrace", () => { var lineNumber = -1; var functionName = ""; var parentLineNumber = -1; @@ -503,9 +523,8 @@ test.todo("err.stack should invoke prepareStackTrace", () => { functionWithAName(); expect(functionName).toBe("functionWithAName"); - expect(lineNumber).toBe(391); - // TODO: this is wrong - expect(parentLineNumber).toBe(394); + expect(lineNumber).toBe(518); + expect(parentLineNumber).toBe(523); }); test("Error.prepareStackTrace inside a node:vm works", () => { @@ -559,3 +578,88 @@ test("Error.prepareStackTrace returns a CallSite object", () => { expect(error.stack[0]).not.toBeString(); expect(error.stack[0][Symbol.toStringTag]).toBe("CallSite"); }); + +test("Error.captureStackTrace updates the stack property each call, even if Error.prepareStackTrace is set", () => { + const prevPrepareStackTrace = Error.prepareStackTrace; + var didCallPrepareStackTrace = false; + + let error = new Error(); + const firstStack = error.stack; + Error.prepareStackTrace = function (err, stack) { + expect(err.stack).not.toBe(firstStack); + didCallPrepareStackTrace = true; + return stack; + }; + function outer() { + inner(); + } + function inner() { + Error.captureStackTrace(error); + } + outer(); + const secondStack = error.stack; + expect(firstStack).not.toBe(secondStack); + expect(firstStack).toBeString(); + expect(firstStack).not.toContain("outer"); + expect(firstStack).not.toContain("inner"); + expect(didCallPrepareStackTrace).toBe(true); + expect(secondStack.find(a => a.getFunctionName() === "outer")).toBeTruthy(); + expect(secondStack.find(a => a.getFunctionName() === "inner")).toBeTruthy(); + Error.prepareStackTrace = prevPrepareStackTrace; +}); + +test("Error.captureStackTrace updates the stack property each call", () => { + let error = new Error(); + const firstStack = error.stack; + function outer() { + inner(); + } + function inner() { + Error.captureStackTrace(error); + } + outer(); + const secondStack = error.stack; + expect(firstStack).not.toBe(secondStack); + expect(firstStack.length).toBeLessThan(secondStack.length); + expect(firstStack).not.toContain("outer"); + expect(firstStack).not.toContain("inner"); + expect(secondStack).toContain("outer"); + expect(secondStack).toContain("inner"); +}); + +test("calling .stack later uses the stored StackTrace", function hey() { + let error = new Error(); + let stack; + function outer() { + inner(); + } + function inner() { + stack = error.stack; + } + outer(); + + expect(stack).not.toContain("outer"); + expect(stack).not.toContain("inner"); + expect(stack).toContain("hey"); +}); + +test("calling .stack on a non-materialized Error updates the stack properly", function hey() { + let error = new Error(); + let stack; + function outer() { + inner(); + } + function inner() { + stack = error.stack; + } + function wrapped() { + Error.captureStackTrace(error); + } + wrapped(); + outer(); + + expect(stack).not.toContain("outer"); + expect(stack).not.toContain("inner"); + expect(stack).toContain("hey"); + expect(stack).toContain("wrapped"); +}); diff --git a/test/js/node/v8/error-prepare-stack-default-fixture.js b/test/js/node/v8/error-prepare-stack-default-fixture.js index 2586758595..17df9c6d9d 100644 --- a/test/js/node/v8/error-prepare-stack-default-fixture.js +++ b/test/js/node/v8/error-prepare-stack-default-fixture.js @@ -5,20 +5,38 @@ const orig = Error.prepareStackTrace; Error.prepareStackTrace = (err, stack) => { return orig(err, stack); }; +var stack2, stack; -const err = new Error(); -Error.captureStackTrace(err); -const stack = err.stack; +function twoWrapperLevel() { + const err = new Error(); + Error.captureStackTrace(err); + stack = err.stack; -Error.prepareStackTrace = undefined; -const err2 = new Error(); -Error.captureStackTrace(err2); -const stack2 = err2.stack; + Error.prepareStackTrace = undefined; + const err2 = new Error(); + Error.captureStackTrace(err2); + stack2 = err2.stack; +} -const stackIgnoringLineAndColumn = stack.replaceAll(":10:24", "N"); -const stack2IgnoringLineAndColumn = stack2.replaceAll(":15:24", "N"); +function oneWrapperLevel() { + // ... + var a = 123; + globalThis.a = a; + // --- + + twoWrapperLevel(); +} + +oneWrapperLevel(); + +// The native line column numbers might differ a bit here. +const stackIgnoringLineAndColumn = stack.replaceAll(":12:26", ":NN:NN").replaceAll(/native:.*$/gm, "native)"); +const stack2IgnoringLineAndColumn = stack2.replaceAll(":17:26", ":NN:NN").replaceAll(/native:.*$/gm, "native)"); if (stackIgnoringLineAndColumn !== stack2IgnoringLineAndColumn) { + console.log("\n-----\n"); console.log(stackIgnoringLineAndColumn); + console.log("\n-----\n"); console.log(stack2IgnoringLineAndColumn); + console.log("\n-----\n"); throw new Error("Stacks are different"); } diff --git a/test/regression/issue/013880-fixture.cjs b/test/regression/issue/013880-fixture.cjs new file mode 100644 index 0000000000..6c246f36fa --- /dev/null +++ b/test/regression/issue/013880-fixture.cjs @@ -0,0 +1,15 @@ +function a() { + try { + new Function("throw new Error(1)")(); + } catch (e) { + console.log(Error.prepareStackTrace); + console.log(e.stack); + } +} + +Error.prepareStackTrace = function abc() { + console.log("trigger"); + a(); +}; + +new Error().stack; diff --git a/test/regression/issue/013880.test.ts b/test/regression/issue/013880.test.ts new file mode 100644 index 0000000000..90b84bebeb --- /dev/null +++ b/test/regression/issue/013880.test.ts @@ -0,0 +1,5 @@ +import { test, expect } from "bun:test"; + +test("regression", () => { + expect(() => require("./013880-fixture.cjs")).not.toThrow(); +});