Skip to content

Support multiple abort signals and refactor signal/timeout controllers#11

Merged
adelrodriguez merged 1 commit intomainfrom
02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers
Feb 26, 2026
Merged

Support multiple abort signals and refactor signal/timeout controllers#11
adelrodriguez merged 1 commit intomainfrom
02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers

Conversation

@adelrodriguez
Copy link
Copy Markdown
Owner

@adelrodriguez adelrodriguez commented Feb 23, 2026

This pull request refactors signal handling to support multiple abort signals and improves the internal architecture of timeout and signal controllers.

Key Changes:

  • Multiple Signal Support: The signal() method now accumulates multiple abort signals instead of replacing them. The builder config now uses signals array instead of a single signal property.

  • Combined Signal Context: When multiple signals are configured via chained .signal() calls, they are combined into a compound signal available as ctx.signal using AbortSignal.any().

  • Controller Refactoring:

    • Converted createTimeoutController() and createSignalController() factory functions to TimeoutController and SignalController classes
    • Implemented proper disposable pattern with Symbol.dispose support
    • Simplified internal signal management using native AbortSignal.any() instead of manual event forwarding
  • API Simplification:

    • Renamed createDisposer to dispose for the exported function
    • Removed separate context.ts module, moving context creation inline
    • Updated method names from raceWithTimeout/raceWithSignal to simply race
  • Enhanced Testing: Added comprehensive tests for multiple signal scenarios, ensuring any configured signal can trigger cancellation during async operations.

The changes maintain backward compatibility for single signal usage while enabling more flexible cancellation patterns through signal composition.

Greptile Summary

Refactored signal and timeout handling to support multiple abort signals with improved architecture using native AbortSignal.any() and class-based controllers.

Major changes:

  • The .signal() method now accumulates signals into an array instead of replacing, enabling signal composition
  • SignalController and TimeoutController converted from factory functions to classes with proper disposable patterns
  • Leverages native AbortSignal.any() to combine multiple signals into ctx.signal, simplifying internal implementation
  • Method names unified from raceWithTimeout/raceWithSignal to race()
  • Removed context.ts module, inlined context creation into RunExecution
  • Renamed createDisposer to dispose for cleaner API
  • Enhanced test coverage for multi-signal scenarios

Confidence Score: 5/5

  • Safe to merge - well-tested refactoring with comprehensive test coverage and backward compatibility
  • The refactoring is thoroughly tested with new tests for multi-signal composition, maintains backward compatibility for single signal usage, uses native browser APIs (AbortSignal.any()), follows modern disposable patterns, and reduces code complexity while adding functionality
  • No files require special attention

Important Files Changed

Filename Overview
src/lib/signal.ts Refactored to class-based controller using AbortSignal.any() for combining signals, with simplified disposal pattern
src/lib/timeout.ts Converted to class-based controller with private fields and improved disposal using DisposableStack
src/lib/runner.ts Updated to use new controller classes, moved context creation inline, and simplified disposal with DisposableStack
src/lib/builder.ts Modified signal() method to accumulate signals into array instead of replacing single signal
src/lib/types/builder.ts Changed signal property to signals array to support multiple abort signals
src/tests/index.test.ts Added test for multiple signal composition verifying ctx.signal combines signals using AbortSignal.any()

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User calls .signal] --> B[Builder accumulates into signals array]
    B --> C[RunExecution creates TimeoutController]
    C --> D[Merges timeout.signal with config.signals]
    D --> E[SignalController created with merged array]
    E --> F[AbortSignal.any combines into compound signal]
    F --> G[ctx.signal available to user code]
    G --> H{Any signal aborts?}
    H -->|Yes| I[Race resolves to CancellationError]
    H -->|No| J[Promise completes normally]
    I --> K[Cleanup via DisposableStack]
    J --> K
Loading

Last reviewed commit: 7e4db2d

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 23, 2026

Warning

Rate limit exceeded

@adelrodriguez has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 58 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between e636621 and eb9688f.

📒 Files selected for processing (14)
  • AGENTS.md
  • src/__tests__/index.test.ts
  • src/index.ts
  • src/lib/__tests__/dispose.test.ts
  • src/lib/__tests__/retry.test.ts
  • src/lib/__tests__/runner.test.ts
  • src/lib/__tests__/timeout.test.ts
  • src/lib/builder.ts
  • src/lib/context.ts
  • src/lib/dispose.ts
  • src/lib/runner.ts
  • src/lib/signal.ts
  • src/lib/timeout.ts
  • src/lib/types/builder.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers

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.

Copy link
Copy Markdown
Owner Author

adelrodriguez commented Feb 23, 2026

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR successfully refactors signal and timeout handling to support multiple abort signals while improving the internal architecture.

Key improvements:

  • Multiple abort signals are now accumulated via chained .signal() calls and combined using native AbortSignal.any() into a compound signal available as ctx.signal
  • Factory functions converted to proper TimeoutController and SignalController classes with Symbol.dispose support for cleaner resource management
  • Simplified method names (raceWithTimeout/raceWithSignalrace) and function names (createDisposerdispose)
  • Context creation moved inline from separate module, reducing unnecessary abstraction
  • Comprehensive test coverage added for multiple signal scenarios

Architecture notes:

  • The implementation correctly prioritizes user cancellation over timeout when both occur simultaneously
  • SignalController.checkDidCancel() filters out TimeoutError reasons to maintain separation of concerns between user cancellation and automatic timeouts
  • Disposal uses DisposableStack pattern to ensure event listeners are properly cleaned up and timeouts are cleared
  • The change from signal to signals array in BuilderConfig is a breaking change, but acceptable per the version policy documented in AGENTS.md (project is in v0)

Confidence Score: 5/5

  • This PR is safe to merge with no blocking issues
  • Well-architected refactoring with proper resource cleanup, comprehensive test coverage for new functionality, and clean separation of concerns between signal and timeout handling
  • No files require special attention

Important Files Changed

Filename Overview
src/lib/signal.ts Refactored from factory function to class using AbortSignal.any() for combining multiple signals, with proper disposal pattern
src/lib/timeout.ts Converted to class-based implementation with proper cleanup via [Symbol.dispose](), timeout lifecycle correctly managed
src/lib/runner.ts Updated to use new controller classes, moved context creation inline, proper disposal stack for cleanup
src/lib/builder.ts Modified .signal() to accumulate signals into array instead of replacing, enabling multiple signal support
src/lib/types/builder.ts Type definition updated from signal?: AbortSignal to signals?: AbortSignal[] to support multiple signals

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class TryBuilder {
        -BuilderConfig config
        +signal(AbortSignal) TryBuilder
        +timeout(TimeoutOptions) TryBuilder
        +run(tryFn) T|E
        +runAsync(tryFn) Promise
    }
    
    class RunExecution {
        -TimeoutController timeout
        -SignalController signal
        -TryCtx ctx
        +execute() Result
        -race(promise) Promise
        +dispose() void
    }
    
    class SignalController {
        +AbortSignal signal
        +constructor(signals)
        +checkDidCancel() CancellationError
        +race(promise) Promise
        +dispose() void
    }
    
    class TimeoutController {
        +AbortSignal signal
        -AbortController controller
        -number timeoutId
        +constructor(TimeoutPolicy)
        +checkDidTimeout() TimeoutError
        +race(promise) Promise
        +dispose() void
    }
    
    class BuilderConfig {
        +signals AbortSignal[]
        +timeout TimeoutPolicy
        +retry RetryPolicy
        +wraps WrapFn[]
    }
    
    TryBuilder --> BuilderConfig : uses
    RunExecution --> TimeoutController : creates
    RunExecution --> SignalController : creates
    SignalController --> AbortSignal : combines via any()
    TimeoutController --> AbortController : manages
    RunExecution --> BuilderConfig : receives
Loading

Last reviewed commit: eb9688f

@adelrodriguez adelrodriguez force-pushed the 02-23-implement_phase_8_dispose_with_asyncdisposablestack branch from b627ab2 to 0d6daf5 Compare February 26, 2026 10:50
@adelrodriguez adelrodriguez force-pushed the 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers branch from 9eac38d to 6a202b7 Compare February 26, 2026 10:51
@adelrodriguez adelrodriguez force-pushed the 02-23-implement_phase_8_dispose_with_asyncdisposablestack branch from 0d6daf5 to c2d4c9d Compare February 26, 2026 10:54
@adelrodriguez adelrodriguez force-pushed the 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers branch from 6a202b7 to 02d9642 Compare February 26, 2026 10:54
Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

14 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@adelrodriguez adelrodriguez changed the base branch from 02-23-implement_phase_8_dispose_with_asyncdisposablestack to graphite-base/11 February 26, 2026 11:31
@adelrodriguez adelrodriguez force-pushed the 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers branch from 02d9642 to 4c46ca6 Compare February 26, 2026 11:31
@adelrodriguez adelrodriguez changed the base branch from graphite-base/11 to 02-23-implement_phase_8_dispose_with_asyncdisposablestack February 26, 2026 11:31
@adelrodriguez adelrodriguez changed the base branch from 02-23-implement_phase_8_dispose_with_asyncdisposablestack to graphite-base/11 February 26, 2026 11:33
@adelrodriguez adelrodriguez force-pushed the 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers branch from 4c46ca6 to 030906e Compare February 26, 2026 11:33
@adelrodriguez adelrodriguez changed the base branch from graphite-base/11 to main February 26, 2026 11:33
@adelrodriguez adelrodriguez force-pushed the 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers branch from 030906e to eb9688f Compare February 26, 2026 11:33
@adelrodriguez adelrodriguez merged commit 79e9900 into main Feb 26, 2026
8 checks passed
@adelrodriguez adelrodriguez deleted the 02-23-support_multiple_abort_signals_and_refactor_signal_timeout_controllers branch February 26, 2026 11:37
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.

1 participant