Fix chunkErrorRecovery.spec.ts's reload-guard race in CI - #397
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
frontend/tests/chunkErrorRecovery.spec.ts'sexpect.poll(() => reloadRequests).toBe(1)failed twice consecutively on CI shard 1/4 of PR #395 (run 30039392833), including a clean re-run, while passing reliably locally.Received: 0- no error thrown anywhere in the test, the reload request simply never arrived.Root cause
useChunkErrorRecovery's guard (chunkErrorRecovery.ts'sCHUNK_RELOAD_GUARD_KEY, a real 10s sessionStorage-backed "only one reload per window" debounce) was consumed by a real chunk hiccup before the test's own synthetic dispatch ever ran.CI's
playwright.config.tswebServer runsnpm run dev(not the static export the site actually deploys), and Next's dev server compiles pages on demand. Pulling the CI trace apart (playwright-reportartifact from run 30039392833) showed the navbar's own "Editor" nav link - visible even while already on/editor- getting prefetched bynext/link's default viewportIntersectionObserverbehaviour, triggering a second on-demand recompile ofpages/editor.jsmid-test (~800ms, network-adjacent in time to the test's own dispatch). A slower/colder 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 (and correctly, per the guard's own design) consumes the one-shot budget first.This is a dev-server/test-harness artifact only - the deployed static export has zero on-demand compilation or HMR, so it structurally can't happen in production. The guard suppressing a second reload within its window is the product working exactly as designed; the test's implicit assumption that no real chunk error has fired by the time it dispatches its own synthetic one is what's invalid in CI's slower/cold-compile environment.
Why the assertions are not weakened
Both guarantees the suite exists to protect stay exactly as strict:
window 'error'andunhandledrejectiontriggers) still assertreloadRequestsbecomes exactly1, unchanged.The fix (
clearReloadGuard, callingpage.evaluate(() => sessionStorage.removeItem(CHUNK_RELOAD_GUARD_KEY))immediately before each test's own dispatch) only establishes a clean precondition - it doesn't touch or bypass the guard logic itself.CHUNK_RELOAD_GUARD_KEYis exported fromchunkErrorRecovery.tsas a pure string constant (no behaviour change) so the test doesn't duplicate/hardcode the key name.Verification
npx prettier@2.7.1 --checkon all three changed files: passnpx tsc --noEmit: passnpx jest(full suite, 64 suites / 565 tests, includeschunkErrorRecovery.test.ts's existing pure-logic coverage, untouched): all passnpx playwright test tests/chunkErrorRecovery.spec.ts --repeat-each=10(warm dev server): 40/40 passedrm -rf .next && CI=true npx playwright test tests/chunkErrorRecovery.spec.ts --repeat-each=5(cold dev-server cache, closer to CI's actual conditions): 20/20 passedrm -rf .next && npx playwright test(full suite, cold cache): 328/328 passedDeferred: reproducing the exact CI shard-1/4 timing on this machine wasn't attempted (single-machine timing races like this are inherently hard to force deterministically) - confidence instead comes from the CI trace's own network/timeline evidence (see
docs/troubleshooting.md's new entry) plus the fix's mechanism directly targeting the exact state (the sessionStorage guard key) that had to be responsible given the observed symptom (zero network requests, zero errors, correct call ordering per the trace).Docs
Added a symptom-first entry to
docs/troubleshooting.md(chunkErrorRecovery.spec.ts's reload-guard race) per repo convention, since diagnosis here took well over 15 minutes.Not merging - per this session's operating rules, PRs are reviewed and merged by the owner.