Repository navigation
fix(storage): stop a full localStorage from hanging the boot splash - #78
Conversation
On a localStorage at the WebView quota every setItem throws. The write in OnboardingProvider's effect was unguarded, so it threw during commit and took the tree down before heroReady could fire onReady: revealRoot() never ran and #harbor-boot stayed up forever over a hidden #root. The app looked frozen on the splash with no error, idle CPU and nothing on stderr. The recovery for this already existed but could never run: - proactiveStorageCleanup() was called from a useEffect in WebhookLoopMount, a component that only mounts after the render that dies. Call it at the top of mount() instead, before anything writes. - It only reached freeStorageSpace() when the prunable total passed 3.5 MB. A store filled mostly with keys we never prune stays under that while every write already throws, so it no-opped exactly when it was needed. Ask the store whether it still accepts a write instead. - The largest caches were not prunable at all. Add the refetchable ones: ebook OpenLibrary v2, ebook NYT, sports boards, MAL hero, anime art and hosted anime hero. Settings, profiles and installed add-ons stay untouched. OnboardingProvider now persists through setItemWithRecovery like the other callers, so a full store degrades instead of bricking the boot. Measured on a profile that reproduced the hang: cleanup takes localStorage from 3,159,696 to 1,093,786 chars and the app reaches Home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks for investigating and fixing the boot splash issue. Moving storage recovery earlier in startup and handling the onboarding write through 1. [P1] Storage recovery can delete persistent user data The new
Since Suggested fix: Separate disposable caches from persistent user data and exclude these keys from automatic eviction. It would also be better to stop pruning once enough space has been recovered rather than deleting every cache unnecessarily. Please add a regression test confirming that playback history and legacy settings survive storage recovery. Once this is addressed, the PR should be in good shape for merging. |
storageRejectsWrites() is what finally makes freeStorageSpace() run at boot, which turned a dormant bug into data loss on the first launch with a full store: the lists held playback history, the last season picked, live-TV watch counts and the legacy parental key, which still carries hiddenTabs and pinHash, so recovery could silently unlock parental controls. Those are out now, and the boot path frees only what it takes to make the store writable again instead of dropping every cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You're right, thanks for catching it. Those keys were already in the prunable lists before this PR - what I added was All three are out now, plus a fourth you didn't flag: I checked every remaining key against On stopping early: The regression test now seeds the user's records as the largest entries, so a wrong list hits them first, and asserts all four survive; a second test covers the early stop. Both fail if I put the respective bug back, I checked. |
|
Thanks for the follow-up. The previous findings have been addressed, and I agree with keeping However, I found one more issue with the remaining prunable keys. 1. [P1] Additional user-generated data is still included in storage cleanup In
There's also Suggested fix: Remove the manual EPG mappings and Discover preference records from automatic eviction, and extend the regression tests to verify that these records survive storage recovery. Checking The rest of the changes look good, including the additional cache entries and early-stop behavior. Once these remaining records are protected, the PR should be ready for merging. |
…he live caches Manual EPG matches come from the Live TV match dialog and Discover's affinity is built from up to 500 recorded interactions, so neither comes back from the network. The build rating goes too, and so do the anti-repeat lists for surprise-me and top picks: a few KB each, nothing to gain by risking them. The other half of the same list was pinning cache versions, so seven live caches had quietly stopped being prunable - jikancatalog2, armkitsucache.v3 and the rest of the anime mapping set, which are exactly the ones that grow. They are prefixes now, matched whatever the owner bumps them to. Ordering in the existing tests could leave a wrongly listed key untouched, so the list has its own guard over the full sweep.
|
Both confirmed and out, along with Two things worth more than the key list, though. My assertions for those keys passed with the bug still in place. Early-stop frees the largest cache and never reaches the rest, so ordering was deciding the outcome, not the list. There's now an order-independent guard over the full sweep, and I checked it fails for each re-listed key individually. And going back through every remaining key turned up the opposite problem. Seven live caches weren't prunable at all, because the list pinned the version before the current one: harbor.jikancatalog → harbor.jikancatalog2 Plus One path neither of us looked at: Three keys in the list have no writer left anywhere ( |
comet776
left a comment
There was a problem hiding this comment.
Reviewed head 4dcb55d0 against beta-branch. The core fix is sound: cleanup now runs before createRoot, onboarding writes through setItemWithRecovery, and the new quota tests exercise the real failure shape. I ran the new test file locally: 5/5 pass.
P1 — harbor.streamBadges.v1 is still in the prunable list, and it is user data
src/lib/storage-recovery.ts (line 29) keeps harbor.streamBadges.v1 in PRUNABLE_EXACT. That key is written by src/lib/stream-badges.ts and holds:
overrideswith user-uploaded badge images — data URLs up to 260 KB (kind-editor-modal.tsx);- custom badge rules the user created or imported — regex, colors, names, enabled state.
None of it is refetchable, and the import source URL is not stored with the rules. The key is not a cache.
This is the same class as playback-history, iptv.stats, and discover.v1: it was already in the list, but this PR is what makes it fire at boot. It is especially likely to be hit because freeUntilWritable() prunes largest-first and stops at the first successful probe. A 250 KB badge image can be the largest prunable entry, so it is deleted first, and the loop stops immediately.
I reproduced it against the PR's own quota stub: with streamBadges at 250 KB, mal.hero at 100 KB, and settings at 500 KB, cleanup logs pruned "harbor.streamBadges.v1" and leaves mal.hero intact. The total > 3.5 MB branch (freeStorageSpace) deletes it too. The full-sweep test does not seed it, so it passes.
Suggested fix:
- Remove
harbor.streamBadges.v1fromPRUNABLE_EXACT. - Add it at ~250 KB to the protected seeds in
tests/storage-quota-boot.test.tsand assert it survives both cleanup paths. - Re-audit the rest against "user-created config", not just history and settings.
P2 — the probe only guarantees room for one character
storageRejectsWrites() writes "1". freeUntilWritable() stops when that succeeds, not when the next boot write fits. A store full of many small prunable entries plus large user data can free only a few hundred bytes; the next settings/profile/hero-cache write can still throw. Onboarding is now guarded, so the original crash is fixed, but the comment's "free just enough to get going" is not guaranteed. Probe with a realistic payload (16–64 KB) or free to a fixed headroom, and add a test where the largest prunable entry is smaller than the follow-up write.
P2 — full cleanup now runs in every Tauri window
src/main.tsx:228 runs before the isPip / isModal / isCaptions / isDjDeck / isVideoPip / isHdrOverlay dispatch. Every auxiliary window loads index.html?harbor-* (pip.rs:93, modal_overlay.rs:75, captions.rs:79, dj_deck.rs:17, pip_window.rs:170, hdr_overlay.rs:65) and shares the same localStorage. Opening a modal or PIP can therefore call freeStorageSpace() and wipe more than 3.5 MB of caches the main window is using, causing mid-session refetches.
Previously this only ran in WebhookLoopMount inside the main App. Gate the proactive prune to the main window, or split "recover if unwritable" (safe everywhere) from "proactive prune" (main window only).
P2 — new entries repeat the version-pinning bug the PR fixes
The new comment says "Prefixes, not exact keys, for every cache whose owner version-stamps it", but the PR adds exact harbor.ebook.nyt.v1, harbor.mal.hero.v1, harbor.anime.art.v2, and harbor.anime.hero.hosted.v5. The version-bump test only covers the seven previously converted prefixes. On the next bump these silently stop being prunable.
Several prefixes also still bake in a version: harbor.sports.board.v3:, harbor.ebook.openlibrary.v1./v2., harbor.manga.cache.v1./v2.. Use version-agnostic prefixes and seed an unshipped version for each, the way the test already does with armkitsucache.v99.
P3 — cleanup is unguarded on the boot path
proactiveStorageCleanup() is now called before createRoot with no try/catch. A non-quota storage error (blocked or corrupt storage) would reject mount(); the catch only logs and never reveals the splash. Wrapping the call is cheap insurance.
Verification
- New
tests/storage-quota-boot.test.ts: 5/5 pass locally and in the CI log. - Typecheck passed in CI, and the new tests pass in the Frontend job.
- The Frontend job is still red on unrelated failures, e.g.
search-scroll-return.test.mjs(useInstallFeaturesmock) andstream-blocker.test.mjsmissingsrc-tauri/resources/harbor-stream-blocker/README.md. Native passed. Worth confirming thebeta-branchbaseline, but I do not see a diff-related failure. - The
parsed.nudgesformatter hunk is semantically equivalent; keep or drop. - The earlier P1s are addressed:
playback-history,iptv.stats,iptv.epgmap,discover.v1,lastseason,parental, andbuild.ratingare out of the list.
Verdict: request changes because of the streamBadges data loss. Once that key is protected and the two P2s are addressed, this is a good fix.
…ws out harbor.streamBadges.v1 holds badge images the user uploaded as data URLs, up to 260KB each, plus the rules they wrote - none of it refetchable, and largest-first pruning reaches for it early. Out of the list. The write probe asked for one character, so recovery could stop with a few hundred bytes free and the next settings or profile write still threw. It asks for 32KB now. Cleanup ran before the window dispatch in main.tsx, so opening a PIP, modal, captions, DJ or overlay window could evict caches the main window was using. Gated to the main window. PRUNABLE_EXACT is gone: one prefix list, nothing with a version stamp, since the exact/prefix split is what let the pinning happen. The boot call is wrapped too - a blocked or corrupt store rejecting mount() is the failure this is meant to prevent.
|
Thorough review, thanks - the window one especially, I'd missed it completely and it's my change that introduced it. Oopsie!
The probe is 32KB now rather than one character, sized like a real settings or profile write. The auxiliary windows are gated behind an On the version pinning: fair, and a bit embarrassing. I wrote "prefixes, not exact keys" and added four exact version-stamped keys in the same commit. Wrapped the boot call in try/catch. That one stung a little, since a blocked or corrupt store rejecting On CI: those are the branch baseline. I compared the failing set with and without the diff stashed and it's identical, 251 lines either way. Keeping the Each new test fails if I put the respective bug back, which I checked one at a time: Also built it and checked the window gate for real rather than by construction: the main window reports |
Preserve the one-time library-name repair marker and the OMDb daily request counter during quota recovery. Keep only rebuildable OMDb rating and miss caches prunable, and test both boot recovery and full-sweep eviction.
Exclude webhook delivery baselines and historical addon-star snapshots from automatic eviction. Add boot quota and full-sweep regressions guarding both records.
Summary
A localStorage that has reached the WebView quota makes Harbor hang forever on the boot splash. This fixes the crash path and makes the existing storage recovery actually reachable.
main.tsx— runproactiveStorageCleanup()at the top ofmount(), before anything writes.storage-recovery.ts— trigger the full prune when the store rejects a write, not only when the prunable total passes a size threshold; add the largest refetchable caches to the prunable lists.onboarding.tsx— persist throughsetItemWithRecovery, like the other callers, instead of a baresetItemduring commit.webhook-loop-mount.tsx— drop the now-redundant cleanup effect.Why
On a full store every
setItemthrows. The write inOnboardingProvider's effect was unguarded, and that provider wraps the whole app, so the throw landed during commit and tore the tree down beforeheroReadycould fireonReady.revealRoot()never ran, so#harbor-bootstayed up over avisibility:hidden#root: the app looks frozen on the splash with idle CPU, nothing on stderr, and no error surface, because the error screen React does render is hidden underneath.The recovery for this already existed but could never run:
proactiveStorageCleanup()was called from auseEffectinWebhookLoopMount, a component that only mounts after the render that dies.freeStorageSpace()abovePROACTIVE_TOTAL_THRESHOLD(3.5 MB of prunable data). A store filled mostly with keys we never prune stays under that while every write already throws, so it no-opped precisely when it was needed. Asking the store whether it still accepts a write is the condition that actually matters.harbor.ebook.openlibrary.v2.*,harbor.ebook.nyt.v1,harbor.sports.board.v3:*,harbor.mal.hero.v1,harbor.anime.art.v2,harbor.anime.hero.hosted.v5. All are refetchable derived caches.harbor.settings,harbor.profiles.v1,harbor.installed-addons.*andharbor.addonOrderBackupsare deliberately left alone.Measured on a profile that reproduced the hang: cleanup takes localStorage from 3,159,696 to 1,093,786 chars and the app reaches Home.
Verification
The new test fails on
beta-branchwithout thestorage-recovery.tschange and passes with it.Manual: with the real profile that reproduced the bug, the pre-fix build sits on the splash indefinitely (confirmed past 80 s, across both the system WebKitGTK 2.54.2 and a private 2.52.6); the post-fix build reaches Home in a few seconds. Diagnosis was done by tracing boot stages and
Storage.prototype.setItemfailures out of the packaged release build.No Rust changes, so no Cargo checks were run.
Platform Impact
Verified on Linux/Wayland (Hyprland, WebKitGTK 2.54.2). The cause is the WebView localStorage quota, which is not Linux-specific, so Windows and macOS are affected the same way and the fix is platform-neutral. Not retested on those, nor on TV-style input; no input, playback or navigation behavior is touched.
Notes
vp check --fixalso rewrote...(parsed.nudges ?? {})to...parsed.nudgesinonboarding.tsx— same semantics, and thevp stagedpre-commit hook reapplies it on any commit touching that file. Happy to drop that hunk if you prefer.Checklist
vp checkfor the changed files.vp run typecheckafter TypeScript changes.🤖 Generated with Claude Code