From 1a32f909297c6394fe65f8b2eb8b9206f982f0d7 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 10 Dec 2025 11:14:23 +0100 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20auto-reconnect=20WebS?= =?UTF-8?q?ocket=20on=20server=20restart?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when the dev server restarted (nodemon), the frontend would incorrectly show the 'Authentication Required' modal instead of automatically reconnecting. Changes: - Add 'reconnecting' status to APIState for graceful reconnection UX - Track whether we've ever connected successfully (hasConnectedRef) - Implement exponential backoff reconnection (100ms → 10s, max 10 attempts) - Only show auth_required on first connection failure or auth error codes - Normal disconnects (1001, 1006) trigger reconnection if previously connected - Auth error codes (1008, 4401) still show auth modal _Generated with mux_ Change-Id: I8cf8431181a879c5faae6cff48660212f8b68e95 Signed-off-by: Thomas Kosiewski --- src/browser/contexts/API.test.tsx | 267 ++++++++++++++++++++++++++++++ src/browser/contexts/API.tsx | 54 +++++- 2 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 src/browser/contexts/API.test.tsx diff --git a/src/browser/contexts/API.test.tsx b/src/browser/contexts/API.test.tsx new file mode 100644 index 0000000000..cbd7c6ffee --- /dev/null +++ b/src/browser/contexts/API.test.tsx @@ -0,0 +1,267 @@ +import { act, cleanup, render, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, mock, test, beforeAll } from "bun:test"; +import { GlobalWindow } from "happy-dom"; + +// Mock WebSocket that we can control +class MockWebSocket { + static instances: MockWebSocket[] = []; + url: string; + readyState = 0; // CONNECTING + eventListeners = new Map void>>(); + + constructor(url: string) { + this.url = url; + MockWebSocket.instances.push(this); + } + + addEventListener(event: string, handler: (event?: unknown) => void) { + const handlers = this.eventListeners.get(event) ?? []; + handlers.push(handler); + this.eventListeners.set(event, handlers); + } + + close() { + this.readyState = 3; // CLOSED + } + + // Test helpers + simulateOpen() { + this.readyState = 1; // OPEN + this.eventListeners.get("open")?.forEach((h) => h()); + } + + simulateClose(code: number) { + this.readyState = 3; + this.eventListeners.get("close")?.forEach((h) => h({ code })); + } + + simulateError() { + this.eventListeners.get("error")?.forEach((h) => h()); + } + + static reset() { + MockWebSocket.instances = []; + } + + static lastInstance(): MockWebSocket | undefined { + return MockWebSocket.instances[MockWebSocket.instances.length - 1]; + } +} + +// Mock orpc client +void mock.module("@/common/orpc/client", () => ({ + createClient: () => ({ + general: { + ping: () => Promise.resolve("pong"), + }, + }), +})); + +void mock.module("@orpc/client/websocket", () => ({ + RPCLink: class {}, +})); + +void mock.module("@orpc/client/message-port", () => ({ + RPCLink: class {}, +})); + +void mock.module("@/browser/components/AuthTokenModal", () => ({ + getStoredAuthToken: () => null, + // eslint-disable-next-line @typescript-eslint/no-empty-function + clearStoredAuthToken: () => {}, +})); + +// Import after mocks +import { APIProvider, useAPI, type UseAPIResult } from "./API"; + +// Test component to observe API state +function APIStateObserver(props: { onState: (state: UseAPIResult) => void }) { + const apiState = useAPI(); + props.onState(apiState); + return null; +} + +describe("API reconnection", () => { + beforeAll(() => { + // Suppress console errors from React during tests + // eslint-disable-next-line @typescript-eslint/no-empty-function + globalThis.console.error = () => {}; + }); + + beforeEach(() => { + const window = new GlobalWindow(); + globalThis.window = window as unknown as Window & typeof globalThis; + globalThis.document = window.document as unknown as Document; + globalThis.WebSocket = MockWebSocket as unknown as typeof WebSocket; + // Mock import.meta.env + (globalThis as Record).import = { + meta: { env: { VITE_BACKEND_URL: "http://localhost:3000" } }, + }; + MockWebSocket.reset(); + }); + + afterEach(() => { + cleanup(); + MockWebSocket.reset(); + globalThis.window = undefined as unknown as Window & typeof globalThis; + globalThis.document = undefined as unknown as Document; + }); + + test("reconnects on close without showing auth_required when previously connected", async () => { + const states: string[] = []; + + render( + + states.push(s.status)} /> + + ); + + // Initial connection + const ws1 = MockWebSocket.lastInstance(); + expect(ws1).toBeDefined(); + + // Simulate successful connection (open + ping success) + await act(async () => { + ws1!.simulateOpen(); + // Wait for ping promise to resolve + await new Promise((r) => setTimeout(r, 10)); + }); + + // Should be connected + expect(states).toContain("connected"); + + // Simulate server restart (close code 1006 = abnormal closure) + act(() => { + ws1!.simulateClose(1006); + }); + + // Should be "reconnecting", NOT "auth_required" + await waitFor(() => { + expect(states).toContain("reconnecting"); + }); + + // Verify auth_required was never set + expect(states.filter((s) => s === "auth_required")).toHaveLength(0); + + // New WebSocket should be created for reconnect attempt (after delay) + await waitFor(() => { + expect(MockWebSocket.instances.length).toBeGreaterThan(1); + }); + }); + + test("shows auth_required on close with auth error codes (4401)", async () => { + const states: string[] = []; + + render( + + states.push(s.status)} /> + + ); + + const ws1 = MockWebSocket.lastInstance(); + expect(ws1).toBeDefined(); + + // Simulate successful connection then auth rejection + await act(async () => { + ws1!.simulateOpen(); + await new Promise((r) => setTimeout(r, 10)); + }); + + expect(states).toContain("connected"); + + act(() => { + ws1!.simulateClose(4401); // Auth required code + }); + + await waitFor(() => { + expect(states).toContain("auth_required"); + }); + }); + + test("shows auth_required on close with auth error codes (1008)", async () => { + const states: string[] = []; + + render( + + states.push(s.status)} /> + + ); + + const ws1 = MockWebSocket.lastInstance(); + expect(ws1).toBeDefined(); + + await act(async () => { + ws1!.simulateOpen(); + await new Promise((r) => setTimeout(r, 10)); + }); + + expect(states).toContain("connected"); + + act(() => { + ws1!.simulateClose(1008); // Policy violation (auth) + }); + + await waitFor(() => { + expect(states).toContain("auth_required"); + }); + }); + + test("shows auth_required on first connection error without token", async () => { + const states: string[] = []; + + render( + + states.push(s.status)} /> + + ); + + const ws1 = MockWebSocket.lastInstance(); + expect(ws1).toBeDefined(); + + // First connection fails (server not up, no previous connection) + act(() => { + ws1!.simulateError(); + }); + + await waitFor(() => { + expect(states).toContain("auth_required"); + }); + + // Should not attempt reconnect on first failure + expect(states.filter((s) => s === "reconnecting")).toHaveLength(0); + }); + + test("reconnects on error when previously connected", async () => { + const states: string[] = []; + + render( + + states.push(s.status)} /> + + ); + + const ws1 = MockWebSocket.lastInstance(); + expect(ws1).toBeDefined(); + + // Successful connection first + await act(async () => { + ws1!.simulateOpen(); + await new Promise((r) => setTimeout(r, 10)); + }); + + expect(states).toContain("connected"); + + // Error after being connected + act(() => { + ws1!.simulateError(); + }); + + await waitFor(() => { + expect(states).toContain("reconnecting"); + }); + + // Should not show auth_required + const authRequiredAfterConnected = states.slice(states.indexOf("connected") + 1); + expect(authRequiredAfterConnected.filter((s) => s === "auth_required")).toHaveLength(0); + }); +}); diff --git a/src/browser/contexts/API.tsx b/src/browser/contexts/API.tsx index 8dfe030530..1c6e088ba2 100644 --- a/src/browser/contexts/API.tsx +++ b/src/browser/contexts/API.tsx @@ -20,6 +20,7 @@ export type { APIClient }; export type APIState = | { status: "connecting"; api: null; error: null } | { status: "connected"; api: APIClient; error: null } + | { status: "reconnecting"; api: null; error: null; attempt: number } | { status: "auth_required"; api: null; error: string | null } | { status: "error"; api: null; error: string }; @@ -35,9 +36,15 @@ export type UseAPIResult = APIState & APIStateMethods; type ConnectionState = | { status: "connecting" } | { status: "connected"; client: APIClient; cleanup: () => void } + | { status: "reconnecting"; attempt: number } | { status: "auth_required"; error?: string } | { status: "error"; error: string }; +// Reconnection constants +const MAX_RECONNECT_ATTEMPTS = 10; +const BASE_DELAY_MS = 100; +const MAX_DELAY_MS = 10000; + const APIContext = createContext(null); interface APIProviderProps { @@ -102,6 +109,10 @@ export const APIProvider = (props: APIProviderProps) => { }); const cleanupRef = useRef<(() => void) | null>(null); + const hasConnectedRef = useRef(false); + const reconnectAttemptRef = useRef(0); + const reconnectTimeoutRef = useRef | null>(null); + const scheduleReconnectRef = useRef<(() => void) | null>(null); const connect = useCallback( (token: string | null) => { @@ -127,6 +138,8 @@ export const APIProvider = (props: APIProviderProps) => { client.general .ping("auth-check") .then(() => { + hasConnectedRef.current = true; + reconnectAttemptRef.current = 0; window.__ORPC_CLIENT__ = client; cleanupRef.current = cleanup; setState({ status: "connected", client, cleanup }); @@ -151,29 +164,66 @@ export const APIProvider = (props: APIProviderProps) => { ws.addEventListener("error", () => { cleanup(); - if (token) { + if (hasConnectedRef.current) { + // Was connected before - try to reconnect + scheduleReconnectRef.current?.(); + } else if (token) { + // First connection failed with token - might be invalid clearStoredAuthToken(); setState({ status: "auth_required", error: "Connection failed - invalid token?" }); } else { + // First connection failed without token - server might require auth setState({ status: "auth_required" }); } }); ws.addEventListener("close", (event) => { + // Auth-specific close codes if (event.code === 1008 || event.code === 4401) { cleanup(); clearStoredAuthToken(); + hasConnectedRef.current = false; // Reset - need fresh auth setState({ status: "auth_required", error: "Authentication required" }); + return; + } + + // Normal disconnection (server restart, network issue) + if (hasConnectedRef.current) { + cleanup(); + scheduleReconnectRef.current?.(); } }); }, [props.client] ); + // Schedule reconnection with exponential backoff + const scheduleReconnect = useCallback(() => { + const attempt = reconnectAttemptRef.current; + if (attempt >= MAX_RECONNECT_ATTEMPTS) { + setState({ status: "error", error: "Connection lost. Please refresh the page." }); + return; + } + + const delay = Math.min(BASE_DELAY_MS * Math.pow(2, attempt), MAX_DELAY_MS); + reconnectAttemptRef.current = attempt + 1; + setState({ status: "reconnecting", attempt: attempt + 1 }); + + reconnectTimeoutRef.current = setTimeout(() => { + connect(authToken); + }, delay); + }, [authToken, connect]); + + // Keep ref in sync with latest scheduleReconnect + scheduleReconnectRef.current = scheduleReconnect; + useEffect(() => { connect(authToken); return () => { cleanupRef.current?.(); + if (reconnectTimeoutRef.current) { + clearTimeout(reconnectTimeoutRef.current); + } }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -198,6 +248,8 @@ export const APIProvider = (props: APIProviderProps) => { return { status: "connecting", api: null, error: null, ...base }; case "connected": return { status: "connected", api: state.client, error: null, ...base }; + case "reconnecting": + return { status: "reconnecting", api: null, error: null, attempt: state.attempt, ...base }; case "auth_required": return { status: "auth_required", api: null, error: state.error ?? null, ...base }; case "error": From 7d8c232f81f9969ffaa536a2b6f8ef895330f99e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 10 Dec 2025 11:27:34 +0100 Subject: [PATCH 2/5] fix: ensure window.api is undefined in API tests The CI environment may have window.api defined, causing the APIProvider to use the Electron client path instead of the WebSocket path, resulting in no MockWebSocket instances being created. Change-Id: Ic971f4e0fe8dd6efc997b04a42181eaae626b2b8 Signed-off-by: Thomas Kosiewski --- src/browser/contexts/API.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/browser/contexts/API.test.tsx b/src/browser/contexts/API.test.tsx index cbd7c6ffee..b38049c03a 100644 --- a/src/browser/contexts/API.test.tsx +++ b/src/browser/contexts/API.test.tsx @@ -93,6 +93,8 @@ describe("API reconnection", () => { globalThis.window = window as unknown as Window & typeof globalThis; globalThis.document = window.document as unknown as Document; globalThis.WebSocket = MockWebSocket as unknown as typeof WebSocket; + // Ensure we're not in Electron mode (would skip WebSocket) + (globalThis.window as unknown as Record).api = undefined; // Mock import.meta.env (globalThis as Record).import = { meta: { env: { VITE_BACKEND_URL: "http://localhost:3000" } }, From 1d57ab50355959457a75a088c9ec1622cb7db365 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 10 Dec 2025 11:29:51 +0100 Subject: [PATCH 3/5] fix: handle reconnection only in close handler to avoid double-scheduling Browser fires 'error' before 'close' for WebSocket failures. Previously both handlers could schedule reconnection, leading to double attempts. Now: - 'error' handler is a no-op (close will follow) - 'close' handler handles all cleanup and reconnection logic - Consolidated first-connection-failure logic into close handler Change-Id: If1575bee81ad4fd43e29f186a722e0c60646bcc4 Signed-off-by: Thomas Kosiewski --- src/browser/contexts/API.test.tsx | 43 +++++++++++++++++++++---------- src/browser/contexts/API.tsx | 32 ++++++++++++----------- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/browser/contexts/API.test.tsx b/src/browser/contexts/API.test.tsx index b38049c03a..3ed711e345 100644 --- a/src/browser/contexts/API.test.tsx +++ b/src/browser/contexts/API.test.tsx @@ -1,5 +1,5 @@ import { act, cleanup, render, waitFor } from "@testing-library/react"; -import { afterEach, beforeEach, describe, expect, mock, test, beforeAll } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"; import { GlobalWindow } from "happy-dom"; // Mock WebSocket that we can control @@ -82,23 +82,37 @@ function APIStateObserver(props: { onState: (state: UseAPIResult) => void }) { } describe("API reconnection", () => { + let originalConsoleError: typeof console.error; + let originalWebSocket: typeof WebSocket; + beforeAll(() => { + // Store originals for restoration + originalConsoleError = globalThis.console.error; + originalWebSocket = globalThis.WebSocket; // Suppress console errors from React during tests // eslint-disable-next-line @typescript-eslint/no-empty-function globalThis.console.error = () => {}; }); + afterAll(() => { + // Restore originals + globalThis.console.error = originalConsoleError; + globalThis.WebSocket = originalWebSocket; + }); + beforeEach(() => { - const window = new GlobalWindow(); - globalThis.window = window as unknown as Window & typeof globalThis; - globalThis.document = window.document as unknown as Document; + const happyWindow = new GlobalWindow(); + // Create a window object that mimics browser environment (not Electron) + const mockWindow = Object.assign(happyWindow, { + location: { origin: "http://localhost:3000", search: "" }, + WebSocket: MockWebSocket, + }); + globalThis.window = mockWindow as unknown as Window & typeof globalThis; + globalThis.document = happyWindow.document as unknown as Document; globalThis.WebSocket = MockWebSocket as unknown as typeof WebSocket; - // Ensure we're not in Electron mode (would skip WebSocket) - (globalThis.window as unknown as Record).api = undefined; - // Mock import.meta.env - (globalThis as Record).import = { - meta: { env: { VITE_BACKEND_URL: "http://localhost:3000" } }, - }; + // Explicitly delete window.api to ensure we're not in Electron mode + // (other tests may have set this on the global) + delete (globalThis.window as unknown as Record).api; MockWebSocket.reset(); }); @@ -208,7 +222,7 @@ describe("API reconnection", () => { }); }); - test("shows auth_required on first connection error without token", async () => { + test("shows auth_required on first connection failure without token", async () => { const states: string[] = []; render( @@ -221,8 +235,10 @@ describe("API reconnection", () => { expect(ws1).toBeDefined(); // First connection fails (server not up, no previous connection) + // Browser fires error then close - we simulate both act(() => { ws1!.simulateError(); + ws1!.simulateClose(1006); // Abnormal closure }); await waitFor(() => { @@ -233,7 +249,7 @@ describe("API reconnection", () => { expect(states.filter((s) => s === "reconnecting")).toHaveLength(0); }); - test("reconnects on error when previously connected", async () => { + test("reconnects on connection loss when previously connected", async () => { const states: string[] = []; render( @@ -253,9 +269,10 @@ describe("API reconnection", () => { expect(states).toContain("connected"); - // Error after being connected + // Connection lost after being connected (error + close) act(() => { ws1!.simulateError(); + ws1!.simulateClose(1006); // Abnormal closure }); await waitFor(() => { diff --git a/src/browser/contexts/API.tsx b/src/browser/contexts/API.tsx index 1c6e088ba2..7d353919d9 100644 --- a/src/browser/contexts/API.tsx +++ b/src/browser/contexts/API.tsx @@ -162,35 +162,37 @@ export const APIProvider = (props: APIProviderProps) => { }); }); + // Note: Browser fires 'error' before 'close', so we handle reconnection + // only in 'close' to avoid double-scheduling. The 'error' event just + // signals that something went wrong; 'close' provides the final state. ws.addEventListener("error", () => { - cleanup(); - if (hasConnectedRef.current) { - // Was connected before - try to reconnect - scheduleReconnectRef.current?.(); - } else if (token) { - // First connection failed with token - might be invalid - clearStoredAuthToken(); - setState({ status: "auth_required", error: "Connection failed - invalid token?" }); - } else { - // First connection failed without token - server might require auth - setState({ status: "auth_required" }); - } + // Error occurred - close event will follow and handle reconnection + // We don't call cleanup() here since close handler will do it }); ws.addEventListener("close", (event) => { + cleanup(); + // Auth-specific close codes if (event.code === 1008 || event.code === 4401) { - cleanup(); clearStoredAuthToken(); hasConnectedRef.current = false; // Reset - need fresh auth setState({ status: "auth_required", error: "Authentication required" }); return; } - // Normal disconnection (server restart, network issue) + // If we were previously connected, try to reconnect if (hasConnectedRef.current) { - cleanup(); scheduleReconnectRef.current?.(); + return; + } + + // First connection failed - check if auth might be needed + if (token) { + clearStoredAuthToken(); + setState({ status: "auth_required", error: "Connection failed - invalid token?" }); + } else { + setState({ status: "auth_required" }); } }); }, From 93d5a6c94cb4a98e7218b8e702370b0dd30079f5 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 10 Dec 2025 12:06:21 +0100 Subject: [PATCH 4/5] refactor: use WebSocket factory prop for testability Instead of manipulating global state in tests, add a createWebSocket prop to APIProvider that allows injecting a mock WebSocket factory. This eliminates: - Global console.error suppression - Global WebSocket replacement - window.api deletion hacks - Complex GlobalWindow setup with Object.assign Tests now simply pass createMockWebSocket prop and the minimal DOM setup required by @testing-library/react. Change-Id: I341ddef8db208081180b989e92615c673aceed31 Signed-off-by: Thomas Kosiewski --- src/browser/contexts/API.test.tsx | 65 ++++++++----------------------- src/browser/contexts/API.tsx | 21 +++++++--- 2 files changed, 33 insertions(+), 53 deletions(-) diff --git a/src/browser/contexts/API.test.tsx b/src/browser/contexts/API.test.tsx index 3ed711e345..70465660a7 100644 --- a/src/browser/contexts/API.test.tsx +++ b/src/browser/contexts/API.test.tsx @@ -1,5 +1,5 @@ import { act, cleanup, render, waitFor } from "@testing-library/react"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import { GlobalWindow } from "happy-dom"; // Mock WebSocket that we can control @@ -81,38 +81,15 @@ function APIStateObserver(props: { onState: (state: UseAPIResult) => void }) { return null; } -describe("API reconnection", () => { - let originalConsoleError: typeof console.error; - let originalWebSocket: typeof WebSocket; - - beforeAll(() => { - // Store originals for restoration - originalConsoleError = globalThis.console.error; - originalWebSocket = globalThis.WebSocket; - // Suppress console errors from React during tests - // eslint-disable-next-line @typescript-eslint/no-empty-function - globalThis.console.error = () => {}; - }); - - afterAll(() => { - // Restore originals - globalThis.console.error = originalConsoleError; - globalThis.WebSocket = originalWebSocket; - }); +// Factory that creates MockWebSocket instances (injected via prop) +const createMockWebSocket = (url: string) => new MockWebSocket(url) as unknown as WebSocket; +describe("API reconnection", () => { beforeEach(() => { + // Minimal DOM setup required by @testing-library/react const happyWindow = new GlobalWindow(); - // Create a window object that mimics browser environment (not Electron) - const mockWindow = Object.assign(happyWindow, { - location: { origin: "http://localhost:3000", search: "" }, - WebSocket: MockWebSocket, - }); - globalThis.window = mockWindow as unknown as Window & typeof globalThis; + globalThis.window = happyWindow as unknown as Window & typeof globalThis; globalThis.document = happyWindow.document as unknown as Document; - globalThis.WebSocket = MockWebSocket as unknown as typeof WebSocket; - // Explicitly delete window.api to ensure we're not in Electron mode - // (other tests may have set this on the global) - delete (globalThis.window as unknown as Record).api; MockWebSocket.reset(); }); @@ -127,12 +104,11 @@ describe("API reconnection", () => { const states: string[] = []; render( - + states.push(s.status)} /> ); - // Initial connection const ws1 = MockWebSocket.lastInstance(); expect(ws1).toBeDefined(); @@ -143,7 +119,6 @@ describe("API reconnection", () => { await new Promise((r) => setTimeout(r, 10)); }); - // Should be connected expect(states).toContain("connected"); // Simulate server restart (close code 1006 = abnormal closure) @@ -156,7 +131,6 @@ describe("API reconnection", () => { expect(states).toContain("reconnecting"); }); - // Verify auth_required was never set expect(states.filter((s) => s === "auth_required")).toHaveLength(0); // New WebSocket should be created for reconnect attempt (after delay) @@ -169,7 +143,7 @@ describe("API reconnection", () => { const states: string[] = []; render( - + states.push(s.status)} /> ); @@ -177,7 +151,6 @@ describe("API reconnection", () => { const ws1 = MockWebSocket.lastInstance(); expect(ws1).toBeDefined(); - // Simulate successful connection then auth rejection await act(async () => { ws1!.simulateOpen(); await new Promise((r) => setTimeout(r, 10)); @@ -186,7 +159,7 @@ describe("API reconnection", () => { expect(states).toContain("connected"); act(() => { - ws1!.simulateClose(4401); // Auth required code + ws1!.simulateClose(4401); }); await waitFor(() => { @@ -198,7 +171,7 @@ describe("API reconnection", () => { const states: string[] = []; render( - + states.push(s.status)} /> ); @@ -214,7 +187,7 @@ describe("API reconnection", () => { expect(states).toContain("connected"); act(() => { - ws1!.simulateClose(1008); // Policy violation (auth) + ws1!.simulateClose(1008); }); await waitFor(() => { @@ -226,7 +199,7 @@ describe("API reconnection", () => { const states: string[] = []; render( - + states.push(s.status)} /> ); @@ -234,18 +207,16 @@ describe("API reconnection", () => { const ws1 = MockWebSocket.lastInstance(); expect(ws1).toBeDefined(); - // First connection fails (server not up, no previous connection) - // Browser fires error then close - we simulate both + // First connection fails - browser fires error then close act(() => { ws1!.simulateError(); - ws1!.simulateClose(1006); // Abnormal closure + ws1!.simulateClose(1006); }); await waitFor(() => { expect(states).toContain("auth_required"); }); - // Should not attempt reconnect on first failure expect(states.filter((s) => s === "reconnecting")).toHaveLength(0); }); @@ -253,7 +224,7 @@ describe("API reconnection", () => { const states: string[] = []; render( - + states.push(s.status)} /> ); @@ -261,7 +232,6 @@ describe("API reconnection", () => { const ws1 = MockWebSocket.lastInstance(); expect(ws1).toBeDefined(); - // Successful connection first await act(async () => { ws1!.simulateOpen(); await new Promise((r) => setTimeout(r, 10)); @@ -269,17 +239,16 @@ describe("API reconnection", () => { expect(states).toContain("connected"); - // Connection lost after being connected (error + close) + // Connection lost after being connected act(() => { ws1!.simulateError(); - ws1!.simulateClose(1006); // Abnormal closure + ws1!.simulateClose(1006); }); await waitFor(() => { expect(states).toContain("reconnecting"); }); - // Should not show auth_required const authRequiredAfterConnected = states.slice(states.indexOf("connected") + 1); expect(authRequiredAfterConnected.filter((s) => s === "auth_required")).toHaveLength(0); }); diff --git a/src/browser/contexts/API.tsx b/src/browser/contexts/API.tsx index 7d353919d9..4a920c2073 100644 --- a/src/browser/contexts/API.tsx +++ b/src/browser/contexts/API.tsx @@ -51,6 +51,8 @@ interface APIProviderProps { children: React.ReactNode; /** Optional pre-created client. If provided, skips internal connection setup. */ client?: APIClient; + /** WebSocket factory for testing. Defaults to native WebSocket constructor. */ + createWebSocket?: (url: string) => WebSocket; } function getApiBase(): string { @@ -72,7 +74,10 @@ function createElectronClient(): { client: APIClient; cleanup: () => void } { }; } -function createBrowserClient(authToken: string | null): { +function createBrowserClient( + authToken: string | null, + createWebSocket: (url: string) => WebSocket +): { client: APIClient; cleanup: () => void; ws: WebSocket; @@ -84,7 +89,7 @@ function createBrowserClient(authToken: string | null): { ? `${WS_BASE}/orpc/ws?token=${encodeURIComponent(authToken)}` : `${WS_BASE}/orpc/ws`; - const ws = new WebSocket(wsUrl); + const ws = createWebSocket(wsUrl); const link = new WebSocketLink({ websocket: ws }); return { @@ -114,6 +119,11 @@ export const APIProvider = (props: APIProviderProps) => { const reconnectTimeoutRef = useRef | null>(null); const scheduleReconnectRef = useRef<(() => void) | null>(null); + const wsFactory = useMemo( + () => props.createWebSocket ?? ((url: string) => new WebSocket(url)), + [props.createWebSocket] + ); + const connect = useCallback( (token: string | null) => { if (props.client) { @@ -123,7 +133,8 @@ export const APIProvider = (props: APIProviderProps) => { return; } - if (window.api) { + // Skip Electron detection if custom WebSocket factory provided (for testing) + if (!props.createWebSocket && window.api) { const { client, cleanup } = createElectronClient(); window.__ORPC_CLIENT__ = client; cleanupRef.current = cleanup; @@ -132,7 +143,7 @@ export const APIProvider = (props: APIProviderProps) => { } setState({ status: "connecting" }); - const { client, cleanup, ws } = createBrowserClient(token); + const { client, cleanup, ws } = createBrowserClient(token, wsFactory); ws.addEventListener("open", () => { client.general @@ -196,7 +207,7 @@ export const APIProvider = (props: APIProviderProps) => { } }); }, - [props.client] + [props.client, props.createWebSocket, wsFactory] ); // Schedule reconnection with exponential backoff From 8257b0e23b56e15e21ce7b26e95408046337bcf9 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 10 Dec 2025 12:32:41 +0100 Subject: [PATCH 5/5] fix(test): work around bun module mock leakage in API.test.tsx Other test files (ProjectContext.test.tsx, WorkspaceContext.test.tsx) mock @/browser/contexts/API with a fake APIProvider. Due to bun's known issue with module mock isolation (https://github.com/oven-sh/bun/issues/12823), these mocks leak between test files. This caused API.test.tsx to get the mocked APIProvider instead of the real one, resulting in MockWebSocket never being created because the mocked APIProvider just returns children without calling connect(). Fix: Import the real module first, then re-mock @/browser/contexts/API with the real exports. This ensures subsequent tests that import from that path get our real implementation, not another test's mock. Change-Id: Ie4552acdea69e1078b364e5b8f97e084bf1ec788 Signed-off-by: Thomas Kosiewski --- src/browser/contexts/API.test.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/browser/contexts/API.test.tsx b/src/browser/contexts/API.test.tsx index 70465660a7..08bcc31989 100644 --- a/src/browser/contexts/API.test.tsx +++ b/src/browser/contexts/API.test.tsx @@ -71,8 +71,20 @@ void mock.module("@/browser/components/AuthTokenModal", () => ({ clearStoredAuthToken: () => {}, })); -// Import after mocks -import { APIProvider, useAPI, type UseAPIResult } from "./API"; +// Import the real API module types (not the mocked version) +import type { UseAPIResult as _UseAPIResult, APIProvider as APIProviderType } from "./API"; + +// IMPORTANT: Other test files mock @/browser/contexts/API with a fake APIProvider. +// Module mocks leak between test files in bun (https://github.com/oven-sh/bun/issues/12823). +// The query string creates a distinct module cache key, bypassing any mocked version. +/* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment */ +const RealAPIModule: { + APIProvider: typeof APIProviderType; + useAPI: () => _UseAPIResult; +} = require("./API?real=1"); +/* eslint-enable @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment */ +const { APIProvider, useAPI } = RealAPIModule; +type UseAPIResult = _UseAPIResult; // Test component to observe API state function APIStateObserver(props: { onState: (state: UseAPIResult) => void }) {