Skip to content

fix: add stack trace to AbortSignal.timeout errorsfix: add stack trace to AbortSignal.timeout errors - #25339

Closed
tennisleng wants to merge 2 commits into
oven-sh:mainfrom
tennisleng:fix/abort-signal-timeout-stack-trace
Closed

tennisleng wants to merge 2 commits into
oven-sh:mainfrom
tennisleng:fix/abort-signal-timeout-stack-trace

Conversation

@tennisleng

Copy link
Copy Markdown

Summary

Fixes #25182: No stack trace on AbortSignal.timeout error

This PR adds stack trace capture to AbortSignal.timeout() so that when the timeout fires and throws a TimeoutError, the stack trace points to where AbortSignal.timeout() was called in user code.

Changes

  • src/bun.js/bindings/webcore/AbortSignal.h: Added m_timeoutCreationStack member variable and timeoutCreationStack() accessor. Updated toJS() signature with optional captured stack parameter.
  • src/bun.js/bindings/webcore/AbortSignal.cpp: Modified timeout() to capture the current stack trace at creation. Modified signalAbort() to pass the captured stack to toJS().
  • src/bun.js/bindings/ErrorCode.cpp: Modified toJS() for CommonAbortReason::Timeout to copy the stack from the captured error to the new TimeoutError.
  • test/js/bun/abort-signal-timeout-stack.test.ts: Added test cases verifying stack trace includes calling function name and file name.

How It Works

  1. When AbortSignal.timeout() is called, we create an error object and capture its stack trace at that point
  2. We store this error in m_timeoutCreationStack
  3. When the timeout fires and we create the TimeoutError, we copy the stack from the stored error
  4. The result is a TimeoutError with a stack trace pointing to user code

Before/After

Before: Error has no stack trace or only internal frames
After: Error.stack shows where AbortSignal.timeout() was calledCapture the stack trace at the point where AbortSignal.timeout() is called and attach it to the TimeoutError that is thrown when the timeout fires.

Previously, the TimeoutError had no stack trace (or only internal frames), making it difficult to debug where the timeout originated in user code.

Now the error's stack property points to the user code that called AbortSignal.timeout(), matching Node.js behavior.

Changes:

  • AbortSignal.h: Added m_timeoutCreationStack member and accessor
  • AbortSignal.cpp: Capture stack in timeout(), pass to signalAbort()
  • ErrorCode.cpp: Copy captured stack to TimeoutError in toJS()
  • Added test case for stack trace verification

Closes #25182

What does this PR do?

How did you verify your code works?

Capture the stack trace at the point where AbortSignal.timeout() is called
and attach it to the TimeoutError that is thrown when the timeout fires.

Previously, the TimeoutError had no stack trace (or only internal frames),
making it difficult to debug where the timeout originated in user code.

Now the error's stack property points to the user code that called
AbortSignal.timeout(), matching Node.js behavior.

Changes:
- AbortSignal.h: Added m_timeoutCreationStack member and accessor
- AbortSignal.cpp: Capture stack in timeout(), pass to signalAbort()
- ErrorCode.cpp: Copy captured stack to TimeoutError in toJS()
- Added test case for stack trace verification

Closes #25182
@coderabbitai

coderabbitai Bot commented Dec 4, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds capture of the stack at AbortSignal.timeout call sites and propagates that captured stack into the produced Timeout DOMException by extending the toJS conversion to accept an optional capturedStack parameter and storing the creation stack on AbortSignal.

Changes

Cohort / File(s) Summary
Error conversion signature
src/bun.js/bindings/ErrorCode.cpp
toJS signature extended to accept an optional JSC::JSObject* capturedStack; conversion copies capturedStack into the created error object when present and the abort reason is Timeout.
AbortSignal capture & propagation
src/bun.js/bindings/webcore/AbortSignal.cpp, src/bun.js/bindings/webcore/AbortSignal.h
AbortSignal::timeout now creates an ErrorInstance and stores its stack in m_timeoutCreationStack; added timeoutCreationStack() accessor; signalAbort now passes the stored stack into toJS when firing a timeout.
Tests
test/js/bun/abort-signal-timeout-stack.test.ts
New tests verifying AbortSignal.timeout errors include the creation-site stack (function and test file name) and validate error type/name/message/stack presence.

Suggested reviewers

  • Jarred-Sumner
  • pfgithub

Pre-merge checks

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and specifically describes adding stack trace functionality to AbortSignal.timeout errors, matching the primary change across all modified files.
Description check ✅ Passed Description contains What does this PR do section with comprehensive explanation of changes and mechanism, plus verification section with test information. Follows template structure.
Linked Issues check ✅ Passed All code changes directly address issue #25182 objectives: stack capture in AbortSignal.timeout(), propagation to TimeoutError, and test validation of stack trace content.
Out of Scope Changes check ✅ Passed All changes are directly scoped to stack trace propagation for AbortSignal.timeout errors. No unrelated modifications detected across the four modified files.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 92ed113 and 1ddf0d0.

📒 Files selected for processing (2)
  • src/bun.js/bindings/webcore/AbortSignal.cpp (2 hunks)
  • src/bun.js/bindings/webcore/AbortSignal.h (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,zig}

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

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

  • src/bun.js/bindings/webcore/AbortSignal.cpp
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: JavaScript class implementations in C++ should create three classes if there's a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties in C++ JavaScript classes using HashTableValue arrays
Add iso subspaces for C++ JavaScript classes with C++ fields
Cache structures in ZigGlobalObject for C++ JavaScript classes

Files:

  • src/bun.js/bindings/webcore/AbortSignal.cpp
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : For classes without Constructor, use `JSC::LazyProperty<JSGlobalObject, Structure>` instead of `JSC::LazyClassStructure` in ZigGlobalObject.h, initialize in `GlobalObject::finishCreation()`, and visit in `GlobalObject::visitChildrenImpl()`

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : JavaScript class implementations in C++ should create three classes if there's a public constructor: `Foo` (JSDestructibleObject), `FooPrototype` (JSNonFinalObject), and `FooConstructor` (InternalFunction)

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ JavaScript classes with C++ fields

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement function definitions using `JSC_DEFINE_HOST_FUNCTION` macro, performing type checking with `jsDynamicCast`, throwing this-type errors when type checking fails, and returning encoded JSValue

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement getter functions using `JSC_DEFINE_CUSTOM_GETTER` macro, performing type checking with `jsDynamicCast`, throwing errors for type mismatches, and returning encoded JSValue

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.h
🧬 Code graph analysis (2)
src/bun.js/bindings/webcore/AbortSignal.cpp (1)
src/bun.js/bindings/ErrorCode.cpp (2)
  • toJS (1645-1677)
  • toJS (1645-1645)
src/bun.js/bindings/webcore/AbortSignal.h (1)
src/bun.js/bindings/ErrorCode.cpp (2)
  • toJS (1645-1677)
  • toJS (1645-1645)
🔇 Additional comments (5)
src/bun.js/bindings/webcore/AbortSignal.h (3)

58-59: LGTM!

The comment clarifies the implementation location in ErrorCode.cpp, and the signature extension with an optional capturedStack parameter is appropriate for propagating the creation-time stack trace.


129-129: LGTM!

The accessor follows the same pattern as the existing reason() accessor, returning a const reference to the JSValueInWrappedObject.


194-196: LGTM!

The FIXME properly documents the GC write-barrier concern, noting that both m_timeoutCreationStack and m_reason use the same JSValueInWrappedObject + setWeakly pattern and need to be addressed together for GC safety.

src/bun.js/bindings/webcore/AbortSignal.cpp (2)

69-78: LGTM!

The stack capture logic is well implemented:

  • Null-checks globalObject for safety
  • Uses jsDynamicCast for safe casting
  • Skips one frame with captureStackTrace(vm, globalObject, 1) to exclude the internal C++ frame
  • Uses setWeakly consistently with the existing m_reason pattern

224-230: LGTM!

The stack propagation logic is clean and correct:

  • Retrieves the stored stack value
  • Safely extracts the JSObject* only if the value is an object
  • Passes nullptr when no captured stack exists, which the toJS function handles gracefully

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

📜 Review details

Configuration used: Path: .coderabbit.yaml

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 4d60b6f and 92ed113.

📒 Files selected for processing (4)
  • src/bun.js/bindings/ErrorCode.cpp (2 hunks)
  • src/bun.js/bindings/webcore/AbortSignal.cpp (2 hunks)
  • src/bun.js/bindings/webcore/AbortSignal.h (3 hunks)
  • test/js/bun/abort-signal-timeout-stack.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*.{js,ts,jsx,tsx}

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

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using -e flag over tempDir
For multi-file tests in Bun test suite, prefer using tempDir and Bun.spawn
Always use port: 0 when spawning servers in tests - do not hardcode ports or use custom random port functions
Use normalizeBunSnapshot to normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
Use tempDir from harness to create temporary directories in tests - do not use tmpdirSync or fs.mkdtempSync
In tests, call expect(stdout).toBe(...) before expect(exitCode).toBe(0) when spawning processes for more useful error messages on failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - tests are not valid if they pass with USE_SYSTEM_BUN=1
Avoid shell commands in tests - do not use find or grep; use Bun's Glob and built-in tools instead
Test files must end in .test.ts or .test.tsx and be created in the appropriate test folder structure

Files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
src/**/*.{cpp,zig}

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

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

  • src/bun.js/bindings/ErrorCode.cpp
  • src/bun.js/bindings/webcore/AbortSignal.cpp
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: JavaScript class implementations in C++ should create three classes if there's a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties in C++ JavaScript classes using HashTableValue arrays
Add iso subspaces for C++ JavaScript classes with C++ fields
Cache structures in ZigGlobalObject for C++ JavaScript classes

Files:

  • src/bun.js/bindings/ErrorCode.cpp
  • src/bun.js/bindings/webcore/AbortSignal.cpp
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Do not write flaky tests - do not use `setTimeout` in tests; instead `await` the condition to be met since you're testing the CONDITION, not TIME PASSING

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/sourcemap.test.ts : Organize source-map tests in sourcemap.test.ts for tests verifying source-maps are correct

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer async/await over callbacks in tests

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

  • test/js/bun/abort-signal-timeout-stack.test.ts
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
  • src/bun.js/bindings/webcore/AbortSignal.h
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
  • src/bun.js/bindings/webcore/AbortSignal.h
  • src/bun.js/bindings/webcore/AbortSignal.cpp
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-09-19T19:55:22.427Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: src/valkey/js_valkey_functions.zig:853-854
Timestamp: 2025-09-19T19:55:22.427Z
Learning: In Bun's JSPromise API, `resolvedPromiseValue(globalThis: *JSGlobalObject, value: JSValue) JSValue` returns a JSValue directly (not a JSPromise that needs `.toJS()` called on it). This is different from `resolvedPromise()` which returns a *JSPromise that would need `.toJS()`.

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-10-17T01:21:35.060Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/bindings/BunIDLConvert.h:79-95
Timestamp: 2025-10-17T01:21:35.060Z
Learning: In JavaScriptCore (used by Bun), the `toBoolean()` method implements ECMAScript's ToBoolean abstract operation, which is side-effect-free and cannot throw exceptions. It does not execute arbitrary JavaScript or invoke user-defined methods like Symbol.toPrimitive. Exception handling is not needed after toBoolean() calls.

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement function definitions using `JSC_DEFINE_HOST_FUNCTION` macro, performing type checking with `jsDynamicCast`, throwing this-type errors when type checking fails, and returning encoded JSValue

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : To create JavaScript objects from Zig, implement C++ functions following the `Bun__ClassName__toJS(Zig::GlobalObject*, NativeType*)` convention that construct and return the JavaScript object as an encoded JSValue

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement getter functions using `JSC_DEFINE_CUSTOM_GETTER` macro, performing type checking with `jsDynamicCast`, throwing errors for type mismatches, and returning encoded JSValue

