diff --git a/packages/appkit/src/plugin/interceptors/retry.ts b/packages/appkit/src/plugin/interceptors/retry.ts index c0a184b8..435e0fde 100644 --- a/packages/appkit/src/plugin/interceptors/retry.ts +++ b/packages/appkit/src/plugin/interceptors/retry.ts @@ -59,11 +59,10 @@ export class RetryInterceptor implements ExecutionInterceptor { } private calculateDelay(attempt: number): number { - // exponential backoff const delay = this.initialDelay * 2 ** (attempt - 1); + const capped = Math.min(delay, this.maxDelay); - // max delay cap - return Math.min(delay, this.maxDelay); + return capped * Math.random(); } private sleep(ms: number): Promise { diff --git a/packages/appkit/src/plugin/tests/retry.test.ts b/packages/appkit/src/plugin/tests/retry.test.ts index 913a8262..b897c585 100644 --- a/packages/appkit/src/plugin/tests/retry.test.ts +++ b/packages/appkit/src/plugin/tests/retry.test.ts @@ -83,6 +83,8 @@ describe("RetryInterceptor", () => { attempts: 4, initialDelay: 1000, }; + + vi.spyOn(Math, "random").mockReturnValue(1); const interceptor = new RetryInterceptor(config); const fn = vi .fn() @@ -93,17 +95,20 @@ describe("RetryInterceptor", () => { interceptor.intercept(fn, context); - // First retry: 1000ms delay (2^0 * 1000) + // With Math.random() = 1, jitter multiplier is 1x (no reduction) + // First retry: 1000ms delay (2^0 * 1000 * 1) await vi.advanceTimersByTimeAsync(1000); expect(fn).toHaveBeenCalledTimes(2); - // Second retry: 2000ms delay (2^1 * 1000) + // Second retry: 2000ms delay (2^1 * 1000 * 1) await vi.advanceTimersByTimeAsync(2000); expect(fn).toHaveBeenCalledTimes(3); - // Third retry: 4000ms delay (2^2 * 1000) + // Third retry: 4000ms delay (2^2 * 1000 * 1) await vi.advanceTimersByTimeAsync(4000); expect(fn).toHaveBeenCalledTimes(4); + + vi.spyOn(Math, "random").mockRestore(); }); test("should respect maxDelay cap", async () => { @@ -111,21 +116,24 @@ describe("RetryInterceptor", () => { enabled: true, attempts: 10, initialDelay: 1000, - maxDelay: 5000, // Cap at 5 seconds + maxDelay: 5000, }; + + vi.spyOn(Math, "random").mockReturnValue(1); const interceptor = new RetryInterceptor(config); const fn = vi.fn().mockRejectedValue(new Error("fail")); interceptor.intercept(fn, context); - // After 3 retries, delay should be capped at maxDelay - // Attempt 4 would normally be 8000ms (2^3 * 1000), but capped at 5000ms + // With Math.random() = 1, delays are at their maximum await vi.advanceTimersByTimeAsync(1000); // 1st retry await vi.advanceTimersByTimeAsync(2000); // 2nd retry await vi.advanceTimersByTimeAsync(4000); // 3rd retry - await vi.advanceTimersByTimeAsync(5000); // 4th retry (capped) + await vi.advanceTimersByTimeAsync(5000); // 4th retry (capped at maxDelay) expect(fn).toHaveBeenCalledTimes(5); + + vi.spyOn(Math, "random").mockRestore(); }); test("should not retry if signal is aborted", async () => { @@ -170,4 +178,67 @@ describe("RetryInterceptor", () => { await expect(interceptor.intercept(fn, context)).rejects.toThrow("fail"); expect(fn).toHaveBeenCalledTimes(1); }); + + test("should apply full jitter: delay between 0 and capped value", async () => { + const config: RetryConfig = { + enabled: true, + attempts: 3, + initialDelay: 1000, + }; + + // At Math.random() = 0, delay = 1000 * 0 = 0ms (minimum) + vi.spyOn(Math, "random").mockReturnValue(0); + const interceptorMin = new RetryInterceptor(config); + const fnMin = vi + .fn() + .mockRejectedValueOnce(new Error("fail")) + .mockResolvedValue("ok"); + + const promiseMin = interceptorMin.intercept(fnMin, context); + await vi.advanceTimersByTimeAsync(0); + await promiseMin; + expect(fnMin).toHaveBeenCalledTimes(2); + + // At Math.random() = 1, delay = 1000 * 1 = 1000ms (maximum) + vi.spyOn(Math, "random").mockReturnValue(1); + const interceptorMax = new RetryInterceptor(config); + const fnMax = vi + .fn() + .mockRejectedValueOnce(new Error("fail")) + .mockResolvedValue("ok"); + + interceptorMax.intercept(fnMax, context); + await vi.advanceTimersByTimeAsync(999); + expect(fnMax).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(fnMax).toHaveBeenCalledTimes(2); + + vi.spyOn(Math, "random").mockRestore(); + }); + + test("should produce jittered delay at midpoint", async () => { + const config: RetryConfig = { + enabled: true, + attempts: 3, + initialDelay: 1000, + }; + + vi.spyOn(Math, "random").mockReturnValue(0.5); + const interceptor = new RetryInterceptor(config); + const fn = vi + .fn() + .mockRejectedValueOnce(new Error("fail")) + .mockResolvedValue("success"); + + interceptor.intercept(fn, context); + + // delay = 1000 * 0.5 = 500ms + await vi.advanceTimersByTimeAsync(499); + expect(fn).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(1); + expect(fn).toHaveBeenCalledTimes(2); + + vi.spyOn(Math, "random").mockRestore(); + }); }); diff --git a/packages/shared/src/execute.ts b/packages/shared/src/execute.ts index 9221ea2f..62ac45bf 100644 --- a/packages/shared/src/execute.ts +++ b/packages/shared/src/execute.ts @@ -13,7 +13,7 @@ export interface StreamConfig { maxActiveStreams?: number; } -/** Retry configuration for the RetryInterceptor. Uses exponential backoff between attempts. */ +/** Retry configuration for the RetryInterceptor. Uses exponential backoff with full jitter between attempts. */ export interface RetryConfig { enabled?: boolean; attempts?: number;