Repository navigation
fix(flaky): complete subscription journey with authentication protection and accessibility - #930
SarahLittlejohn wants to merge 2 commits into
Conversation
The nightly E2E test "Email Subscriptions - Location > complete subscription journey with authentication protection and accessibility" was flaky on 2026-08-09 (failed on attempt 1, passed on retry) against the same commit that ran clean the day before and after. Root cause: exchangeCodeForToken() made a single, non-retried fetch call to the IDAM token endpoint. Any transient network blip or 5xx from the AAT test IDAM service surfaced as a thrown error, which the callback handler in cft-login/return/index.ts catches and turns into a `/sign-in?error=auth_failed` redirect - masking the real cause and failing the login journey outright. Add a single bounded retry (2 attempts, 250ms backoff) for network failures and 5xx responses only; 4xx responses (e.g. an already-used authorization code) still fail immediately since retrying those can never succeed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DmhgNiZ4QP83JrXCWLJKAn
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe token client now retries transient network and 5xx failures up to two attempts with a 250 ms delay. It fails immediately for 4xx responses. Tests cover successful retries and exhausted retries. ChangesToken exchange retries
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e04dcb78-726b-4047-a6fc-6cf9009c9825
📒 Files selected for processing (2)
libs/auth/src/cft-idam/token-client.test.tslibs/auth/src/cft-idam/token-client.ts
… mock - Cancel the 5xx response body before retrying so the underlying connection isn't held open across the retry attempt. - Type the fetch mock once via vi.mocked(global.fetch) instead of repeating (fetch as any) casts at each call site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DmhgNiZ4QP83JrXCWLJKAn
|
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
🎭 Playwright E2E Test Results82 tests 52 ✅ 7m 30s ⏱️ Results for commit 50a58c7. |



Summary
Weekly flaky-test scan of the last 7 days of CI on
masterfound one flaky test:tests/verified-user/email-subscriptions.spec.ts:72— Email Subscriptions - Location › complete subscription journey with authentication protection and accessibilityOn the Nightly Tests run of 2026-08-09 this test failed on its first attempt and passed on Playwright's automatic retry — against the exact same commit (
64a344d9) that had run clean on 2026-08-08 and 2026-08-10. Flake rate: 1/7 relevant nightly runs (~14%).Root cause
The failure was:
exchangeCodeForToken()inlibs/auth/src/cft-idam/token-client.tsmakes a single, non-retriedfetchcall to the CFT IDAM (AAT) token endpoint. Any transient network blip or 5xx response from that external test environment throws, which the callback handler inapps/web/src/pages/(auth)/cft-login/return/index.tscatches and turns into a/sign-in?error=auth_failedredirect — the login journey fails outright instead of retrying, and the real cause (a one-off network/server hiccup) is masked.This is external-network-reliance flakiness, not application logic — the redirect target of
waitForURLin the e2e login helper is loose enough to also match the failure redirect, which is how the failure surfaced downstream in the test assertion rather than inside the helper itself.Fix
Added a single bounded retry (2 attempts, 250ms backoff) around the token exchange call, retrying only on:
fetchthrowing — DNS, connection reset, timeout), and5xxresponses from IDAM (transient service errors)4xxresponses (e.g. an already-used, single-use authorization code) still fail immediately, since retrying those can never succeed and would just add latency to a genuine failure.Verification
The sandboxed environment couldn't reach
hmcts's private Azure DevOps package feed (@hmcts-cft/cloud-native-platform) needed for a fullyarn install, so the vitest suite couldn't be run directly here. Instead the exact committedtoken-client.tswas executed directly (viatsx, type-only imports elided) against 7 mocked scenarios covering success, 4xx (no retry), 5xx (retry then succeed / retry then fail), and network failure (retry then succeed / retry then fail) — run 5 times consecutively with consistent, deterministic results. New unit tests covering the same scenarios were added tolibs/auth/src/cft-idam/token-client.test.tsfor CI to run withyarn test.Other findings (not flaky, noted for visibility)
COURTEL_API_URL is not set). Worth a separate ticket to fix or quarantine, but out of scope here.Test Changed Packages/ Vitest) were found onmasterpushes in the last 7 days — all green except one run where only the Terraform Apply jobs failed (infra, unrelated to tests).Test plan
token-client.test.tsbiome checkclean on both changed filesyarn test) — could not run locally due to private package feed access; will be verified by the pipeline on this PRGenerated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests