fix: harden presentation sync lifecycle

This commit is contained in:
lda
2026-07-14 13:33:32 +07:00 Verified
parent ff6cce6b5c
commit 27c127fbd1
4 changed files with 390 additions and 20 deletions
@@ -46,6 +46,7 @@ class FakeSocket {
static readonly OPEN = 1;
static readonly CLOSED = 3;
readonly sent: string[] = [];
readonly closeCalls: Array<readonly [number | undefined, string | undefined]> = [];
readyState = 0;
onopen: (() => void) | null = null;
onmessage: ((event: { readonly data: unknown }) => void) | null = null;
@@ -67,9 +68,10 @@ class FakeSocket {
this.onmessage?.({ data: JSON.stringify(message) });
}
close(code = 1000, reason = "closed"): void {
close(code?: number, reason?: string): void {
this.closeCalls.push([code, reason]);
this.readyState = FakeSocket.CLOSED;
this.onclose?.({ code, reason });
this.onclose?.({ code: code ?? 1000, reason: reason ?? "closed" });
}
}
@@ -229,4 +231,16 @@ describe("presentation sync client", () => {
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).toBeNull();
expect(sockets).toHaveLength(1);
});
it("closes socket errors without using reserved close code 1006", () => {
const storage = new MemoryStorage();
const sockets: FakeSocket[] = [];
const client = createPresentationSyncClient(makeDependencies(storage, sockets));
client.connect(grant, () => {});
sockets[0]?.onerror?.();
expect(sockets[0]?.closeCalls).toEqual([[undefined, undefined]]);
expect(sockets[0]?.closeCalls.some(([code]) => code === 1006)).toBe(false);
});
});
@@ -196,7 +196,9 @@ export const createPresentationSyncClient = (
handleServerMessage(nextSocket, event.data);
};
nextSocket.onerror = () => {
if (nextSocket === socket && active) nextSocket.close(1006, "socket error");
// 1006 is reserved for browser-reported abnormal closes and cannot be
// supplied to WebSocket.close; closing without arguments uses 1000.
if (nextSocket === socket && active) nextSocket.close();
};
nextSocket.onclose = (event) => {
if (nextSocket !== socket) return;
@@ -1,6 +1,8 @@
import { act, renderHook, waitFor } from "@testing-library/react";
import { StrictMode, type ReactNode } from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
createPresentationSyncClient,
PRESENTATION_SYNC_GRANT_STORAGE_KEY,
type PresentationSyncClientDependencies,
} from "./presentation-sync-client.js";
@@ -112,6 +114,15 @@ const makeDependencies = () => {
const resolvedGrant = () =>
new Response(JSON.stringify(grant), { status: 201 });
const strictWrapper = ({ children }: { readonly children: ReactNode }) => (
<StrictMode>{children}</StrictMode>
);
const settleAsync = async (): Promise<void> => {
await Promise.resolve();
await Promise.resolve();
};
const connectSession = async (
result: { readonly current: ReturnType<typeof usePresentationSync> },
sockets: FakeSocket[],
@@ -188,6 +199,37 @@ describe("usePresentationSync", () => {
)).toHaveLength(0);
});
it("publishes an intervening local hash instead of keeping remote suppression armed", async () => {
const { dependencies, sockets, fetch } = makeDependencies();
fetch.mockImplementation(async () => resolvedGrant());
const applyRemoteHash = vi.fn();
const { result, rerender } = renderHook(
({ hash }) =>
usePresentationSync({
role: "audience",
currentHash: hash,
applyRemoteHash,
dependencies,
}),
{ initialProps: { hash: "#scene/thesis/title" } },
);
await connectSession(result, sockets);
await act(async () => {
sockets[0]?.serverMessage(snapshot("#scene/problem/direct-actions", 1));
});
rerender({ hash: "#scene/architecture/runtime" });
expect(sockets[0]?.sent.filter((message) =>
JSON.parse(message).type === "location.publish",
)).toHaveLength(1);
expect(JSON.parse(sockets[0]?.sent[0] ?? "{}")).toMatchObject({
type: "location.publish",
hash: "#scene/architecture/runtime",
});
});
it("applies stale rejection snapshots and does not echo convergence", async () => {
const { dependencies, sockets, fetch } = makeDependencies();
fetch.mockImplementation(async () => resolvedGrant());
@@ -322,6 +364,254 @@ describe("usePresentationSync", () => {
expect(result.current.state.kind).toBe("waiting");
});
it("ignores a stale create result after a newer create operation", async () => {
const { dependencies, sockets, fetch } = makeDependencies();
const releases: Array<(response: Response) => void> = [];
fetch.mockImplementation(
() => new Promise<Response>((resolve) => releases.push(resolve)),
);
const { result } = renderHook(() =>
usePresentationSync({
role: "presenter",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies,
}),
);
await act(async () => {
void result.current.startSession();
void result.current.startSession();
});
expect(releases).toHaveLength(2);
await act(async () => {
releases[0]?.(resolvedGrant());
await settleAsync();
});
expect(sockets).toHaveLength(0);
expect(result.current.state.kind).toBe("creating");
await act(async () => {
releases[1]?.(resolvedGrant());
await settleAsync();
});
expect(sockets).toHaveLength(1);
expect(result.current.state.kind).toBe("waiting");
});
it("ignores a stale join result after a newer join operation", async () => {
const { dependencies, sockets, fetch } = makeDependencies();
const releases: Array<(response: Response) => void> = [];
fetch.mockImplementation(
() => new Promise<Response>((resolve) => releases.push(resolve)),
);
const { result } = renderHook(() =>
usePresentationSync({
role: "audience",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies,
}),
);
await act(async () => {
void result.current.joinSession("AAA111");
void result.current.joinSession("BBB222");
});
expect(releases).toHaveLength(2);
expect(result.current.state).toMatchObject({ kind: "joining", code: "BBB222" });
await act(async () => {
releases[0]?.(resolvedGrant());
await settleAsync();
});
expect(sockets).toHaveLength(0);
expect(result.current.state).toMatchObject({ kind: "joining", code: "BBB222" });
await act(async () => {
releases[1]?.(resolvedGrant());
await settleAsync();
});
expect(sockets).toHaveLength(1);
});
it("invalidates an in-flight operation when retry starts a newer operation", async () => {
const { dependencies, sockets, fetch } = makeDependencies();
const releases: Array<(response: Response) => void> = [];
fetch.mockImplementation(
() => new Promise<Response>((resolve) => releases.push(resolve)),
);
const { result } = renderHook(() =>
usePresentationSync({
role: "presenter",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies,
}),
);
await act(async () => {
void result.current.startSession();
result.current.retry();
void result.current.joinSession("CCC333");
});
expect(releases).toHaveLength(3);
await act(async () => {
releases[0]?.(resolvedGrant());
releases[1]?.(resolvedGrant());
await settleAsync();
});
expect(sockets).toHaveLength(0);
expect(result.current.state).toMatchObject({ kind: "joining", code: "CCC333" });
await act(async () => {
releases[2]?.(resolvedGrant());
await settleAsync();
});
expect(sockets).toHaveLength(1);
});
it("ignores a deferred result after leave, end, or unmount", async () => {
const makeDeferredHook = () => {
const { dependencies, sockets, fetch } = makeDependencies();
let resolvePending: ((response: Response) => void) | null = null;
fetch.mockImplementation(
() => new Promise<Response>((resolve) => { resolvePending = resolve; }),
);
const rendered = renderHook(() =>
usePresentationSync({
role: "presenter",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies,
}),
);
return {
...rendered,
release: (response: Response) => resolvePending?.(response),
sockets,
};
};
const left = makeDeferredHook();
await act(async () => { void left.result.current.startSession(); });
await act(async () => { left.result.current.leaveSession(); });
await act(async () => { left.release(resolvedGrant()); await settleAsync(); });
expect(left.sockets).toHaveLength(0);
expect(left.result.current.state).toEqual({ kind: "ended", reason: "left" });
left.unmount();
const ended = makeDeferredHook();
await act(async () => { void ended.result.current.startSession(); });
await act(async () => { ended.result.current.endSession(); });
await act(async () => { ended.release(resolvedGrant()); await settleAsync(); });
expect(ended.sockets).toHaveLength(0);
expect(ended.result.current.state).toEqual({
kind: "ended",
reason: "presenter_ended",
});
ended.unmount();
const unmounted = makeDeferredHook();
await act(async () => { void unmounted.result.current.startSession(); });
unmounted.unmount();
await act(async () => { unmounted.release(resolvedGrant()); await settleAsync(); });
expect(unmounted.sockets).toHaveLength(0);
});
it("does not read browser globals when a complete client and dependency set is injected", () => {
const { dependencies, sockets } = makeDependencies();
const client = createPresentationSyncClient(dependencies);
const realWindow = globalThis.window;
vi.stubGlobal(
"window",
new Proxy(realWindow, {
get(target, property, receiver) {
if (
property === "fetch" ||
property === "sessionStorage" ||
property === "location" ||
property === "WebSocket" ||
property === "setTimeout" ||
property === "clearTimeout"
) {
throw new Error(`unexpected browser dependency read: ${String(property)}`);
}
return Reflect.get(target, property, receiver);
},
}),
);
try {
const { unmount } = renderHook(() =>
usePresentationSync({
role: "audience",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies,
client,
}),
);
expect(sockets).toHaveLength(0);
unmount();
} finally {
vi.unstubAllGlobals();
}
});
it("reconnects a saved grant when Strict Mode replays the effect", () => {
const { dependencies, sockets, storage } = makeDependencies();
storage.setItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY, JSON.stringify(grant));
const { unmount } = renderHook(
() =>
usePresentationSync({
role: "audience",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies,
}),
{ reactStrictMode: true, wrapper: strictWrapper },
);
expect(sockets).toHaveLength(2);
expect(sockets[0]?.readyState).toBe(FakeSocket.CLOSED);
expect(storage.getItem(PRESENTATION_SYNC_GRANT_STORAGE_KEY)).not.toBeNull();
unmount();
});
it("re-runs query auto-join during Strict Mode effect replay", async () => {
const { dependencies, sockets, fetch } = makeDependencies();
const pairDependencies = {
...dependencies,
location: {
search: "?pair=ab-c%20123",
href: "http://console.test/present?pair=ab-c%20123",
},
};
const releases: Array<(response: Response) => void> = [];
fetch.mockImplementation(
() => new Promise<Response>((resolve) => releases.push(resolve)),
);
const { unmount } = renderHook(
() =>
usePresentationSync({
role: "audience",
currentHash: "#scene/thesis/title",
applyRemoteHash: () => {},
dependencies: pairDependencies,
}),
{ reactStrictMode: true, wrapper: strictWrapper },
);
expect(fetch).toHaveBeenCalledTimes(2);
releases[0]?.(resolvedGrant());
releases[1]?.(resolvedGrant());
await act(async () => { await settleAsync(); });
expect(sockets).toHaveLength(1);
unmount();
});
it("closes the socket and reconnect timer on unmount", async () => {
vi.useFakeTimers();
const { dependencies, sockets, fetch, storage } = makeDependencies();
@@ -40,6 +40,11 @@ type BrowserUrlState = {
readonly history: { readonly replaceState: unknown };
};
const emptyBrowserUrlState: BrowserUrlState = {
location: { href: "http://localhost/", search: "" },
history: { replaceState: () => {} },
};
type InternalController = PresentationSyncController & {
readonly subscribe: (listener: () => void) => () => void;
readonly getSnapshot: () => PresentationSyncState;
@@ -78,11 +83,48 @@ const defaultBrowserDependencies = (): {
};
};
const hasCompleteClientDependencies = (
dependencies: PresentationSyncHookDependencies | undefined,
): dependencies is PresentationSyncClientDependencies &
Pick<PresentationSyncHookDependencies, "location" | "history"> =>
dependencies?.fetch !== undefined &&
dependencies.createWebSocket !== undefined &&
dependencies.storage !== undefined &&
dependencies.origin !== undefined &&
dependencies.protocol !== undefined &&
dependencies.setTimeout !== undefined &&
dependencies.clearTimeout !== undefined;
const browserDependenciesFor = (
dependencies: PresentationSyncHookDependencies | undefined,
): { readonly client: PresentationSyncClientDependencies; readonly url: BrowserUrlState } => {
const defaults = defaultBrowserDependencies();
needsClient: boolean,
): {
readonly client: PresentationSyncClientDependencies | null;
readonly url: BrowserUrlState;
} => {
const { location, history, ...clientOverrides } = dependencies ?? {};
const completeClient = hasCompleteClientDependencies(dependencies);
if (!needsClient) {
return {
client: null,
url: {
location: location ?? emptyBrowserUrlState.location,
history: history ?? emptyBrowserUrlState.history,
},
};
}
if (completeClient) {
return {
client: dependencies,
url: {
location: location ?? emptyBrowserUrlState.location,
history: history ?? emptyBrowserUrlState.history,
},
};
}
const defaults = defaultBrowserDependencies();
return {
client: { ...defaults.client, ...clientOverrides } as PresentationSyncClientDependencies,
url: {
@@ -104,6 +146,7 @@ const createController = (options: {
}): InternalController => {
let state: PresentationSyncState = initialPresentationSyncState;
let mounted = true;
let operationGeneration = 0;
let currentGrant: SessionGrant | null = null;
let lastOperation: LastOperation = null;
const listeners = new Set<() => void>();
@@ -122,6 +165,14 @@ const createController = (options: {
notify();
};
const beginOperation = (): number => {
operationGeneration += 1;
return operationGeneration;
};
const isCurrentOperation = (generation: number): boolean =>
mounted && generation === operationGeneration;
const applySnapshot = (snapshot: PresentationSnapshot): void => {
if (snapshot.hash === options.getCurrentHash()) return;
@@ -209,6 +260,7 @@ const createController = (options: {
};
const startSession = async (): Promise<void> => {
const generation = beginOperation();
lastOperation = { kind: "create" };
currentGrant = null;
pendingMessageIds.clear();
@@ -219,10 +271,10 @@ const createController = (options: {
options.getRole(),
options.getCurrentHash(),
);
if (!mounted) return;
if (!isCurrentOperation(generation)) return;
connectGrant(grant);
} catch (error) {
if (mounted) {
if (isCurrentOperation(generation)) {
dispatch({
type: "failed",
message: errorMessage(error),
@@ -233,6 +285,7 @@ const createController = (options: {
};
const joinSession = async (code: string): Promise<void> => {
const generation = beginOperation();
const normalizedCode = normalizeJoinCode(code);
lastOperation = { kind: "join", code: normalizedCode };
currentGrant = null;
@@ -241,10 +294,10 @@ const createController = (options: {
dispatch({ type: "start_join", code: normalizedCode });
try {
const grant = await options.client.join(options.getRole(), normalizedCode);
if (!mounted) return;
if (!isCurrentOperation(generation)) return;
connectGrant(grant);
} catch (error) {
if (mounted) {
if (isCurrentOperation(generation)) {
dispatch({
type: "failed",
message: errorMessage(error),
@@ -263,6 +316,7 @@ const createController = (options: {
};
const leaveSession = (): void => {
beginOperation();
currentGrant = null;
pendingMessageIds.clear();
options.client.leave();
@@ -270,6 +324,7 @@ const createController = (options: {
};
const endSession = (): void => {
beginOperation();
currentGrant = null;
pendingMessageIds.clear();
options.client.end();
@@ -288,9 +343,9 @@ const createController = (options: {
};
const dispose = (): void => {
beginOperation();
mounted = false;
options.client.dispose();
listeners.clear();
};
return {
@@ -346,11 +401,14 @@ export const usePresentationSync = ({
applyRemoteHashRef.current = applyRemoteHash;
const controllerRef = useRef<InternalController | null>(null);
const autoJoinConsumedRef = useRef(false);
const pairCodeRef = useRef<string | null>(null);
if (controllerRef.current === null) {
const browser = browserDependenciesFor(dependencies);
const browser = browserDependenciesFor(dependencies, injectedClient === undefined);
const client =
injectedClient ?? createPresentationSyncClient(browser.client);
injectedClient ??
(browser.client === null
? (() => { throw new Error("presentation sync client dependencies unavailable"); })()
: createPresentationSyncClient(browser.client));
controllerRef.current = createController({
client,
getRole: () => roleRef.current,
@@ -374,13 +432,11 @@ export const usePresentationSync = ({
useEffect(() => {
controller.mount();
if (autoJoinConsumedRef.current) return () => controller.dispose();
autoJoinConsumedRef.current = true;
const restoredGrant = controller.restoreSavedGrant();
if (restoredGrant !== null) controller.restoreGrant(restoredGrant);
else {
const pairCode = pairCodeFromUrl(controller.browserUrl);
const pairCode = pairCodeRef.current ?? pairCodeFromUrl(controller.browserUrl);
pairCodeRef.current = pairCode;
if (pairCode !== null) void controller.joinSession(pairCode);
}
return () => controller.dispose();
@@ -389,9 +445,17 @@ export const usePresentationSync = ({
useEffect(() => {
const remoteHash = remoteHashInFlightRef.current;
if (remoteHash !== null) {
lastObservedHashRef.current = currentHash;
if (remoteHash === currentHash) remoteHashInFlightRef.current = null;
return;
if (remoteHash === currentHash) {
lastObservedHashRef.current = currentHash;
remoteHashInFlightRef.current = null;
return;
}
// A rerender caused by connection state can observe the old hash before
// the route callback updates it. Keep suppression armed for that case,
// but treat a genuinely changed hash as local navigation.
if (lastObservedHashRef.current === currentHash) return;
remoteHashInFlightRef.current = null;
}
if (lastObservedHashRef.current === currentHash) return;