Refactor: Move error stack trace code to FormatStackTraceForJS #23558
Conversation
…Info This commit extracts all error/stack trace-related code from ZigGlobalObject.cpp into a new dedicated JSErrorInfo.cpp file for better code organization. Changes: - Created JSErrorInfo.h and JSErrorInfo.cpp with all error/stack functions - Moved Error.captureStackTrace implementation - Moved Error.prepareStackTrace handling - Moved computeErrorInfoWithPrepareStackTrace and related functions - Moved Bun::formatStackTrace and stack formatting utilities - Moved errorInstanceLazyStackCustomGetter/Setter - Moved GlobalObject::createCallSitesFromFrames to Zig namespace - Moved errorConstructorFuncAppendStackTrace - Moved jsFunctionDefaultErrorPrepareStackTrace - Moved DEFAULT_ERROR_STACK_TRACE_LIMIT constant - Updated ZigGlobalObject.h to remove moved function declarations - Updated ZigGlobalObject.cpp to remove moved implementations - Added JSErrorInfo.h include to BunProcess.cpp All error/stack trace functionality remains unchanged, this is purely a code organization refactor to improve maintainability.
The new name better describes what the code does - it formats stack traces for JavaScript error objects.
|
Updated 6:17 PM PT - Oct 12th, 2025
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 23558That installs a local version of the PR into your bun-23558 --bun |
WalkthroughIntroduces a new FormatStackTraceForJS module (header + implementation) that centralizes JS error stack formatting, CallSite creation, prepareStackTrace support, and lazy error.stack handling. Removes prior in-file formatting helpers from ZigGlobalObject files and updates includes to use the new module. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{cpp,h}📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
Files:
**/*.cpp📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
Files:
src/bun.js/bindings/**/*.cpp📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-09-08T00:41:12.052ZApplied to files:
📚 Learning: 2025-09-08T00:41:12.052ZApplied to files:
📚 Learning: 2025-08-30T00:13:36.815ZApplied to files:
📚 Learning: 2025-08-30T00:11:00.890ZApplied to files:
📚 Learning: 2025-08-30T00:13:36.815ZApplied to files:
📚 Learning: 2025-08-30T00:11:57.076ZApplied to files:
📚 Learning: 2025-08-30T00:11:00.890ZApplied to files:
📚 Learning: 2025-08-30T00:13:36.815ZApplied to files:
📚 Learning: 2025-08-30T00:11:00.890ZApplied to files:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/bun.js/bindings/BunProcess.cpp(1 hunks)src/bun.js/bindings/FormatStackTraceForJS.cpp(1 hunks)src/bun.js/bindings/FormatStackTraceForJS.h(1 hunks)src/bun.js/bindings/ZigGlobalObject.cpp(1 hunks)src/bun.js/bindings/ZigGlobalObject.h(0 hunks)
💤 Files with no reviewable changes (1)
- src/bun.js/bindings/ZigGlobalObject.h
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{cpp,h}
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible
Files:
src/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FormatStackTraceForJS.cpp
**/*.cpp
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue
Files:
src/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FormatStackTraceForJS.cpp
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: When a JS class has a public constructor, implement Foo, FooPrototype, and FooConstructor (JSDestructibleObject, JSNonFinalObject, InternalFunction)
Define class properties using HashTableValue arrays in C++ bindings
Add iso subspaces for classes with C++ fields
Cache structures in ZigGlobalObject for C++-backed classes
Files:
src/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FormatStackTraceForJS.cpp
src/bun.js/bindings/ZigGlobalObject.cpp
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
Initialize the LazyClassStructure in GlobalObject::finishCreation and visit it in GlobalObject::visitChildrenImpl
Files:
src/bun.js/bindings/ZigGlobalObject.cpp
src/bun.js/bindings/ZigGlobalObject.{h,cpp}
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
If only a class (no public constructor/prototype), use JSC::LazyProperty<JSGlobalObject, Structure>, initialize it in finishCreation, and visit it in visitChildren
Files:
src/bun.js/bindings/ZigGlobalObject.cpp
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.zig : Wrap the Bun__<Type>__toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
Applied to files:
src/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FormatStackTraceForJS.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Applied to files:
src/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FormatStackTraceForJS.cpp
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Applied to files:
src/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FormatStackTraceForJS.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Applied to files:
src/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.zig : Wrap the Bun__<Type>__toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Applied to files:
src/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.{h,cpp} : If only a class (no public constructor/prototype), use JSC::LazyProperty<JSGlobalObject, Structure>, initialize it in finishCreation, and visit it in visitChildren
Applied to files:
src/bun.js/bindings/FormatStackTraceForJS.hsrc/bun.js/bindings/FormatStackTraceForJS.cpp
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for C++-backed classes
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.h : When there is a class, prototype, and constructor, add a JSC::LazyClassStructure field for the class to ZigGlobalObject.h
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : Add new V8 API method mangled symbols to the V8API struct in src/napi/napi.zig for both GCC/Clang and MSVC
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.{cpp,h} : If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
🧬 Code graph analysis (2)
src/bun.js/bindings/FormatStackTraceForJS.h (3)
src/bun.js/bindings/ZigGlobalObject.cpp (3)
GlobalObject(779-792)GlobalObject(794-807)GlobalObject(809-815)src/bun.js/bindings/ErrorStackTrace.h (1)
JSCStackTrace(162-209)src/bun.js/bindings/FormatStackTraceForJS.cpp (8)
formatStackTrace(163-380)formatStackTrace(163-173)computeErrorInfoWrapperToString(547-567)computeErrorInfoWrapperToString(547-547)computeErrorInfoWrapperToJSValue(569-580)computeErrorInfoWrapperToJSValue(569-569)createCallSitesFromFrames(720-742)createCallSitesFromFrames(720-720)
src/bun.js/bindings/FormatStackTraceForJS.cpp (3)
src/bun.js/bindings/FormatStackTraceForJS.h (1)
JSC(33-40)src/bun.js/bindings/BunClientData.h (1)
clientData(200-203)src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(363-368)create(363-363)create(370-375)create(370-370)create(377-382)create(377-377)create(384-389)create(384-384)
🔇 Additional comments (5)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
69-69: LGTM! Include directive correctly added.The include for
FormatStackTraceForJS.his appropriately placed and necessary for accessing the stack-trace formatting functions that were moved out of this file.src/bun.js/bindings/FormatStackTraceForJS.cpp (4)
153-157: Verify exception handling for property access.The code uses
CLEAR_IF_EXCEPTIONwhen accessingprepareStackTracefrom a non-Zig global object. While this defensive approach prevents crashes, it silently swallows all exceptions (including potential programming errors beyond missing properties).Consider whether specific exception types should be logged or handled differently for debugging purposes.
188-189: Track the unresolved FIXME.The FIXME comment about vector capacity has been carried over from the original code location. This references a known issue from October 2023 but doesn't appear to cause functional problems.
Consider either:
- Investigating and resolving the root cause
- Removing the FIXME if the issue is not impactful
- Creating a tracking issue if it's a known limitation
383-415: Verify memory allocation safety in finalizer context.Line 398 states "we are not allowed to allocate memory" because this function can be called from a finalizer. However, the function:
- Calls
sanitizedNameString()andsanitizedMessageString()(lines 403-406)- Calls
formatStackTrace()which usesStringBuilder(line 414)Ensure these operations are safe in a finalizer context. If
StringBuilderallocations are permissible but other allocations aren't, consider adding clarifying comments about what types of allocations are restricted.
691-703: Verify DeletePropertyMode for stack property removal.When re-capturing a stack trace on an ErrorInstance, the code deletes the existing
stackproperty usingVM::DeletePropertyMode::IgnoreConfigurable(line 695).Confirm this is the intended behavior. According to the V8 API,
Error.captureStackTrace()should overwrite the stack property regardless of its configurability, so this appears correct. However, verify that this doesn't cause issues with stack properties that have special attributes.
| #include "root.h" | ||
| #include "JavaScriptCore/ArgList.h" | ||
| #include "JavaScriptCore/JSCJSValue.h" | ||
| #include "JavaScriptCore/JSObject.h" | ||
|
|
||
| namespace JSC { | ||
| class VM; | ||
| class JSGlobalObject; | ||
| class StackFrame; | ||
| class ErrorInstance; | ||
| class CallFrame; | ||
| enum class PropertyAttribute : unsigned; | ||
| } // namespace JSC | ||
|
|
||
| namespace WTF { | ||
| class String; | ||
| class OrdinalNumber; | ||
| } // namespace WTF | ||
|
|
||
| namespace Zig { | ||
| class GlobalObject; | ||
| class JSCStackTrace; | ||
| } // namespace Zig | ||
|
|
||
| using JSC::EncodedJSValue; | ||
| using JSC::PropertyName; | ||
|
|
||
| namespace Bun { | ||
|
|
||
| // Constants | ||
| constexpr size_t DEFAULT_ERROR_STACK_TRACE_LIMIT = 10; | ||
|
|
||
| // Main stack trace formatting function | ||
| WTF::String formatStackTrace( | ||
| JSC::VM& vm, | ||
| Zig::GlobalObject* globalObject, | ||
| JSC::JSGlobalObject* lexicalGlobalObject, | ||
| const WTF::String& name, | ||
| const WTF::String& message, | ||
| WTF::OrdinalNumber& line, | ||
| WTF::OrdinalNumber& column, | ||
| WTF::String& sourceURL, | ||
| WTF::Vector<JSC::StackFrame>& stackTrace, | ||
| JSC::JSObject* errorInstance); | ||
|
|
||
| // JSC Host Functions - Error constructor methods | ||
| JSC_DECLARE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace); | ||
| JSC_DECLARE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace); | ||
| JSC_DECLARE_HOST_FUNCTION(jsFunctionDefaultErrorPrepareStackTrace); | ||
|
|
||
| // JSC Custom Accessors - error.stack getter/setter | ||
| JSC_DECLARE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter); | ||
| JSC_DECLARE_CUSTOM_SETTER(errorInstanceLazyStackCustomSetter); | ||
|
|
||
| // 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); | ||
| JSC::JSValue computeErrorInfoWrapperToJSValue(JSC::VM& vm, WTF::Vector<JSC::StackFrame>& stackTrace, unsigned int& line_in, unsigned int& column_in, WTF::String& sourceURL, JSC::JSObject* errorInstance, void* bunErrorData); | ||
|
|
||
| } // namespace Bun | ||
|
|
||
| namespace Zig { | ||
|
|
||
| // GlobalObject member function for creating CallSite objects | ||
| void createCallSitesFromFrames( | ||
| Zig::GlobalObject* globalObject, | ||
| JSC::JSGlobalObject* lexicalGlobalObject, | ||
| Zig::JSCStackTrace& stackTrace, | ||
| JSC::MarkedArgumentBuffer& callSites); |
There was a problem hiding this comment.
Missing includes for WTF::Vector and JSC::MarkedArgumentBuffer.
This header uses WTF::Vector<StackFrame> and JSC::MarkedArgumentBuffer&, but it neither includes <wtf/Vector.h> nor declares MarkedArgumentBuffer. The template cannot be forward declared (signature depends on multiple template parameters), so compilation will fail as soon as this header is consumed elsewhere. Please add the required headers (or forward declarations where legal). Example fix:
#include "root.h"
#include "JavaScriptCore/ArgList.h"
#include "JavaScriptCore/JSCJSValue.h"
#include "JavaScriptCore/JSObject.h"
+#include "JavaScriptCore/MarkedArgumentBuffer.h"
+#include <wtf/Vector.h>
@@
namespace JSC {
class VM;
class JSGlobalObject;
class StackFrame;
class ErrorInstance;
class CallFrame;
enum class PropertyAttribute : unsigned;
+class MarkedArgumentBuffer;
} // namespace JSC📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include "root.h" | |
| #include "JavaScriptCore/ArgList.h" | |
| #include "JavaScriptCore/JSCJSValue.h" | |
| #include "JavaScriptCore/JSObject.h" | |
| namespace JSC { | |
| class VM; | |
| class JSGlobalObject; | |
| class StackFrame; | |
| class ErrorInstance; | |
| class CallFrame; | |
| enum class PropertyAttribute : unsigned; | |
| } // namespace JSC | |
| namespace WTF { | |
| class String; | |
| class OrdinalNumber; | |
| } // namespace WTF | |
| namespace Zig { | |
| class GlobalObject; | |
| class JSCStackTrace; | |
| } // namespace Zig | |
| using JSC::EncodedJSValue; | |
| using JSC::PropertyName; | |
| namespace Bun { | |
| // Constants | |
| constexpr size_t DEFAULT_ERROR_STACK_TRACE_LIMIT = 10; | |
| // Main stack trace formatting function | |
| WTF::String formatStackTrace( | |
| JSC::VM& vm, | |
| Zig::GlobalObject* globalObject, | |
| JSC::JSGlobalObject* lexicalGlobalObject, | |
| const WTF::String& name, | |
| const WTF::String& message, | |
| WTF::OrdinalNumber& line, | |
| WTF::OrdinalNumber& column, | |
| WTF::String& sourceURL, | |
| WTF::Vector<JSC::StackFrame>& stackTrace, | |
| JSC::JSObject* errorInstance); | |
| // JSC Host Functions - Error constructor methods | |
| JSC_DECLARE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace); | |
| JSC_DECLARE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace); | |
| JSC_DECLARE_HOST_FUNCTION(jsFunctionDefaultErrorPrepareStackTrace); | |
| // JSC Custom Accessors - error.stack getter/setter | |
| JSC_DECLARE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter); | |
| JSC_DECLARE_CUSTOM_SETTER(errorInstanceLazyStackCustomSetter); | |
| // 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); | |
| JSC::JSValue computeErrorInfoWrapperToJSValue(JSC::VM& vm, WTF::Vector<JSC::StackFrame>& stackTrace, unsigned int& line_in, unsigned int& column_in, WTF::String& sourceURL, JSC::JSObject* errorInstance, void* bunErrorData); | |
| } // namespace Bun | |
| namespace Zig { | |
| // GlobalObject member function for creating CallSite objects | |
| void createCallSitesFromFrames( | |
| Zig::GlobalObject* globalObject, | |
| JSC::JSGlobalObject* lexicalGlobalObject, | |
| Zig::JSCStackTrace& stackTrace, | |
| JSC::MarkedArgumentBuffer& callSites); | |
| #include "root.h" | |
| #include "JavaScriptCore/ArgList.h" | |
| #include "JavaScriptCore/JSCJSValue.h" | |
| #include "JavaScriptCore/JSObject.h" | |
| #include "JavaScriptCore/MarkedArgumentBuffer.h" | |
| #include <wtf/Vector.h> | |
| namespace JSC { | |
| class VM; | |
| class JSGlobalObject; | |
| class StackFrame; | |
| class ErrorInstance; | |
| class CallFrame; | |
| enum class PropertyAttribute : unsigned; | |
| class MarkedArgumentBuffer; | |
| } // namespace JSC | |
| namespace WTF { | |
| class String; | |
| class OrdinalNumber; | |
| } // namespace WTF | |
| namespace Zig { | |
| class GlobalObject; | |
| class JSCStackTrace; | |
| } // namespace Zig | |
| using JSC::EncodedJSValue; | |
| using JSC::PropertyName; | |
| // … rest of the file unchanged … |
🤖 Prompt for AI Agents
In src/bun.js/bindings/FormatStackTraceForJS.h around lines 28 to 95, the header
uses WTF::Vector<...> and JSC::MarkedArgumentBuffer but doesn't include their
definitions which breaks compilation; add #include <wtf/Vector.h> and #include
"JavaScriptCore/MarkedArgumentBuffer.h" near the other includes (or, if you
prefer, forward-declare JSC::MarkedArgumentBuffer and only include the Vector
header since the template cannot be forward declared) so the signatures compile
wherever this header is consumed.
BunProcessReportObjectWindows.cpp had an extern forward declaration of formatStackTrace with the old signature. This caused a linker error on Windows because the actual implementation has a different signature (references instead of pass-by-value for OrdinalNumber params). The proper declaration is now in FormatStackTraceForJS.h which is already included, so the extern declaration is no longer needed.
2 participants
Summary
This PR extracts all error/stack trace-related code from
ZigGlobalObject.cppinto a new dedicatedFormatStackTraceForJS.{h,cpp}file for better code organization and maintainability.Changes
New Files Created
src/bun.js/bindings/FormatStackTraceForJS.h- Header with all error/stack trace function declarationssrc/bun.js/bindings/FormatStackTraceForJS.cpp- Implementation file with all moved functionsFunctions Moved (18 total)
Core Formatting Functions:
Bun::formatStackTrace- Main stack trace formatting with source mapping supportformatStackTraceToJSValue(2 overloads) - Default stack trace formattingformatStackTraceToJSValueWithoutPrepareStackTrace- Handles prepareStackTrace retrievalError Info Computation:
computeErrorInfoWithoutPrepareStackTrace- Called by error.stack propertycomputeErrorInfoWithPrepareStackTrace- Handles Error.prepareStackTrace callbackcomputeErrorInfoToString,computeErrorInfoToJSValue- Wrapper functionscomputeErrorInfoToJSValueWithoutSkipping- Main JSValue computation logiccomputeErrorInfoWrapperToString,computeErrorInfoWrapperToJSValue- JSC callback wrappersError Constructor Functions:
errorConstructorFuncCaptureStackTrace- Error.captureStackTrace() (V8/Node.js compatibility)errorConstructorFuncAppendStackTrace- Error.appendStackTrace()jsFunctionDefaultErrorPrepareStackTrace- Default Error.prepareStackTrace implementationError Instance Accessors:
errorInstanceLazyStackCustomGetter- Lazy getter for error.stack propertyerrorInstanceLazyStackCustomSetter- Setter for error.stack propertyUtility:
Zig::createCallSitesFromFrames- Creates CallSite objects from stack framesDEFAULT_ERROR_STACK_TRACE_LIMITconstantFiles Modified
ZigGlobalObject.h- Removed moved function declarationsZigGlobalObject.cpp- Removed moved implementations (~630 lines), added FormatStackTraceForJS.h includeBunProcess.cpp- Added FormatStackTraceForJS.h include (usesBun::formatStackTrace)Testing
Rationale
This refactor improves code organization by:
Note: This is a pure refactor with no functional changes - all existing behavior remains identical.
🤖 Generated with Claude Code