Skip to content

Conversation

@sosukesuzuki
Copy link
Member

@sosukesuzuki sosukesuzuki commented Sep 9, 2025

What does this PR do?

Enables async stack traces

How did you verify your code works?

Added tests

@robobun
Copy link
Collaborator

robobun commented Sep 9, 2025

Updated 5:06 AM PT - Sep 11th, 2025

@autofix-ci[bot], your commit bf70b53 has 1 failures in Build #25790:


🧪   To try this PR locally:

bunx bun-pr 22517

That installs a local version of the PR into your bun-22517 executable, so you can run:

bun-22517 --bun

@sosukesuzuki sosukesuzuki changed the title Enable async stack traces WIP: Enable async stack traces Sep 9, 2025
@sosukesuzuki sosukesuzuki force-pushed the eng/enable-async-stack-trace branch from ed2255a to 0b890d7 Compare September 11, 2025 00:15
@sosukesuzuki sosukesuzuki changed the title WIP: Enable async stack traces Enable async stack traces Sep 11, 2025
@sosukesuzuki sosukesuzuki marked this pull request as ready for review September 11, 2025 05:40
frame.isGlobalCode = true;
}

if (functionName.startsWith("async "_s)) {
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe the

 if (functionName == "<anonymous>"_s) {
            functionName = StringView();
        }

check should be moved after this? incase async <anonymous>?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines +4692 to +4694
if (functionName.startsWith("async "_s)) {
frame.isAsync = true;
functionName = functionName.substring(6);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jarred-Sumner do you have any idea to write tests for this?

Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this:

const foo = async function foo() {
  const err = new Error();
  err.stack;
  return err;
};
Object.defineProperty(foo, "name", {  value: "foobar", configurable: true, enumerable: true, writable: true });
expect(Bun.inspect(await foo())).toMatchInlineSnapshot();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bun.inspect(error) show the only latest frame whether asynchronous or synchronous?

Fo example,

{
  const foo = function foo() {
    return bar();
  }
  function bar() {
    return baz();
  }
  function baz() {
    return qux();
  }
  function qux() {
    const error = new Error("error from qux");
    error.stack;
    return error;
  }

  Object.defineProperty(foo, "name", { value: "foobar", configurable: true, enumerable: true, writable: true });
  console.log(Bun.inspect(foo()));
}

outputs

28 |   }
29 |   function baz() {
30 |     return qux();
31 |   }
32 |   function qux() {
33 |     const error = new Error("error from qux");
                           ^
error: error from qux
      at qux (/Users/sosukesuzuki/ghq/github.com/oven-sh/bun/build/debug/test.js:33:23)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should show several frames. But those frames are probably being inlined since it's a single function call.

const error = await foo();
const stack = error.stack!;

const quxIdx = stack.indexOf("at qux (")!;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use an inline snapshot test instead and then you can normalize it with normalizeBunSnapshot? and then run it once to generate it:

expect(normalizeBunSnapshot(stack)).toMatchInlineSnapshot()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

Walkthrough

Updated default WebKit commit and enabled end-to-end async stack-trace support by adding async flags to parsed frames, propagating them through C++/Zig bindings, labeling formatted frames with "async ", and adding tests and defaults to surface async frames.

Changes

Cohort / File(s) Summary of changes
WebKit default version
cmake/tools/SetupWebKit.cmake
Updated default WEBKIT_VERSION commit hash; no other logic changed.
CallSite flags & accessors
src/bun.js/bindings/CallSite.h, src/bun.js/bindings/CallSite.cpp, src/bun.js/bindings/CallSitePrototype.cpp
Added IsAsync flag and isAsync() accessor; set IsAsync on call-site flags for async frames; CallSite prototype isAsync now returns actual async status.
ErrorStackTrace types & accessors
src/bun.js/bindings/ErrorStackTrace.h, src/bun.js/bindings/ErrorStackTrace.cpp
Added m_isAsync to JSCStackFrame, initialized from frame.isAsyncFrame(); added isAsync() accessor.
Bindings glue & parsed frames
src/bun.js/bindings/bindings.cpp, src/bun.js/bindings/headers-handwritten.h
Added isAsync / is_async to parsed and Zig-visible frames; detect and strip "async " prefix in parsed function names and set async state; propagate to Zig frames.
Zig stack frame & formatting
src/bun.js/bindings/ZigStackFrame.zig
Added is_async: bool to ZigStackFrame and NameFormatter; default false; formatting now prefixes "async " (with color tag when enabled) for async frames; zero/init updated.
Global init & rendering
src/bun.js/bindings/ZigGlobalObject.cpp
Enabled async stack traces via JSC::Options::useAsyncStackTrace() and prepend "async " when rendering async frames in formatted stack traces.
Dev server defaults
src/bake/DevServer/ErrorReportRequest.zig
When appending ZigStackFrame, explicitly set .is_async = false for constructed frames.
Tests & harness
test/js/bun/test/stack.test.ts, test/js/node/v8/capture-stack-trace.test.js
Added tests asserting async function frames appear and are ordered; added/imported normalizeBunSnapshot for stable snapshots; added test inspecting CallSite.isAsync().

Possibly related PRs

  • Upgrade WebKit #22499: Overlaps in updating cmake/tools/SetupWebKit.cmake default WEBKIT_VERSION and touches async stack-trace handling.
  • bump webkit #22256: Modifies the same cmake/tools/SetupWebKit.cmake default WEBKIT_VERSION commit hash.

Suggested reviewers

  • Jarred-Sumner

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Enable async stack traces" is a concise, single-sentence summary that directly reflects the primary change in the diff (introducing async-frame flags, plumbing, formatting, and tests) and is clear enough for a reviewer to understand the main intent.
Description Check ✅ Passed The PR description follows the repository template by including both required sections ("What does this PR do?" and "How did you verify your code works?") and succinctly states that async stack traces are enabled and that tests were added, satisfying the template's structure.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1247795 and bf70b53.

📒 Files selected for processing (1)
  • test/js/node/v8/capture-stack-trace.test.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/js/node/v8/capture-stack-trace.test.js
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch eng/enable-async-stack-trace

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/bun.js/bindings/ErrorStackTrace.cpp (2)

6-8: Include root.h first to satisfy lints.

All binding .cpp files should include root.h at the top.

Apply:

+#include "root.h"
 #include "config.h"
 #include "ErrorStackTrace.h"

366-374: Possible null deref of frame.codeBlock().

Within the hasLineAndColumnInfo branch you dereference frame.codeBlock() without checking for null; this can be null for native/builtin frames with synthesized positions.

Use a guard:

-        auto codeType = frame.codeBlock()->codeType();
-        if (codeType == JSC::FunctionCode || codeType == JSC::EvalCode) {
-            m_isFunctionOrEval = true;
-        }
+        if (auto* cb = frame.codeBlock()) {
+            auto codeType = cb->codeType();
+            if (codeType == JSC::FunctionCode || codeType == JSC::EvalCode) {
+                m_isFunctionOrEval = true;
+            }
+        }
src/bun.js/bindings/CallSite.cpp (2)

6-8: Include root.h first to satisfy lints.

Apply:

+#include "root.h"
 #include "config.h"
 #include "CallSite.h"

127-147: Prefix “async ” in CallSite formatting for parity with default formatter.

Default stack formatting (Bun::formatStackTrace) already prefixes async frames. CallSite::formatAsString should do the same so Error.prepareStackTrace users see consistent output.

Apply:

     if (functionName.length() > 0) {
 
+        if (isAsync()) {
+            sb.append("async "_s);
+        }
+
         if (isConstructor()) {
             sb.append("new "_s);
         }
 
         if (auto* object = thisValue.getObject()) {
🧹 Nitpick comments (5)
cmake/tools/SetupWebKit.cmake (4)

8-9: Guard against short/invalid WEBKIT_VERSION before SUBSTRING.

If a user passes a non-SHA (or a BOOL via -DWEBKIT_VERSION=ON), SUBSTRING will yield a bad prefix (“ON”), breaking cache paths and downloads. Add a length check.

-string(SUBSTRING ${WEBKIT_VERSION} 0 16 WEBKIT_VERSION_PREFIX)
+string(LENGTH "${WEBKIT_VERSION}" WEBKIT_VERSION_LENGTH)
+if(WEBKIT_VERSION_LENGTH LESS 16)
+  message(FATAL_ERROR "WEBKIT_VERSION must be a 16+ char git SHA (got length ${WEBKIT_VERSION_LENGTH}).")
+endif()
+string(SUBSTRING "${WEBKIT_VERSION}" 0 16 WEBKIT_VERSION_PREFIX)

10-14: Avoid cross-OS/arch cache collisions for prebuilt WebKit.

DEFAULT_WEBKIT_PATH only keys by commit prefix. On shared caches (CI, dev machines switching targets), macOS/Linux/asan/lto artifacts can overwrite each other and the regex check below won’t catch it. Include OS/arch/suffix in the cache path and set it after those vars are computed.

-else()
-  set(DEFAULT_WEBKIT_PATH ${CACHE_PATH}/webkit-${WEBKIT_VERSION_PREFIX})
-endif()
-
-option(WEBKIT_PATH "The path to the WebKit directory")
-
-if(NOT WEBKIT_PATH)
-  set(WEBKIT_PATH ${DEFAULT_WEBKIT_PATH})
-endif()
+else()
+  # Defer DEFAULT_WEBKIT_PATH for remote builds until after OS/ARCH/SUFFIX are known.
+endif()
+
+option(WEBKIT_PATH "The path to the WebKit directory")

Add after ASAN suffix is finalized (right after Line 80):

+if(NOT WEBKIT_LOCAL)
+  set(DEFAULT_WEBKIT_PATH
+      ${CACHE_PATH}/webkit-${WEBKIT_VERSION_PREFIX}-${WEBKIT_OS}-${WEBKIT_ARCH}${WEBKIT_SUFFIX})
+  if(NOT WEBKIT_PATH)
+    set(WEBKIT_PATH ${DEFAULT_WEBKIT_PATH})
+  endif()
+endif()

Also applies to: 18-20


1-1: Use CACHE STRING/PATH instead of option() for non-boolean vars.

option() is BOOL; it can coerce values to ON/OFF in GUIs and confuses type. Prefer typed cache vars.

-option(WEBKIT_VERSION "The version of WebKit to use")
+set(WEBKIT_VERSION "" CACHE STRING "The git SHA of WebKit to use")-option(WEBKIT_PATH "The path to the WebKit directory")
+set(WEBKIT_PATH "" CACHE PATH "The path to the WebKit directory")

Also applies to: 16-16


85-91: Use CMake JSON parsing for an exact package.json version check (repo uses CMake 3.24)

File: cmake/tools/SetupWebKit.cmake (lines 85–91) — top-level CMakeLists.txt declares cmake_minimum_required(VERSION 3.24), so replace the loose MATCHES check with a JSON parse + exact string comparison.

-  if(WEBKIT_PACKAGE_JSON MATCHES ${WEBKIT_VERSION})
-    return()
-  endif()
+  # CMake 3.19+: parse JSON and compare exactly
+  string(JSON WEBKIT_JSON_VERSION GET "${WEBKIT_PACKAGE_JSON}" version)
+  if(WEBKIT_JSON_VERSION STREQUAL "${WEBKIT_VERSION}")
+    return()
+  endif()
src/bun.js/bindings/bindings.cpp (1)

4603-4603: Nit: parentheses checks and potential underflow in function-name slicing

Minor cleanup and safety:

  • The condition duplicates closingParentheses on both sides; it should likely also check openingParentheses.
  • If openingParentheses == WTF::notFound, openingParentheses - 1 can underflow.

Suggested tweak:

-        if (closingParentheses == WTF::notFound || closingParentheses == WTF::notFound) {
+        if (openingParentheses == WTF::notFound || closingParentheses == WTF::notFound) {
             offset = stack.length();
             return false;
         }
@@
-        StringView functionName = line.substring(0, openingParentheses - 1);
+        const auto func_end = openingParentheses != WTF::notFound && openingParentheses > 0
+            ? openingParentheses - 1
+            : line.length();
+        StringView functionName = line.substring(0, func_end);

Also applies to: 4681-4683

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5551d and 3daa574.

📒 Files selected for processing (11)
  • cmake/tools/SetupWebKit.cmake (1 hunks)
  • src/bake/DevServer/ErrorReportRequest.zig (1 hunks)
  • src/bun.js/bindings/CallSite.cpp (1 hunks)
  • src/bun.js/bindings/CallSite.h (2 hunks)
  • src/bun.js/bindings/ErrorStackTrace.cpp (2 hunks)
  • src/bun.js/bindings/ErrorStackTrace.h (2 hunks)
  • src/bun.js/bindings/ZigGlobalObject.cpp (2 hunks)
  • src/bun.js/bindings/ZigStackFrame.zig (4 hunks)
  • src/bun.js/bindings/bindings.cpp (3 hunks)
  • src/bun.js/bindings/headers-handwritten.h (1 hunks)
  • test/js/bun/test/stack.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{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/ErrorStackTrace.cpp
  • src/bun.js/bindings/headers-handwritten.h
  • src/bun.js/bindings/bindings.cpp
  • src/bun.js/bindings/CallSite.h
  • src/bun.js/bindings/ZigGlobalObject.cpp
  • src/bun.js/bindings/CallSite.cpp
  • src/bun.js/bindings/ErrorStackTrace.h
**/*.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/ErrorStackTrace.cpp
  • src/bun.js/bindings/bindings.cpp
  • src/bun.js/bindings/ZigGlobalObject.cpp
  • src/bun.js/bindings/CallSite.cpp
**/*.{cpp,cc,cxx,h,hpp,hxx}

📄 CodeRabbit inference engine (CLAUDE.md)

Format C/C++ sources/headers with clang-format (bun run clang-format)

Files:

  • src/bun.js/bindings/ErrorStackTrace.cpp
  • src/bun.js/bindings/headers-handwritten.h
  • src/bun.js/bindings/bindings.cpp
  • src/bun.js/bindings/CallSite.h
  • src/bun.js/bindings/ZigGlobalObject.cpp
  • src/bun.js/bindings/CallSite.cpp
  • src/bun.js/bindings/ErrorStackTrace.h
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: For JS classes with a public constructor, implement Foo, FooPrototype, and FooConstructor (JSC patterns)
Define class properties using HashTableValue arrays in JSC bindings
Add iso subspaces for classes with C++ fields in JSC bindings
Cache structures in ZigGlobalObject for JSC-bound classes

Files:

  • src/bun.js/bindings/ErrorStackTrace.cpp
  • src/bun.js/bindings/bindings.cpp
  • src/bun.js/bindings/ZigGlobalObject.cpp
  • src/bun.js/bindings/CallSite.cpp
test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

  • test/js/bun/test/stack.test.ts
test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

  • test/js/bun/test/stack.test.ts
test/js/bun/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Files:

  • test/js/bun/test/stack.test.ts
test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

  • test/js/bun/test/stack.test.ts
test/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.ts: Name test files *.test.ts and use bun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; use port: 0 to get a random port
When spawning Bun in tests, use bunExe() and bunEnv from harness
Prefer async/await in tests; for a single callback, use Promise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Use tempDir/tempDirWithFiles from harness for temporary files and directories in tests
For large/repetitive strings in tests, prefer Buffer.alloc(count, fill).toString() over "A".repeat(count)
Import common test utilities from harness (e.g., bunExe, bunEnv, tempDirWithFiles, tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and use toThrow for synchronous errors
Use describe blocks for grouping, describe.each for parameterized tests, snapshots with toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach); track resources for cleanup in afterEach
Use using/await using with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests

Files:

  • test/js/bun/test/stack.test.ts
test/js/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Organize unit tests for specific features under test/js/ by module

Files:

  • test/js/bun/test/stack.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions

Files:

  • test/js/bun/test/stack.test.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Format JavaScript/TypeScript files with Prettier (bun run prettier)

Files:

  • test/js/bun/test/stack.test.ts
src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Implement debug logs in Zig using const log = bun.Output.scoped(.${SCOPE}, false); and invoking log("...", .{})

Files:

  • src/bake/DevServer/ErrorReportRequest.zig
  • src/bun.js/bindings/ZigStackFrame.zig
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup

Files:

  • src/bake/DevServer/ErrorReportRequest.zig
  • src/bun.js/bindings/ZigStackFrame.zig
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
src/bun.js/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy

Files:

  • src/bun.js/bindings/ZigStackFrame.zig
🧠 Learnings (22)
📚 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/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : In error tests, assert non-zero exit codes for failing processes and use `toThrow` for synchronous errors

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : Never write tests that merely assert absence of "panic" or "uncaught exception" in output

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Prefer `async/await` in tests; for a single callback, use `Promise.withResolvers()`

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 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/headers-handwritten.h
  • 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/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt

Applied to files:

  • src/bun.js/bindings/bindings.cpp
  • 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 : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors

Applied to files:

  • src/bun.js/bindings/bindings.cpp
  • src/bun.js/bindings/ZigGlobalObject.cpp
  • src/bun.js/bindings/ZigStackFrame.zig
📚 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/bindings.cpp
  • src/bun.js/bindings/ZigStackFrame.zig
📚 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/symbols.dyn : Add new V8 API mangled symbols (with leading underscore and semicolons) to src/symbols.dyn

Applied to files:

  • src/bun.js/bindings/bindings.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/bindings.cpp
  • 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/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility

Applied to files:

  • src/bun.js/bindings/bindings.cpp
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JSC-bound classes

Applied to files:

  • src/bun.js/bindings/bindings.cpp
  • 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/bindings.cpp
  • src/bun.js/bindings/ZigStackFrame.zig
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for classes with C++ fields in JSC bindings

Applied to files:

  • src/bun.js/bindings/bindings.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/V8*.cpp : Implement new V8 API classes in src/bun.js/bindings/v8/ as V8<Class>.cpp and include v8_compatibility_assertions.h with ASSERT_V8_TYPE_LAYOUT_MATCHES for the class

Applied to files:

  • src/bun.js/bindings/bindings.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/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/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
  • src/bun.js/bindings/ZigStackFrame.zig
📚 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/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/ZigGlobalObject.cpp
📚 Learning: 2025-09-02T17:14:46.924Z
Learnt from: taylordotfish
PR: oven-sh/bun#22227
File: src/safety/alloc.zig:93-95
Timestamp: 2025-09-02T17:14:46.924Z
Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.

Applied to files:

  • src/bun.js/bindings/ZigStackFrame.zig
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
PR: oven-sh/bun#22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.

Applied to files:

  • src/bun.js/bindings/ZigStackFrame.zig
🧬 Code graph analysis (4)
src/bun.js/bindings/ErrorStackTrace.cpp (1)
src/bun.js/bindings/bindings.cpp (2)
  • frame (4568-4705)
  • frame (4568-4568)
src/bun.js/bindings/bindings.cpp (1)
src/bun.js/bindings/ErrorStackTrace.cpp (8)
  • functionName (391-398)
  • functionName (391-391)
  • functionName (589-608)
  • functionName (589-589)
  • functionName (610-666)
  • functionName (610-610)
  • functionName (668-810)
  • functionName (668-668)
src/bun.js/bindings/CallSite.h (1)
src/bun.js/bindings/webcore/AbortSignal.h (1)
  • m_flags (190-190)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
src/bun.js/bindings/bindings.cpp (2)
  • frame (4568-4705)
  • frame (4568-4568)
🔇 Additional comments (15)
cmake/tools/SetupWebKit.cmake (2)

5-5: WebKit revision bump: OK.

Pinned SHA looks fine and aligns with enabling async stack traces.


81-84: Verified: autobuild assets exist for SHA 2d2e8dd5b020cc165e2bc1d284461b4504d624e5 — OK to merge.
Found matching assets for linux/amd64: bun-webkit-linux-amd64-asan.tar.gz, bun-webkit-linux-amd64-debug.tar.gz, bun-webkit-linux-amd64-lto.tar.gz, bun-webkit-linux-amd64-musl-debug.tar.gz, bun-webkit-linux-amd64-musl-lto.tar.gz, bun-webkit-linux-amd64-musl.tar.gz, bun-webkit-linux-amd64.tar.gz.

src/bun.js/bindings/ErrorStackTrace.cpp (2)

295-295: LGTM: initialize async flag.

Defaulting m_isAsync to false in the visitor-based ctor is correct.


344-345: LGTM: propagate async from JSC::StackFrame.

m_isAsync(frame.isAsyncFrame()) correctly mirrors the engine’s async bit.

src/bake/DevServer/ErrorReportRequest.zig (1)

80-83: LGTM: explicit default for async bit.

Setting .is_async = false on frames in error reports keeps wire format stable and avoids accidental async labeling.

src/bun.js/bindings/headers-handwritten.h (1)

180-186: Zig/C ABI layout verified — ZigStackFrame matches C header

src/bun.js/bindings/ZigStackFrame.zig declares the extern struct with fields in the same order (function_name, source_url, position, code_type, is_async, remapped) and the Zero initializer sets .is_async = false — no action required.

src/bun.js/bindings/CallSite.cpp (1)

79-81: LGTM: set async flag on CallSite.

Flags::IsAsync mirrors stackFrame.isAsync() correctly.

src/bun.js/bindings/ErrorStackTrace.h (1)

68-69: LGTM: async accessor and storage.

The new m_isAsync plus isAsync() accessor integrates cleanly with existing flags.

src/bun.js/bindings/ZigGlobalObject.cpp (2)

303-304: LGTM: enable async stack traces globally.

JSC::Options::useAsyncStackTrace() = true aligns with the new feature and expected output.


631-635: LGTM: prefix “async ” in formatted stack.

Matches Node/V8-style rendering and your tests.

src/bun.js/bindings/CallSite.h (1)

30-31: LGTM: add IsAsync flag and accessor.

Bit value 64 fits the current flags layout and avoids overlap.

test/js/bun/test/stack.test.ts (1)

125-157: LGTM: async-frame coverage and ordering checks.

Good assertions on presence and order of “async ” frames.

src/bun.js/bindings/bindings.cpp (1)

4431-4432: LGTM: async flag propagation for JSC frames

Setting frame->is_async = stackFrame->isAsyncFrame(); correctly carries async-ness into Zig frames for structured JSC stack traces.

src/bun.js/bindings/ZigStackFrame.zig (2)

123-123: Name formatting: async prefix applied only for function frames

Prefixing “async ” (colored and plain) when code_type == .Function is correct and avoids illegal “async constructor/global”. Nice.

Also applies to: 146-156


9-9: ABI add: is_async field wired correctly — verified
headers-handwritten.h declares bool is_async (line 184); ZigStackFrame.zig declares is_async (line 9) and Zero initializer sets .is_async = false (line 191); bindings.cpp writes frame->is_async = stackFrame->isAsyncFrame() (line 4431).


expect(normalizeBunSnapshot(error.stack!)).toMatchInlineSnapshot(`
"Error: error from qux
at asyncFunctionResume (file:NN:NN)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we hide asyncFunctionResume from the traces? Is that showing up if you do BUN_JSC_showPrivateScriptsInStackframes=0?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/js/bun/test/stack.test.ts (1)

125-152: Trim noisy logging; minor polish; optional stronger coverage.

  • Remove console.log to keep CI output clean.
  • Minor wording nit for the test title.
  • Readability: prefer await Promise.resolve() over await 1.

Apply within this hunk:

-test("Async functions frame should be included in stack trace", async () => {
+test("Async function frames are included in stack trace", async () => {
@@
-  async function baz() {
-    await 1;
+  async function baz() {
+    await Promise.resolve();
@@
-  console.log(error.stack);
+  // no-op: avoid noisy logging in CI

Optional: add a second test that throws (rather than returns) to exercise propagation across async boundaries:

test("Thrown async error includes async frames", async () => {
  async function foo() { return await bar(); }
  async function bar() { return await baz(); }
  async function baz() { await Promise.resolve(); return await qux(); }
  async function qux() { throw new Error("error from qux"); }

  try {
    await foo();
    throw new Error("expected to throw");
  } catch (err) {
    expect(normalizeBunSnapshot((err as Error).stack!)).toMatchInlineSnapshot();
  }
});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f957c87 and 1b0e87a.

📒 Files selected for processing (1)
  • test/js/bun/test/stack.test.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

  • test/js/bun/test/stack.test.ts
test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

  • test/js/bun/test/stack.test.ts
test/js/bun/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Files:

  • test/js/bun/test/stack.test.ts
test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

  • test/js/bun/test/stack.test.ts
test/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.ts: Name test files *.test.ts and use bun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; use port: 0 to get a random port
When spawning Bun in tests, use bunExe() and bunEnv from harness
Prefer async/await in tests; for a single callback, use Promise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Use tempDir/tempDirWithFiles from harness for temporary files and directories in tests
For large/repetitive strings in tests, prefer Buffer.alloc(count, fill).toString() over "A".repeat(count)
Import common test utilities from harness (e.g., bunExe, bunEnv, tempDirWithFiles, tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and use toThrow for synchronous errors
Use describe blocks for grouping, describe.each for parameterized tests, snapshots with toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach); track resources for cleanup in afterEach
Use using/await using with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests

Files:

  • test/js/bun/test/stack.test.ts
test/js/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Organize unit tests for specific features under test/js/ by module

Files:

  • test/js/bun/test/stack.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions

Files:

  • test/js/bun/test/stack.test.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Format JavaScript/TypeScript files with Prettier (bun run prettier)

Files:

  • test/js/bun/test/stack.test.ts
🧠 Learnings (14)
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, use normalizeBunSnapshot when asserting snapshots

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 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/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Use shared utilities from test/harness.ts where applicable

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot tests over exact stdout equality assertions

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`

Applied to files:

  • test/js/bun/test/stack.test.ts
📚 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:

  • test/js/bun/test/stack.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : In error tests, assert non-zero exit codes for failing processes and use `toThrow` for synchronous errors

Applied to files:

  • test/js/bun/test/stack.test.ts
🧬 Code graph analysis (1)
test/js/bun/test/stack.test.ts (1)
test/harness.ts (1)
  • normalizeBunSnapshot (1783-1814)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Format
🔇 Additional comments (1)
test/js/bun/test/stack.test.ts (1)

4-4: LGTM on explicit harness imports.

Importing normalizeBunSnapshot (and bunEnv/bunExe) directly aligns with our testing guidelines while keeping the side-effect import for custom matchers.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/bun.js/bindings/CallSitePrototype.cpp (1)

6-18: Include root.h to satisfy lints

Repo guideline for *.cpp requires root.h near the top. Add it to avoid lint failures.

Apply:

-#include "config.h"
+#include "config.h"
+#include "root.h"
test/js/node/v8/capture-stack-trace.test.js (2)

756-756: Terminate the test with a semicolon (Prettier/style)

Keep consistency with surrounding tests and formatter expectations.

-})
+});

748-755: Restore Error.prepareStackTrace in finally for robustness

Guard restoration even if an assertion throws before reaching the assignment.

-  try {
-    await foo();
-  } catch (e) {
-    e.stack;
-  }
-  Error.prepareStackTrace = prevPrepareStackTrace;
+  try {
+    await foo();
+  } catch (e) {
+    e.stack;
+  } finally {
+    Error.prepareStackTrace = prevPrepareStackTrace;
+  }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1b0e87a and 1247795.

📒 Files selected for processing (2)
  • src/bun.js/bindings/CallSitePrototype.cpp (1 hunks)
  • test/js/node/v8/capture-stack-trace.test.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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/CallSitePrototype.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/CallSitePrototype.cpp
**/*.{cpp,cc,cxx,h,hpp,hxx}

📄 CodeRabbit inference engine (CLAUDE.md)

Format C/C++ sources/headers with clang-format (bun run clang-format)

Files:

  • src/bun.js/bindings/CallSitePrototype.cpp
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: For JS classes with a public constructor, implement Foo, FooPrototype, and FooConstructor (JSC patterns)
Define class properties using HashTableValue arrays in JSC bindings
Add iso subspaces for classes with C++ fields in JSC bindings
Cache structures in ZigGlobalObject for JSC-bound classes

Files:

  • src/bun.js/bindings/CallSitePrototype.cpp
test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

  • test/js/node/v8/capture-stack-trace.test.js
test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

  • test/js/node/v8/capture-stack-trace.test.js
test/js/node/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Node.js module compatibility tests under test/js/node/, separated by module (e.g., test/js/node/assert/)

Files:

  • test/js/node/v8/capture-stack-trace.test.js
test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

  • test/js/node/v8/capture-stack-trace.test.js
test/js/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Organize unit tests for specific features under test/js/ by module

Files:

  • test/js/node/v8/capture-stack-trace.test.js
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Format JavaScript/TypeScript files with Prettier (bun run prettier)

Files:

  • test/js/node/v8/capture-stack-trace.test.js
🧠 Learnings (3)
📚 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 : Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue

Applied to files:

  • src/bun.js/bindings/CallSitePrototype.cpp
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : In error tests, assert non-zero exit codes for failing processes and use `toThrow` for synchronous errors

Applied to files:

  • test/js/node/v8/capture-stack-trace.test.js
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Prefer `async/await` in tests; for a single callback, use `Promise.withResolvers()`

Applied to files:

  • test/js/node/v8/capture-stack-trace.test.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Format
🔇 Additional comments (2)
src/bun.js/bindings/CallSitePrototype.cpp (1)

227-228: Return actual async status — LGTM

Correctly forwards CallSite::isAsync() through the proto function.

test/js/node/v8/capture-stack-trace.test.js (1)

728-756: Async-frame expectations — nice addition

Good, focused check that async propagation flags frames as expected, with a single prepareStackTrace invocation.

@Jarred-Sumner Jarred-Sumner merged commit 9479bb8 into main Sep 12, 2025
60 of 61 checks passed
@Jarred-Sumner Jarred-Sumner deleted the eng/enable-async-stack-trace branch September 12, 2025 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants