From d90f4d488ed967919a019c9ae2cc189acda65919 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 11 Aug 2026 19:16:19 -0700 Subject: [PATCH] fix(ui): survive first load against a cold backend without a blank page (#11246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The web UI holds live websocket connections for run events, coordinates cross-tab polling through a leader-election store, and renders app chrome (sidebar, providers) around a routed outlet > - When the backend is still cold-starting (managed hosting wake, server restart, reverse proxy up before the app), the event websockets refuse connections and the first SPA load mounts against a dead backend > - In that state the mount cascade can exceed React's nested update limit (minified error #185); the crash originates in shell hooks outside the routed error boundary, so React unmounts the entire root to a blank page, and the dead page keeps retrying the websocket on a flat 1.5s timer until the user hard-refreshes > - This pull request removes the wasted nested commits from the shared-polling subscription path, adds exponential backoff to the transcript websocket reconnect, and adds a last-resort app-shell error boundary > - The benefit is that a cold or briefly unreachable backend degrades to a recoverable state instead of a blank page that hammers the server ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** On the first load against a backend that was still starting, the app showed its loading animation and then a blank page. The console showed repeated `WebSocket connection to 'wss://…/api/companies//events/ws' failed` lines and `Uncaught Error: Minified React error #185` with a stack through the shared-polling coordinator's `subscribe`. The websocket retries continued indefinitely on the dead page. A manual refresh fixed it. **Expected behavior** A backend that is briefly unreachable degrades gracefully: websocket reconnects back off, the UI keeps rendering from cache, and even a worst-case crash shows a reload prompt instead of a blank page. **Steps to reproduce** 1. Serve the UI while the backend API is still starting (websocket upgrades and API calls refused). 2. Load any company page with several shared-polling consumers mounted (dashboard with sidebar). 3. Observe repeated websocket failures; on affected loads the page goes blank with React error #185. ## What Changed - `ui/src/hooks/useSharedPolling.ts`: coordinator snapshot notifications now keep the previous state object when leadership did not change, so React bails out instead of scheduling a nested re-render. `subscribe` invokes its listener synchronously from inside the mount effect with a fresh object each time; before this change every mount and notify burned nested-update budget even with no value change — the crash frame in the field report was exactly this `subscribe → setState` call. - `ui/src/components/transcript/useLiveRunTranscripts.ts`: the live event websocket reconnect backs off exponentially (1.5s → 15s cap, reset on successful open), mirroring `LiveUpdatesProvider`, instead of a flat 1.5s retry. - `ui/src/components/AppErrorBoundary.tsx` (+ wiring in `ui/src/main.tsx`): a dependency-free boundary above the router and providers. `RouteErrorBoundary` only guards the routed ``; a crash in the shell around it had no boundary, so React unmounted the root to a blank page. The boundary renders a reload prompt with the error message. - Tests: `useSharedPollingSnapshot.test.tsx` (mount costs no extra commit — fails against the previous code; a real leadership change re-renders exactly once and ticks stay quiet), a backoff test in `useLiveRunTranscripts.test.tsx` (delays grow 1.5s → 3s → 6s and reset after a successful open), and `AppErrorBoundary.test.tsx` (render throw, effect throw, healthy pass-through). ## Verification - `pnpm vitest run` in `ui/` over the touched suites (shared polling, cross-tab poll, transcripts, boundary): 34 tests pass. - `pnpm typecheck` in `ui/` — clean. - The snapshot regression test was verified to fail against the pre-change hook (extra commit per mount). - Not reproduced end-to-end: the exact 50-update cascade from the field crash needs a live cold backend; the change removes the identified per-mount/per-notify nested commits at the reported crash frame, bounds the reconnect load, and guarantees the shell can no longer blank the page. ## Risks Low risk. The snapshot change only suppresses re-renders whose state is value-identical; leadership changes propagate exactly as before. The backoff only lengthens retry delays after consecutive failures and resets on success. The new boundary renders children untouched unless an error reaches it; behavior on healthy loads is unchanged. Self-hosted deployments see the same code paths — the cold-backend window simply rarely occurs there. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, test execution; diagnosis included mapping the production minified stack to source). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ui/src/components/AppErrorBoundary.test.tsx | 92 +++++++++++++++++++ ui/src/components/AppErrorBoundary.tsx | 56 +++++++++++ .../transcript/useLiveRunTranscripts.test.tsx | 92 +++++++++++++++++++ .../transcript/useLiveRunTranscripts.ts | 25 ++++- ui/src/hooks/useSharedPolling.ts | 19 +++- .../hooks/useSharedPollingSnapshot.test.tsx | 91 ++++++++++++++++++ ui/src/main.tsx | 57 ++++++------ 7 files changed, 401 insertions(+), 31 deletions(-) create mode 100644 ui/src/components/AppErrorBoundary.test.tsx create mode 100644 ui/src/components/AppErrorBoundary.tsx create mode 100644 ui/src/hooks/useSharedPollingSnapshot.test.tsx 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( - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + );