Skip to content

Propagate abort reason from external signal in runner - #17

Merged
lambdalisue merged 1 commit into
mainfrom
fix/timeout
Jan 8, 2026
Merged

Propagate abort reason from external signal in runner#17
lambdalisue merged 1 commit into
mainfrom
fix/timeout

Conversation

@lambdalisue

Copy link
Copy Markdown
Member

Summary

  • Pass abort reason from external signal to internal abort controller

Why

Previously, when an external abort signal was received, the runner would abort its internal controller without preserving the original abort reason. This meant that timeout errors or other abort reasons were lost during propagation, making it difficult to distinguish between different types of cancellation (timeout vs. manual cancellation).

By propagating the reason property from the external signal, the abort context is preserved throughout the execution chain, enabling proper error handling and timeout detection downstream.

Test Plan

  • Verify timeout errors are properly detected when external signal times out
  • Verify manual cancellation still works correctly
  • Run deno task verify to ensure all tests pass

Copilot AI review requested due to automatic review settings January 8, 2026 12:03
@lambdalisue
lambdalisue enabled auto-merge January 8, 2026 12:04
@lambdalisue
lambdalisue merged commit 3c6d04c into main Jan 8, 2026
6 checks passed
@lambdalisue
lambdalisue deleted the fix/timeout branch January 8, 2026 12:06
@codecov

codecov Bot commented Jan 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves abort signal handling by propagating the abort reason from external signals to the internal abort controller. Previously, when an external signal was aborted (e.g., due to a timeout), the reason for the abort was lost when propagating to the internal controller, making it difficult to distinguish between different types of cancellation.

Key Changes:

  • Modified the abort event listener to pass the external signal's reason when aborting the internal controller
  • This enables proper timeout detection downstream via the isTimeoutError function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +73 to +76
options?.signal?.addEventListener("abort", () => {
// Pass the reason from external signal to internal controller
controller.abort(options.signal?.reason);
});

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

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

This change introduces abort reason propagation but lacks test coverage. The PR description mentions tests that should verify timeout error detection and manual cancellation, but these tests haven't been added yet. Consider adding a test that:

  1. Creates an external AbortSignal with AbortSignal.timeout()
  2. Passes it to runner.run() via options.signal
  3. Verifies that when the timeout fires, the internal signal receives the TimeoutError reason
  4. Confirms that isTimeoutError() correctly identifies the error as a timeout

This would ensure the abort reason propagation works as intended and doesn't break in future changes.

Copilot uses AI. Check for mistakes.
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.

2 participants