diff --git a/app/hooks/__tests__/useDraftSocket.reconnect.test.ts b/app/hooks/__tests__/useDraftSocket.reconnect.test.ts new file mode 100644 index 0000000..0b25f6b --- /dev/null +++ b/app/hooks/__tests__/useDraftSocket.reconnect.test.ts @@ -0,0 +1,369 @@ +/** + * Tests for useDraftSocket reconnection logic. + * + * Covers the Disconnect → State-Change-in-DB → Reconnect flow and ensures + * reconnectCount (which the draft route uses to trigger revalidate()) behaves + * correctly across reconnects, visibility changes, and network flapping. + * + * We mock socket.io-client so the tests run in jsdom with no real network. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; + +// --------------------------------------------------------------------------- +// socket.io-client mock +// --------------------------------------------------------------------------- + +type EventHandler = (...args: any[]) => void; + +/** + * Minimal, controllable EventEmitter that mimics the socket.io-client Socket + * and its Manager (socket.io) interface. + */ +function makeSocketMock() { + const socketHandlers: Record = {}; + const managerHandlers: Record = {}; + + const socketMock = { + connected: false, + id: "socket-test-id", + on: vi.fn((event: string, handler: EventHandler) => { + socketHandlers[event] ??= []; + socketHandlers[event].push(handler); + }), + off: vi.fn((event: string, handler?: EventHandler) => { + if (!handler) { + socketHandlers[event] = []; + } else { + socketHandlers[event] = (socketHandlers[event] ?? []).filter( + (h) => h !== handler + ); + } + }), + emit: vi.fn(), + connect: vi.fn(() => { + socketMock.connected = true; + // Simulate the "connect" event firing on the next tick + setTimeout(() => { + socketMock._trigger("connect"); + }, 0); + }), + disconnect: vi.fn(() => { + socketMock.connected = false; + }), + // Manager-level events (socket.io.on / socket.io.off) + io: { + on: vi.fn((event: string, handler: EventHandler) => { + managerHandlers[event] ??= []; + managerHandlers[event].push(handler); + }), + off: vi.fn((event: string, handler?: EventHandler) => { + if (!handler) { + managerHandlers[event] = []; + } else { + managerHandlers[event] = (managerHandlers[event] ?? []).filter( + (h) => h !== handler + ); + } + }), + }, + + // Test helpers — call these to simulate server-side events + _trigger(event: string, ...args: any[]) { + (socketHandlers[event] ?? []).forEach((h) => h(...args)); + }, + _triggerManager(event: string, ...args: any[]) { + (managerHandlers[event] ?? []).forEach((h) => h(...args)); + }, + }; + + return socketMock; +} + +type SocketMock = ReturnType; +let socketMock: SocketMock; + +vi.mock("socket.io-client", () => ({ + io: vi.fn(() => socketMock), +})); + +// --------------------------------------------------------------------------- +// Import the hook under test (after mocking) +// --------------------------------------------------------------------------- + +import { useDraftSocket } from "../useDraftSocket"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function renderDraftSocket(seasonId = "season-1", teamId?: string) { + return renderHook(() => useDraftSocket(seasonId, teamId)); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("useDraftSocket – reconnection", () => { + beforeEach(() => { + socketMock = makeSocketMock(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); + }); + + // ------------------------------------------------------------------------- + // Basic connection + // ------------------------------------------------------------------------- + + it("starts disconnected and connects on mount", async () => { + const { result } = renderDraftSocket(); + + expect(result.current.isConnected).toBe(false); + + // Simulate the socket emitting "connect" + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + + expect(result.current.isConnected).toBe(true); + expect(result.current.reconnectCount).toBe(0); + expect(result.current.isReconnecting).toBe(false); + }); + + // ------------------------------------------------------------------------- + // reconnectCount increments on true socket reconnect + // ------------------------------------------------------------------------- + + it("increments reconnectCount on socket reconnect (not initial connect)", async () => { + const { result } = renderDraftSocket(); + + // First connection — reconnectCount must stay 0 + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + expect(result.current.reconnectCount).toBe(0); + + // Server-initiated disconnect + act(() => { + socketMock.connected = false; + socketMock._trigger("disconnect", "transport error"); + }); + expect(result.current.isConnected).toBe(false); + expect(result.current.isReconnecting).toBe(true); + + // Reconnect (second call to "connect" on the same socket instance) + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + + expect(result.current.isConnected).toBe(true); + expect(result.current.reconnectCount).toBe(1); + }); + + // ------------------------------------------------------------------------- + // Visibility-change triggers reconnect counter when socket is alive + // ------------------------------------------------------------------------- + + it("increments reconnectCount on visibilitychange when socket is already connected", () => { + const { result } = renderDraftSocket("season-1", "team-1"); + + // Initial connect + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + expect(result.current.reconnectCount).toBe(0); + + // App comes back to foreground while socket is still alive + act(() => { + Object.defineProperty(document, "visibilityState", { + value: "visible", + configurable: true, + }); + document.dispatchEvent(new Event("visibilitychange")); + }); + + expect(result.current.reconnectCount).toBe(1); + // join-draft re-emitted so the server knows we're back + expect(socketMock.emit).toHaveBeenCalledWith("join-draft", "season-1", "team-1"); + }); + + it("triggers socket.connect() on visibilitychange when socket is disconnected", () => { + const { result } = renderDraftSocket(); + + // Connect then lose the connection + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + act(() => { + socketMock.connected = false; + socketMock._trigger("disconnect", "transport close"); + }); + + expect(result.current.isReconnecting).toBe(true); + + // Tab becomes visible — hook should call socket.connect() + act(() => { + Object.defineProperty(document, "visibilityState", { + value: "visible", + configurable: true, + }); + document.dispatchEvent(new Event("visibilitychange")); + }); + + expect(socketMock.connect).toHaveBeenCalled(); + }); + + // ------------------------------------------------------------------------- + // Network flapping — multiple rapid reconnects in succession + // ------------------------------------------------------------------------- + + it("correctly counts reconnects during network flapping (5 rapid cycles)", () => { + const { result } = renderDraftSocket(); + + // Initial connect + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + expect(result.current.reconnectCount).toBe(0); + + const FLAP_CYCLES = 5; + for (let i = 0; i < FLAP_CYCLES; i++) { + act(() => { + socketMock.connected = false; + socketMock._trigger("disconnect", "transport error"); + }); + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + } + + // Each reconnect (not the initial connect) increments the counter + expect(result.current.reconnectCount).toBe(FLAP_CYCLES); + expect(result.current.isConnected).toBe(true); + expect(result.current.isReconnecting).toBe(false); + }); + + it("does not set a permanent error state during transient network flapping", () => { + const { result } = renderDraftSocket(); + + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + + // Flap 3 times + for (let i = 0; i < 3; i++) { + act(() => { + socketMock.connected = false; + socketMock._trigger("disconnect", "transport error"); + }); + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + } + + expect(result.current.connectionError).toBeNull(); + expect(result.current.isReconnecting).toBe(false); + }); + + // ------------------------------------------------------------------------- + // Disconnect → DB state change → Reconnect simulation + // + // This is the primary regression test for the stale-state bug. + // The hook's responsibility is to increment reconnectCount so that the + // draft route's useEffect calls revalidate(). The actual state merge + // (picks + currentPick sync) lives in the route component and is tested + // via the sync effect in the route; here we verify the contract the hook + // fulfils. + // ------------------------------------------------------------------------- + + it("emits join-draft on every reconnect so the server re-sends presence data", () => { + const { result } = renderDraftSocket("season-42", "team-7"); + + // Initial connect + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + + const joinCallsBefore = socketMock.emit.mock.calls.filter( + ([ev]) => ev === "join-draft" + ).length; + + // Simulate DB picks being added while client is disconnected + act(() => { + socketMock.connected = false; + socketMock._trigger("disconnect", "ping timeout"); + }); + + // Client reconnects — the hook must re-emit join-draft + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + + const joinCallsAfter = socketMock.emit.mock.calls.filter( + ([ev]) => ev === "join-draft" + ).length; + + expect(joinCallsAfter).toBe(joinCallsBefore + 1); + expect(result.current.reconnectCount).toBe(1); + }); + + // ------------------------------------------------------------------------- + // Error handling — exhausted reconnect attempts + // ------------------------------------------------------------------------- + + it("sets connectionError and clears isReconnecting on reconnect_failed", () => { + const { result } = renderDraftSocket(); + + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + act(() => { + socketMock.connected = false; + socketMock._trigger("disconnect", "transport error"); + }); + + // All reconnect attempts exhausted + act(() => { + socketMock._triggerManager("reconnect_failed"); + }); + + expect(result.current.connectionError).toBeTruthy(); + expect(result.current.isReconnecting).toBe(false); + }); + + // ------------------------------------------------------------------------- + // Cleanup — no listeners leak after unmount + // ------------------------------------------------------------------------- + + it("emits leave-draft and disconnects on unmount", () => { + const { unmount } = renderDraftSocket("season-1", "team-1"); + + act(() => { + socketMock.connected = true; + socketMock._trigger("connect"); + }); + + unmount(); + + expect(socketMock.emit).toHaveBeenCalledWith("leave-draft", "season-1"); + expect(socketMock.disconnect).toHaveBeenCalled(); + }); +}); diff --git a/app/routes/leagues/$leagueId.draft.$seasonId.tsx b/app/routes/leagues/$leagueId.draft.$seasonId.tsx index 95b2738..f911301 100644 --- a/app/routes/leagues/$leagueId.draft.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.draft.$seasonId.tsx @@ -227,7 +227,7 @@ export default function DraftRoom() { numFlexPicks, ownerMap, } = useLoaderData(); - const { revalidate } = useRevalidator(); + const { revalidate, state: revalidatorState } = useRevalidator(); const { isConnected, connectionError, isReconnecting, reconnectCount, on, off } = useDraftSocket(season.id, userTeam?.id); const { permissionState: notificationsPermission, @@ -267,6 +267,53 @@ export default function DraftRoom() { } }, [reconnectCount, revalidate]); + // Track revalidation lifecycle to sync local state from fresh loader data. + // + // The problem: `picks` and `currentPick` are local state initialized once + // from useLoaderData(). When revalidate() runs after a reconnect, React + // Router re-fetches `draftPicks`/`season` — but those updates never flow + // back into the local useState copies automatically. + // + // Race condition: a `pick-made` socket event that arrives *during* the + // revalidation window would normally be applied to the stale prev state and + // then overwritten when we sync from the DB snapshot. We prevent that by + // buffering live picks while revalidation is in flight and merging them + // (deduplicated by ID) after the DB snapshot lands. + const revalidatorStateRef = useRef(revalidatorState); + const isRevalidatingRef = useRef(false); + const pendingPicksDuringRevalidationRef = useRef([]); + + useEffect(() => { + const prev = revalidatorStateRef.current; + revalidatorStateRef.current = revalidatorState; + + if (prev !== "loading" && revalidatorState === "loading") { + // Revalidation just started — buffer any live picks to avoid loss + isRevalidatingRef.current = true; + pendingPicksDuringRevalidationRef.current = []; + } else if (prev === "loading" && revalidatorState === "idle") { + // Revalidation just completed — merge DB snapshot with buffered picks + isRevalidatingRef.current = false; + const dbPickIds = new Set(draftPicks.map((p: any) => p.id)); + const missedPicks = pendingPicksDuringRevalidationRef.current.filter( + (p: any) => !dbPickIds.has(p.id) + ); + pendingPicksDuringRevalidationRef.current = []; + + setPicks([...draftPicks, ...missedPicks]); + setCurrentPick(season.currentPickNumber || 1); + setIsPaused(season.draftPaused || false); + setIsDraftComplete( + season.status === "active" || season.status === "completed" + ); + // Sync the queue — participants drafted while we were away will have + // been removed server-side, but we never received those + // `participant-removed-from-queues` events. The loader re-fetches + // userQueue fresh on every revalidation so we can trust it here. + setQueue(userQueue); + } + }, [revalidatorState, draftPicks, season]); + const [picks, setPicks] = useState(draftPicks); const [currentPick, setCurrentPick] = useState(season.currentPickNumber || 1); const [searchQuery, setSearchQuery] = useState(""); @@ -442,8 +489,17 @@ export default function DraftRoom() { // Listen for new picks from other users useEffect(() => { const handlePickMade = (data: any) => { - setPicks((prev: any) => [...prev, data.pick]); - setCurrentPick(data.nextPickNumber); + if (isRevalidatingRef.current) { + // Buffer this pick; the revalidation-complete handler will merge it + // into the DB snapshot so it isn't silently dropped. + // Don't update currentPick either — the sync effect will set it from + // fresh season.currentPickNumber once loading → idle, keeping the + // pick counter and the picks list in sync with each other. + pendingPicksDuringRevalidationRef.current.push(data.pick); + } else { + setPicks((prev: any) => [...prev, data.pick]); + setCurrentPick(data.nextPickNumber); + } // No meaningful "next turn" notification once the draft is over if (data.isDraftComplete) return;