diff --git a/.changeset/fix-offline-retry-and-online-gating.md b/.changeset/fix-offline-retry-and-online-gating.md new file mode 100644 index 000000000..47f1f3953 --- /dev/null +++ b/.changeset/fix-offline-retry-and-online-gating.md @@ -0,0 +1,5 @@ +--- +'@tanstack/offline-transactions': patch +--- + +Fix retry behavior to not cap at 10 attempts and not burn retries while offline. Default retry policy now retries indefinitely with exponential backoff. Execution loop and retry timer check online status before attempting transactions. `OnlineDetector.isOnline()` is now a required method on the interface. `OfflineExecutor.notifyOnline()` has been removed — the online detector handles connectivity changes automatically. diff --git a/packages/offline-transactions/README.md b/packages/offline-transactions/README.md index 351480258..d73c1b20c 100644 --- a/packages/offline-transactions/README.md +++ b/packages/offline-transactions/README.md @@ -5,7 +5,7 @@ Offline-first transaction capabilities for TanStack DB that provides durable per ## Features - **Outbox Pattern**: Persist mutations before dispatch for zero data loss -- **Automatic Retry**: Exponential backoff with jitter for failed transactions +- **Automatic Retry**: Configurable retry behavior with exponential backoff + jitter by default - **Multi-tab Coordination**: Leader election ensures safe storage access - **FIFO Sequential Processing**: Transactions execute one at a time in creation order - **Flexible Storage**: IndexedDB with localStorage fallback @@ -129,6 +129,7 @@ interface OfflineConfig { beforeRetry?: (transactions: OfflineTransaction[]) => OfflineTransaction[] onUnknownMutationFn?: (name: string, tx: OfflineTransaction) => void onLeadershipChange?: (isLeader: boolean) => void + onlineDetector?: OnlineDetector } ``` @@ -144,7 +145,6 @@ interface OfflineConfig { - `waitForTransactionCompletion(id)` - Wait for a specific transaction to complete - `removeFromOutbox(id)` - Manually remove transaction from outbox - `peekOutbox()` - View all pending transactions -- `notifyOnline()` - Manually trigger retry execution - `dispose()` - Clean up resources ### Error Handling @@ -183,21 +183,6 @@ const executor = startOfflineExecutor({ }) ``` -### Custom Retry Policy - -```typescript -const executor = startOfflineExecutor({ - maxConcurrency: 5, - jitter: true, - beforeRetry: (transactions) => { - // Filter out old transactions - const cutoff = Date.now() - 24 * 60 * 60 * 1000 // 24 hours - return transactions.filter((tx) => tx.createdAt.getTime() > cutoff) - }, - // ... other config -}) -``` - ### Manual Transaction Control ```typescript diff --git a/packages/offline-transactions/src/OfflineExecutor.ts b/packages/offline-transactions/src/OfflineExecutor.ts index ccfb08c66..cc537b3a3 100644 --- a/packages/offline-transactions/src/OfflineExecutor.ts +++ b/packages/offline-transactions/src/OfflineExecutor.ts @@ -220,7 +220,6 @@ export class OfflineExecutor { this.unsubscribeOnline = this.onlineDetector.subscribe(() => { if (this.isOfflineEnabled && this.executor) { - // Reset retry delays so transactions can execute immediately when back online this.executor.resetRetryDelays() this.executor.executeAll().catch((error) => { console.warn( @@ -546,10 +545,6 @@ export class OfflineExecutor { this.executor.clear() } - notifyOnline(): void { - this.onlineDetector.notifyOnline() - } - getPendingCount(): number { if (!this.executor) { return 0 @@ -568,6 +563,10 @@ export class OfflineExecutor { return this.onlineDetector } + isOnline(): boolean { + return this.onlineDetector.isOnline() + } + dispose(): void { if (this.unsubscribeOnline) { this.unsubscribeOnline() diff --git a/packages/offline-transactions/src/connectivity/ReactNativeOnlineDetector.ts b/packages/offline-transactions/src/connectivity/ReactNativeOnlineDetector.ts index e969732df..d18f4dc7c 100644 --- a/packages/offline-transactions/src/connectivity/ReactNativeOnlineDetector.ts +++ b/packages/offline-transactions/src/connectivity/ReactNativeOnlineDetector.ts @@ -27,10 +27,19 @@ export class ReactNativeOnlineDetector implements OnlineDetector { this.isListening = true + if (typeof NetInfo.fetch === `function`) { + void NetInfo.fetch() + .then((state) => { + this.wasConnected = this.toConnectivityState(state) + }) + .catch(() => { + // Ignore initial fetch failures and rely on subscription updates. + }) + } + // Subscribe to network state changes this.netInfoUnsubscribe = NetInfo.addEventListener((state) => { - const isConnected = - state.isConnected === true && state.isInternetReachable !== false + const isConnected = this.toConnectivityState(state) // Only notify when transitioning to online if (isConnected && !this.wasConnected) { @@ -98,8 +107,19 @@ export class ReactNativeOnlineDetector implements OnlineDetector { this.notifyListeners() } + isOnline(): boolean { + return this.wasConnected + } + dispose(): void { this.stopListening() this.listeners.clear() } + + private toConnectivityState(state: { + isConnected: boolean | null + isInternetReachable: boolean | null + }): boolean { + return state.isConnected === true && state.isInternetReachable !== false + } } diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index f3e2324c9..71328443f 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -4,7 +4,11 @@ import { NonRetriableError } from '../types' import { withNestedSpan } from '../telemetry/tracer' import type { KeyScheduler } from './KeyScheduler' import type { OutboxManager } from '../outbox/OutboxManager' -import type { OfflineConfig, OfflineTransaction } from '../types' +import type { + OfflineConfig, + OfflineTransaction, + TransactionSignaler, +} from '../types' const HANDLED_EXECUTION_ERROR = Symbol(`HandledExecutionError`) @@ -15,19 +19,22 @@ export class TransactionExecutor { private retryPolicy: DefaultRetryPolicy private isExecuting = false private executionPromise: Promise | null = null - private offlineExecutor: any // Reference to OfflineExecutor for signaling + private offlineExecutor: TransactionSignaler private retryTimer: ReturnType | null = null constructor( scheduler: KeyScheduler, outbox: OutboxManager, config: OfflineConfig, - offlineExecutor: any, + offlineExecutor: TransactionSignaler, ) { this.scheduler = scheduler this.outbox = outbox this.config = config - this.retryPolicy = new DefaultRetryPolicy(10, config.jitter ?? true) + this.retryPolicy = new DefaultRetryPolicy( + Number.POSITIVE_INFINITY, + config.jitter ?? true, + ) this.offlineExecutor = offlineExecutor } @@ -54,6 +61,10 @@ export class TransactionExecutor { private async runExecution(): Promise { while (this.scheduler.getPendingCount() > 0) { + if (!this.isOnline()) { + break + } + const transaction = this.scheduler.getNext() if (!transaction) { @@ -178,7 +189,10 @@ export class TransactionExecutor { return } - const delay = this.retryPolicy.calculateDelay(transaction.retryCount) + const delay = Math.max( + 0, + this.retryPolicy.calculateDelay(transaction.retryCount), + ) const updatedTransaction: OfflineTransaction = { ...transaction, retryCount: transaction.retryCount + 1, @@ -320,6 +334,10 @@ export class TransactionExecutor { // Clear existing timer this.clearRetryTimer() + if (!this.isOnline()) { + return + } + // Find the earliest retry time among pending transactions const earliestRetryTime = this.getEarliestRetryTime() @@ -353,6 +371,10 @@ export class TransactionExecutor { } } + private isOnline(): boolean { + return this.offlineExecutor.isOnline() + } + getRunningCount(): number { return this.scheduler.getRunningCount() } diff --git a/packages/offline-transactions/src/retry/RetryPolicy.ts b/packages/offline-transactions/src/retry/RetryPolicy.ts index 9fb4f6bf5..420fe057d 100644 --- a/packages/offline-transactions/src/retry/RetryPolicy.ts +++ b/packages/offline-transactions/src/retry/RetryPolicy.ts @@ -6,7 +6,7 @@ export class DefaultRetryPolicy implements RetryPolicy { private backoffCalculator: BackoffCalculator private maxRetries: number - constructor(maxRetries = 10, jitter = true) { + constructor(maxRetries = Number.POSITIVE_INFINITY, jitter = true) { this.backoffCalculator = new BackoffCalculator(jitter) this.maxRetries = maxRetries } diff --git a/packages/offline-transactions/src/types.ts b/packages/offline-transactions/src/types.ts index e0b1f6df7..e18a287cb 100644 --- a/packages/offline-transactions/src/types.ts +++ b/packages/offline-transactions/src/types.ts @@ -2,6 +2,7 @@ import type { Collection, MutationFnParams, PendingMutation, + Transaction, } from '@tanstack/db' // Extended mutation function that includes idempotency key @@ -104,7 +105,7 @@ export interface OfflineConfig { /** * Custom online detector implementation. * Defaults to WebOnlineDetector for browser environments. - * Use ReactNativeOnlineDetector from '@tanstack/offline-transactions/react-native' for RN/Expo. + * The '@tanstack/offline-transactions/react-native' entry point uses ReactNativeOnlineDetector automatically. */ onlineDetector?: OnlineDetector } @@ -129,9 +130,20 @@ export interface LeaderElection { onLeadershipChange: (callback: (isLeader: boolean) => void) => () => void } +export interface TransactionSignaler { + resolveTransaction: (transactionId: string, result: any) => void + rejectTransaction: (transactionId: string, error: Error) => void + registerRestorationTransaction: ( + offlineTransactionId: string, + restorationTransaction: Transaction, + ) => void + isOnline: () => boolean +} + export interface OnlineDetector { subscribe: (callback: () => void) => () => void notifyOnline: () => void + isOnline: () => boolean dispose: () => void } diff --git a/packages/offline-transactions/tests/harness.ts b/packages/offline-transactions/tests/harness.ts index f12a026c6..b971a7670 100644 --- a/packages/offline-transactions/tests/harness.ts +++ b/packages/offline-transactions/tests/harness.ts @@ -228,6 +228,7 @@ export function createTestOfflineEnvironment( } const config: OfflineConfig = { + ...options.config, collections: { ...(options.config?.collections ?? {}), [collection.id]: collection, @@ -237,11 +238,6 @@ export function createTestOfflineEnvironment( [mutationFnName]: wrappedMutation, }, storage, - maxConcurrency: options.config?.maxConcurrency, - jitter: options.config?.jitter, - beforeRetry: options.config?.beforeRetry, - onUnknownMutationFn: options.config?.onUnknownMutationFn, - onLeadershipChange: options.config?.onLeadershipChange, leaderElection: options.config?.leaderElection ?? leader, } diff --git a/packages/offline-transactions/tests/offline-e2e.test.ts b/packages/offline-transactions/tests/offline-e2e.test.ts index 290ef07ce..981315c26 100644 --- a/packages/offline-transactions/tests/offline-e2e.test.ts +++ b/packages/offline-transactions/tests/offline-e2e.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it } from 'vitest' import { NonRetriableError } from '../src/types' +import { DefaultRetryPolicy } from '../src/retry/RetryPolicy' import { FakeStorageAdapter, createTestOfflineEnvironment } from './harness' import type { TestItem } from './harness' -import type { OfflineMutationFnParams } from '../src/types' +import type { OfflineMutationFnParams, OnlineDetector } from '../src/types' import type { PendingMutation } from '@tanstack/db' const flushMicrotasks = () => new Promise((resolve) => setTimeout(resolve, 0)) @@ -22,6 +23,45 @@ const waitUntil = async ( throw new Error(`Timed out waiting for condition`) } +class ManualOnlineDetector implements OnlineDetector { + private listeners = new Set<() => void>() + private online: boolean + + constructor(initialOnline: boolean) { + this.online = initialOnline + } + + subscribe(callback: () => void): () => void { + this.listeners.add(callback) + + return () => { + this.listeners.delete(callback) + } + } + + notifyOnline(): void { + for (const listener of this.listeners) { + listener() + } + } + + isOnline(): boolean { + return this.online + } + + setOnline(isOnline: boolean): void { + this.online = isOnline + + if (isOnline) { + this.notifyOnline() + } + } + + dispose(): void { + this.listeners.clear() + } +} + describe(`offline executor end-to-end`, () => { it(`resolves waiting promises for successful transactions`, async () => { const env = createTestOfflineEnvironment() @@ -112,7 +152,7 @@ describe(`offline executor end-to-end`, () => { // Now bring the system back online online = true - env.executor.notifyOnline() + env.executor.getOnlineDetector().notifyOnline() // Wait for the retry to succeed await waitUntil(() => env.mutationCalls.length >= 2) @@ -132,6 +172,58 @@ describe(`offline executor end-to-end`, () => { env.executor.dispose() }) + it(`retries beyond 10 attempts by default`, () => { + const policy = new DefaultRetryPolicy() + const error = new Error(`transient`) + + for (let i = 0; i < 50; i++) { + expect(policy.shouldRetry(error, i)).toBe(true) + } + + expect(policy.shouldRetry(new NonRetriableError(`permanent`), 0)).toBe( + false, + ) + }) + + it(`does not execute mutations while offline`, async () => { + const onlineDetector = new ManualOnlineDetector(false) + const env = createTestOfflineEnvironment({ + config: { + onlineDetector, + }, + }) + + await env.waitForLeader() + + const offlineTx = env.executor.createOfflineTransaction({ + mutationFnName: env.mutationFnName, + autoCommit: false, + }) + + offlineTx.mutate(() => { + env.collection.insert({ + id: `queued-while-offline`, + value: `queued`, + completed: false, + updatedAt: new Date(), + }) + }) + + const commitPromise = offlineTx.commit() + + await flushMicrotasks() + expect(env.mutationCalls).toHaveLength(0) + expect(await env.executor.peekOutbox()).toHaveLength(1) + + onlineDetector.setOnline(true) + + await expect(commitPromise).resolves.toBeDefined() + expect(env.mutationCalls).toHaveLength(1) + expect(await env.executor.peekOutbox()).toEqual([]) + + env.executor.dispose() + }) + it(`rejects waiting promises for permanent failures and rolls back optimistic state`, async () => { const error = new NonRetriableError(`permanent`) const env = createTestOfflineEnvironment({