Skip to content

Commit

Permalink
Call Error.prepareStackTrace on new Error().stack (#5802)
Browse files Browse the repository at this point in the history 
* Always call `Error.prepareStackTrace`

* Support node:vm

* Remove this

* Handle more cases

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
  • Loading branch information
@Jarred-Sumner
Jarred-Sumner and Jarred-Sumner authored Sep 20, 2023
1 parent 1456c72 commit ff7f642
Show file tree
Hide file tree
Showing 6 changed files with 277 additions and 155 deletions.
76 changes: 23 additions & 53 deletions src/bun.js/bindings/ErrorStackTrace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ JSCStackFrame::JSCStackFrame(JSC::VM& vm, JSC::StackVisitor& visitor)
: m_vm(vm)
, m_codeBlock(nullptr)
, m_bytecodeIndex(JSC::BytecodeIndex())
, m_sourceURL(nullptr)
, m_functionName(nullptr)
, m_sourceURL()
, m_functionName()
, m_isWasmFrame(false)
, m_sourcePositionsState(SourcePositionsState::NotCalculated)
{
Expand Down Expand Up @@ -163,8 +163,8 @@ JSCStackFrame::JSCStackFrame(JSC::VM& vm, const JSC::StackFrame& frame)
, m_callFrame(nullptr)
, m_codeBlock(nullptr)
, m_bytecodeIndex(JSC::BytecodeIndex())
, m_sourceURL(nullptr)
, m_functionName(nullptr)
, m_sourceURL()
, m_functionName()
, m_isWasmFrame(false)
, m_sourcePositionsState(SourcePositionsState::NotCalculated)
{
Expand Down Expand Up @@ -193,7 +193,7 @@ JSC::JSString* JSCStackFrame::sourceURL()
m_sourceURL = retrieveSourceURL();
}

return m_sourceURL;
return jsString(this->m_vm, m_sourceURL);
}

JSC::JSString* JSCStackFrame::functionName()
Expand All @@ -202,7 +202,7 @@ JSC::JSString* JSCStackFrame::functionName()
m_functionName = retrieveFunctionName();
}

return m_functionName;
return jsString(this->m_vm, m_functionName);
}

JSC::JSString* JSCStackFrame::typeName()
Expand All @@ -211,7 +211,7 @@ JSC::JSString* JSCStackFrame::typeName()
m_typeName = retrieveTypeName();
}

return m_typeName;
return jsString(this->m_vm, m_typeName);
}

JSCStackFrame::SourcePositions* JSCStackFrame::getSourcePositions()
Expand All @@ -223,91 +223,61 @@ JSCStackFrame::SourcePositions* JSCStackFrame::getSourcePositions()
return (SourcePositionsState::Calculated == m_sourcePositionsState) ? &m_sourcePositions : nullptr;
}

ALWAYS_INLINE JSC::JSString* JSCStackFrame::retrieveSourceURL()
ALWAYS_INLINE String JSCStackFrame::retrieveSourceURL()
{
static auto sourceURLWasmString = MAKE_STATIC_STRING_IMPL("[wasm code]");
static auto sourceURLNativeString = MAKE_STATIC_STRING_IMPL("[native code]");

if (m_isWasmFrame) {
return jsOwnedString(m_vm, sourceURLWasmString);
return String(sourceURLWasmString);
}

if (!m_codeBlock) {
return jsOwnedString(m_vm, sourceURLNativeString);
return String(sourceURLNativeString);
}

String sourceURL = m_codeBlock->ownerExecutable()->sourceURL();
return sourceURL.isNull() ? m_vm.smallStrings.emptyString() : JSC::jsString(m_vm, sourceURL);
return m_codeBlock->ownerExecutable()->sourceURL();
}

ALWAYS_INLINE JSC::JSString* JSCStackFrame::retrieveFunctionName()
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");

if (m_isWasmFrame) {
return jsString(m_vm, JSC::Wasm::makeString(m_wasmFunctionIndexOrName));
return JSC::Wasm::makeString(m_wasmFunctionIndexOrName);
}

if (m_codeBlock) {
switch (m_codeBlock->codeType()) {
case JSC::EvalCode:
return JSC::jsOwnedString(m_vm, functionNameEvalCodeString);
return String(functionNameEvalCodeString);
case JSC::ModuleCode:
return JSC::jsOwnedString(m_vm, functionNameModuleCodeString);
return String(functionNameModuleCodeString);
case JSC::FunctionCode:
break;
case JSC::GlobalCode:
return JSC::jsOwnedString(m_vm, functionNameGlobalCodeString);
return String(functionNameGlobalCodeString);
default:
ASSERT_NOT_REACHED();
}
}

