Db issue 1122 #1125
Draft
Db issue 1122 #1125
+923
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These tests reproduce scenarios related to the bug described in Issue #1122 where SyncTransactionAlreadyCommittedError occurs after browser visibility changes in progressive mode. One failing test demonstrates a related bug: when there's a persisting (optimistic) transaction, committed sync transactions are not removed from pendingSyncedTransactions because commitPendingTransactions() skips processing when hasPersistingTransaction is true. Test scenarios covered: - Visibility resume after atomic swap - New changes after visibility resume - Duplicate messages during buffering phase - Visibility change during active sync - Move-out messages during atomic swap - Multiple rapid visibility changes - Up-to-date in separate batch after changes - Orphaned committed transactions - Sync messages while optimistic mutation is persisting (FAILING)
…ive mode This commit fixes two issues related to transaction state management in progressive mode that could contribute to Issue #1122: 1. Fix duplicate begin() calls during atomic swap: - During atomic swap, processMoveOutEvent was called with transactionStarted=false - But begin() was already called at the start of the atomic swap - If processMoveOutEvent had rows to delete, it would call begin() again - This created duplicate transactions, with only the last one being committed - Fix: Pass true to processMoveOutEvent during atomic swap 2. Fix transactionStarted reset order: - Previously, transactionStarted was reset to false AFTER commit() - If commit() threw an exception, transactionStarted would remain true - This could cause subsequent batches to skip begin() and try to use an already-committed or non-existent transaction - Fix: Reset transactionStarted BEFORE calling commit() Also updates tests to verify: - Sync messages work correctly while optimistic mutations are persisting - Subsequent sync messages don't throw SyncTransactionAlreadyCommittedError
Add comprehensive documentation of the bug analysis, root cause, and solution for the SyncTransactionAlreadyCommittedError issue in progressive mode.
|
Cursor Agent can help with this pull request. Just |
|
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: 0 B Total Size: 90.5 kB ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 3.47 kB ℹ️ View Unchanged
|
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Addresses Issue #1122, fixing
SyncTransactionAlreadyCommittedErrorin progressive sync mode after browser visibility changes.The bug stemmed from two main issues:
begin()calls:processMoveOutEventwas incorrectly callingbegin()again during atomic swap, leading to orphaned transactions. This is fixed by ensuringtransactionStartedis correctly passed astrue.transactionStartedflag: ThetransactionStartedflag was reset aftercommit(), risking a staletruestate ifcommit()failed. This is fixed by resetting the flag beforecommit().New comprehensive unit tests have been added to cover visibility resume scenarios and ensure the fix. A detailed bug report is also included.
✅ Checklist
pnpm test:pr.🚀 Release Impact