From 08ffa503ddfc4a2a28a9d7cfae754797e440992d Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 25 Feb 2026 10:58:04 -0700 Subject: [PATCH 1/5] Fix KeyScheduler strict FIFO ordering on retries --- .../src/executor/KeyScheduler.ts | 23 ++-- .../tests/KeyScheduler.test.ts | 104 ++++++++++++++++++ 2 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 packages/offline-transactions/tests/KeyScheduler.test.ts diff --git a/packages/offline-transactions/src/executor/KeyScheduler.ts b/packages/offline-transactions/src/executor/KeyScheduler.ts index 1243db565..77a9b67ab 100644 --- a/packages/offline-transactions/src/executor/KeyScheduler.ts +++ b/packages/offline-transactions/src/executor/KeyScheduler.ts @@ -33,19 +33,22 @@ export class KeyScheduler { return [] } - // Find the first transaction that's ready to run - const readyTransaction = this.pendingTransactions.find((tx) => - this.isReadyToRun(tx), - ) + const firstTransaction = this.pendingTransactions[0] + + if (!firstTransaction) { + span.setAttribute(`result`, `empty`) + return [] + } - if (readyTransaction) { - span.setAttribute(`result`, `found`) - span.setAttribute(`transaction.id`, readyTransaction.id) - } else { - span.setAttribute(`result`, `none_ready`) + if (!this.isReadyToRun(firstTransaction)) { + span.setAttribute(`result`, `waiting_for_first`) + span.setAttribute(`transaction.id`, firstTransaction.id) + return [] } - return readyTransaction ? [readyTransaction] : [] + span.setAttribute(`result`, `found`) + span.setAttribute(`transaction.id`, firstTransaction.id) + return [firstTransaction] }, ) } diff --git a/packages/offline-transactions/tests/KeyScheduler.test.ts b/packages/offline-transactions/tests/KeyScheduler.test.ts new file mode 100644 index 000000000..984a49c4b --- /dev/null +++ b/packages/offline-transactions/tests/KeyScheduler.test.ts @@ -0,0 +1,104 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { KeyScheduler } from '../src/executor/KeyScheduler' +import type { OfflineTransaction } from '../src/types' + +function createTransaction({ + id, + createdAt, + nextAttemptAt, + retryCount = 0, +}: { + id: string + createdAt: Date + nextAttemptAt: number + retryCount?: number +}): OfflineTransaction { + return { + id, + mutationFnName: `syncData`, + mutations: [], + keys: [], + idempotencyKey: `idempotency-${id}`, + createdAt, + retryCount, + nextAttemptAt, + metadata: {}, + version: 1, + } +} + +describe(`KeyScheduler`, () => { + afterEach(() => { + vi.useRealTimers() + }) + + it(`does not execute a later ready transaction while an earlier retry is pending`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const first = createTransaction({ + id: `first`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + const second = createTransaction({ + id: `second`, + createdAt: new Date(now.getTime() + 1), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(first) + scheduler.schedule(second) + + const firstBatch = scheduler.getNextBatch(1) + expect(firstBatch.map((tx) => tx.id)).toEqual([`first`]) + + scheduler.markStarted(first) + scheduler.markFailed(first) + scheduler.updateTransaction({ + ...first, + retryCount: 1, + nextAttemptAt: now.getTime() + 5000, + lastError: { + name: `Error`, + message: `network timeout`, + }, + }) + + const secondBatch = scheduler.getNextBatch(1) + expect(secondBatch).toEqual([]) + }) + + it(`executes the first transaction once its retry delay has elapsed`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const first = createTransaction({ + id: `first`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime() + 5000, + retryCount: 1, + }) + const second = createTransaction({ + id: `second`, + createdAt: new Date(now.getTime() + 1), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(first) + scheduler.schedule(second) + + expect(scheduler.getNextBatch(1)).toEqual([]) + + vi.advanceTimersByTime(5000) + + const batch = scheduler.getNextBatch(1) + expect(batch.map((tx) => tx.id)).toEqual([`first`]) + }) +}) From c190184f2680ab5b7d4efeb7187e0d9cc7bc47a5 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 25 Feb 2026 11:18:11 -0700 Subject: [PATCH 2/5] Apply code simplification and expand test coverage Co-Authored-By: Claude Opus 4.6 --- .../src/executor/KeyScheduler.ts | 7 +- .../tests/KeyScheduler.test.ts | 156 ++++++++++++++++++ 2 files changed, 157 insertions(+), 6 deletions(-) diff --git a/packages/offline-transactions/src/executor/KeyScheduler.ts b/packages/offline-transactions/src/executor/KeyScheduler.ts index 77a9b67ab..9057be4e9 100644 --- a/packages/offline-transactions/src/executor/KeyScheduler.ts +++ b/packages/offline-transactions/src/executor/KeyScheduler.ts @@ -33,12 +33,7 @@ export class KeyScheduler { return [] } - const firstTransaction = this.pendingTransactions[0] - - if (!firstTransaction) { - span.setAttribute(`result`, `empty`) - return [] - } + const firstTransaction = this.pendingTransactions[0]! if (!this.isReadyToRun(firstTransaction)) { span.setAttribute(`result`, `waiting_for_first`) diff --git a/packages/offline-transactions/tests/KeyScheduler.test.ts b/packages/offline-transactions/tests/KeyScheduler.test.ts index 984a49c4b..c3e2f1067 100644 --- a/packages/offline-transactions/tests/KeyScheduler.test.ts +++ b/packages/offline-transactions/tests/KeyScheduler.test.ts @@ -101,4 +101,160 @@ describe(`KeyScheduler`, () => { const batch = scheduler.getNextBatch(1) expect(batch.map((tx) => tx.id)).toEqual([`first`]) }) + + it(`processes the second transaction after the first completes`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const first = createTransaction({ + id: `first`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + const second = createTransaction({ + id: `second`, + createdAt: new Date(now.getTime() + 1), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(first) + scheduler.schedule(second) + + const batch1 = scheduler.getNextBatch(1) + expect(batch1.map((tx) => tx.id)).toEqual([`first`]) + + scheduler.markStarted(first) + scheduler.markCompleted(first) + + const batch2 = scheduler.getNextBatch(1) + expect(batch2.map((tx) => tx.id)).toEqual([`second`]) + }) + + it(`returns empty batch while a transaction is running`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const tx = createTransaction({ + id: `tx1`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(tx) + scheduler.markStarted(tx) + + expect(scheduler.getNextBatch(1)).toEqual([]) + }) + + it(`processes second transaction after first retries and succeeds`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const first = createTransaction({ + id: `first`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + const second = createTransaction({ + id: `second`, + createdAt: new Date(now.getTime() + 1), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(first) + scheduler.schedule(second) + + scheduler.markStarted(first) + scheduler.markFailed(first) + scheduler.updateTransaction({ + ...first, + retryCount: 1, + nextAttemptAt: now.getTime() + 5000, + }) + + expect(scheduler.getNextBatch(1)).toEqual([]) + + vi.advanceTimersByTime(5000) + + const retryBatch = scheduler.getNextBatch(1) + expect(retryBatch.map((tx) => tx.id)).toEqual([`first`]) + + scheduler.markStarted(retryBatch[0]!) + scheduler.markCompleted(retryBatch[0]!) + + const finalBatch = scheduler.getNextBatch(1) + expect(finalBatch.map((tx) => tx.id)).toEqual([`second`]) + }) + + it(`maintains FIFO order regardless of scheduling order`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const older = createTransaction({ + id: `older`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + const newer = createTransaction({ + id: `newer`, + createdAt: new Date(now.getTime() + 1), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(newer) + scheduler.schedule(older) + + const batch = scheduler.getNextBatch(1) + expect(batch.map((tx) => tx.id)).toEqual([`older`]) + }) + + it(`preserves FIFO order after updateTransaction`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const first = createTransaction({ + id: `first`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + const second = createTransaction({ + id: `second`, + createdAt: new Date(now.getTime() + 1), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(first) + scheduler.schedule(second) + + scheduler.updateTransaction({ + ...first, + retryCount: 1, + nextAttemptAt: now.getTime() + 5000, + }) + + vi.advanceTimersByTime(5000) + + const batch = scheduler.getNextBatch(1) + expect(batch.map((tx) => tx.id)).toEqual([`first`]) + }) + + it(`returns empty batch when no transactions are scheduled`, () => { + const scheduler = new KeyScheduler() + expect(scheduler.getNextBatch(1)).toEqual([]) + }) }) From 3dbc965940e9a2703d61fa8ae3344eb0ca4a932f Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 25 Feb 2026 11:18:42 -0700 Subject: [PATCH 3/5] Add changeset for KeyScheduler FIFO fix Co-Authored-By: Claude Opus 4.6 --- .changeset/fix-keyscheduler-strict-fifo.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-keyscheduler-strict-fifo.md diff --git a/.changeset/fix-keyscheduler-strict-fifo.md b/.changeset/fix-keyscheduler-strict-fifo.md new file mode 100644 index 000000000..3d9d55068 --- /dev/null +++ b/.changeset/fix-keyscheduler-strict-fifo.md @@ -0,0 +1,5 @@ +--- +'@tanstack/offline-transactions': patch +--- + +Fix KeyScheduler to enforce strict FIFO ordering on retries. Previously, `getNextBatch` could skip past a transaction waiting for retry and execute a later one, causing dependent UPDATE-before-INSERT failures. From 9dce67929c4355424ec67c33e0d66b49a4dfa86b Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 25 Feb 2026 11:20:00 -0700 Subject: [PATCH 4/5] Add test for identical createdAt timestamp ordering Co-Authored-By: Claude Opus 4.6 --- .../tests/KeyScheduler.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/offline-transactions/tests/KeyScheduler.test.ts b/packages/offline-transactions/tests/KeyScheduler.test.ts index c3e2f1067..e3c18c2e6 100644 --- a/packages/offline-transactions/tests/KeyScheduler.test.ts +++ b/packages/offline-transactions/tests/KeyScheduler.test.ts @@ -257,4 +257,35 @@ describe(`KeyScheduler`, () => { const scheduler = new KeyScheduler() expect(scheduler.getNextBatch(1)).toEqual([]) }) + + it(`processes transactions with identical createdAt in scheduling order`, () => { + vi.useFakeTimers() + + const now = new Date(`2026-01-01T00:00:00.000Z`) + vi.setSystemTime(now) + + const scheduler = new KeyScheduler() + const first = createTransaction({ + id: `first`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + const second = createTransaction({ + id: `second`, + createdAt: new Date(now.getTime()), + nextAttemptAt: now.getTime(), + }) + + scheduler.schedule(first) + scheduler.schedule(second) + + const batch1 = scheduler.getNextBatch(1) + expect(batch1.map((tx) => tx.id)).toEqual([`first`]) + + scheduler.markStarted(first) + scheduler.markCompleted(first) + + const batch2 = scheduler.getNextBatch(1) + expect(batch2.map((tx) => tx.id)).toEqual([`second`]) + }) }) From af3e50a812f354de86dc78c6dbc8c3b9dd5d518c Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 26 Feb 2026 08:01:20 -0700 Subject: [PATCH 5/5] Refactor getNextBatch to getNext returning single transaction The KeyScheduler always returns at most one transaction, so the array return type was misleading. Changed to OfflineTransaction | undefined for a more precise API contract per review feedback. Co-Authored-By: Claude Opus 4.6 --- .changeset/fix-keyscheduler-strict-fifo.md | 2 +- .../src/executor/KeyScheduler.ts | 11 ++-- .../src/executor/TransactionExecutor.ts | 11 +--- .../tests/KeyScheduler.test.ts | 60 +++++++++---------- 4 files changed, 39 insertions(+), 45 deletions(-) diff --git a/.changeset/fix-keyscheduler-strict-fifo.md b/.changeset/fix-keyscheduler-strict-fifo.md index 3d9d55068..5e3992e9b 100644 --- a/.changeset/fix-keyscheduler-strict-fifo.md +++ b/.changeset/fix-keyscheduler-strict-fifo.md @@ -2,4 +2,4 @@ '@tanstack/offline-transactions': patch --- -Fix KeyScheduler to enforce strict FIFO ordering on retries. Previously, `getNextBatch` could skip past a transaction waiting for retry and execute a later one, causing dependent UPDATE-before-INSERT failures. +Fix KeyScheduler to enforce strict FIFO ordering on retries. Previously, the scheduler could skip past a transaction waiting for retry and execute a later one, causing dependent UPDATE-before-INSERT failures. Also refactored `getNextBatch` to `getNext` to reflect that it always returns at most one transaction. diff --git a/packages/offline-transactions/src/executor/KeyScheduler.ts b/packages/offline-transactions/src/executor/KeyScheduler.ts index 9057be4e9..da59b904b 100644 --- a/packages/offline-transactions/src/executor/KeyScheduler.ts +++ b/packages/offline-transactions/src/executor/KeyScheduler.ts @@ -22,15 +22,14 @@ export class KeyScheduler { ) } - getNextBatch(_maxConcurrency: number): Array { + getNext(): OfflineTransaction | undefined { return withSyncSpan( - `scheduler.getNextBatch`, + `scheduler.getNext`, { pendingCount: this.pendingTransactions.length }, (span) => { - // For sequential processing, we ignore maxConcurrency and only process one transaction at a time if (this.isRunning || this.pendingTransactions.length === 0) { span.setAttribute(`result`, `empty`) - return [] + return undefined } const firstTransaction = this.pendingTransactions[0]! @@ -38,12 +37,12 @@ export class KeyScheduler { if (!this.isReadyToRun(firstTransaction)) { span.setAttribute(`result`, `waiting_for_first`) span.setAttribute(`transaction.id`, firstTransaction.id) - return [] + return undefined } span.setAttribute(`result`, `found`) span.setAttribute(`transaction.id`, firstTransaction.id) - return [firstTransaction] + return firstTransaction }, ) } diff --git a/packages/offline-transactions/src/executor/TransactionExecutor.ts b/packages/offline-transactions/src/executor/TransactionExecutor.ts index 5853a243b..f3e2324c9 100644 --- a/packages/offline-transactions/src/executor/TransactionExecutor.ts +++ b/packages/offline-transactions/src/executor/TransactionExecutor.ts @@ -53,19 +53,14 @@ export class TransactionExecutor { } private async runExecution(): Promise { - const maxConcurrency = this.config.maxConcurrency ?? 3 - while (this.scheduler.getPendingCount() > 0) { - const batch = this.scheduler.getNextBatch(maxConcurrency) + const transaction = this.scheduler.getNext() - if (batch.length === 0) { + if (!transaction) { break } - const executions = batch.map((transaction) => - this.executeTransaction(transaction), - ) - await Promise.allSettled(executions) + await this.executeTransaction(transaction) } // Schedule next retry after execution completes diff --git a/packages/offline-transactions/tests/KeyScheduler.test.ts b/packages/offline-transactions/tests/KeyScheduler.test.ts index e3c18c2e6..d7226a0a0 100644 --- a/packages/offline-transactions/tests/KeyScheduler.test.ts +++ b/packages/offline-transactions/tests/KeyScheduler.test.ts @@ -53,8 +53,8 @@ describe(`KeyScheduler`, () => { scheduler.schedule(first) scheduler.schedule(second) - const firstBatch = scheduler.getNextBatch(1) - expect(firstBatch.map((tx) => tx.id)).toEqual([`first`]) + const firstTx = scheduler.getNext() + expect(firstTx?.id).toBe(`first`) scheduler.markStarted(first) scheduler.markFailed(first) @@ -68,8 +68,8 @@ describe(`KeyScheduler`, () => { }, }) - const secondBatch = scheduler.getNextBatch(1) - expect(secondBatch).toEqual([]) + const secondTx = scheduler.getNext() + expect(secondTx).toBeUndefined() }) it(`executes the first transaction once its retry delay has elapsed`, () => { @@ -94,12 +94,12 @@ describe(`KeyScheduler`, () => { scheduler.schedule(first) scheduler.schedule(second) - expect(scheduler.getNextBatch(1)).toEqual([]) + expect(scheduler.getNext()).toBeUndefined() vi.advanceTimersByTime(5000) - const batch = scheduler.getNextBatch(1) - expect(batch.map((tx) => tx.id)).toEqual([`first`]) + const result = scheduler.getNext() + expect(result?.id).toBe(`first`) }) it(`processes the second transaction after the first completes`, () => { @@ -123,17 +123,17 @@ describe(`KeyScheduler`, () => { scheduler.schedule(first) scheduler.schedule(second) - const batch1 = scheduler.getNextBatch(1) - expect(batch1.map((tx) => tx.id)).toEqual([`first`]) + const tx1 = scheduler.getNext() + expect(tx1?.id).toBe(`first`) scheduler.markStarted(first) scheduler.markCompleted(first) - const batch2 = scheduler.getNextBatch(1) - expect(batch2.map((tx) => tx.id)).toEqual([`second`]) + const tx2 = scheduler.getNext() + expect(tx2?.id).toBe(`second`) }) - it(`returns empty batch while a transaction is running`, () => { + it(`returns nothing while a transaction is running`, () => { vi.useFakeTimers() const now = new Date(`2026-01-01T00:00:00.000Z`) @@ -149,7 +149,7 @@ describe(`KeyScheduler`, () => { scheduler.schedule(tx) scheduler.markStarted(tx) - expect(scheduler.getNextBatch(1)).toEqual([]) + expect(scheduler.getNext()).toBeUndefined() }) it(`processes second transaction after first retries and succeeds`, () => { @@ -181,18 +181,18 @@ describe(`KeyScheduler`, () => { nextAttemptAt: now.getTime() + 5000, }) - expect(scheduler.getNextBatch(1)).toEqual([]) + expect(scheduler.getNext()).toBeUndefined() vi.advanceTimersByTime(5000) - const retryBatch = scheduler.getNextBatch(1) - expect(retryBatch.map((tx) => tx.id)).toEqual([`first`]) + const retryTx = scheduler.getNext() + expect(retryTx?.id).toBe(`first`) - scheduler.markStarted(retryBatch[0]!) - scheduler.markCompleted(retryBatch[0]!) + scheduler.markStarted(retryTx!) + scheduler.markCompleted(retryTx!) - const finalBatch = scheduler.getNextBatch(1) - expect(finalBatch.map((tx) => tx.id)).toEqual([`second`]) + const finalTx = scheduler.getNext() + expect(finalTx?.id).toBe(`second`) }) it(`maintains FIFO order regardless of scheduling order`, () => { @@ -216,8 +216,8 @@ describe(`KeyScheduler`, () => { scheduler.schedule(newer) scheduler.schedule(older) - const batch = scheduler.getNextBatch(1) - expect(batch.map((tx) => tx.id)).toEqual([`older`]) + const next = scheduler.getNext() + expect(next?.id).toBe(`older`) }) it(`preserves FIFO order after updateTransaction`, () => { @@ -249,13 +249,13 @@ describe(`KeyScheduler`, () => { vi.advanceTimersByTime(5000) - const batch = scheduler.getNextBatch(1) - expect(batch.map((tx) => tx.id)).toEqual([`first`]) + const next = scheduler.getNext() + expect(next?.id).toBe(`first`) }) - it(`returns empty batch when no transactions are scheduled`, () => { + it(`returns undefined when no transactions are scheduled`, () => { const scheduler = new KeyScheduler() - expect(scheduler.getNextBatch(1)).toEqual([]) + expect(scheduler.getNext()).toBeUndefined() }) it(`processes transactions with identical createdAt in scheduling order`, () => { @@ -279,13 +279,13 @@ describe(`KeyScheduler`, () => { scheduler.schedule(first) scheduler.schedule(second) - const batch1 = scheduler.getNextBatch(1) - expect(batch1.map((tx) => tx.id)).toEqual([`first`]) + const tx1 = scheduler.getNext() + expect(tx1?.id).toBe(`first`) scheduler.markStarted(first) scheduler.markCompleted(first) - const batch2 = scheduler.getNextBatch(1) - expect(batch2.map((tx) => tx.id)).toEqual([`second`]) + const tx2 = scheduler.getNext() + expect(tx2?.id).toBe(`second`) }) })