From c8f88a8b38338e2eeacc599cbbe6354986ff2a9a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 20 Aug 2026 04:21:57 -0700 Subject: [PATCH] fix(ui): recover newer builds after stale probes (#126628) * fix(ui): recover newer builds after stale probes * fix(ui): elect newest stale-build recovery target --- ui/src/app/stale-chunk-reload.test.ts | 98 ++++++++++++++++++++++++--- ui/src/app/stale-chunk-reload.ts | 58 ++++++++++------ 2 files changed, 127 insertions(+), 29 deletions(-) diff --git a/ui/src/app/stale-chunk-reload.test.ts b/ui/src/app/stale-chunk-reload.test.ts index 171b90176138..795e84c07880 100644 --- a/ui/src/app/stale-chunk-reload.test.ts +++ b/ui/src/app/stale-chunk-reload.test.ts @@ -104,7 +104,7 @@ describe("scheduleStaleChunkReload", () => { expect(storage.getItem(GUARD_KEY)).toBe("build-a"); }); - it("never auto-reloads twice for the same build, but recovers on a newer build", async () => { + it("never lets a persisted build guard suppress recovery for a newer build", async () => { const reload = vi.fn(); const storage = memoryStorage({ [GUARD_KEY]: "build-a" }); stubDocumentFetch(new Response(null, { status: 200 })); @@ -119,7 +119,7 @@ describe("scheduleStaleChunkReload", () => { expect(reload).not.toHaveBeenCalled(); await expect( scheduleStaleChunkReload({ - now: () => 7000, + now: () => 2000, buildId: "build-b", storage, reload, @@ -179,20 +179,102 @@ describe("scheduleStaleChunkReload", () => { await expect( scheduleStaleChunkReload({ now: () => 1000, + buildId: "build-a", storage, reload, }), ).resolves.toBe(false); - await expect(scheduleStaleChunkReload({ now: () => 2000, storage, reload })).resolves.toBe( - false, - ); + await expect( + scheduleStaleChunkReload({ now: () => 2000, buildId: "build-a", storage, reload }), + ).resolves.toBe(false); expect(fetchMock).toHaveBeenCalledTimes(1); - await expect(scheduleStaleChunkReload({ now: () => 7000, storage, reload })).resolves.toBe( - true, - ); + await expect( + scheduleStaleChunkReload({ now: () => 7000, buildId: "build-a", storage, reload }), + ).resolves.toBe(true); expect(reload).toHaveBeenCalledTimes(1); }); + it("probes a newer build immediately after an older build was unreachable", async () => { + const reload = vi.fn(); + const storage = memoryStorage(); + const fetchMock = stubDocumentFetch( + new Response(null, { status: 503 }), + new Response(null, { status: 200 }), + ); + + await expect( + scheduleStaleChunkReload({ now: () => 1000, buildId: "build-a", storage, reload }), + ).resolves.toBe(false); + await expect( + scheduleStaleChunkReload({ now: () => 2000, buildId: "build-b", storage, reload }), + ).resolves.toBe(true); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(reload).toHaveBeenCalledTimes(1); + expect(storage.getItem(GUARD_KEY)).toBe("build-b"); + }); + + it("reprobes a newer build after its joined older-build probe fails", async () => { + const olderProbe = deferred(); + const fetchMock = vi + .fn() + .mockImplementationOnce(async () => olderProbe.promise) + .mockResolvedValueOnce(new Response(null, { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + const reload = vi.fn(); + const storage = memoryStorage(); + + const olderBuild = scheduleStaleChunkReload({ + now: () => 1000, + buildId: "build-a", + storage, + reload, + }); + const newerBuild = scheduleStaleChunkReload({ + now: () => 2000, + buildId: "build-b", + storage, + reload, + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + + olderProbe.resolve(new Response(null, { status: 503 })); + await expect(Promise.all([olderBuild, newerBuild])).resolves.toEqual([false, true]); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(reload).toHaveBeenCalledTimes(1); + expect(storage.getItem(GUARD_KEY)).toBe("build-b"); + }); + + it("reloads only the newest build after a shared document probe succeeds", async () => { + const sharedProbe = deferred(); + const fetchMock = vi.fn(async () => sharedProbe.promise); + vi.stubGlobal("fetch", fetchMock); + const reload = vi.fn(); + const storage = memoryStorage(); + + const olderBuild = scheduleStaleChunkReload({ + now: () => 1000, + buildId: "build-a", + storage, + reload, + }); + const newerBuild = scheduleStaleChunkReload({ + now: () => 2000, + buildId: "build-b", + storage, + reload, + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + + sharedProbe.resolve(new Response(null, { status: 200 })); + await expect(Promise.all([olderBuild, newerBuild])).resolves.toEqual([false, true]); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(reload).toHaveBeenCalledTimes(1); + expect(storage.getItem(GUARD_KEY)).toBe("build-b"); + }); + it("settles and aborts a hanging document probe after its deadline", async () => { vi.useFakeTimers(); const reload = vi.fn(); diff --git a/ui/src/app/stale-chunk-reload.ts b/ui/src/app/stale-chunk-reload.ts index d44c3bc4d8a8..0394296947f4 100644 --- a/ui/src/app/stale-chunk-reload.ts +++ b/ui/src/app/stale-chunk-reload.ts @@ -38,9 +38,12 @@ type MissingStylesheetRecoveryDeps = { retry?: () => Promise; }; -const lastAttemptAtByStorage = new WeakMap(); -let lastAttemptWithoutStorage: number | null = null; -let inFlightDocumentProbe: Promise | null = null; +const recoveryByStorage = new WeakMap< + object, + { attemptsByBuild: Map; latestBuildId: string } +>(); +const unavailableStorage = {}; +let inFlightDocumentProbe: { buildId?: string; promise: Promise } | null = null; export function isStaleChunkImportError(error: unknown): boolean { return ( @@ -65,9 +68,9 @@ function sessionStorageOrNull(): Pick | null { } } -function probeControlUiDocument(): Promise { +function probeControlUiDocument(buildId?: string): Promise { if (inFlightDocumentProbe) { - return inFlightDocumentProbe; + return inFlightDocumentProbe.promise; } const probe = (async () => { const controller = new AbortController(); @@ -86,11 +89,11 @@ function probeControlUiDocument(): Promise { } })(); const settledProbe = probe.finally(() => { - if (inFlightDocumentProbe === settledProbe) { + if (inFlightDocumentProbe?.promise === settledProbe) { inFlightDocumentProbe = null; } }); - inFlightDocumentProbe = settledProbe; + inFlightDocumentProbe = { buildId, promise: settledProbe }; return settledProbe; } @@ -125,19 +128,7 @@ function persistGuardBuildId( * app webviews) instead of the recoverable panel error. */ export async function scheduleStaleChunkReload(deps: StaleChunkReloadDeps = {}): Promise { - const now = deps.now?.() ?? Date.now(); const storage = deps.storage === undefined ? sessionStorageOrNull() : deps.storage; - const lastAttemptAt = storage - ? (lastAttemptAtByStorage.get(storage) ?? null) - : lastAttemptWithoutStorage; - if (lastAttemptAt !== null && now - lastAttemptAt < ATTEMPT_COOLDOWN_MS) { - return false; - } - if (storage) { - lastAttemptAtByStorage.set(storage, now); - } else { - lastAttemptWithoutStorage = now; - } const buildId = deps.buildId ?? CONTROL_UI_BUILD_INFO.buildId; // One automatic reload per build id: if the reloaded document still fails // with the same build, the build itself is broken and reloading cannot help. @@ -145,13 +136,38 @@ export async function scheduleStaleChunkReload(deps: StaleChunkReloadDeps = {}): if (readGuardBuildId(storage) === buildId) { return false; } - if (!(await probeControlUiDocument())) { + const now = deps.now?.() ?? Date.now(); + const storageIdentity = storage ?? unavailableStorage; + const recovery = recoveryByStorage.get(storageIdentity) ?? { + attemptsByBuild: new Map(), + latestBuildId: buildId, + }; + const { attemptsByBuild } = recovery; + for (const [attemptedBuildId, attemptedAt] of attemptsByBuild) { + if (now - attemptedAt >= ATTEMPT_COOLDOWN_MS) { + attemptsByBuild.delete(attemptedBuildId); + } + } + if (attemptsByBuild.has(buildId)) { + return false; + } + attemptsByBuild.set(buildId, now); + recovery.latestBuildId = buildId; + recoveryByStorage.set(storageIdentity, recovery); + // A newer build cannot inherit the failed probe started for an older build. + const joinedOlderBuildProbe = Boolean( + inFlightDocumentProbe && inFlightDocumentProbe.buildId !== buildId, + ); + if ( + !(await probeControlUiDocument(buildId)) && + (!joinedOlderBuildProbe || !(await probeControlUiDocument(buildId))) + ) { return false; } // A reload resets the in-memory state, so without a persisted guard a broken // build would reload forever. When storage is unavailable or rejects the // write, leave recovery to the manual Retry path instead of reloading. - if (!persistGuardBuildId(storage, buildId)) { + if (recovery.latestBuildId !== buildId || !persistGuardBuildId(storage, buildId)) { return false; } (deps.reload ?? reloadControlUiDocument)();