Skip to content

Commit aaf6348

Browse files
abueideclaude
andauthored
test(core): add comprehensive RetryManager test suite (#1159)
* fix: add persistence validation, atomic backoff calc, and 429 precedence - Validate persisted state in canRetry() to handle clock changes/corruption per SDD §Metadata Lifecycle - Move backoff calculation inside dispatch to avoid stale retryCount from concurrent batch failures (handleErrorWithBackoff) - Ensure RATE_LIMITED state is never downgraded to BACKING_OFF - Update reset() docstring to clarify when it should be called Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: clean up RetryManager comments for post-merge readability Simplify class docstring to describe current architecture without referencing SDD deviations. Remove redundant inline comments, compact JSDoc to single-line where appropriate, and ensure all comments use present tense. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: consolidate error handlers, fix getState race, add limit signaling - Use getState(true) for queue-safe reads to prevent race conditions between concurrent canRetry/handle429/handleTransientError calls - Consolidate handleError and handleErrorWithBackoff into a single method that accepts a computeWaitUntilTime function - Extract side effects (logging, Math.random) from dispatch reducers - Return RetryResult ('rate_limited'|'backed_off'|'limit_exceeded') from handle429/handleTransientError so callers can drop events on limit exceeded - Clear auto-flush timer in transitionToReady - Validate state string in isPersistedStateValid Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: trim verbose inline comments in RetryManager Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: use enums and switch statements for clearer state handling Replace string literals with TypeScript enums: - RetryState enum (READY, RATE_LIMITED, BACKING_OFF) - RetryResult enum (RATE_LIMITED, BACKED_OFF, LIMIT_EXCEEDED) Extract helper methods for clarity: - resolveStatePrecedence(): handles 429 taking priority over backoff - consolidateWaitTime(): uses switch statement for clear wait time logic - getStateDisplayName(): maps state to display names Benefits: - Type-safe state handling (no magic strings) - Switch statements make control flow explicit - Each helper method has a single, named responsibility - Easier to test and maintain Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: remove VALID_STATES Set, use Object.values for enum validation Use Object.values(RetryState).includes() instead of maintaining a duplicate Set of valid states. More idiomatic TypeScript and eliminates maintenance burden of keeping Set in sync with enum. * test(core): add comprehensive RetryManager test suite Add 29 tests covering all RetryManager states and transitions: canRetry, handle429 rate limiting, handleTransientError backoff, reset, retry strategies (eager/lazy), autoFlush callbacks, and mixed error scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: update test for 429 overriding BACKING_OFF state Updated test to verify that 429 now correctly overrides BACKING_OFF state rather than being silently ignored. The 429's retry-after takes precedence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add 429 authority test and fix autoFlush timer cleanup - Add test verifying 429 Retry-After overrides long transient backoff - Track RetryManager instances in autoFlush tests and destroy in afterEach to prevent timer leaks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: update tests for refactored API and add missing coverage Update log assertions for consolidated limit-exceeded message. Add tests for: RetryResult return values, jitter calculation, isPersistedStateValid clock skew detection, handle429(0) edge case, and strengthened assertions that verify behavioral state after clamping/rejection (not just log output). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: remove autoFlush test suite from RetryManager tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: update tests for eager-only behavior after master merge --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1f56a4d commit aaf6348

File tree

2 files changed

+761
-0
lines changed

2 files changed

+761
-0
lines changed

packages/core/src/backoff/RetryManager.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ export class RetryManager {
126126
return true;
127127
}
128128

129+
if (!this.isPersistedStateValid(state, now)) {
130+
this.logger?.warn(
131+
'Persisted retry state failed validation, resetting to READY'
132+
);
133+
await this.reset();
134+
return true;
135+
}
136+
129137
if (now >= state.waitUntilTime) {
130138
await this.transitionToReady();
131139
return true;

0 commit comments

Comments
 (0)