fix(query-sync-storage-persister): recover from throttle errors instead of permanently disabling persistence

[koreahghg]

🎯 Changes

createSyncStoragePersister's internal throttle() helper does not guard against the wrapped persistClient callback throwing:

timer = timeoutManager.setTimeout(() => {
  func(...params)
  timer = null   // never reached if func throws
}, wait)

If func throws — which happens whenever the retry option (including the built-in removeOldestQuery, which does [...persistedClient.clientState.mutations] and throws on malformed/undefined clientState) throws while trying to recover from a failed storage.setItem — timer is left non-null forever. Every subsequent persistClient() call then sees timer !== null and silently no-ops, permanently disabling persistence for the rest of the page session with no error surfaced anywhere.

The async counterpart (@tanstack/query-async-storage-persister's asyncThrottle) already guards against this with a try/catch — this brings the sync version in line.

• Wrapped the timer callback in try/catch/finally so timer is always reset, and the thrown error doesn't escape as an unhandled exception either (verified: without the catch, the error propagates out of the setTimeout callback and shows up as an unhandled rejection/exception).
• Added a regression test (storageIsFull.test.ts) that reproduces the stuck state.
• Added a changeset (patch).

Testing notes

I wasn't able to run nx-based commands (pnpm run test:pr / nx run @tanstack/query-sync-storage-persister:test:lib) in my local environment — this repo's config files rely on symlinks (per the note in CONTRIBUTING.md about symlink support), and on my Windows checkout they materialized as plain text files instead of real symlinks, which breaks the tsc --build step nx depends on for this package's dependency graph. This is an environment limitation, unrelated to the change itself.

As a substitute, I verified the fix by running the package's tests directly with vitest run inside packages/query-sync-storage-persister:

• With the fix: all tests pass, no unhandled errors, exit code 0.
• With only the src/index.ts fix reverted (test kept): the new test fails exactly as expected (stuck timer / unhandled exception), confirming the test actually catches the regression.

I'd appreciate it if a maintainer (or CI) could confirm with the full nx pipeline on a symlink-capable environment.

✅ Checklist

• I have followed the steps in the Contributing guide.
• I have tested this code locally with pnpm run test:pr. (see "Testing notes" above — ran the package's vitest suite directly instead, due to a local symlink/environment limitation)

🚀 Release Impact

• This change affects published code, and I have generated a changeset.
• This change is docs/CI/dev-only (no release).

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes the @tanstack/query-sync-storage-persister throttle mechanism so persistence errors no longer permanently disable future persist attempts. The timer callback now wraps the throttled function call in try/catch/finally, ensuring the timer is always reset. A test and changeset accompany the fix.

Changes

Throttle Error Recovery Fix

Layer / File(s)	Summary
Timer error handling and recovery test packages/query-sync-storage-persister/src/index.ts, packages/query-sync-storage-persister/src/__tests__/storageIsFull.test.ts, .changeset/sync-storage-persister-throttle-error-recovery.md	Wraps the throttle timer's function call in try/catch/finally so the internal timer is reset even if the function or retry handler throws; adds a test confirming persistence continues across subsequent calls; adds a patch changeset describing the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Persister as createSyncStoragePersister
  participant Timer as ThrottleTimer
  participant Storage

  Client->>Persister: persistClient()
  Persister->>Timer: invoke throttled func
  Timer->>Storage: setItem(...)
  Storage-->>Timer: throws error
  Timer->>Timer: catch error, finally reset timer to null
  Client->>Persister: persistClient() (next call)
  Persister->>Timer: invoke throttled func
  Timer->>Storage: setItem(...)

Loading

Poem

• A rabbit hopped through storage lanes,
• Found errors clogging up the drains.
• With try, catch, finally in tow,
• The timer resets and off we go.
• Persist again, no more delay— 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the main change to sync-storage persistence recovery.
Description check	✅ Passed	The description follows the template and includes changes, testing notes, checklist items, and release impact.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/query-sync-storage-persister/src/index.ts (1)

113-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider surfacing swallowed errors for observability.

The empty catch silently discards all errors from func, including repeated storage.setItem/retry failures. Since this mirrors the async persister's behavior it's likely intentional, but callers currently have no way to detect a persistently failing storage backend.

♻️ Optional: log the caught error

-        try {
-          func(...params)
-        } catch {
-          // Prevent an error thrown by `func` (e.g. a `retry` handler) from
-          // leaving `timer` stuck, which would silently disable all future
-          // persist attempts for the rest of the session.
-        } finally {
+        try {
+          func(...params)
+        } catch (error) {
+          // Prevent an error thrown by `func` (e.g. a `retry` handler) from
+          // leaving `timer` stuck, which would silently disable all future
+          // persist attempts for the rest of the session.
+          if (process.env.NODE_ENV !== 'production') {
+            console.error(error)
+          }
+        } finally {

🤖 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 `@packages/query-sync-storage-persister/src/index.ts` around lines 113 - 116,
The empty catch in the persister flow silently swallows errors from func, making
repeated storage.setItem/retry failures invisible. Update the catch in the
timer/reset logic in query-sync-storage-persister’s main persistence path to
optionally surface the caught error through the existing logger or a similar
observability hook, while still preserving the current behavior of clearing
timer and avoiding session-wide failure.

🤖 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 `@packages/query-sync-storage-persister/src/index.ts`:
- Around line 113-116: The empty catch in the persister flow silently swallows
errors from func, making repeated storage.setItem/retry failures invisible.
Update the catch in the timer/reset logic in query-sync-storage-persister’s main
persistence path to optionally surface the caught error through the existing
logger or a similar observability hook, while still preserving the current
behavior of clearing timer and avoiding session-wide failure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4aa8a398-bdb3-4ee1-862f-2887a382c96a

📥 Commits

Reviewing files that changed from the base of the PR and between be8f11b and 6f42708.

📒 Files selected for processing (3)

• .changeset/sync-storage-persister-throttle-error-recovery.md
• packages/query-sync-storage-persister/src/__tests__/storageIsFull.test.ts
• packages/query-sync-storage-persister/src/index.ts