diff --git a/ui/src/components/AppErrorBoundary.test.tsx b/ui/src/components/AppErrorBoundary.test.tsx new file mode 100644 index 0000000000..8de874f7e4 --- /dev/null +++ b/ui/src/components/AppErrorBoundary.test.tsx @@ -0,0 +1,92 @@ +// @vitest-environment jsdom + +import { act, useEffect } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { AppErrorBoundary } from "./AppErrorBoundary"; + +function BoomRender(): never { + throw new Error("Maximum update depth exceeded"); +} + +function BoomEffect() { + useEffect(() => { + throw new Error("effect exploded"); + }, []); + return null; +} + +describe("AppErrorBoundary", () => { + let container: HTMLDivElement; + let consoleErrorSpy: ReturnType; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + // React logs caught render errors to console.error; silence the expected noise. + consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + container.remove(); + }); + + it("renders a reload prompt instead of a blank page when the shell throws in render", () => { + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + + expect(container.textContent).toContain("Paperclip hit an error"); + expect(container.textContent).toContain("Maximum update depth exceeded"); + expect( + Array.from(container.querySelectorAll("button")).some( + (button) => button.textContent === "Reload page", + ), + ).toBe(true); + + act(() => { + root.unmount(); + }); + }); + + it("catches errors thrown from effects, not just render", () => { + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + + expect(container.textContent).toContain("Paperclip hit an error"); + expect(container.textContent).toContain("effect exploded"); + + act(() => { + root.unmount(); + }); + }); + + it("renders children untouched when nothing throws", () => { + const root = createRoot(container); + act(() => { + root.render( + +
healthy app
+ , + ); + }); + + expect(container.textContent).toBe("healthy app"); + + act(() => { + root.unmount(); + }); + }); +}); diff --git a/ui/src/components/AppErrorBoundary.tsx b/ui/src/components/AppErrorBoundary.tsx new file mode 100644 index 0000000000..12717b7747 --- /dev/null +++ b/ui/src/components/AppErrorBoundary.tsx @@ -0,0 +1,56 @@ +import { Component, type ErrorInfo, type ReactNode } from "react"; + +type AppErrorBoundaryState = { + error: Error | null; +}; + +/** + * Last-resort boundary above the router and every provider that renders app + * chrome. `RouteErrorBoundary` only guards the routed ``; a crash in + * the shell around it (sidebar, providers, layout hooks) has no boundary, so + * React unmounts the entire root and the user is left staring at a blank + * page with no way forward but knowing to hard-refresh. This boundary trades + * that blank page for a reload prompt. + * + * Deliberately dependency-free: no router, no toast, no query client — the + * crash being handled may have originated inside any of those providers. + */ +export class AppErrorBoundary extends Component<{ children: ReactNode }, AppErrorBoundaryState> { + override state: AppErrorBoundaryState = { error: null }; + + static getDerivedStateFromError(error: unknown): AppErrorBoundaryState { + return { error: error instanceof Error ? error : new Error(String(error)) }; + } + + override componentDidCatch(error: unknown, info: ErrorInfo): void { + console.error("App shell crashed", { error, componentStack: info.componentStack }); + } + + override render() { + const { error } = this.state; + if (!error) return this.props.children; + + return ( +
+
+

Paperclip hit an error

+

+ Something went wrong while running the app. Reloading usually fixes this. +

+
+ 
+          {error.message}
+
+
+ +
+
+ ); + } +} diff --git a/ui/src/components/transcript/useLiveRunTranscripts.test.tsx b/ui/src/components/transcript/useLiveRunTranscripts.test.tsx index dbe9744d70..29d3364770 100644 --- a/ui/src/components/transcript/useLiveRunTranscripts.test.tsx +++ b/ui/src/components/transcript/useLiveRunTranscripts.test.tsx @@ -63,6 +63,11 @@ class FakeWebSocket { this.readyState = FakeWebSocket.OPEN; this.onopen?.(new Event("open")); } + + triggerClose() { + this.readyState = FakeWebSocket.CLOSED; + this.onclose?.(new CloseEvent("close")); + } } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -623,4 +628,91 @@ describe("useLiveRunTranscripts", () => { vi.useRealTimers(); } }); + + it("backs off exponentially when the live event socket keeps failing", async () => { + vi.useFakeTimers(); + try { + function Harness({ lastOutputBytes }: { lastOutputBytes?: number }) { + useLiveRunTranscripts({ + companyId: "company-1", + runs: [{ id: "run-1", status: "running", adapterType: "codex_local", lastOutputBytes }], + }); + return null; + } + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + expect(FakeWebSocket.instances).toHaveLength(1); + + // Cold backend: every handshake fails. Delays must grow 1.5s → 3s → 6s + // instead of hammering a flat interval. + await act(async () => { + FakeWebSocket.instances[0].triggerClose(); + await vi.advanceTimersByTimeAsync(1_499); + }); + expect(FakeWebSocket.instances).toHaveLength(1); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + expect(FakeWebSocket.instances).toHaveLength(2); + + await act(async () => { + FakeWebSocket.instances[1].triggerClose(); + await vi.advanceTimersByTimeAsync(2_999); + }); + expect(FakeWebSocket.instances).toHaveLength(2); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + expect(FakeWebSocket.instances).toHaveLength(3); + + await act(async () => { + FakeWebSocket.instances[2].triggerClose(); + await vi.advanceTimersByTimeAsync(5_999); + }); + expect(FakeWebSocket.instances).toHaveLength(3); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + expect(FakeWebSocket.instances).toHaveLength(4); + + // Run-metadata changes restart the socket effect; the progressed delay + // must survive the restart instead of resetting to the base delay. + await act(async () => { + root.render(); + await Promise.resolve(); + }); + expect(FakeWebSocket.instances).toHaveLength(5); + await act(async () => { + FakeWebSocket.instances[4].triggerClose(); + await vi.advanceTimersByTimeAsync(11_999); + }); + expect(FakeWebSocket.instances).toHaveLength(5); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + expect(FakeWebSocket.instances).toHaveLength(6); + + // A successful connection resets the backoff to the base delay. + await act(async () => { + FakeWebSocket.instances[5].triggerOpen(); + FakeWebSocket.instances[5].triggerClose(); + await vi.advanceTimersByTimeAsync(1_500); + }); + expect(FakeWebSocket.instances).toHaveLength(7); + + act(() => { + root.unmount(); + }); + container.remove(); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/ui/src/components/transcript/useLiveRunTranscripts.ts b/ui/src/components/transcript/useLiveRunTranscripts.ts index 4eaa5dbffb..8cd2d1e46d 100644 --- a/ui/src/components/transcript/useLiveRunTranscripts.ts +++ b/ui/src/components/transcript/useLiveRunTranscripts.ts @@ -132,6 +132,10 @@ export function useLiveRunTranscripts({ // the effect again once the nearest deadline elapses so a run that stays gone // is eventually cleaned up even if the `runs` list never changes again. const absenceDeadlineByRunRef = useRef(new Map()); + // Backoff state for the live event socket. Held outside the socket effect + // because that effect restarts on run-metadata changes; per-effect state + // would reset a progressed delay back to its base mid-outage. + const reconnectStateRef = useRef<{ companyId: string; attempt: number } | null>(null); const prevKnownRunIdsRef = useRef(new Set()); const [pruneTick, setPruneTick] = useState(0); const transcriptCacheRef = useRef(new Map { if (closed) return; - reconnectTimer = window.setTimeout(connect, 1500); + reconnectState.attempt += 1; + const delayMs = Math.min(15_000, 1_500 * 2 ** Math.min(reconnectState.attempt - 1, 4)); + reconnectTimer = window.setTimeout(connect, delayMs); }; const connect = () => { @@ -354,6 +372,11 @@ export function useLiveRunTranscripts({ ); socket = new WebSocket(url); + socket.onopen = () => { + if (closed) return; + reconnectState.attempt = 0; + }; + socket.onmessage = (message) => { const raw = typeof message.data === "string" ? message.data : ""; if (!raw) return; diff --git a/ui/src/hooks/useSharedPolling.ts b/ui/src/hooks/useSharedPolling.ts index e58cefcb58..e1d09f1ded 100644 --- a/ui/src/hooks/useSharedPolling.ts +++ b/ui/src/hooks/useSharedPolling.ts @@ -93,18 +93,31 @@ export function useSharedPollingQuery({ const queryKeyRef = useRef(queryKey); const [snapshot, setSnapshot] = useState({ isLeader: true }); + // Coordinator notifications arrive as fresh snapshot objects, and + // `subscribe` invokes its listener synchronously from inside the mount + // effect. Passing `setSnapshot` straight through would schedule a nested + // re-render for every notification — including value-equal ones — and those + // wasted commits count toward React's nested-update limit. Under a hostile + // mount cascade (many shared-polling hooks mounting while the backend is + // cold) that budget is shared with every other effect-driven update, so + // keep the previous state object whenever leadership did not change and + // let React bail out instead. + const applySnapshot = useCallback((next: SharedPollingSnapshot) => { + setSnapshot((prev) => (prev.isLeader === next.isLeader ? prev : next)); + }, []); + useEffect(() => { queryKeyRef.current = queryKey; }, [queryKey, queryKeyHash]); useEffect(() => { if (!activeCompanyId || !fullResourceKey) { - setSnapshot({ isLeader: true }); + applySnapshot({ isLeader: true }); return; } const coordinator = acquireCoordinator(activeCompanyId); - const unsubscribeState = coordinator.subscribe(setSnapshot); + const unsubscribeState = coordinator.subscribe(applySnapshot); const unsubscribeResource = coordinator.subscribeResource(fullResourceKey, (message) => { applySharedPollingResult(queryClient, queryKeyRef.current, message); }); @@ -115,7 +128,7 @@ export function useSharedPollingQuery({ unsubscribeState(); releaseCoordinator(activeCompanyId); }; - }, [activeCompanyId, fullResourceKey, leaderOnly, queryClient, queryKeyHash]); + }, [activeCompanyId, applySnapshot, fullResourceKey, leaderOnly, queryClient, queryKeyHash]); const isLeader = !leaderOnly || snapshot.isLeader; const queryEnabled = enabled && (!leaderOnly || snapshot.isLeader); diff --git a/ui/src/hooks/useSharedPollingSnapshot.test.tsx b/ui/src/hooks/useSharedPollingSnapshot.test.tsx new file mode 100644 index 0000000000..29c7f3ecf9 --- /dev/null +++ b/ui/src/hooks/useSharedPollingSnapshot.test.tsx @@ -0,0 +1,91 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { sharedPollingLeaseKey } from "../lib/cross-tab-poll"; +import { useSharedPollingQuery } from "./useSharedPolling"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +let renderCount = 0; +let lastIsLeader: boolean | null = null; + +function Harness({ companyId }: { companyId: string }) { + renderCount += 1; + const shared = useSharedPollingQuery({ + companyId, + resourceKey: "res", + queryKey: ["res", companyId], + leaderOnly: true, + }); + lastIsLeader = shared.isLeader; + return null; +} + +describe("useSharedPollingQuery snapshot updates", () => { + let container: HTMLDivElement; + let root: Root | null = null; + let queryClient: QueryClient; + + beforeEach(() => { + vi.useFakeTimers(); + localStorage.clear(); + sessionStorage.clear(); + renderCount = 0; + lastIsLeader = null; + container = document.createElement("div"); + document.body.appendChild(container); + queryClient = new QueryClient(); + }); + + afterEach(() => { + act(() => root?.unmount()); + root = null; + container.remove(); + vi.useRealTimers(); + }); + + function mount(companyId: string) { + root = createRoot(container); + act(() => { + root!.render( + + + , + ); + }); + } + + it("does not schedule an extra commit when the subscribed snapshot is value-equal", () => { + // No foreign lease: this tab claims leadership on the coordinator's first + // tick, so the snapshot delivered at subscribe time equals the hook's + // initial optimistic state. That delivery is a fresh object; it must not + // cost a re-render. + mount("company-a"); + expect(lastIsLeader).toBe(true); + expect(renderCount).toBe(1); + }); + + it("re-renders exactly once for a real leadership change and stays quiet after", () => { + // Another tab holds an unexpired lease: this tab mounts optimistic-leader + // and is demoted to follower by the subscribe-time snapshot. + localStorage.setItem( + sharedPollingLeaseKey("company-b"), + JSON.stringify({ leader: "other-tab", expiresAt: Date.now() + 60_000, visible: true }), + ); + + mount("company-b"); + expect(lastIsLeader).toBe(false); + expect(renderCount).toBe(2); + + // Ticks with unchanged leadership must not render at all. + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(lastIsLeader).toBe(false); + expect(renderCount).toBe(2); + }); +}); diff --git a/ui/src/main.tsx b/ui/src/main.tsx index 3da35e44c8..2766c51e1e 100644 --- a/ui/src/main.tsx +++ b/ui/src/main.tsx @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { BrowserRouter } from "@/lib/router"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { App } from "./App"; +import { AppErrorBoundary } from "./components/AppErrorBoundary"; import { CompanyProvider, useCompany } from "./context/CompanyContext"; import { LiveUpdatesProvider } from "./context/LiveUpdatesProvider"; import { BreadcrumbProvider } from "./context/BreadcrumbContext"; @@ -54,32 +55,34 @@ function CompanyAwareBreadcrumbProvider({ children }: { children: React.ReactNod createRoot(document.getElementById("root")!).render( - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + );