diff --git a/src/lib/__tests__/api.test.ts b/src/lib/__tests__/api.test.ts new file mode 100644 index 00000000..0baa9fee --- /dev/null +++ b/src/lib/__tests__/api.test.ts @@ -0,0 +1,28 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { getRetryDelay } from '../api'; + +describe('getRetryDelay', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('uses full jitter across the exponential backoff window', () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5); + + expect(getRetryDelay(1, 100)).toBe(50); + expect(getRetryDelay(2, 100)).toBe(100); + expect(getRetryDelay(3, 100)).toBe(200); + }); + + it('can produce the minimum delay', () => { + vi.spyOn(Math, 'random').mockReturnValue(0); + + expect(getRetryDelay(3, 100)).toBe(0); + }); + + it('can produce the maximum backoff window', () => { + vi.spyOn(Math, 'random').mockReturnValue(1); + + expect(getRetryDelay(3, 100)).toBe(400); + }); +}); diff --git a/src/lib/api.ts b/src/lib/api.ts index 23c2f048..8be069d4 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -84,8 +84,9 @@ function shouldRetry(status: number, attempt: number, maxRetries: number): boole return [408, 429, 500, 502, 503, 504].includes(status); } -function getRetryDelay(attempt: number, baseDelay: number): number { - return baseDelay * Math.pow(2, attempt - 1) + Math.random() * 1000; +export function getRetryDelay(attempt: number, baseDelay: number): number { + const exponentialDelay = baseDelay * Math.pow(2, attempt - 1); + return Math.random() * exponentialDelay; } // --------------------------------------------------------------------------- diff --git a/src/utils/__tests__/circuitBreaker.test.ts b/src/utils/__tests__/circuitBreaker.test.ts index 14e6bd98..13731a24 100644 --- a/src/utils/__tests__/circuitBreaker.test.ts +++ b/src/utils/__tests__/circuitBreaker.test.ts @@ -135,6 +135,54 @@ describe('CircuitBreaker', () => { }); }); + describe('Recovery jitter', () => { + it('uses full jitter for the recovery window', async () => { + vi.useFakeTimers(); + vi.spyOn(Math, 'random').mockReturnValue(0.5); + + const breaker = new CircuitBreaker(config); + const operation = vi.fn().mockRejectedValue(new Error('test error')); + + for (let i = 0; i < config.failureThreshold; i++) { + await expect(breaker.execute(operation)).rejects.toThrow(); + } + + expect(breaker.getState()).toBe('OPEN'); + + await vi.advanceTimersByTimeAsync(config.timeout / 2 - 1); + await expect(breaker.execute(vi.fn())).rejects.toThrow('Circuit breaker is OPEN'); + + await vi.advanceTimersByTimeAsync(2); + const successOperation = vi.fn().mockResolvedValue('success'); + await breaker.execute(successOperation); + + expect(breaker.getState()).toBe('HALF_OPEN'); + + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('can recover immediately when full jitter selects zero', async () => { + vi.useFakeTimers(); + vi.spyOn(Math, 'random').mockReturnValue(0); + + const breaker = new CircuitBreaker(config); + const operation = vi.fn().mockRejectedValue(new Error('test error')); + + for (let i = 0; i < config.failureThreshold; i++) { + await expect(breaker.execute(operation)).rejects.toThrow(); + } + + const successOperation = vi.fn().mockResolvedValue('success'); + await breaker.execute(successOperation); + + expect(breaker.getState()).toBe('HALF_OPEN'); + + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + }); + describe('Recovery (HALF_OPEN state)', () => { it('should transition to HALF_OPEN after timeout', async () => { const operation = vi.fn().mockRejectedValue(new Error('test error')); diff --git a/src/utils/circuitBreaker.ts b/src/utils/circuitBreaker.ts index b96f7ccc..d219f1af 100644 --- a/src/utils/circuitBreaker.ts +++ b/src/utils/circuitBreaker.ts @@ -50,6 +50,7 @@ export class CircuitBreaker { private totalSuccesses: number = 0; private activeRequests: number = 0; private failureHistory: number[] = []; + private recoveryDeadline?: number; constructor(private config: CircuitBreakerConfig = DEFAULT_CONFIG) {} @@ -120,13 +121,16 @@ export class CircuitBreaker { */ private onFailure(): void { this.totalFailures++; - this.lastFailureTime = Date.now(); - this.failureHistory.push(Date.now()); + const now = Date.now(); + this.lastFailureTime = now; - // Clean up old failures outside monitoring period + // Clean up old failures outside monitoring period before recording + // the current failure so the count represents the active window. this.failureHistory = this.failureHistory.filter( - (time) => Date.now() - time < this.config.monitoringPeriod, + (time) => now - time < this.config.monitoringPeriod, ); + this.failureCount = this.failureHistory.length; + this.failureHistory.push(now); if (this.state === 'HALF_OPEN') { this.transitionTo('OPEN'); @@ -142,8 +146,8 @@ export class CircuitBreaker { * Check if we should attempt to reset the circuit */ private shouldAttemptReset(): boolean { - if (!this.lastFailureTime) return false; - return Date.now() - this.lastFailureTime > this.config.timeout; + if (!this.recoveryDeadline) return false; + return Date.now() >= this.recoveryDeadline; } /** @@ -160,6 +164,8 @@ export class CircuitBreaker { this.successCount = 0; } else if (newState === 'OPEN') { this.successCount = 0; + const recoveryDelay = Math.random() * this.config.timeout; + this.recoveryDeadline = Date.now() + recoveryDelay; } else if (newState === 'HALF_OPEN') { this.successCount = 0; } @@ -191,6 +197,7 @@ export class CircuitBreaker { this.lastFailureTime = undefined; this.lastStateChange = Date.now(); this.failureHistory = []; + this.recoveryDeadline = undefined; } /**