Repository navigation
fix(desktop): guard localStorage collections against quota-exceeded rollback loops - #6038
Conversation
…ollback loops Reworked from PR #5965 by @snehendu098 (refs #5496). When localStorage is full, @tanstack/db's localStorageCollectionOptions rethrows QuotaExceededError on every write; the library rolls the optimistic write back, the live query re-emits, the queuing effect retries - an infinite loop (~40x/s) that pegs the renderer and survives restarts. withQuotaGuard wraps every localStorage-backed collection's storage so setItem absorbs quota failures: reclaim space, retry once, then drop the write instead of throwing. No throw means no rollback, so the loop cannot form at any write site. Changes from the original PR: - Reclaim is driven by the terminal GC's persisted-at index (24h pressure TTL, terminal-buffer-gc.ts) instead of registry membership plus an in-memory preserved-set that reset on relaunch and could delete restorable snapshots of not-yet-opened workspaces. - Key prefixes stay canonical in terminal-buffer-gc.ts; the shared/constants.ts copies and the registry additions (getRegisteredTerminalIds, preservedSnapshotTerminalIds) are dropped. - QUOTA_NOTICE_MODE ships as offer-reclaim: genuine exhaustion surfaces a toast with a working "Free up space" action instead of a silent console warning. Co-authored-by: snehendu098 <snehendu098@users.noreply.github.com>
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit b2a9e17 on Jul 29, 2026 3:24am UTC. |
📝 WalkthroughWalkthroughAdds terminal snapshot quota reclamation, a guarded localStorage write path with soft quota failures, and user notifications with optional reclaim actions. Multiple desktop collections now use the quota guard, with tests covering reclamation, retries, notifications, and persistence behavior. ChangesLocalStorage quota recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Collection as LocalStorage collection
participant Guard as withQuotaGuard
participant GC as reclaimTerminalStateForQuota
participant Notice as notifyQuotaExhausted
Collection->>Guard: Attempt setItem
Guard->>GC: Reclaim persisted terminal snapshots
GC-->>Guard: Return removed-key count
Guard->>Collection: Retry setItem
Guard->>Notice: Report failed persistence
Notice-->>Collection: Show quota notification
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withQuotaGuard.ts (1)
61-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider guarding
reclaim()/onPersistFailed()against throwing.The module's entire purpose is "no throw means no rollback." Today
reclaim(reclaimTerminalStateForQuota) andonPersistFailed(notifyQuotaExhausted) happen to swallow their own errors, but nothing here enforces that — if either handler throws in the future, the exception propagates straight out ofsetItemand reinstates the exact rollback/retry loop this file was built to eliminate.🛡️ Proposed defensive wrapping
setItem: (key, value) => { try { base.setItem(key, value); return; } catch (error) { if (!isQuotaExceeded(error)) throw error; - if (reclaim() === 0) { - onPersistFailed(key, error); - return; - } + let reclaimed = 0; + try { + reclaimed = reclaim(); + } catch {} + if (reclaimed === 0) { + try { + onPersistFailed(key, error); + } catch {} + return; + } } try { base.setItem(key, value); } catch (retryError) { if (!isQuotaExceeded(retryError)) throw retryError; - onPersistFailed(key, retryError); + try { + onPersistFailed(key, retryError); + } catch {} } },🤖 Prompt for 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. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withQuotaGuard.ts` around lines 61 - 78, Update the setItem method’s reclaim and onPersistFailed handling so exceptions from either callback are caught and do not escape setItem, preserving the no-throw behavior during both initial quota recovery and retry failure paths. Keep non-quota errors from base.setItem propagating as they do now.
🤖 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.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withQuotaGuard.ts`:
- Around line 61-78: Update the setItem method’s reclaim and onPersistFailed
handling so exceptions from either callback are caught and do not escape
setItem, preserving the no-throw behavior during both initial quota recovery and
retry failure paths. Keep non-quota errors from base.setItem propagating as they
do now.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f4a8995-bcbe-417c-ad9b-a7257ede0120
📒 Files selected for processing (7)
apps/desktop/src/renderer/lib/terminal/terminal-buffer-gc.test.tsapps/desktop/src/renderer/lib/terminal/terminal-buffer-gc.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/notifyQuotaExhausted.test.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/notifyQuotaExhausted.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withQuotaGuard.test.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withQuotaGuard.ts
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Post-merge CDP verification (merged
|
| Observation | Result |
|---|---|
| Reload into the exhausted store | boot completes, evals alive, macrotask RTT p50 5.0 ms (pre-fix: renderer pegged >110% CPU, CDP evals timed out) |
v2SidebarSections.insert() on the full store |
no throw, row stays in memory, disk write dropped (blob stays 0 bytes) — no rollback loop (RTT p50 4.2 ms during) |
| Guard telemetry | deduped console warning fired; "Storage is full" toast with "Free up space" action rendered (offer-reclaim mode live) |
| Recovery | filler removed → reload → same insert persists to disk (row id found in the blob); test row cleaned up |
Caveats, stated plainly: the toast's action button presence was verified but its click wasn't exercised live (toast auto-dismissed between steps; the handler clearAllTerminalState is unit-tested). The insert was driven through the real guarded collection via CDP rather than a UI click — the dev instance's sign-in is currently 500ing server-side (environmental, reproduces on empty storage and pre-dates these changes), so an authenticated UI journey wasn't available. The before-state (freeze loop) was reproduced on unguarded main in the 2026-07-28 session that motivated #5965's verification, not re-reproduced today.
Reworked from #5965 by @snehendu098 — thank you for the excellent diagnosis and the
withQuotaGuarddesign, both kept here. Refs #5496 (fixes the freeze mechanism; the growth contributors are tracked separately in SUPER-1686). Opened as a new PR because the original fork doesn't allow maintainer edits and the branch needed reworking on top of #6036.Problem (from #5965)
When localStorage is full,
@tanstack/db'slocalStorageCollectionOptionsre-serializes the whole collection on every mutation and rethrowsQuotaExceededError. The library rolls the optimistic write back → the live query re-emits → the effect that queued the write retries — a synchronous ~40x/s loop that pegs the renderer at >110% CPU and survives restarts because the full store is on disk.Fix
withQuotaGuard(unchanged from #5965) wraps every localStorage-backed collection'sstorage: on quota error it reclaims space, retries once, then returns without throwing. No throw → no rollback → the loop cannot form at any of the write sites. Non-quota errors still rethrow.Changes from #5965
terminal-buffer-persisted-atindex from fix(desktop): garbage-collect persisted terminal scrollback in localStorage #6036 (24h pressure TTL, unstamped included) instead of registry membership + an in-memorypreservedSnapshotTerminalIdsset. The in-memory set reset on relaunch, so a quota event shortly after boot could delete restorable snapshots of terminals in not-yet-opened workspaces. This also deletesselectOrphanedTerminalSnapshots,terminalSnapshotStorage, and the two registry additions outright (−4 files, −36 registry lines).terminal-buffer-gc.ts— theshared/constants.tscopies are dropped.QUOTA_NOTICE_MODEships asoffer-reclaiminstead ofsilent: genuine exhaustion surfaces a toast with a working "Free up space" action (clears all persisted scrollback viaclearAllTerminalState) rather than a console-only warning the user never sees.Testing
withQuotaGuardtests kept verbatim from fix(renderer): the infinite loop due to which the app ui stays frozen #5965 (quota absorb, retry-once, non-quota rethrow, reclaim-0 skip).notifyQuotaExhaustedtests kept (warn dedupe, mode branches, toast id reuse).terminal-buffer-gctests forreclaimTerminalStateForQuota(24h TTL, fresh kept, key counts, index hygiene — mutation-checked) andclearAllTerminalState.bun run typecheck --filter=desktopandbun run lintclean.Summary by cubic
Prevents renderer-freezing retry loops when
localStorageis full by guarding all@tanstack/react-dbcollections: on quota errors we reclaim space, retry once, then drop the write instead of throwing. Refs SUPER-1686 (fixes the freeze mechanism; growth contributors tracked separately).Bug Fixes
withQuotaGuardto alllocalStorage-backed collections; non-quota errors still throw, quota errors reclaim once and return without throwing.New Features
terminal-buffer-persisted-atindex with a 24h pressure TTL viareclaimTerminalStateForQuota(plusclearAllTerminalState).offer-reclaim: deduped warning and a toast with “Free up space” to clear saved terminal scrollback.Written for commit b2a9e17. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes