fix: fully sync local draft state after socket reconnection (#41)
On reconnect, revalidate() re-fetches fresh loader data but the local useState copies (picks, currentPick, isPaused, isDraftComplete, queue) never synced with the new values — leaving the UI stale until a manual refresh. - Watch revalidatorState (idle→loading→idle) to detect when a revalidation completes and apply the fresh loader snapshot to all affected local state - Buffer pick-made socket events received during the revalidation window (instead of discarding or double-applying them); merge into the DB snapshot on completion, deduplicated by pick ID - Keep setCurrentPick paired with setPicks in handlePickMade so the "on the clock" indicator and the picks list never desync - Sync queue state from fresh userQueue after revalidation so participants drafted while the user was away disappear from their queue even if the participant-removed-from-queues events were missed Adds useDraftSocket reconnect test suite (9 tests) covering initial connect, socket reconnect, visibility-change triggering, network flapping, error exhaustion, and cleanup. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Chris Parsons
GitHub
parent
1ba50828f7
commit
6dfe56e178
2 changed files with 428 additions and 3 deletions
369
app/hooks/__tests__/useDraftSocket.reconnect.test.ts
Normal file
369
app/hooks/__tests__/useDraftSocket.reconnect.test.ts
Normal file
|
|
@ -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<string, EventHandler[]> = {};
|
||||
const managerHandlers: Record<string, EventHandler[]> = {};
|
||||
|
||||
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<typeof makeSocketMock>;
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
@ -227,7 +227,7 @@ export default function DraftRoom() {
|
|||
numFlexPicks,
|
||||
ownerMap,
|
||||
} = useLoaderData<typeof loader>();
|
||||
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<any[]>([]);
|
||||
|
||||
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;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue