diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx index c6fd57230b..6093ae74ef 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx @@ -2089,6 +2089,7 @@ describe("useChatStore", () => { it("sets reconnectState on WebSocket disconnect and clears it after reconnect", async () => { immediateAnimationFrame(); + vi.spyOn(Math, "random").mockReturnValue(0.5); const chatID = "chat-disconnect"; const mockSocket1 = createMockSocket(); diff --git a/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts b/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts index 5cafc47b05..bb9151be34 100644 --- a/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts +++ b/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts @@ -283,6 +283,7 @@ describe("useGitWatcher", () => { vi.useFakeTimers(); try { + vi.spyOn(Math, "random").mockReturnValue(0.5); const socket1 = createMockSocket(); renderHook(() => diff --git a/site/src/utils/reconnectingWebSocket.test.ts b/site/src/utils/reconnectingWebSocket.test.ts index 47444c0461..79560897ec 100644 --- a/site/src/utils/reconnectingWebSocket.test.ts +++ b/site/src/utils/reconnectingWebSocket.test.ts @@ -40,6 +40,8 @@ const expectReconnectSchedule = ( ); }; +const deterministicRandom = () => 0.5; + beforeEach(() => { vi.useFakeTimers(); vi.setSystemTime(new Date("2025-01-01T00:00:00.000Z")); @@ -95,6 +97,7 @@ describe("createReconnectingWebSocket", () => { baseMs: 1000, maxMs: 10000, factor: 2, + random: deterministicRandom, }); expect(connect).toHaveBeenCalledTimes(1); @@ -128,6 +131,189 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(4); }); + it("applies the minimum jitter bound when random returns 0", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0.3, + random: () => 0, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 700 }); + + vi.advanceTimersByTime(699); + expect(connect).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(connect).toHaveBeenCalledTimes(2); + }); + + it("applies the maximum jitter bound when random returns 1", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0.3, + random: () => 1, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1300 }); + + vi.advanceTimersByTime(1299); + expect(connect).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(connect).toHaveBeenCalledTimes(2); + }); + + it("preserves legacy exact timing when jitter is disabled", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0, + random: () => 1, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + + vi.advanceTimersByTime(999); + expect(connect).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(connect).toHaveBeenCalledTimes(2); + }); + + it("clamps jitter greater than 1 before delay math", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 1.5, + random: () => 1, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 2000 }); + }); + + it("treats negative jitter as no jitter", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: -0.5, + random: () => 0, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + }); + + it("treats NaN jitter as no jitter", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: Number.NaN, + random: () => 0.5, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + }); + + it("treats NaN from random() as the midpoint with no jitter offset", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0.3, + random: () => Number.NaN, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + }); + it("caps backoff delay at maxMs", () => { let activeSocket = createMockSocket(); const connect = vi.fn(() => { @@ -140,6 +326,7 @@ describe("createReconnectingWebSocket", () => { baseMs: 1000, maxMs: 5000, factor: 2, + random: () => 1, }); // Disconnect enough times that the uncapped delay would exceed @@ -158,6 +345,81 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(5); }); + it("preserves jitter spread after raw backoff exceeds maxMs", () => { + const makeReconnect = (random: () => number) => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + maxMs: 10000, + factor: 2, + jitter: 0.3, + random, + }); + + for (let i = 0; i < 4; i++) { + activeSocket.emit("close"); + vi.runOnlyPendingTimers(); + } + activeSocket.emit("close"); + + return disconnects[4]!; + }; + + const minReconnect = makeReconnect(() => 0); + expectReconnectSchedule(minReconnect, { attempt: 5, delayMs: 7000 }); + + vi.clearAllTimers(); + vi.setSystemTime(new Date("2025-01-01T00:00:00.000Z")); + + const maxReconnect = makeReconnect(() => 1); + expectReconnectSchedule(maxReconnect, { attempt: 5, delayMs: 10000 }); + expect(maxReconnect.reconnect.delayMs).toBeGreaterThan( + minReconnect.reconnect.delayMs, + ); + }); + + it("saturates overflowed backoff at maxMs instead of 0ms", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + maxMs: 5000, + factor: 1e308, + jitter: 0, + random: deterministicRandom, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + vi.runOnlyPendingTimers(); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[1]!, { attempt: 2, delayMs: 5000 }); + }); + it("resets backoff on successful connection", () => { let activeSocket = createMockSocket(); const connect = vi.fn(() => { @@ -170,6 +432,7 @@ describe("createReconnectingWebSocket", () => { baseMs: 1000, maxMs: 10000, factor: 2, + random: deterministicRandom, }); // Disconnect twice to bump the attempt counter. @@ -213,28 +476,6 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(2); }); - it("closes previous socket when reconnecting", () => { - let activeSocket = createMockSocket(); - const sockets: ReturnType[] = []; - const connect = vi.fn(() => { - activeSocket = createMockSocket(); - sockets.push(activeSocket); - return activeSocket; - }); - - createReconnectingWebSocket({ connect }); - - const firstSocket = sockets[0]!; - firstSocket.emit("close"); - vi.runOnlyPendingTimers(); - - // The connect function creates a new socket. The old socket was - // already "closed" by the browser, but on a fresh reconnection - // the utility closes the previous one if it's still the active - // reference. - expect(connect).toHaveBeenCalledTimes(2); - }); - it("dispose stops reconnection and closes the socket", () => { const socket = createMockSocket(); const connect = vi.fn(() => socket); @@ -289,25 +530,17 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(1); }); - it("passes socket to onOpen callback", () => { - const socket = createMockSocket(); - const connect = vi.fn(() => socket); - const onOpen = vi.fn(); - - createReconnectingWebSocket({ connect, onOpen }); - - socket.emit("open"); - expect(onOpen).toHaveBeenCalledWith(socket); - }); - - it("uses default backoff values when none provided", () => { + it("uses default backoff values when backoff options are omitted", () => { let activeSocket = createMockSocket(); const connect = vi.fn(() => { activeSocket = createMockSocket(); return activeSocket; }); - createReconnectingWebSocket({ connect }); + createReconnectingWebSocket({ + connect, + random: deterministicRandom, + }); // Default: baseMs=1000, factor=2, maxMs=10000. activeSocket.emit("close"); diff --git a/site/src/utils/reconnectingWebSocket.ts b/site/src/utils/reconnectingWebSocket.ts index 47ed0c976c..24841f78eb 100644 --- a/site/src/utils/reconnectingWebSocket.ts +++ b/site/src/utils/reconnectingWebSocket.ts @@ -32,12 +32,19 @@ /** Default base delay for exponential backoff (milliseconds). */ const RECONNECT_BASE_MS = 1_000; -/** Default maximum delay cap for exponential backoff (milliseconds). */ +/** Default maximum base delay cap for exponential backoff (milliseconds). */ const RECONNECT_MAX_MS = 10_000; /** Default multiplier applied to the base delay on each retry. */ const RECONNECT_FACTOR = 2; +/** + * Default symmetric jitter applied to the computed reconnect delay. + * `0.3` means the final delay is randomized within ±30% of the base + * exponential-backoff value. + */ +const RECONNECT_JITTER = 0.3; + /** * Metadata for the reconnect attempt that was just scheduled. * `attempt` is 1-based and user-facing: `1` means the first retry after @@ -92,25 +99,83 @@ interface ReconnectingWebSocketOptions { /** Base delay in milliseconds. Defaults to {@link RECONNECT_BASE_MS}. */ baseMs?: number; - /** Maximum delay cap in milliseconds. Defaults to {@link RECONNECT_MAX_MS}. */ + /** + * Hard upper bound on the reconnect delay in milliseconds. Jitter is + * applied to the capped backoff base, so the final delay never exceeds + * this value. + */ maxMs?: number; /** Multiplier applied per attempt. Defaults to {@link RECONNECT_FACTOR}. */ factor?: number; + + /** + * Symmetric jitter applied to the computed delay. `0.3` means the + * final delay may vary within ±30% of the base exponential-backoff + * value. Set to `0` to preserve exact legacy timing. Values are + * clamped to `[0, 1]`; non-finite values are treated as `0`. + */ + jitter?: number; + + /** + * Random-number source used for jitter. Defaults to `Math.random` and + * exists primarily as a deterministic test seam. Output is normalized + * to `[0, 1]`; non-finite values fall back to `0.5`. + */ + random?: () => number; } +const normalizeUnitInterval = (value: number, fallback: number): number => + Number.isFinite(value) ? Math.min(Math.max(value, 0), 1) : fallback; + +const normalizeDelayMs = (value: number, fallback: number): number => + Number.isFinite(value) ? Math.max(0, value) : fallback; + +const applyReconnectJitter = ({ + delayMs, + jitter, + random, +}: { + delayMs: number; + jitter: number; + random: () => number; +}): number => { + const safeJitter = normalizeUnitInterval(jitter, 0); + if (safeJitter <= 0) { + return delayMs; + } + const safeRandom = normalizeUnitInterval(random(), 0.5); + const jitterOffset = (safeRandom * 2 - 1) * safeJitter; + return normalizeDelayMs(Math.round(delayMs * (1 + jitterOffset)), delayMs); +}; + const getReconnectSchedule = ({ attempt, baseMs, maxMs, factor, + jitter, + random, }: { attempt: number; baseMs: number; maxMs: number; factor: number; + jitter: number; + random: () => number; }): ReconnectSchedule => { - const delayMs = Math.min(baseMs * factor ** (attempt - 1), maxMs); + const safeMaxMs = normalizeDelayMs(maxMs, 0); + const rawDelayMs = normalizeDelayMs( + baseMs * factor ** (attempt - 1), + safeMaxMs, + ); + const cappedDelayMs = Math.min(rawDelayMs, safeMaxMs); + const jitteredDelayMs = applyReconnectJitter({ + delayMs: cappedDelayMs, + jitter, + random, + }); + const delayMs = Math.min(jitteredDelayMs, safeMaxMs); return { attempt, delayMs, @@ -129,7 +194,11 @@ const getReconnectSchedule = ({ * * Backoff delay formula: * ``` - * delay = min(baseMs * factor ^ (attempt - 1), maxMs) + * rawDelay = baseMs * factor ^ (attempt - 1) + * cappedDelay = min(rawDelay, maxMs) + * jitteredDelay = round(cappedDelay * (1 + offset)) + * delay = min(jitteredDelay, maxMs) + * offset ∈ [-jitter, +jitter] * ``` * * The reconnect attempt counter resets after a successful `open`. @@ -146,6 +215,8 @@ export function createReconnectingWebSocket( baseMs = RECONNECT_BASE_MS, maxMs = RECONNECT_MAX_MS, factor = RECONNECT_FACTOR, + jitter = RECONNECT_JITTER, + random = Math.random, } = options; let disposed = false; @@ -195,6 +266,8 @@ export function createReconnectingWebSocket( baseMs, maxMs, factor, + jitter, + random, }); onDisconnect?.(reconnect); scheduleReconnect(reconnect);