Fix public leaderboard network recovery - #652
Conversation
WalkthroughThe leaderboard fallback fetch now retries transient network failures with short delays. After repeated failure, the page shows a connection error and a “Try again” button. Tests and architecture documentation cover the updated behavior. ChangesLeaderboard retry recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
actor Viewer
participant LeaderboardPageContent
participant LeaderboardAPI
Viewer->>LeaderboardPageContent: Open leaderboard
LeaderboardPageContent->>LeaderboardAPI: Fetch leaderboard
LeaderboardAPI-->>LeaderboardPageContent: Transient network failure
LeaderboardPageContent->>LeaderboardAPI: Retry after backoff
LeaderboardAPI-->>LeaderboardPageContent: Success or repeated failure
Viewer->>LeaderboardPageContent: Click “Try again”
LeaderboardPageContent->>LeaderboardAPI: Fetch leaderboard again
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/wodsmith-start/test/components/leaderboard-page-content.test.tsx`:
- Around line 803-826: Add exactly one adjacent `@lat`: comment to each relevant
recovery test in the leaderboard test suite, including the tests around
getCompetitionLeaderboardFn retry behavior. The reference should identify the
documented public leaderboard recovery behavior, and no test should receive more
than one LAT comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 72264ee1-0bba-49cc-90d0-3a65e26c3acf
📒 Files selected for processing (3)
apps/wodsmith-start/src/components/leaderboard-page-content.tsxapps/wodsmith-start/test/components/leaderboard-page-content.test.tsxlat.md/architecture.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/test/components/leaderboard-page-content.test.tsx">
<violation number="1" location="apps/wodsmith-start/test/components/leaderboard-page-content.test.tsx:818">
P3: The 'retries transient network failures' test only validates the first 250ms retry followed by success. The PR's core behavior includes a second 750ms retry and, when both retries are exhausted, falling through to the 'Error loading leaderboard' recovery UI — and that exhaustion path is exactly what turns a brief outage into the manual 'Try again' flow. Suggest adding a case that rejects all three attempts (TypeError twice) and asserts the final error state, plus one that verifies the second retry fires after the cumulative backoff. This guards against regressions in the retry-count/backoff and terminal-state logic.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| expect(getCompetitionLeaderboardFn).toHaveBeenCalledTimes(1) | ||
|
|
||
| await act(async () => { |
There was a problem hiding this comment.
P3: The 'retries transient network failures' test only validates the first 250ms retry followed by success. The PR's core behavior includes a second 750ms retry and, when both retries are exhausted, falling through to the 'Error loading leaderboard' recovery UI — and that exhaustion path is exactly what turns a brief outage into the manual 'Try again' flow. Suggest adding a case that rejects all three attempts (TypeError twice) and asserts the final error state, plus one that verifies the second retry fires after the cumulative backoff. This guards against regressions in the retry-count/backoff and terminal-state logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/test/components/leaderboard-page-content.test.tsx, line 817:
<comment>The 'retries transient network failures' test only validates the first 250ms retry followed by success. The PR's core behavior includes a second 750ms retry and, when both retries are exhausted, falling through to the 'Error loading leaderboard' recovery UI — and that exhaustion path is exactly what turns a brief outage into the manual 'Try again' flow. Suggest adding a case that rejects all three attempts (TypeError twice) and asserts the final error state, plus one that verifies the second retry fires after the cumulative backoff. This guards against regressions in the retry-count/backoff and terminal-state logic.</comment>
<file context>
@@ -795,6 +800,63 @@ describe("LeaderboardPageContent", () => {
+
+ expect(getCompetitionLeaderboardFn).toHaveBeenCalledTimes(1)
+
+ await act(async () => {
+ await vi.advanceTimersByTimeAsync(250)
+ })
</file context>
1 participant
Summary
Load failedmessage with a connection-focused recovery stateRoot cause
Production telemetry showed intermittent iOS browser
TypeError: Load failedfailures while fetching leaderboard data through the server function. The Worker and leaderboard computation remained healthy; the failure occurs in the browser transport during client navigation or refetch.iOS Simulator testing against production through a read-only localhost fault proxy isolated the trigger:
The implementation intentionally does not add another application retry loop on top of those existing attempts. That would increase one failed division change from as many as 8 reads to as many as 16. Instead, it gives the viewer a clear manual recovery action once connectivity returns.
Validation
lat checkpassedgit diff --checkpassedThe full workspace type-check remains blocked by two unrelated pre-existing untracked test files (
my-schedule-event-section.test.tsxandmwfc-round-breakdown.test.ts).