fix: stop retry work on caller cancellation - #1766
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthrough재시도 API가 Changes재시도 취소 지원
Generated-app smoke 증거 처리
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Retryable
participant RetryOrchestrator
participant RetryEngine
participant BackoffPolicy
Caller->>Retryable: 호출 인자와 AbortSignal 전달
Retryable->>RetryOrchestrator: signalResolver 결과 전달
RetryOrchestrator->>RetryEngine: signal을 포함한 retry loop 실행
RetryEngine->>BackoffPolicy: wait(attempt, signal)
BackoffPolicy-->>RetryEngine: 정상 완료 또는 abort reason
RetryEngine-->>Caller: 결과 또는 RetryAbortedProblem
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-08-07T15:15:31.115Z · Commit: a52a305 |
kang-heewon
force-pushed
the
fix/1712-abortable-retry
branch
from
August 5, 2026 17:09
a2eb675 to
db45505
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/abort-retry-work.md:
- Line 7: Update the release note in abort-retry-work.md to document the
cancellation contract for custom backoff policies and injected sleepers: they
must declare cancellation support, otherwise an already-cancelled call fails
with retry-core/backoff-cancellation-unsupported before the callback runs.
Include the migration guidance for existing custom implementations.
In `@packages/retry-core/src/libs/BackoffPolicy.ts`:
- Around line 140-147: ExponentialBackoff.wait()의 signal 조건 분기를 제거하고, delayMs가
양수일 때 선택적 signal을 그대로 this.sleep(delayMs, signal)에 전달하십시오. 동일한 중복 로직이 있는
FixedBackoff.wait()에도 같은 변경을 적용하고, 나머지 대기 동작은 유지하십시오.
In `@packages/retry-core/src/tests/Retryable.spec.ts`:
- Around line 65-66: Retryable.spec.ts의 비동기 오류 검증을
rejects.toBeInstanceOf(RetryAbortedProblem)에서
rejects.toThrow(RetryAbortedProblem)로 변경하세요. 언급된 두 테스트 구간의 RetryAbortedProblem
클래스 검증만 업데이트하고, callback이 호출되지 않았는지 확인하는 expect(callback).not.toHaveBeenCalled()
검증은 그대로 유지하세요.
In `@packages/retry-core/src/tests/RetryEngine.spec.ts`:
- Around line 185-189: Update each cancellation-error assertion in RetryEngine
tests to include await expect(execution).rejects.toThrow(...) with the expected
error message. Preserve the existing code and methodName checks in the first
test, and retain any other current assertions.
- Around line 15-25: Replace the definite-assignment assertion on resolve in
createDeferred with an implementation that initializes the resolver without
non-null assertions, while preserving the existing promise and resolve API used
by the tests.
In `@packages/retry-core/src/tests/RetryTemplate.spec.ts`:
- Around line 46-55: Update the async rejection assertion in the “forwards
caller cancellation to the retry engine” test to use rejects.toThrow with
RetryAbortedProblem instead of rejects.toBeInstanceOf. Keep the callback
non-invocation assertion unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cb5289fc-cb72-422c-a8cb-dd300d705135
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (36)
.changeset/abort-retry-work.mddocs/problem-code-registry.jsonpackages/docs/src/content/docs/api/retry-core/src/classes/ExponentialBackoff.mdpackages/docs/src/content/docs/api/retry-core/src/classes/FixedBackoff.mdpackages/docs/src/content/docs/api/retry-core/src/classes/NoBackoff.mdpackages/docs/src/content/docs/api/retry-core/src/classes/RetryAbortedProblem.mdpackages/docs/src/content/docs/api/retry-core/src/classes/RetryCancellationUnsupportedProblem.mdpackages/docs/src/content/docs/api/retry-core/src/functions/executeRetryLoop.mdpackages/docs/src/content/docs/api/retry-core/src/interfaces/BackoffDependencies.mdpackages/docs/src/content/docs/api/retry-core/src/interfaces/BackoffPolicy.mdpackages/docs/src/content/docs/api/retry-core/src/interfaces/RetryTemplateOptions.mdpackages/docs/src/content/docs/api/retry-core/src/interfaces/RetryableOptions.mdpackages/docs/src/content/docs/api/retry-core/src/type-aliases/RetryOrchestratorOptions.mdpackages/docs/src/content/docs/api/retry-core/src/type-aliases/RetrySignalResolverContext.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/retry-core/README.mdpackages/retry-core/src/index.tspackages/retry-core/src/libs/BackoffPolicy.tspackages/retry-core/src/libs/RetryEngine.tspackages/retry-core/src/libs/RetryOrchestrator.tspackages/retry-core/src/libs/RetryTemplate.tspackages/retry-core/src/libs/Retryable.tspackages/retry-core/src/libs/errors/RetryAbortedProblem.tspackages/retry-core/src/libs/errors/RetryCancellationUnsupportedProblem.tspackages/retry-core/src/libs/errors/index.tspackages/retry-core/src/tests/BackoffPolicy.spec.tspackages/retry-core/src/tests/RetryEngine.spec.tspackages/retry-core/src/tests/RetryTemplate.spec.tspackages/retry-core/src/tests/Retryable.spec.tspackages/testing/src/libs/TestRuntime.tspackages/testing/src/tests/TestRuntime.spec.tspublic-api-surface.snapshot.jsonscripts/release-spine-evidence.mtsscripts/tests/release-spine-evidence.spec.tsscripts/tests/verification-manifest.spec.tsscripts/verification-manifest.mts
kang-heewon
force-pushed
the
fix/1712-abortable-retry
branch
from
August 7, 2026 15:05
c165ec3 to
6ad0e44
Compare
Member
Author
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Caller-provided
AbortSignalvalues now stop retry execution before an attempt or during backoff. Built-in backoff timers and the testing virtual clock release abort listeners and scheduled work promptly.Custom backoff policies and injected sleepers must explicitly advertise abort support. When cancellation is requested for an unsupported implementation, retry execution fails before invoking the callback with the stable
retry-core/backoff-cancellation-unsupportedProblem code. Calls without a signal retain their previous behavior.@Retryablesupports both a fixed signal and a per-invocationsignalResolver, so concurrent method calls can be cancelled independently.Fixes #1712
Verification
pnpm --filter @croco/retry-core test— 286 passedpnpm --filter @croco/testing test— 168 passedpnpm docs:api:checkcontinues to report unrelated repository-wide documentation drift already present on trunk; no changed retry-core page remains divergent from the isolated generated output.Review gates
Summary by CodeRabbit
AbortSignal을 전달해 첫 시도 전이나 백오프 대기 중에도 즉시 취소할 수 있습니다.