useLocalStorage swallows write errors, risking silent data loss
In src/hooks/useLocalStorage.ts, the set function calls localStorage.setItem(key, JSON.stringify(s)) with no try/catch, while the initial read is wrapped in one.
In private browsing/quota-exceeded scenarios (used by useContacts and useTheme), this throws uncaught, updates React state without persisting it, and can bubble into ErrorBoundary, wiping the in-progress UI. Wrap the write in a try/catch and surface a toast/error state instead of throwing.
Additional Notes
Consumers at risk: grepping call sites confirms useLocalStorage backs at least useContacts (the address book), useTheme (dark-mode preference), and the wallet auto-refresh interval value referenced from WalletContext (see the new issue about the unvalidated stellarsend_refresh_interval value — that value is read directly via localStorage.getItem, not through this hook, but shares the same underlying storage-reliability problem). A write failure in useContacts is the most user-visible: a user adds a contact, sees it appear in local React state immediately (optimistic update), but on a quota-exceeded Safari private-browsing session the write throws, state is now out of sync with storage, and a page refresh loses the "saved" contact with no warning ever shown.
Implementation sketch: wrap localStorage.setItem in try/catch inside set; on failure, keep the in-memory state update (so the UI doesn't regress mid-session) but surface a non-blocking toast via the existing useToast hook ("Couldn't save — your changes may be lost on refresh"), and return a boolean/throw a typed error from set so callers like useContacts.addContact can decide whether to roll back the optimistic update.
Edge cases: QuotaExceededError (Safari private mode, or simply a full 5-10MB quota) vs. a SecurityError (storage fully disabled by browser policy/extension) — these may warrant different messaging (retry vs. "storage unavailable, contact won't persist this session"); JSON circular-reference errors from JSON.stringify if a caller ever stores a non-serializable value — same catch block handles it but the resulting toast message should not claim "storage full" for what's actually a caller bug.
Testing strategy: mock Storage.prototype.setItem to throw QuotaExceededError and assert the hook does not throw synchronously, state still updates, and (once useToast integration exists) the toast function is called; add a regression test for the already-correct read-path try/catch to lock in that behavior too, since it's easy to accidentally regress both paths together in a future refactor.
Cross-references: shares its "unvalidated/unsafe localStorage access" theme with the new issue about WalletContext's unguarded parseInt on stellarsend_refresh_interval — worth considering a single small safeLocalStorage wrapper module used by both fixes instead of two independent patches.
useLocalStorage swallows write errors, risking silent data loss
In
src/hooks/useLocalStorage.ts, thesetfunction callslocalStorage.setItem(key, JSON.stringify(s))with no try/catch, while the initial read is wrapped in one.In private browsing/quota-exceeded scenarios (used by
useContactsanduseTheme), this throws uncaught, updates React state without persisting it, and can bubble intoErrorBoundary, wiping the in-progress UI. Wrap the write in a try/catch and surface a toast/error state instead of throwing.Additional Notes
Consumers at risk: grepping call sites confirms
useLocalStoragebacks at leastuseContacts(the address book),useTheme(dark-mode preference), and the wallet auto-refresh interval value referenced fromWalletContext(see the new issue about the unvalidatedstellarsend_refresh_intervalvalue — that value is read directly vialocalStorage.getItem, not through this hook, but shares the same underlying storage-reliability problem). A write failure inuseContactsis the most user-visible: a user adds a contact, sees it appear in local React state immediately (optimistic update), but on a quota-exceeded Safari private-browsing session the write throws, state is now out of sync with storage, and a page refresh loses the "saved" contact with no warning ever shown.Implementation sketch: wrap
localStorage.setItemin try/catch insideset; on failure, keep the in-memory state update (so the UI doesn't regress mid-session) but surface a non-blocking toast via the existinguseToasthook ("Couldn't save — your changes may be lost on refresh"), and return a boolean/throw a typed error fromsetso callers likeuseContacts.addContactcan decide whether to roll back the optimistic update.Edge cases:
QuotaExceededError(Safari private mode, or simply a full 5-10MB quota) vs. aSecurityError(storage fully disabled by browser policy/extension) — these may warrant different messaging (retry vs. "storage unavailable, contact won't persist this session"); JSON circular-reference errors fromJSON.stringifyif a caller ever stores a non-serializable value — same catch block handles it but the resulting toast message should not claim "storage full" for what's actually a caller bug.Testing strategy: mock
Storage.prototype.setItemto throwQuotaExceededErrorand assert the hook does not throw synchronously, state still updates, and (onceuseToastintegration exists) the toast function is called; add a regression test for the already-correct read-path try/catch to lock in that behavior too, since it's easy to accidentally regress both paths together in a future refactor.Cross-references: shares its "unvalidated/unsafe localStorage access" theme with the new issue about
WalletContext's unguardedparseIntonstellarsend_refresh_interval— worth considering a single smallsafeLocalStoragewrapper module used by both fixes instead of two independent patches.