diff --git a/tests/e2e/chunk-load-recovery.spec.ts b/tests/e2e/chunk-load-recovery.spec.ts index 637825a52ca..8c66138ec51 100644 --- a/tests/e2e/chunk-load-recovery.spec.ts +++ b/tests/e2e/chunk-load-recovery.spec.ts @@ -45,7 +45,7 @@ test("a failed lazy CSS preload recovers with one reload", async ({ page, reques expect(counts.documents).toBe(2); }); -test("a lazy CSS asset that stays broken reloads once, then shows the error", async ({ page, request }) => { +test("a lazy CSS asset that stays broken reloads up to the retry limit, then shows the error", async ({ page, request }) => { const company = await json(await request.post("/api/companies", { data: { name: `Chunk recovery ${randomUUID()}` }, })); @@ -61,9 +61,10 @@ test("a lazy CSS asset that stays broken reloads once, then shows the error", as await page.goto(`/${company.issuePrefix}${environmentsPath}`); - await expect(page.getByText("This page hit an error")).toBeVisible(); + // Three reloads wait 0 s, 2 s and 5 s before the error page shows. + await expect(page.getByText("This page hit an error")).toBeVisible({ timeout: 30_000 }); await expect(page.getByText(/Unable to preload CSS for .*CompanyEnvironments-/)).toBeVisible(); await page.waitForTimeout(1_000); - expect(documents).toBe(2); - expect(failed).toBe(2); + expect(documents).toBe(4); + expect(failed).toBe(4); }); diff --git a/ui/src/components/AppErrorBoundary.test.tsx b/ui/src/components/AppErrorBoundary.test.tsx index f5bdd90fa4c..47ab5ed26e0 100644 --- a/ui/src/components/AppErrorBoundary.test.tsx +++ b/ui/src/components/AppErrorBoundary.test.tsx @@ -3,7 +3,7 @@ import { act, lazy, Suspense, useEffect } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createChunkLoadRecovery, RELOAD_ABANDONED_MS } from "@/lib/chunk-load-recovery"; +import { createChunkLoadRecovery, RELOAD_ABANDONED_MS, RELOAD_DELAYS_MS } from "@/lib/chunk-load-recovery"; import { AppErrorBoundary } from "./AppErrorBoundary"; const captureBrowserExceptionMock = vi.hoisted(() => vi.fn()); @@ -23,6 +23,7 @@ vi.mock("@/lib/chunk-load-recovery", async (importOriginal) => { ...actual, reloadForChunkLoadError: (error: unknown) => chunkRecovery.current!.reloadForChunkLoadError(error), onChunkLoadReloadAbandoned: (listener: () => void) => chunkRecovery.current!.onChunkLoadReloadAbandoned(listener), + cancelPendingChunkLoadReload: () => chunkRecovery.current!.cancelPendingChunkLoadReload(), }; }); @@ -86,7 +87,7 @@ describe("AppErrorBoundary", () => { }); }); - it("reloads once for a failed lazy chunk, then shows the prompt if it fails again", async () => { + it("reloads for a failed lazy chunk, then shows the prompt once the retries are used up", async () => { const chunkError = () => new Error("Failed to fetch dynamically imported module: https://paperclip.example/assets/layout.js"); const root = createRoot(container); const FailedLayout = lazy(async () => { throw chunkError(); }); @@ -98,6 +99,11 @@ describe("AppErrorBoundary", () => { expect(container.querySelector('[role="status"]')?.textContent).toContain("Reloading Paperclip"); act(() => root.unmount()); + // The reloaded pages keep failing, as when an asset stays broken. + for (let attempt = 1; attempt < RELOAD_DELAYS_MS.length; attempt += 1) { + expect(createChunkLoadRecovery({ reload: vi.fn() }).reloadForChunkLoadError(chunkError())).toBe(true); + } + chunkRecovery.current = createChunkLoadRecovery({ reload: reloadMock }); const nextRoot = createRoot(container); const StillFailedLayout = lazy(async () => { throw chunkError(); }); diff --git a/ui/src/components/RouteErrorBoundary.test.tsx b/ui/src/components/RouteErrorBoundary.test.tsx index 81f5ee80fea..a0dbf345cff 100644 --- a/ui/src/components/RouteErrorBoundary.test.tsx +++ b/ui/src/components/RouteErrorBoundary.test.tsx @@ -3,7 +3,7 @@ import { act, lazy } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createChunkLoadRecovery, RELOAD_ABANDONED_MS } from "@/lib/chunk-load-recovery"; +import { createChunkLoadRecovery, RELOAD_ABANDONED_MS, RELOAD_DELAYS_MS } from "@/lib/chunk-load-recovery"; import { RouteErrorBoundary } from "./RouteErrorBoundary"; const navigateMock = vi.hoisted(() => vi.fn()); @@ -30,9 +30,16 @@ vi.mock("@/lib/chunk-load-recovery", async (importOriginal) => { ...actual, reloadForChunkLoadError: (error: unknown) => chunkRecovery.current!.reloadForChunkLoadError(error), onChunkLoadReloadAbandoned: (listener: () => void) => chunkRecovery.current!.onChunkLoadReloadAbandoned(listener), + cancelPendingChunkLoadReload: () => chunkRecovery.current!.cancelPendingChunkLoadReload(), }; }); +function seedReloadAttempts(attempts: number) { + const startedAt = Date.now(); + window.sessionStorage.setItem("paperclip:chunk-load-reload-at", String(startedAt)); + window.sessionStorage.setItem("paperclip:chunk-load-reload-attempts", `${startedAt}:${attempts}`); +} + function Boom(): never { throw new Error("Maximum update depth exceeded"); } @@ -125,8 +132,53 @@ describe("RouteErrorBoundary", () => { } }); + it("keeps loading through a delayed retry, then reports once if it never leaves the page", async () => { + vi.useFakeTimers(); + try { + seedReloadAttempts(1); + const FailedPage = lazy(async () => { throw new Error("Failed to fetch dynamically imported module"); }); + const root = createRoot(container); + await act(async () => { root.render(); }); + const delayMs = RELOAD_DELAYS_MS[1]!; + act(() => { vi.advanceTimersByTime(delayMs - 1); }); + expect(reloadMock).not.toHaveBeenCalled(); + act(() => { vi.advanceTimersByTime(1); }); + expect(reloadMock).toHaveBeenCalledTimes(1); + + act(() => { vi.advanceTimersByTime(RELOAD_ABANDONED_MS - 1); }); + expect(container.querySelector('[role="status"]')?.textContent).toContain("Loading"); + expect(captureBrowserExceptionMock).not.toHaveBeenCalled(); + act(() => { vi.advanceTimersByTime(1); }); + expect(container.textContent).toContain("This page hit an error"); + expect(captureBrowserExceptionMock).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + } finally { + vi.useRealTimers(); + } + }); + + it("cancels a delayed retry when the user navigates away from the failed route", async () => { + vi.useFakeTimers(); + try { + seedReloadAttempts(1); + const FailedPage = lazy(async () => { throw new Error("Failed to fetch dynamically imported module"); }); + const root = createRoot(container); + await act(async () => { root.render(); }); + routerLocation.current = { pathname: "/co/issues/CO-1", search: "" }; + await act(async () => { root.render(
Saved conversation
); }); + + act(() => { vi.advanceTimersByTime(RELOAD_DELAYS_MS[1]! + RELOAD_ABANDONED_MS); }); + expect(reloadMock).not.toHaveBeenCalled(); + expect(captureBrowserExceptionMock).not.toHaveBeenCalled(); + expect(container.textContent).toContain("Saved conversation"); + act(() => root.unmount()); + } finally { + vi.useRealTimers(); + } + }); + it("contains a repeated chunk load failure and recovers when navigating back to a task", async () => { - window.sessionStorage.setItem("paperclip:chunk-load-reload-at", String(Date.now())); + seedReloadAttempts(RELOAD_DELAYS_MS.length); const FailedPage = lazy(async () => { throw new Error("Failed to fetch dynamically imported module"); }); const root = createRoot(container); await act(async () => { root.render(); }); diff --git a/ui/src/components/RouteErrorBoundary.tsx b/ui/src/components/RouteErrorBoundary.tsx index e9fb2373fb0..1ef00c9ff35 100644 --- a/ui/src/components/RouteErrorBoundary.tsx +++ b/ui/src/components/RouteErrorBoundary.tsx @@ -2,7 +2,11 @@ import { describeError } from "@/api/errors"; import { Component, Suspense, type ErrorInfo, type ReactNode } from "react"; import { useLocation, useNavigate } from "@/lib/router"; import { Button } from "@/components/ui/button"; -import { onChunkLoadReloadAbandoned, reloadForChunkLoadError } from "@/lib/chunk-load-recovery"; +import { + cancelPendingChunkLoadReload, + onChunkLoadReloadAbandoned, + reloadForChunkLoadError, +} from "@/lib/chunk-load-recovery"; import { captureBrowserException } from "@/lib/sentry"; import { PaperclipLoading } from "./AnimatedPaperclipIcon"; @@ -40,9 +44,15 @@ class RouteErrorBoundaryInner extends Component, so reset whenever the route changes. if (this.state.error && prevProps.resetKey !== this.props.resetKey) { + this.cancelPendingReload(); this.stopWaiting(); this.setState({ error: null, reloading: false }); } diff --git a/ui/src/lib/chunk-load-recovery.test.ts b/ui/src/lib/chunk-load-recovery.test.ts index 63e5460d8cf..bc611c1f675 100644 --- a/ui/src/lib/chunk-load-recovery.test.ts +++ b/ui/src/lib/chunk-load-recovery.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { createChunkLoadRecovery, isChunkLoadError, RELOAD_ABANDONED_MS } from "./chunk-load-recovery"; +import { createChunkLoadRecovery, isChunkLoadError, RELOAD_ABANDONED_MS, RELOAD_DELAYS_MS } from "./chunk-load-recovery"; function memoryStorage(initial: Record = {}) { const values = new Map(Object.entries(initial)); @@ -47,15 +47,98 @@ describe("reloadForChunkLoadError", () => { expect(reload).toHaveBeenCalledTimes(1); }); - it("defers to the error UI when the fresh page fails again", () => { + it("retries a fresh page that fails again, waiting longer each time for the deploy to settle", () => { + vi.useFakeTimers(); const storage = memoryStorage(); const reload = vi.fn(); let now = 1_000_000; createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }).reloadForChunkLoadError(cssPreloadError); + expect(reload).toHaveBeenCalledTimes(1); + for (const delayMs of RELOAD_DELAYS_MS.slice(1)) { + now += 10_000; + const freshPage = createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }); + const reloadsBefore = reload.mock.calls.length; + expect(freshPage.reloadForChunkLoadError(cssPreloadError)).toBe(true); + vi.advanceTimersByTime(delayMs - 1); + expect(reload).toHaveBeenCalledTimes(reloadsBefore); + vi.advanceTimersByTime(1); + expect(reload).toHaveBeenCalledTimes(reloadsBefore + 1); + } + }); + + it("defers to the error UI once the retries in the guard window are used up", () => { + const storage = memoryStorage(); + const reload = vi.fn(); + let now = 1_000_000; + for (let attempt = 0; attempt < RELOAD_DELAYS_MS.length; attempt += 1) { + expect(createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }).reloadForChunkLoadError(cssPreloadError)).toBe(true); + now += 10_000; + } + + const exhaustedPage = createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }); + expect(exhaustedPage.reloadForChunkLoadError(cssPreloadError)).toBe(false); + }); + + it("counts a reload recorded by an older build as the first retry", () => { + vi.useFakeTimers(); + const reload = vi.fn(); + const storage = memoryStorage({ "paperclip:chunk-load-reload-at": "1000000" }); + const { reloadForChunkLoadError } = createChunkLoadRecovery({ storage: () => storage, reload, now: () => 1_010_000 }); + + expect(reloadForChunkLoadError(cssPreloadError)).toBe(true); + expect(reload).not.toHaveBeenCalled(); + vi.advanceTimersByTime(RELOAD_DELAYS_MS[1]!); + expect(reload).toHaveBeenCalledTimes(1); + }); + + it("keeps the stored reload time readable by builds without retries", () => { + const storage = memoryStorage(); + const reload = vi.fn(); + let now = 1_000_000; + createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }).reloadForChunkLoadError(cssPreloadError); now += 10_000; - const freshPage = createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }); - expect(freshPage.reloadForChunkLoadError(cssPreloadError)).toBe(false); + createChunkLoadRecovery({ storage: () => storage, reload, now: () => now }).reloadForChunkLoadError(cssPreloadError); + + // An older build reads this key with Number() and keeps its one-reload guard. + expect(Number(storage.getItem("paperclip:chunk-load-reload-at"))).toBe(1_000_000); + }); + + it("ignores a retry count left from another guard window", () => { + vi.useFakeTimers(); + const reload = vi.fn(); + const storage = memoryStorage({ + "paperclip:chunk-load-reload-at": "2000000", + "paperclip:chunk-load-reload-attempts": `1000000:${RELOAD_DELAYS_MS.length}`, + }); + const { reloadForChunkLoadError } = createChunkLoadRecovery({ storage: () => storage, reload, now: () => 2_010_000 }); + + expect(reloadForChunkLoadError(cssPreloadError)).toBe(true); + vi.advanceTimersByTime(RELOAD_DELAYS_MS[1]!); + expect(reload).toHaveBeenCalledTimes(1); + }); + + it("cancels a delayed retry that has not started", () => { + vi.useFakeTimers(); + const reload = vi.fn(); + const storage = memoryStorage({ "paperclip:chunk-load-reload-at": "1000000" }); + const recovery = createChunkLoadRecovery({ storage: () => storage, reload, now: () => 1_010_000 }); + expect(recovery.reloadForChunkLoadError(cssPreloadError)).toBe(true); + const abandoned = vi.fn(); + recovery.onChunkLoadReloadAbandoned(abandoned); + + expect(recovery.cancelPendingChunkLoadReload()).toBe(true); + vi.advanceTimersByTime(RELOAD_DELAYS_MS[1]! + RELOAD_ABANDONED_MS); + expect(reload).not.toHaveBeenCalled(); + expect(abandoned).not.toHaveBeenCalled(); + expect(recovery.cancelPendingChunkLoadReload()).toBe(false); + }); + + it("cannot cancel a reload that already started", () => { + const reload = vi.fn(); + const recovery = createChunkLoadRecovery({ storage: () => memoryStorage(), reload }); + expect(recovery.reloadForChunkLoadError(cssPreloadError)).toBe(true); + expect(recovery.cancelPendingChunkLoadReload()).toBe(false); expect(reload).toHaveBeenCalledTimes(1); }); @@ -101,10 +184,13 @@ describe("reloadForChunkLoadError", () => { }); it("ignores an unreadable stored marker", () => { - const reload = vi.fn(); - const storage = memoryStorage({ "paperclip:chunk-load-reload-at": "not-a-time" }); - const { reloadForChunkLoadError } = createChunkLoadRecovery({ storage: () => storage, reload, now: () => 1_000 }); - expect(reloadForChunkLoadError(cssPreloadError)).toBe(true); + for (const marker of ["not-a-time", "{\"windowStartedAt\":\"soon\"}", "null"]) { + const reload = vi.fn(); + const storage = memoryStorage({ "paperclip:chunk-load-reload-at": marker }); + const { reloadForChunkLoadError } = createChunkLoadRecovery({ storage: () => storage, reload, now: () => 1_000 }); + expect(reloadForChunkLoadError(cssPreloadError)).toBe(true); + expect(reload).toHaveBeenCalledTimes(1); + } }); it("never reloads for other errors", () => { diff --git a/ui/src/lib/chunk-load-recovery.ts b/ui/src/lib/chunk-load-recovery.ts index a7f380817c4..8d541d80a0f 100644 --- a/ui/src/lib/chunk-load-recovery.ts +++ b/ui/src/lib/chunk-load-recovery.ts @@ -1,6 +1,14 @@ +// Holds the bare start time of the guard window. Builds without retries read +// this key too, so it keeps their single-reload guard working during a deploy. const RELOAD_AT_KEY = "paperclip:chunk-load-reload-at"; +// Holds ":"; it only counts for the matching window. +const RELOAD_ATTEMPTS_KEY = "paperclip:chunk-load-reload-attempts"; const RELOAD_GUARD_MS = 5 * 60 * 1000; export const RELOAD_ABANDONED_MS = 10_000; +// During a deploy, old and new servers can both answer for a few seconds, so a +// freshly reloaded page can still miss a chunk. Each retry in the guard window +// waits longer for the rollout to settle; after the last one the error shows. +export const RELOAD_DELAYS_MS = [0, 2_000, 5_000]; const CHUNK_LOAD_ERROR_PATTERNS = [ /^Unable to preload CSS for /, @@ -16,6 +24,17 @@ export function isChunkLoadError(error: unknown): boolean { type ReloadStorage = Pick; +function isPositiveTime(value: number): boolean { + return Number.isFinite(value) && value > 0; +} + +function readAttempts(raw: string | null, windowStartedAt: number): number { + const [startedAt, attempts] = (raw ?? "").split(":").map(Number); + // A window started by an older build has no count yet; its reload was the first. + if (startedAt !== windowStartedAt || !Number.isInteger(attempts) || attempts! < 1) return 1; + return attempts!; +} + type ChunkLoadRecoveryOptions = { storage?: () => ReloadStorage | null; reload?: () => void; @@ -35,10 +54,17 @@ export function createChunkLoadRecovery(options: ChunkLoadRecoveryOptions = {}) const reload = options.reload ?? (() => window.location.reload()); const now = options.now ?? Date.now; let reloading = false; + // A reload that never left this document (cancelled, blocked or stalled) + // is not retried from the same document. + let reloadAbandoned = false; + let pendingReloadTimer: ReturnType | null = null; + let abandonTimer: ReturnType | null = null; const abandonedListeners = new Set<() => void>(); function abandonReload() { + abandonTimer = null; reloading = false; + reloadAbandoned = true; const listeners = [...abandonedListeners]; abandonedListeners.clear(); for (const listener of listeners) listener(); @@ -47,21 +73,46 @@ export function createChunkLoadRecovery(options: ChunkLoadRecoveryOptions = {}) function reloadForChunkLoadError(error: unknown): boolean { if (!isChunkLoadError(error)) return false; if (reloading) return true; + if (reloadAbandoned) return false; const storage = readStorage(); if (!storage) return false; const at = now(); + let delayMs: number; try { - const lastReloadAt = Number(storage.getItem(RELOAD_AT_KEY)); - if (Number.isFinite(lastReloadAt) && lastReloadAt > 0 && at - lastReloadAt < RELOAD_GUARD_MS) { - return false; - } - storage.setItem(RELOAD_AT_KEY, String(at)); + const startedAt = Number(storage.getItem(RELOAD_AT_KEY)); + const inWindow = isPositiveTime(startedAt) && at - startedAt < RELOAD_GUARD_MS; + const attempt = inWindow ? readAttempts(storage.getItem(RELOAD_ATTEMPTS_KEY), startedAt) : 0; + if (attempt >= RELOAD_DELAYS_MS.length) return false; + delayMs = RELOAD_DELAYS_MS[attempt]!; + const windowStartedAt = inWindow ? startedAt : at; + if (!inWindow) storage.setItem(RELOAD_AT_KEY, String(windowStartedAt)); + storage.setItem(RELOAD_ATTEMPTS_KEY, `${windowStartedAt}:${attempt + 1}`); } catch { return false; } reloading = true; - reload(); - globalThis.setTimeout(abandonReload, RELOAD_ABANDONED_MS); + if (delayMs > 0) { + pendingReloadTimer = globalThis.setTimeout(() => { + pendingReloadTimer = null; + reload(); + }, delayMs); + } else { + reload(); + } + abandonTimer = globalThis.setTimeout(abandonReload, delayMs + RELOAD_ABANDONED_MS); + return true; + } + + // Stops a delayed retry that has not started yet, for example when the user + // navigates away from the failed route. A started reload cannot be stopped. + function cancelPendingChunkLoadReload(): boolean { + if (pendingReloadTimer === null) return false; + globalThis.clearTimeout(pendingReloadTimer); + pendingReloadTimer = null; + if (abandonTimer !== null) globalThis.clearTimeout(abandonTimer); + abandonTimer = null; + reloading = false; + abandonedListeners.clear(); return true; } @@ -84,11 +135,12 @@ export function createChunkLoadRecovery(options: ChunkLoadRecoveryOptions = {}) return () => target.removeEventListener("vite:preloadError", onPreloadError); } - return { reloadForChunkLoadError, onChunkLoadReloadAbandoned, startChunkLoadRecovery }; + return { reloadForChunkLoadError, cancelPendingChunkLoadReload, onChunkLoadReloadAbandoned, startChunkLoadRecovery }; } const browserRecovery = createChunkLoadRecovery(); export const reloadForChunkLoadError = browserRecovery.reloadForChunkLoadError; +export const cancelPendingChunkLoadReload = browserRecovery.cancelPendingChunkLoadReload; export const onChunkLoadReloadAbandoned = browserRecovery.onChunkLoadReloadAbandoned; export const startChunkLoadRecovery = browserRecovery.startChunkLoadRecovery;