Fix infinite loop in replay mode when CDN returns same cursor #3773
+107
−2
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
This test reproduces a bug where the client gets stuck in an infinite loop when the CDN keeps returning cached responses with the same cursor that's stored in localStorage from a previous session. The bug occurs because #lastSeenCursor is not cleared after suppressing an up-to-date notification, causing the client to stay in replay mode forever and make rapid requests in a tight loop. Also adds vitest.unit.config.ts to run unit tests without requiring the Electric server. https://claude.ai/code/session_01Gc8cwwR7XDsbX5D3jXQKvF
When the CDN keeps returning cached responses with the same cursor as stored in localStorage, the client would get stuck in an infinite loop because #lastSeenCursor was never cleared after suppressing an up-to-date. The fix clears #lastSeenCursor immediately after the first suppression, which exits replay mode and allows subsequent requests to notify subscribers normally. Also fixes the test to properly use the upToDateTracker singleton instead of just setting localStorage (the singleton doesn't reload from localStorage after initialization). https://claude.ai/code/session_01Gc8cwwR7XDsbX5D3jXQKvF
Update the CDN infinite loop test to properly test the loop behavior by: - Using subscribe: true to allow multiple requests - Using Promise.race with a timeout as a safety valve - Verifying gotUpToDate is true (which fails without the fix) - Verifying fetchCallCount is ≤3 (would be 10+ without fix) The test now properly fails when the fix is removed and passes when the fix is in place. https://claude.ai/code/session_01Gc8cwwR7XDsbX5D3jXQKvF
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3773 +/- ##
==========================================
+ Coverage 87.36% 87.42% +0.05%
==========================================
Files 23 23
Lines 2011 2012 +1
Branches 528 533 +5
==========================================
+ Hits 1757 1759 +2
+ Misses 252 251 -1
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Clarify comment explaining replay mode exit behavior - Refactor Promise.race pattern in test to use named variables and Array.some() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
Comment |
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.
Summary
Fixes a bug where the UpToDateTracker could get stuck in an infinite loop when the CDN returns cached responses with the same cursor value as stored in localStorage. The issue occurred during replay mode when the client would repeatedly suppress notifications without exiting replay mode.
Changes
Added regression test in
up-to-date-tracker.test.tsthat reproduces the infinite loop scenario:Added unit test configuration (
vitest.unit.config.ts):Implementation Details
The test documents the bug scenario and expected fix:
lastSeenCursor=1000from localStoragelastSeenCursormust be cleared after first suppression to exit replay modeThe safety limit of 3 fetch calls allows for:
Without the fix, the test would hit the 10-call limit, indicating the infinite loop.