Applied to files:

  • src/bun.js/bindings/ErrorCode.cpp
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method

Applied to files:

  • src/bun.js/bindings/webcore/AbortSignal.cpp
🧬 Code graph analysis (4)
test/js/bun/abort-signal-timeout-stack.test.ts (1)
src/bun.js/bindings/webcore/AbortSignal.cpp (2)
  • AbortSignal (104-110)
  • AbortSignal (112-115)
src/bun.js/bindings/ErrorCode.cpp (1)
src/bun.js/bindings/webcore/JSCookie.cpp (4)
  • toJS (974-977)
  • toJS (974-974)
  • toJS (993-1009)
  • toJS (993-993)
src/bun.js/bindings/webcore/AbortSignal.h (1)
src/bun.js/bindings/ErrorCode.cpp (2)
  • toJS (1645-1677)
  • toJS (1645-1645)
src/bun.js/bindings/webcore/AbortSignal.cpp (2)
src/bun.js/bindings/ZigGlobalObject.h (1)
  • globalObject (127-127)
src/bun.js/bindings/ScriptExecutionContext.cpp (2)
  • globalObject (93-96)
  • globalObject (93-93)
🔇 Additional comments (3)
src/bun.js/bindings/ErrorCode.cpp (2)

1679-1682: Wrapper preserves ABI by explicitly passing nullptr for capturedStack

Updating WebCore__CommonAbortReason__toJS to call toJS(globalObject, abortReason, nullptr) keeps the C API behavior identical for existing callers; this looks correct and avoids accidentally binding to the new stack-capture behavior where it isn’t intended.


1645-1663: > Likely an incorrect or invalid review comment.

test/js/bun/abort-signal-timeout-stack.test.ts (1)

19-36: Async error assertion style could use Bun’s expect(async () => …).toThrow() pattern

The try/catch + expect.unreachable pattern works, but for consistency with other Bun tests you might wrap await myFunction() in expect(async () => { await myFunction(); }).toThrow(); which directly encodes the expectation that an async call rejects/throws.

⛔ Skipped due to learnings
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer async/await over callbacks in tests
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Do not write flaky tests - do not use `setTimeout` in tests; instead `await` the condition to be met since you're testing the CONDITION, not TIME PASSING
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test

Comment on lines +9 to +17
async function myFunction() {
const signal = AbortSignal.timeout(50);
await new Promise((resolve, reject) => {
signal.addEventListener('abort', () => {
reject(signal.reason);
});
setTimeout(resolve, 1000);
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid setTimeout in tests; rely on the signal or test runner timeout instead

Both tests use setTimeout(resolve, 1000) as a fallback, which goes against the Bun test guidelines to not wait for time to pass in tests; you can safely drop these fallbacks and let the tests either resolve via the abort event or fail via the runner’s own timeout mechanism, reducing flakiness and keeping test timing deterministic.

Also applies to: 41-45

🤖 Prompt for AI Agents
In test/js/bun/abort-signal-timeout-stack.test.ts around lines 9-17 and 41-45,
the tests include a fallback setTimeout(resolve, 1000) which violates the
guideline to avoid waiting in tests; remove the setTimeout fallback from both
places and let the Promise resolve only via the abort event (rejecting with
signal.reason) or time out via the test runner/AbortSignal.timeout so the test
is deterministic and doesn't rely on sleeping.

- Remove unused errorForStack variable in AbortSignal::timeout()
- Simplify captured-stack handling in signalAbort()
- Add implementation location comment for toJS() declaration
- Add FIXME for GC/write-barrier handling of m_timeoutCreationStack
@tennisleng tennisleng closed this by deleting the head repository Dec 4, 2025
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.

No stack trace on AbortSignal.timeout error

1 participant