if (!m_callee || !m_callee->isObject()) {
return m_vm.smallStrings.emptyString();
String name;
if (m_callee) {
if (m_callee->isObject())
name = getCalculatedDisplayName(m_vm, jsCast<JSObject*>(m_callee)).impl();
}

JSC::JSObject* calleeAsObject = JSC::jsCast<JSC::JSObject*>(m_callee);

// First, try the "displayName" property
JSC::JSValue displayName = calleeAsObject->getDirect(m_vm, m_vm.propertyNames->displayName);
if (displayName && isJSString(displayName)) {
return JSC::asString(displayName);
}

// Our addition - if there's no "dispalyName" property, try the "name" property
JSC::JSValue name = calleeAsObject->getDirect(m_vm, m_vm.propertyNames->name);
if (name && isJSString(name)) {
return JSC::asString(name);
}

/* For functions (either JSFunction or InternalFunction), fallback to their "native" name property.
* Based on JSC::getCalculatedDisplayName, "inlining" the
* JSFunction::calculatedDisplayName\InternalFunction::calculatedDisplayName calls */
if (JSC::JSFunction* function = JSC::jsDynamicCast<JSC::JSFunction*>(calleeAsObject)) {
// Based on JSC::JSFunction::calculatedDisplayName, skipping the "displayName" property check
WTF::String actualName = function->name(m_vm);
if (!actualName.isEmpty() || function->isHostOrBuiltinFunction()) {
return JSC::jsString(m_vm, actualName);
}

return JSC::jsString(m_vm, function->jsExecutable()->name().string());
}
if (JSC::InternalFunction* function = JSC::jsDynamicCast<JSC::InternalFunction*>(calleeAsObject)) {
// Based on JSC::InternalFunction::calculatedDisplayName, skipping the "displayName" property check
return JSC::jsString(m_vm, function->name());
}

return m_vm.smallStrings.emptyString();
return name.isNull() ? emptyString() : name;
}

ALWAYS_INLINE JSC::JSString* JSCStackFrame::retrieveTypeName()
ALWAYS_INLINE String JSCStackFrame::retrieveTypeName()
{
JSC::JSObject* calleeObject = JSC::jsCast<JSC::JSObject*>(m_callee);
// return JSC::jsTypeStringForValue(m_globalObjectcalleeObject->toThis()
return jsString(m_vm, makeString(calleeObject->className()));
return calleeObject->className();
}

// General flow here is based on JSC's appendSourceToError (ErrorInstance.cpp)
Expand Down
12 changes: 6 additions & 6 deletions src/bun.js/bindings/ErrorStackTrace.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ class JSCStackFrame {
JSC::BytecodeIndex m_bytecodeIndex;

// Lazy-initialized
JSC::JSString* m_sourceURL;
JSC::JSString* m_functionName;
JSC::JSString* m_typeName;
WTF::String m_sourceURL;
WTF::String m_functionName;
WTF::String m_typeName;

// m_wasmFunctionIndexOrName has meaning only when m_isWasmFrame is set
JSC::Wasm::IndexOrName m_wasmFunctionIndexOrName;
Expand Down Expand Up @@ -105,7 +105,7 @@ class JSCStackFrame {
bool isConstructor() const { return m_codeBlock && (JSC::CodeForConstruct == m_codeBlock->specializationKind()); }

private:
ALWAYS_INLINE JSC::JSString* retrieveSourceURL();
ALWAYS_INLINE String retrieveSourceURL();

/* Regarding real functions (not eval\module\global code), both v8 and JSC seem to follow
* the same logic, which is to first try the function's "display name", and if it's not defined,
Expand All @@ -119,9 +119,9 @@ class JSCStackFrame {
* and just try to use the "name" property when needed, so our lookup will be:
* "display name" property -> "name" property -> JSFunction\InternalFunction "name" methods.
*/
ALWAYS_INLINE JSC::JSString* retrieveFunctionName();
ALWAYS_INLINE String retrieveFunctionName();

ALWAYS_INLINE JSC::JSString* retrieveTypeName();
ALWAYS_INLINE String retrieveTypeName();

bool calculateSourcePositions();
};
Expand Down
Loading

0 comments on commit ff7f642

Please sign in to comment.