app(sync): keep backendBusy cooldown in-memory only#7527
Conversation
The "Omi servers are busy" message persisted across app restarts because the SyncRateLimiter saved its until/reason to SharedPreferences for both rate-limit (HTTP 429) and backendBusy (stale-guard) cases. backendBusy reflects transient backend pipeline pressure. If a user restarts the app to retry, the cooldown should clear and the next sync should re-probe the backend instead of trusting a local timer set during an earlier rough patch. Keep persistence for rateLimit (mirrors server-side fair-use enforcement which IS sticky across restarts) and move backendBusy to an in-memory field. On construction, clear any pre-existing persisted backendBusy entry so users coming from older app versions get unstuck on next launch.
Greptile SummaryThis PR changes
Confidence Score: 3/5Safe to merge for the common case (single cooldown active), but the dual-cooldown scenario produces a misleading UI state. The core improvement — clearing backendBusy on restart and migrating stuck state from older installs — is sound and well-reasoned. The concern is in the reason and until getters: when both a backendBusy and a rateLimit cooldown are simultaneously active, until returns the later (max) expiry while reason always surfaces backendBusy first. The user would see "Omi servers are busy — wait until [rateLimit deadline]" even though the backendBusy ends sooner and the displayed deadline belongs to the rateLimit — then the message silently flips to "Fair-use limit" at backendBusy expiry with no change to the timer. app/lib/services/wals/sync_rate_limiter.dart — specifically the reason and until getters Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[markLimited called] --> B{reason == backendBusy?}
B -- Yes --> C[_backendBusyUntilMs = untilMs in-memory only]
B -- No --> D[SharedPreferences persisted]
C --> E[notifyListeners]
D --> E
F[isLimited getter] --> G{_backendBusyUntilMs > now?}
G -- Yes --> H[return true]
G -- No --> I{persisted until > now?}
I -- Yes --> H
I -- No --> J[return false]
K[until getter] --> L[return max of active candidates]
N[reason getter] --> O{_backendBusyUntilMs > now?}
O -- Yes --> P[return backendBusy]
O -- No --> Q{persisted > now?}
Q -- Yes --> R[return persisted reason]
Q -- No --> S[return null]
Reviews (1): Last reviewed commit: "app(sync): keep backendBusy cooldown in-..." | Re-trigger Greptile |
| DateTime? get until { | ||
| final ms = SharedPreferencesUtil().getInt(_prefKeyUntil); | ||
| return ms > 0 ? DateTime.fromMillisecondsSinceEpoch(ms) : null; | ||
| final now = DateTime.now().millisecondsSinceEpoch; | ||
| final persisted = SharedPreferencesUtil().getInt(_prefKeyUntil); | ||
| final inMemory = _backendBusyUntilMs; | ||
| final candidates = <int>[if (inMemory > now) inMemory, if (persisted > now) persisted]; | ||
| if (candidates.isEmpty) return null; | ||
| return DateTime.fromMillisecondsSinceEpoch(candidates.reduce((a, b) => a > b ? a : b)); | ||
| } | ||
|
|
||
| RateLimitReason? get reason { | ||
| if (!isLimited) return null; | ||
| final name = SharedPreferencesUtil().getString(_prefKeyReason); | ||
| return RateLimitReason.values.asNameMap()[name] ?? RateLimitReason.rateLimit; | ||
| final now = DateTime.now().millisecondsSinceEpoch; | ||
| if (_backendBusyUntilMs > now) return RateLimitReason.backendBusy; | ||
| final persisted = SharedPreferencesUtil().getInt(_prefKeyUntil); | ||
| if (persisted > now) { | ||
| final name = SharedPreferencesUtil().getString(_prefKeyReason); | ||
| return RateLimitReason.values.asNameMap()[name] ?? RateLimitReason.rateLimit; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
reason and until can report an inconsistent pair when both cooldowns are active
When a backendBusy in-memory cooldown and a persisted rateLimit cooldown are both active (e.g., a stale-guard fired during a 429 window), until returns the max expiry (likely the rateLimit end time), while reason returns backendBusy because _backendBusyUntilMs > now takes priority. The UI would then show "Omi servers are busy — wait until [rateLimit deadline]", which is wrong on both counts: the labelled reason expires sooner than displayed, and the displayed deadline belongs to a different reason. After _backendBusyUntilMs expires, the reason silently flips to rateLimit with no change in until, but until that flip the user sees an incorrect message. The reason getter should return the reason whose expiry matches the value returned by until (i.e., the reason with the later deadline), consistent with until's max-based logic.
kodjima33
left a comment
There was a problem hiding this comment.
thanks — keeping backendBusy cooldown in-memory makes sense
Summary
SyncRateLimiternow persists only therateLimitcooldown.backendBusycooldowns live in an in-memory field that clears on app restart.backendBusyentry from older app versions, so users coming from a healthy server get unstuck on next launch.Why
The "Omi servers are busy" banner was surviving app restarts even when the backend had long since recovered. The cooldown was saved to
SharedPreferencesfor bothrateLimit(HTTP 429) andbackendBusy(stale-guard) cases.rateLimitcooldowns SHOULD persist — they mirror server-side fair-use enforcement that is itself sticky across restarts (30-day restrict window). ButbackendBusyreflects transient backend pipeline pressure. If a user restarts the app to retry, the cooldown should clear and the next sync should re-probe the backend, not trust a local timer set during an earlier rough patch.Backend data (last 7 days): zero stale-guard fires globally, yet some users still report seeing the banner. Their app state is stuck from an earlier server hiccup that has long since resolved.
Behavior
Test plan