diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 8aae22453..1c0c6514e 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -1554,3 +1554,44 @@ occurrence**: before writing this off as "expected," reproduce deliberately (recreate the container, watch whether the session survives) and confirm which layer is actually responsible, then replace this entry's cause with the confirmed one. + +## `chunkErrorRecovery.spec.ts`'s `expect.poll(() => reloadRequests).toBe(1)` fails with "Received: 0" in CI, passes locally + +**Symptom**: one of the first three `chunkErrorRecovery.spec.ts` tests +(the ones that dispatch a synthetic `ChunkLoadError` and expect exactly +one intercepted reload request) times out at 0 reload requests on a CI +shard, including on a clean re-run, while passing reliably on a local +machine. No error is thrown anywhere in the test itself (`page.route()` +registers fine, `page.evaluate()` dispatching the synthetic error +completes fine) — the reload request simply never arrives. + +**Cause**: `useChunkErrorRecovery`'s guard +(`chunkErrorRecovery.ts`'s `CHUNK_RELOAD_GUARD_KEY`, a real 10s +sessionStorage-backed "only one reload per window" debounce, working +exactly as designed) gets consumed by a **real** chunk hiccup before +the test's own synthetic dispatch ever runs. CI's `playwright.config.ts` +webServer runs `npm run dev` (not a prebuilt static export), and Next's +dev server compiles pages on demand — the navbar's own "Editor" nav +link (visible even while already on `/editor`) gets prefetched by +`next/link`'s default viewport `IntersectionObserver` behaviour, which +can trigger a second on-demand recompile of `pages/editor.js` mid-test +(confirmed via a CI trace: an ~800ms second compile of that exact +chunk, network-adjacent in time to the test's own dispatch). A slow/ +cold CI runner is more likely to still be mid-churn from that when the +test body reaches its own dispatch, and any real transient chunk error +during that churn legitimately consumes the guard first. This is a +dev-server/test-harness artifact only — the deployed static export has +no on-demand compilation or HMR at all, so it can't happen in +production. + +**Fix**: the test now imports `CHUNK_RELOAD_GUARD_KEY` from +`chunkErrorRecovery.ts` and calls +`page.evaluate((key) => window.sessionStorage.removeItem(key), CHUNK_RELOAD_GUARD_KEY)` +immediately before each test's own synthetic dispatch, establishing a +clean precondition regardless of whatever real dev-server chunk noise +happened during page load. The one test that specifically exercises the +guard-suppression behaviour (two dispatches in one test) only clears it +once, before the first dispatch, so the guard is still genuinely +exercised for real on the second one. See `chunkErrorRecovery.spec.ts`'s +own comment above `clearReloadGuard` for the full trace-based diagnosis +(root-caused against CI run 30039392833, shard 1/4). diff --git a/frontend/src/common/chunkErrorRecovery.ts b/frontend/src/common/chunkErrorRecovery.ts index 2c0ae88ec..3a97aa32b 100644 --- a/frontend/src/common/chunkErrorRecovery.ts +++ b/frontend/src/common/chunkErrorRecovery.ts @@ -35,7 +35,11 @@ export function isRecoveryReloadInFlight(): boolean { return recoveryReloadInFlight; } -const CHUNK_RELOAD_GUARD_KEY = "chunkReloadAttemptedAt"; +// Exported (pure data, no behaviour change) so chunkErrorRecovery.spec.ts can defensively clear +// this key before dispatching its own synthetic error - see that file's own comment for why a +// real, unrelated chunk hiccup during dev-server on-demand compilation can otherwise pre-consume +// this one-shot guard before the test's synthetic dispatch ever runs. +export const CHUNK_RELOAD_GUARD_KEY = "chunkReloadAttemptedAt"; // A real deploy-caused chunk failure is fixed by exactly one reload (the browser fetches the // fresh HTML/chunk manifest). This window exists only to stop a reload LOOP if reloading somehow // doesn't fix it (e.g. a mid-deploy race, or a genuinely broken deploy) - not a retry budget. diff --git a/frontend/tests/chunkErrorRecovery.spec.ts b/frontend/tests/chunkErrorRecovery.spec.ts index 68f106c4d..02496f93d 100644 --- a/frontend/tests/chunkErrorRecovery.spec.ts +++ b/frontend/tests/chunkErrorRecovery.spec.ts @@ -1,5 +1,7 @@ import { expect, Page } from "@playwright/test"; +import { CHUNK_RELOAD_GUARD_KEY } from "@/common/chunkErrorRecovery"; + import { test } from "../playwright.setup"; import { loadPageWithDefaultBackend } from "./test-utils"; @@ -25,6 +27,31 @@ const dispatchChunkError = (page: Page) => ); }); +// CI diagnosis (PR #395, run 30039392833, shard 1/4 - failed twice consecutively, including a +// clean re-run, while passing locally): the recovery mechanism's own sessionStorage guard +// (chunkErrorRecovery.ts's CHUNK_RELOAD_GUARD_KEY, a real 10s "don't loop" debounce, not a bug) +// can legitimately already be set by the time a test gets around to dispatching its own synthetic +// error. `npm run dev`'s webserver compiles /editor on demand (its own network trace showed a +// second, ~800ms recompile of pages/editor.js mid-test, triggered by the navbar's own "Editor" +// nav link - visible while already ON /editor - being prefetched by next/link's default viewport +// IntersectionObserver behaviour), and that on-demand-compilation churn is exactly the class of +// transient chunk hiccup this whole mechanism exists to recover from - it's plausible for a real +// one to fire and consume the guard before a slow/cold CI runner's test body gets to its own +// dispatch. This is a dev-server/test-harness artifact only: the deployed static export has no +// on-demand compilation or HMR at all, so this can't happen in production, and the guard +// suppressing a second reload within its window is the product working exactly as designed - see +// docs/troubleshooting.md's chunkErrorRecovery.spec.ts entry. Clearing the guard immediately +// before each test's own dispatch establishes the clean precondition the assertion actually means +// to test ("a synthetic ChunkLoadError triggers exactly one reload"), independent of whatever +// unrelated real chunk noise the dev server produced getting the page ready - it does not touch +// the guard-suppression behaviour itself, which the last test below still exercises for real via +// two dispatches inside the same clean window. +const clearReloadGuard = (page: Page) => + page.evaluate( + (key) => window.sessionStorage.removeItem(key), + CHUNK_RELOAD_GUARD_KEY + ); + test.describe("Chunk-load-error recovery", () => { test("a ChunkLoadError dispatched as a window 'error' event triggers a reload", async ({ page, @@ -36,6 +63,7 @@ test.describe("Chunk-load-error recovery", () => { await route.abort(); }); + await clearReloadGuard(page); await dispatchChunkError(page); await expect.poll(() => reloadRequests).toBe(1); @@ -51,6 +79,7 @@ test.describe("Chunk-load-error recovery", () => { await route.abort(); }); + await clearReloadGuard(page); await page.evaluate(() => { const error = new Error("Loading CSS chunk 2 failed."); // PromiseRejectionEvent isn't constructible directly in most browsers - a plain object @@ -95,11 +124,14 @@ test.describe("Chunk-load-error recovery", () => { await route.abort(); }); + await clearReloadGuard(page); await dispatchChunkError(page); await expect.poll(() => reloadRequests).toBe(1); // The aborted reload never completed, so the same document/listeners are still live - // dispatch a second chunk error and confirm the guard window suppresses a second attempt. + // (Deliberately no clearReloadGuard() call here - this second dispatch is exactly what's + // meant to hit the still-live guard from the first one above.) await dispatchChunkError(page); await page.waitForTimeout(1_000); expect(reloadRequests).toBe(1);