From 72f94f9d31597601423db97a22ca5b4e60a774d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Jan 2026 19:32:10 +0000 Subject: [PATCH 1/7] test: add bug reproduction for optimistic state not restored on page refresh Add a failing test that demonstrates the issue where offline transactions do not restore optimistic state to the collection when the page is refreshed while offline. Users have to manually handle this in beforeRetry by replaying all transactions into the collection. The test asserts the expected behavior (optimistic data should be present after page refresh) and currently fails, demonstrating the bug. --- .../tests/offline-e2e.test.ts | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/packages/offline-transactions/tests/offline-e2e.test.ts b/packages/offline-transactions/tests/offline-e2e.test.ts index b4f9442df..6fce21763 100644 --- a/packages/offline-transactions/tests/offline-e2e.test.ts +++ b/packages/offline-transactions/tests/offline-e2e.test.ts @@ -466,4 +466,129 @@ describe(`offline executor end-to-end`, () => { env.executor.dispose() }) + + it(`should restore optimistic state to collection on startup (BUG: currently does not)`, async () => { + // This test reproduces the bug where optimistic state is NOT restored + // to the collection when the page is refreshed while offline. + // + // The user expects: After page refresh, the collection should still show + // the optimistic data from pending transactions. + // + // What actually happens: The collection is empty until the transaction + // succeeds and syncs. + + const storage = new FakeStorageAdapter() + + // Create a promise that never resolves - simulates offline/stuck mutation + const mutationPromise = () => new Promise(() => {}) + + // First environment: Create a transaction that gets stuck (simulating offline) + const firstEnv = createTestOfflineEnvironment({ + storage, + mutationFn: async () => { + // This mutation will hang forever, simulating offline state + await mutationPromise() + throw new Error(`Should not reach here in first env`) + }, + }) + + await firstEnv.waitForLeader() + + const offlineTx = firstEnv.executor.createOfflineTransaction({ + mutationFnName: firstEnv.mutationFnName, + autoCommit: false, + }) + + // Apply optimistic update + offlineTx.mutate(() => { + firstEnv.collection.insert({ + id: `optimistic-item`, + value: `should-persist`, + completed: false, + updatedAt: new Date(), + }) + }) + + // Verify the optimistic update is visible in the collection + expect(firstEnv.collection.get(`optimistic-item`)?.value).toBe( + `should-persist`, + ) + + // Start commit - it will persist to outbox and start (but not complete) execution + offlineTx.commit() + + // Wait for the transaction to be persisted to outbox + await waitUntil(async () => { + const pendingEntries = await firstEnv.executor.peekOutbox() + return pendingEntries.length === 1 + }, 5000) + + // Verify it's in the outbox + const outboxEntries = await firstEnv.executor.peekOutbox() + expect(outboxEntries.length).toBe(1) + expect(outboxEntries[0].id).toBe(offlineTx.id) + + // Verify the mutation data is properly serialized + expect(outboxEntries[0].mutations.length).toBe(1) + expect(outboxEntries[0].mutations[0].type).toBe(`insert`) + + // Dispose first environment (simulating page refresh) + firstEnv.executor.dispose() + + // Create a new environment with a deferred mutation to control timing + let secondEnvResolveMutation: (() => void) | null = null + const secondEnvMutationPromise = () => + new Promise((resolve) => { + secondEnvResolveMutation = resolve + }) + + const secondEnv = createTestOfflineEnvironment({ + storage, + mutationFn: async (params) => { + // Wait for explicit resolution + await secondEnvMutationPromise() + const mutations = params.transaction.mutations as Array< + PendingMutation + > + secondEnv.applyMutations(mutations) + return { ok: true, mutations } + }, + }) + + await secondEnv.waitForLeader() + + // BUG: At this point, we should have restored the optimistic state + // to the collection, but we haven't. + // + // The transaction is loaded from storage, but the optimistic mutations + // are NOT applied to the collection. + // + // Expected behavior: collection should have the optimistic item + // Actual behavior: collection is empty (BUG) + + const restoredItem = secondEnv.collection.get(`optimistic-item`) + + // The optimistic state should be restored from persisted transactions + expect(restoredItem?.value).toBe(`should-persist`) + + // Verify the transaction IS still in the outbox (data was persisted correctly) + const secondEnvOutbox = await secondEnv.executor.peekOutbox() + expect(secondEnvOutbox.length).toBe(1) + expect(secondEnvOutbox[0].mutations[0].type).toBe(`insert`) + + // Now complete the mutation to verify the data eventually syncs + secondEnvResolveMutation!() + + await waitUntil(async () => { + const entries = await secondEnv.executor.peekOutbox() + return entries.length === 0 + }) + + // After sync completes, the item appears (but this is too late for offline UX) + expect(secondEnv.serverState.get(`optimistic-item`)?.value).toBe( + `should-persist`, + ) + + secondEnv.executor.dispose() + }) }) From aad8aea19ce52678986b69873a92025e1b6c7be3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Jan 2026 20:12:31 +0000 Subject: [PATCH 2/7] fix: restore optimistic state on page refresh while offline When the page is refreshed while offline with pending transactions, the optimistic state was not being restored to collections. Users had to manually replay transactions in `beforeRetry` to restore UI state. This fix: 1. In `loadPendingTransactions()`, creates restoration transactions that hold the deserialized mutations and registers them with the collection's state manager to display optimistic data immediately 2. Properly reconstructs the mutation `key` during deserialization using the collection's `getKeyFromItem()` method, which is needed for optimistic state lookup 3. Cleans up restoration transactions when the offline transaction completes or fails, allowing sync data to take over 4. Adds `waitForInit()` method to allow waiting for full initialization including pending transaction loading 5. Updates `loadAndReplayTransactions()` to not block on execution, so initialization completes as soon as optimistic state is restored --- .../src/OfflineExecutor.ts | 64 ++++++++++++++++++- .../src/executor/TransactionExecutor.ts | 55 ++++++++++++++++ .../src/outbox/TransactionSerializer.ts | 10 ++- .../tests/TransactionSerializer.test.ts | 1 + .../offline-transactions/tests/harness.ts | 10 ++- .../tests/offline-e2e.test.ts | 25 +++----- 6 files changed, 139 insertions(+), 26 deletions(-) diff --git a/packages/offline-transactions/src/OfflineExecutor.ts b/packages/offline-transactions/src/OfflineExecutor.ts index e515938c1..7254fe05f 100644 --- a/packages/offline-transactions/src/OfflineExecutor.ts +++ b/packages/offline-transactions/src/OfflineExecutor.ts @@ -69,6 +69,9 @@ export class OfflineExecutor { } > = new Map() + // Track restoration transactions for cleanup when offline transactions complete + private restorationTransactions: Map = new Map() + constructor(config: OfflineConfig) { this.config = config this.scheduler = new KeyScheduler() @@ -298,8 +301,14 @@ export class OfflineExecutor { } try { + // Load pending transactions and restore optimistic state await this.executor.loadPendingTransactions() - await this.executor.executeAll() + + // Start execution in the background - don't await to avoid blocking initialization + // The transactions will execute and complete asynchronously + this.executor.executeAll().catch((error) => { + console.warn(`Failed to execute transactions:`, error) + }) } catch (error) { console.warn(`Failed to load and replay transactions:`, error) } @@ -309,6 +318,14 @@ export class OfflineExecutor { return this.mode === `offline` && this.isLeaderState } + /** + * Wait for the executor to fully initialize. + * This ensures that pending transactions are loaded and optimistic state is restored. + */ + async waitForInit(): Promise { + return this.initPromise + } + createOfflineTransaction( options: CreateOfflineTransactionOptions, ): Transaction | OfflineTransactionAPI { @@ -441,6 +458,9 @@ export class OfflineExecutor { deferred.resolve(result) this.pendingTransactionPromises.delete(transactionId) } + + // Clean up the restoration transaction - the sync will provide authoritative data + this.cleanupRestorationTransaction(transactionId) } // Method for TransactionExecutor to signal failure @@ -450,6 +470,48 @@ export class OfflineExecutor { deferred.reject(error) this.pendingTransactionPromises.delete(transactionId) } + + // Clean up the restoration transaction and rollback optimistic state + this.cleanupRestorationTransaction(transactionId, true) + } + + // Method for TransactionExecutor to register restoration transactions + registerRestorationTransaction( + offlineTransactionId: string, + restorationTransaction: Transaction, + ): void { + this.restorationTransactions.set(offlineTransactionId, restorationTransaction) + } + + // Clean up restoration transaction when offline transaction completes or fails + private cleanupRestorationTransaction( + transactionId: string, + shouldRollback: boolean = false, + ): void { + const restorationTx = this.restorationTransactions.get(transactionId) + if (!restorationTx) return + + if (shouldRollback) { + // Rollback the restoration transaction to remove optimistic state + restorationTx.rollback() + } else { + // Mark as completed so recomputeOptimisticState removes it from consideration + // The actual data will come from the sync + restorationTx.setState(`completed`) + + // Manually remove from each collection's transaction map and recompute + const touchedCollections = new Set() + for (const mutation of restorationTx.mutations) { + const collectionId = mutation.collection.id + if (!touchedCollections.has(collectionId)) { + touchedCollections.add(collectionId) + mutation.collection._state.transactions.delete(restorationTx.id) + mutation.collection._state.recomputeOptimisticState(false) + } + } + } + + this.restorationTransactions.delete(transactionId) } async removeFromOutbox(id: string): Promise { diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index 441a87d88..97183f5ed 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -1,3 +1,4 @@ +import { createTransaction } from '@tanstack/db' import { DefaultRetryPolicy } from '../retry/RetryPolicy' import { NonRetriableError } from '../types' import { withNestedSpan } from '../telemetry/tracer' @@ -227,6 +228,10 @@ export class TransactionExecutor { this.scheduler.schedule(transaction) } + // Restore optimistic state for loaded transactions + // This ensures the UI shows the optimistic data while transactions are pending + this.restoreOptimisticState(filteredTransactions) + // Reset retry delays for all loaded transactions so they can run immediately this.resetRetryDelays() @@ -242,6 +247,56 @@ export class TransactionExecutor { } } + /** + * Restore optimistic state from loaded transactions + * Creates internal transactions to hold the mutations so the collection's + * state manager can show optimistic data while waiting for sync + */ + private restoreOptimisticState(transactions: Array): void { + for (const offlineTx of transactions) { + if (offlineTx.mutations.length === 0) continue + + // Create a restoration transaction that will never commit + // Its only purpose is to hold the mutations for optimistic state display + const restorationTx = createTransaction({ + id: offlineTx.id, + autoCommit: false, + mutationFn: async () => { + // This mutation function should never be called + // The real mutation is handled by the offline executor + }, + }) + + // Apply mutations to the restoration transaction + restorationTx.applyMutations(offlineTx.mutations) + + // Register with each affected collection's state manager + const touchedCollections = new Set() + for (const mutation of offlineTx.mutations) { + const collectionId = mutation.collection.id + if (!touchedCollections.has(collectionId)) { + touchedCollections.add(collectionId) + + // Register the transaction with the collection's state manager + mutation.collection._state.transactions.set( + restorationTx.id, + restorationTx, + ) + + // Recompute optimistic state to show the restored data + mutation.collection._state.recomputeOptimisticState(true) + } + } + + // Store the restoration transaction so we can clean it up when the + // offline transaction completes + this.offlineExecutor.registerRestorationTransaction( + offlineTx.id, + restorationTx, + ) + } + } + clear(): void { this.scheduler.clear() this.clearRetryTimer() diff --git a/packages/offline-transactions/src/outbox/TransactionSerializer.ts b/packages/offline-transactions/src/outbox/TransactionSerializer.ts index 67a62d41f..cbbea8ece 100644 --- a/packages/offline-transactions/src/outbox/TransactionSerializer.ts +++ b/packages/offline-transactions/src/outbox/TransactionSerializer.ts @@ -77,18 +77,24 @@ export class TransactionSerializer { throw new Error(`Collection with id ${data.collectionId} not found`) } + const modified = this.deserializeValue(data.modified) + + // Extract the key from the modified data using the collection's getKey function + // This is needed for optimistic state restoration to work correctly + const key = modified ? collection.getKeyFromItem(modified) : null + // Create a partial PendingMutation - we can't fully reconstruct it but // we provide what we can. The executor will need to handle the rest. return { globalKey: data.globalKey, type: data.type as any, - modified: this.deserializeValue(data.modified), + modified, original: this.deserializeValue(data.original), changes: this.deserializeValue(data.changes) ?? {}, collection, // These fields would need to be reconstructed by the executor mutationId: ``, // Will be regenerated - key: null, // Will be extracted from the data + key, metadata: undefined, syncMetadata: {}, optimistic: true, diff --git a/packages/offline-transactions/tests/TransactionSerializer.test.ts b/packages/offline-transactions/tests/TransactionSerializer.test.ts index 0ef83581a..c1ce716fe 100644 --- a/packages/offline-transactions/tests/TransactionSerializer.test.ts +++ b/packages/offline-transactions/tests/TransactionSerializer.test.ts @@ -6,6 +6,7 @@ import type { PendingMutation } from '@tanstack/db' describe(`TransactionSerializer`, () => { const mockCollection = { id: `test-collection`, + getKeyFromItem: (item: any) => item.id, } const createSerializer = () => { diff --git a/packages/offline-transactions/tests/harness.ts b/packages/offline-transactions/tests/harness.ts index 485f37e81..f12a026c6 100644 --- a/packages/offline-transactions/tests/harness.ts +++ b/packages/offline-transactions/tests/harness.ts @@ -248,12 +248,10 @@ export function createTestOfflineEnvironment( const executor = startOfflineExecutor(config) const waitForLeader = async () => { - const start = Date.now() - while (!executor.isOfflineEnabled) { - if (Date.now() - start > 1000) { - throw new Error(`Executor did not become leader within timeout`) - } - await new Promise((resolve) => setTimeout(resolve, 10)) + // Wait for full initialization including loading pending transactions + await executor.waitForInit() + if (!executor.isOfflineEnabled) { + throw new Error(`Executor did not become leader`) } } diff --git a/packages/offline-transactions/tests/offline-e2e.test.ts b/packages/offline-transactions/tests/offline-e2e.test.ts index 6fce21763..035753688 100644 --- a/packages/offline-transactions/tests/offline-e2e.test.ts +++ b/packages/offline-transactions/tests/offline-e2e.test.ts @@ -467,15 +467,12 @@ describe(`offline executor end-to-end`, () => { env.executor.dispose() }) - it(`should restore optimistic state to collection on startup (BUG: currently does not)`, async () => { - // This test reproduces the bug where optimistic state is NOT restored - // to the collection when the page is refreshed while offline. + it(`should restore optimistic state to collection on startup`, async () => { + // This test verifies that optimistic state IS restored to the collection + // when the page is refreshed while offline (e.g., with pending transactions). // - // The user expects: After page refresh, the collection should still show - // the optimistic data from pending transactions. - // - // What actually happens: The collection is empty until the transaction - // succeeds and syncs. + // After page refresh, the collection should still show the optimistic data + // from pending transactions, providing a seamless offline UX. const storage = new FakeStorageAdapter() @@ -557,15 +554,9 @@ describe(`offline executor end-to-end`, () => { await secondEnv.waitForLeader() - // BUG: At this point, we should have restored the optimistic state - // to the collection, but we haven't. - // - // The transaction is loaded from storage, but the optimistic mutations - // are NOT applied to the collection. - // - // Expected behavior: collection should have the optimistic item - // Actual behavior: collection is empty (BUG) - + // At this point, the optimistic state should be restored to the collection + // The transaction is loaded from storage, and the optimistic mutations + // are applied to the collection immediately const restoredItem = secondEnv.collection.get(`optimistic-item`) // The optimistic state should be restored from persisted transactions From 504da203b76b8135f1ea99fb42ef5b074710a992 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 20 Jan 2026 20:13:50 +0000 Subject: [PATCH 3/7] ci: apply automated fixes --- packages/offline-transactions/src/OfflineExecutor.ts | 5 ++++- .../offline-transactions/src/executor/TransactionExecutor.ts | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/offline-transactions/src/OfflineExecutor.ts b/packages/offline-transactions/src/OfflineExecutor.ts index 7254fe05f..cc5a58a7d 100644 --- a/packages/offline-transactions/src/OfflineExecutor.ts +++ b/packages/offline-transactions/src/OfflineExecutor.ts @@ -480,7 +480,10 @@ export class OfflineExecutor { offlineTransactionId: string, restorationTransaction: Transaction, ): void { - this.restorationTransactions.set(offlineTransactionId, restorationTransaction) + this.restorationTransactions.set( + offlineTransactionId, + restorationTransaction, + ) } // Clean up restoration transaction when offline transaction completes or fails diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index 97183f5ed..a6981b761 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -252,7 +252,9 @@ export class TransactionExecutor { * Creates internal transactions to hold the mutations so the collection's * state manager can show optimistic data while waiting for sync */ - private restoreOptimisticState(transactions: Array): void { + private restoreOptimisticState( + transactions: Array, + ): void { for (const offlineTx of transactions) { if (offlineTx.mutations.length === 0) continue From ecb401307227fb72bd52edc7503232bbecde0f54 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 20 Jan 2026 13:24:02 -0700 Subject: [PATCH 4/7] chore: add changeset for optimistic state restoration fix Co-Authored-By: Claude Opus 4.5 --- .changeset/restore-optimistic-state-offline.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/restore-optimistic-state-offline.md diff --git a/.changeset/restore-optimistic-state-offline.md b/.changeset/restore-optimistic-state-offline.md new file mode 100644 index 000000000..82976f7d7 --- /dev/null +++ b/.changeset/restore-optimistic-state-offline.md @@ -0,0 +1,5 @@ +--- +'@tanstack/offline-transactions': patch +--- + +Fix optimistic state not being restored to collections on page refresh while offline. Pending transactions are now automatically rehydrated from storage and their optimistic mutations applied to the UI immediately on startup, providing a seamless offline experience. From e59691aa8a91eca621b12950517843a05961221d Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 20 Jan 2026 13:24:24 -0700 Subject: [PATCH 5/7] refactor: simplify restoration transaction cleanup - Consolidate restorationTransactions.delete() to single point - Improve comments on restoration transaction purpose Co-Authored-By: Claude Opus 4.5 --- .../src/OfflineExecutor.ts | 42 ++++++++++--------- .../src/executor/TransactionExecutor.ts | 40 ++++++++---------- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/packages/offline-transactions/src/OfflineExecutor.ts b/packages/offline-transactions/src/OfflineExecutor.ts index cc5a58a7d..c2122b55c 100644 --- a/packages/offline-transactions/src/OfflineExecutor.ts +++ b/packages/offline-transactions/src/OfflineExecutor.ts @@ -486,35 +486,37 @@ export class OfflineExecutor { ) } - // Clean up restoration transaction when offline transaction completes or fails private cleanupRestorationTransaction( transactionId: string, - shouldRollback: boolean = false, + shouldRollback = false, ): void { const restorationTx = this.restorationTransactions.get(transactionId) - if (!restorationTx) return + if (!restorationTx) { + return + } + + this.restorationTransactions.delete(transactionId) if (shouldRollback) { - // Rollback the restoration transaction to remove optimistic state restorationTx.rollback() - } else { - // Mark as completed so recomputeOptimisticState removes it from consideration - // The actual data will come from the sync - restorationTx.setState(`completed`) - - // Manually remove from each collection's transaction map and recompute - const touchedCollections = new Set() - for (const mutation of restorationTx.mutations) { - const collectionId = mutation.collection.id - if (!touchedCollections.has(collectionId)) { - touchedCollections.add(collectionId) - mutation.collection._state.transactions.delete(restorationTx.id) - mutation.collection._state.recomputeOptimisticState(false) - } - } + return } - this.restorationTransactions.delete(transactionId) + // Mark as completed so recomputeOptimisticState removes it from consideration. + // The actual data will come from the sync. + restorationTx.setState(`completed`) + + // Remove from each collection's transaction map and recompute + const touchedCollections = new Set() + for (const mutation of restorationTx.mutations) { + const collectionId = mutation.collection.id + if (touchedCollections.has(collectionId)) { + continue + } + touchedCollections.add(collectionId) + mutation.collection._state.transactions.delete(restorationTx.id) + mutation.collection._state.recomputeOptimisticState(false) + } } async removeFromOutbox(id: string): Promise { diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index a6981b761..ddda8f5a9 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -248,50 +248,44 @@ export class TransactionExecutor { } /** - * Restore optimistic state from loaded transactions + * Restore optimistic state from loaded transactions. * Creates internal transactions to hold the mutations so the collection's - * state manager can show optimistic data while waiting for sync + * state manager can show optimistic data while waiting for sync. */ private restoreOptimisticState( transactions: Array, ): void { for (const offlineTx of transactions) { - if (offlineTx.mutations.length === 0) continue + if (offlineTx.mutations.length === 0) { + continue + } - // Create a restoration transaction that will never commit - // Its only purpose is to hold the mutations for optimistic state display + // Create a restoration transaction that holds mutations for optimistic state display. + // It will never commit - the real mutation is handled by the offline executor. const restorationTx = createTransaction({ id: offlineTx.id, autoCommit: false, - mutationFn: async () => { - // This mutation function should never be called - // The real mutation is handled by the offline executor - }, + mutationFn: async () => {}, }) - // Apply mutations to the restoration transaction restorationTx.applyMutations(offlineTx.mutations) // Register with each affected collection's state manager const touchedCollections = new Set() for (const mutation of offlineTx.mutations) { const collectionId = mutation.collection.id - if (!touchedCollections.has(collectionId)) { - touchedCollections.add(collectionId) - - // Register the transaction with the collection's state manager - mutation.collection._state.transactions.set( - restorationTx.id, - restorationTx, - ) - - // Recompute optimistic state to show the restored data - mutation.collection._state.recomputeOptimisticState(true) + if (touchedCollections.has(collectionId)) { + continue } + touchedCollections.add(collectionId) + + mutation.collection._state.transactions.set( + restorationTx.id, + restorationTx, + ) + mutation.collection._state.recomputeOptimisticState(true) } - // Store the restoration transaction so we can clean it up when the - // offline transaction completes this.offlineExecutor.registerRestorationTransaction( offlineTx.id, restorationTx, From cc5d8bacbc2b38e212cbb762c97e96101e1c8c8e Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 20 Jan 2026 13:34:24 -0700 Subject: [PATCH 6/7] fix: improve error handling and add rollback test - Add try-catch isolation in restoreOptimisticState to prevent one bad transaction from breaking all restoration - Add defensive null check for mutation.collection in cleanup methods - Add test for rollback of restored optimistic state on permanent failure Co-Authored-By: Claude Opus 4.5 --- .../src/OfflineExecutor.ts | 5 ++ .../src/executor/TransactionExecutor.ts | 60 ++++++++------ .../tests/offline-e2e.test.ts | 81 +++++++++++++++++++ 3 files changed, 122 insertions(+), 24 deletions(-) diff --git a/packages/offline-transactions/src/OfflineExecutor.ts b/packages/offline-transactions/src/OfflineExecutor.ts index c2122b55c..ccfb08c66 100644 --- a/packages/offline-transactions/src/OfflineExecutor.ts +++ b/packages/offline-transactions/src/OfflineExecutor.ts @@ -509,6 +509,11 @@ export class OfflineExecutor { // Remove from each collection's transaction map and recompute const touchedCollections = new Set() for (const mutation of restorationTx.mutations) { + // Defensive check for corrupted deserialized data + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (!mutation.collection) { + continue + } const collectionId = mutation.collection.id if (touchedCollections.has(collectionId)) { continue diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index ddda8f5a9..f2ee5f690 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -260,36 +260,48 @@ export class TransactionExecutor { continue } - // Create a restoration transaction that holds mutations for optimistic state display. - // It will never commit - the real mutation is handled by the offline executor. - const restorationTx = createTransaction({ - id: offlineTx.id, - autoCommit: false, - mutationFn: async () => {}, - }) - - restorationTx.applyMutations(offlineTx.mutations) + try { + // Create a restoration transaction that holds mutations for optimistic state display. + // It will never commit - the real mutation is handled by the offline executor. + const restorationTx = createTransaction({ + id: offlineTx.id, + autoCommit: false, + mutationFn: async () => {}, + }) + + restorationTx.applyMutations(offlineTx.mutations) + + // Register with each affected collection's state manager + const touchedCollections = new Set() + for (const mutation of offlineTx.mutations) { + // Defensive check for corrupted deserialized data + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (!mutation.collection) { + continue + } + const collectionId = mutation.collection.id + if (touchedCollections.has(collectionId)) { + continue + } + touchedCollections.add(collectionId) - // Register with each affected collection's state manager - const touchedCollections = new Set() - for (const mutation of offlineTx.mutations) { - const collectionId = mutation.collection.id - if (touchedCollections.has(collectionId)) { - continue + mutation.collection._state.transactions.set( + restorationTx.id, + restorationTx, + ) + mutation.collection._state.recomputeOptimisticState(true) } - touchedCollections.add(collectionId) - mutation.collection._state.transactions.set( - restorationTx.id, + this.offlineExecutor.registerRestorationTransaction( + offlineTx.id, restorationTx, ) - mutation.collection._state.recomputeOptimisticState(true) + } catch (error) { + console.warn( + `Failed to restore optimistic state for transaction ${offlineTx.id}:`, + error, + ) } - - this.offlineExecutor.registerRestorationTransaction( - offlineTx.id, - restorationTx, - ) } } diff --git a/packages/offline-transactions/tests/offline-e2e.test.ts b/packages/offline-transactions/tests/offline-e2e.test.ts index 035753688..290ef07ce 100644 --- a/packages/offline-transactions/tests/offline-e2e.test.ts +++ b/packages/offline-transactions/tests/offline-e2e.test.ts @@ -582,4 +582,85 @@ describe(`offline executor end-to-end`, () => { secondEnv.executor.dispose() }) + + it(`should rollback restored optimistic state on permanent failure after page refresh`, async () => { + // This test verifies that optimistic state restored from persisted transactions + // is properly rolled back when the transaction fails permanently (NonRetriableError). + // + // Scenario: User creates transaction offline, refreshes page, transaction fails permanently + // Expected: Optimistic state should be visible initially, then removed after failure + + const storage = new FakeStorageAdapter() + + // First environment: Create a transaction that gets stuck (simulating offline) + const mutationPromise = () => new Promise(() => {}) + + const firstEnv = createTestOfflineEnvironment({ + storage, + mutationFn: async () => { + await mutationPromise() + throw new Error(`Should not reach here in first env`) + }, + }) + + await firstEnv.waitForLeader() + + const offlineTx = firstEnv.executor.createOfflineTransaction({ + mutationFnName: firstEnv.mutationFnName, + autoCommit: false, + }) + + // Apply optimistic update + offlineTx.mutate(() => { + firstEnv.collection.insert({ + id: `ghost-item`, + value: `will-fail`, + completed: false, + updatedAt: new Date(), + }) + }) + + // Verify the optimistic update is visible + expect(firstEnv.collection.get(`ghost-item`)?.value).toBe(`will-fail`) + + // Start commit - it will persist to outbox + offlineTx.commit() + + // Wait for the transaction to be persisted + await waitUntil(async () => { + const pendingEntries = await firstEnv.executor.peekOutbox() + return pendingEntries.length === 1 + }, 5000) + + // Dispose first environment (simulating page refresh) + firstEnv.executor.dispose() + + // Create a new environment where the mutation fails permanently + const secondEnv = createTestOfflineEnvironment({ + storage, + mutationFn: () => { + throw new NonRetriableError(`Server rejected mutation permanently`) + }, + }) + + await secondEnv.waitForLeader() + + // Optimistic state should be restored immediately after initialization + expect(secondEnv.collection.get(`ghost-item`)?.value).toBe(`will-fail`) + + // Wait for the transaction to fail and be removed from outbox + await waitUntil(async () => { + const entries = await secondEnv.executor.peekOutbox() + return entries.length === 0 + }, 5000) + + // After permanent failure, the optimistic state should be rolled back + // The item should no longer exist in the collection + expect(secondEnv.collection.get(`ghost-item`)).toBeUndefined() + + // And it should not exist on the server either + expect(secondEnv.serverState.get(`ghost-item`)).toBeUndefined() + + secondEnv.executor.dispose() + }) }) From 1eda746745e9836a9305da4613c6bf3a1fe194f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Jan 2026 21:16:08 +0000 Subject: [PATCH 7/7] fix: prevent unhandled promise rejection from restoration transaction cleanup Add catch handler to restoration transaction's isPersisted promise to prevent unhandled rejection when rollback() is called during cleanup. The rollback calls reject(undefined) which would otherwise cause an unhandled rejection error. --- .../src/executor/TransactionExecutor.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index f2ee5f690..5853a243b 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -269,6 +269,13 @@ export class TransactionExecutor { mutationFn: async () => {}, }) + // Prevent unhandled promise rejection when cleanup calls rollback() + // We don't care about this promise - it's just for holding mutations + restorationTx.isPersisted.promise.catch(() => { + // Intentionally ignored - restoration transactions are cleaned up + // via cleanupRestorationTransaction, not through normal commit flow + }) + restorationTx.applyMutations(offlineTx.mutations) // Register with each affected collection's state manager