Add bounded write-retry for transient Firestore failures (#25)#28
Add bounded write-retry for transient Firestore failures (#25)#28TrystonPerry wants to merge 2 commits into
Conversation
…lures When a document or collection sync() fails, the subscription now automatically reschedules the write up to `maxWriteRetries` times (default 3) at `retryInterval` ms intervals before surfacing the error to React. This closes the silent data-durability gap where a single network blip would strand local edits indefinitely. Key changes: - New `maxWriteRetries` option on DocumentDefinition, CollectionDefinition, DocEntry, and ColEntry (default 3; set 0 to disable). - document.ts / collection.ts: on each catch, check the retry budget; if budget remains, schedule a write-retry timeout and return early (error stays undefined, isSynced stays false). Reset the budget and surface the error only after all retries are exhausted. - scheduleAutosave() cancels any pending write-retry timer and resets the counter so a fresh user edit always starts with a full budget. - stop() cancels the write-retry timer to prevent post-unmount writes. - handleSnapshot / handleMissingDocument clear the retry state when a write is confirmed by the listener. - firestate.ts forwards maxWriteRetries through buildDocumentDefinition and buildCollectionDefinition. - Four new integration tests pin the retry-before-error contract for both document and collection subscriptions. Closes #25 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNQAMEEK1nGotGSM1LipDx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 149656d078
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (writeRetryTimeout) { | ||
| clearTimeout(writeRetryTimeout) | ||
| writeRetryTimeout = null | ||
| } | ||
| writeRetryCount = 0 |
There was a problem hiding this comment.
This retry cancellation is in scheduleAutosave(), but that helper is also invoked from handleSnapshot() whenever localState is still pending. If a write fails, schedules writeRetryTimeout, and then any document snapshot arrives before retryInterval, this clears the pending retry and resets the budget; with autosave: 0 no replacement timer is scheduled, so the edit can remain unsynced with no surfaced error. Scope the retry reset to actual user mutations instead of every autosave scheduling path.
Useful? React with 👍 / 👎.
| if (writeRetryTimeout) { | ||
| clearTimeout(writeRetryTimeout) | ||
| writeRetryTimeout = null | ||
| } | ||
| writeRetryCount = 0 |
There was a problem hiding this comment.
This has the same issue for collections: handleSnapshot() calls scheduleAutosave() while pending local state exists, so an ordinary collection snapshot before the retry fires clears writeRetryTimeout and resets the retry count. In manual-sync configurations (autosave: 0), that leaves the failed batch with no retry and no error notification, so pending collection edits can stay stuck indefinitely.
Useful? React with 👍 / 👎.
handleSnapshot() calls scheduleAutosave() when localState is pending. The previous implementation reset writeRetryCount and cleared writeRetryTimeout unconditionally on every scheduleAutosave() call. This meant a Firestore listener snapshot arriving between a write failure and its retry would silently kill the retry — with autosave: 0 no replacement timer was scheduled, leaving the edit stranded with no error surfaced. Fix: add a `fromMutation` parameter (default false). Only the three user-mutation paths (updateState/setData/deleteDocument in document.ts; updateState/addDocument/removeDocument in collection.ts) pass true, resetting the retry budget. Snapshot-driven calls leave the budget intact. Adds a regression test that fires a snapshot between failure and retry with autosave: 0 and asserts the retry still fires. Addresses Codex review comments on PR #28. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNQAMEEK1nGotGSM1LipDx
2 participants
Summary
Fixes the silent data-durability gap described in #25: when a
sync()call fails, Firestate now automatically retries the write up tomaxWriteRetriestimes (default 3) before surfacing the error to React.onErrorcallback fires during the retry windowisSyncedstaysfalseandlocalStateis preserved throughout — the pending edit is never loststop()cancels the retry timer so unmounting never triggers a post-lifecycle writeCloses #25
Changes
src/types.tsmaxWriteRetries?: numbertoDocumentDefinitionandCollectionDefinitionsrc/firestate.tsmaxWriteRetriestoCommonEntryOptions, forwarded inbuildDocumentDefinitionandbuildCollectionDefinitionsrc/document.tssync()catch blocks;scheduleAutosaveresets counter;stop()cancels retry timer;handleSnapshot/handleMissingDocumentclear retry state on confirmed writesrc/collection.tssrc/firestate.integration.test.tsmaxWriteRetries: 0disables retry, new edit resets budget, collection batch also retriesREADME.mdmaxWriteRetriesin thedefineDocumentanddoc()examplesHow to verify
useDocumentoruseCollectionsubscription.localStateis pending.setDoc/updateDoc/batch.commit()rejects once or twice.erroris set immediately, the edit is stranded.errorstaysundefinedwhile retries fire atretryIntervalms; only aftermaxWriteRetriesconsecutive failures doeserrorsurface.Set
maxWriteRetries: 0on any definition to restore the previous fail-fast behavior (useful for permission errors that are never transient).Follow-ups / risks
retryIntervalsetting (default 5 s), shared with the listener-restart delay. A future PR could introduce a dedicatedwriteRetryIntervalif callers need independent tuning.retryInterval. The issue noted this was acceptable ("re-arming the autosave debounce").PERMISSION_DENIED) will still exhaust retries and surface aftermaxWriteRetries * retryIntervalms. Callers that want instant failure on permanent errors should setmaxWriteRetries: 0.🤖 Generated with Claude Code
https://claude.ai/code/session_01GNQAMEEK1nGotGSM1LipDx
Generated by Claude Code