Skip to content

Commit fdf9d2d

Browse files
KyleAMathewsclaude
KyleAMathews
and
claude
authored
fix(offline-transactions): enforce strict FIFO when retries are pending (#1300)
* Fix KeyScheduler strict FIFO ordering on retries * Apply code simplification and expand test coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add changeset for KeyScheduler FIFO fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add test for identical createdAt timestamp ordering Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent be3921e commit fdf9d2d

File tree

4 files changed

+310
-22
lines changed

4 files changed

+310
-22
lines changed

‎.changeset/fix-keyscheduler-strict-fifo.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/offline-transactions': patch
3+
---
4+
5+
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.

‎packages/offline-transactions/src/executor/KeyScheduler.ts‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,27 @@ export class KeyScheduler {
2222
)
2323
}
2424

25-
getNextBatch(_maxConcurrency: number): Array<OfflineTransaction> {
25+
getNext(): OfflineTransaction | undefined {
2626
return withSyncSpan(
27-
`scheduler.getNextBatch`,
27+
`scheduler.getNext`,
2828
{ pendingCount: this.pendingTransactions.length },
2929
(span) => {
30-
// For sequential processing, we ignore maxConcurrency and only process one transaction at a time
3130
if (this.isRunning || this.pendingTransactions.length === 0) {
3231
span.setAttribute(`result`, `empty`)
33-
return []
32+
return undefined
3433
}
3534

36-
// Find the first transaction that's ready to run
37-
const readyTransaction = this.pendingTransactions.find((tx) =>
38-
this.isReadyToRun(tx),
39-
)
35+
const firstTransaction = this.pendingTransactions[0]!
4036

41-
if (readyTransaction) {
42-
span.setAttribute(`result`, `found`)
43-
span.setAttribute(`transaction.id`, readyTransaction.id)
44-
} else {
45-
span.setAttribute(`result`, `none_ready`)
37+
if (!this.isReadyToRun(firstTransaction)) {
38+
span.setAttribute(`result`, `waiting_for_first`)
39+
span.setAttribute(`transaction.id`, firstTransaction.id)
40+
return undefined
4641
}
4742

48-
return readyTransaction ? [readyTransaction] : []
43+
span.setAttribute(`result`, `found`)
44+
span.setAttribute(`transaction.id`, firstTransaction.id)
45+
return firstTransaction
4946
},
5047
)
5148
}

‎packages/offline-transactions/src/executor/TransactionExecutor.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,14 @@ export class TransactionExecutor {
5353
}
5454

5555
private async runExecution(): Promise<void> {
56-
const maxConcurrency = this.config.maxConcurrency ?? 3
57-
5856
while (this.scheduler.getPendingCount() > 0) {
59-
const batch = this.scheduler.getNextBatch(maxConcurrency)
57+
const transaction = this.scheduler.getNext()
6058

61-
if (batch.length === 0) {
59+
if (!transaction) {
6260
break
6361
}
6462

65-
const executions = batch.map((transaction) =>
66-
this.executeTransaction(transaction),
67-
)
68-
await Promise.allSettled(executions)
63+
await this.executeTransaction(transaction)
6964
}
7065

7166
// Schedule next retry after execution completes

‎packages/offline-transactions/tests/KeyScheduler.test.ts‎

Lines changed: 291 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,291 @@
1+
import { afterEach, describe, expect, it, vi } from 'vitest'
2+
import { KeyScheduler } from '../src/executor/KeyScheduler'
3+
import type { OfflineTransaction } from '../src/types'
4+
5+
function createTransaction({
6+
id,
7+
createdAt,
8+
nextAttemptAt,
9+
retryCount = 0,
10+
}: {
11+
id: string
12+
createdAt: Date
13+
nextAttemptAt: number
14+
retryCount?: number
15+
}): OfflineTransaction {
16+
return {
17+
id,
18+
mutationFnName: `syncData`,
19+
mutations: [],
20+
keys: [],
21+
idempotencyKey: `idempotency-${id}`,
22+
createdAt,
23+
retryCount,
24+
nextAttemptAt,
25+
metadata: {},
26+
version: 1,
27+
}
28+
}
29+
30+
describe(`KeyScheduler`, () => {
31+
afterEach(() => {
32+
vi.useRealTimers()
33+
})
34+
35+
it(`does not execute a later ready transaction while an earlier retry is pending`, () => {
36+
vi.useFakeTimers()
37+
38+
const now = new Date(`2026-01-01T00:00:00.000Z`)
39+
vi.setSystemTime(now)
40+
41+
const scheduler = new KeyScheduler()
42+
const first = createTransaction({
43+
id: `first`,
44+
createdAt: new Date(now.getTime()),
45+
nextAttemptAt: now.getTime(),
46+
})
47+
const second = createTransaction({
48+
id: `second`,
49+
createdAt: new Date(now.getTime() + 1),
50+
nextAttemptAt: now.getTime(),
51+
})
52+
53+
scheduler.schedule(first)
54+
scheduler.schedule(second)
55+
56+
const firstTx = scheduler.getNext()
57+
expect(firstTx?.id).toBe(`first`)
58+
59+
scheduler.markStarted(first)
60+
scheduler.markFailed(first)
61+
scheduler.updateTransaction({
62+
...first,
63+
retryCount: 1,
64+
nextAttemptAt: now.getTime() + 5000,
65+
lastError: {
66+
name: `Error`,
67+
message: `network timeout`,
68+
},
69+
})
70+
71+
const secondTx = scheduler.getNext()
72+
expect(secondTx).toBeUndefined()
73+
})
74+
75+
it(`executes the first transaction once its retry delay has elapsed`, () => {
76+
vi.useFakeTimers()
77+
78+
const now = new Date(`2026-01-01T00:00:00.000Z`)
79+
vi.setSystemTime(now)
80+
81+
const scheduler = new KeyScheduler()
82+
const first = createTransaction({
83+
id: `first`,
84+
createdAt: new Date(now.getTime()),
85+
nextAttemptAt: now.getTime() + 5000,
86+
retryCount: 1,
87+
})
88+
const second = createTransaction({
89+
id: `second`,
90+
createdAt: new Date(now.getTime() + 1),
91+
nextAttemptAt: now.getTime(),
92+
})
93+
94+
scheduler.schedule(first)
95+
scheduler.schedule(second)
96+
97+
expect(scheduler.getNext()).toBeUndefined()
98+
99+
vi.advanceTimersByTime(5000)
100+
101+
const result = scheduler.getNext()
102+
expect(result?.id).toBe(`first`)
103+
})
104+
105+
it(`processes the second transaction after the first completes`, () => {
106+
vi.useFakeTimers()
107+
108+
const now = new Date(`2026-01-01T00:00:00.000Z`)
109+
vi.setSystemTime(now)
110+
111+
const scheduler = new KeyScheduler()
112+
const first = createTransaction({
113+
id: `first`,
114+
createdAt: new Date(now.getTime()),
115+
nextAttemptAt: now.getTime(),
116+
})
117+
const second = createTransaction({
118+
id: `second`,
119+
createdAt: new Date(now.getTime() + 1),
120+
nextAttemptAt: now.getTime(),
121+
})
122+
123+
scheduler.schedule(first)
124+
scheduler.schedule(second)
125+
126+
const tx1 = scheduler.getNext()
127+
expect(tx1?.id).toBe(`first`)
128+
129+
scheduler.markStarted(first)
130+
scheduler.markCompleted(first)
131+
132+
const tx2 = scheduler.getNext()
133+
expect(tx2?.id).toBe(`second`)
134+
})
135+
136+
it(`returns nothing while a transaction is running`, () => {
137+
vi.useFakeTimers()
138+
139+
const now = new Date(`2026-01-01T00:00:00.000Z`)
140+
vi.setSystemTime(now)
141+
142+
const scheduler = new KeyScheduler()
143+
const tx = createTransaction({
144+
id: `tx1`,
145+
createdAt: new Date(now.getTime()),
146+
nextAttemptAt: now.getTime(),
147+
})
148+
149+
scheduler.schedule(tx)
150+
scheduler.markStarted(tx)
151+
152+
expect(scheduler.getNext()).toBeUndefined()
153+
})
154+
155+
it(`processes second transaction after first retries and succeeds`, () => {
156+
vi.useFakeTimers()
157+
158+
const now = new Date(`2026-01-01T00:00:00.000Z`)
159+
vi.setSystemTime(now)
160+
161+
const scheduler = new KeyScheduler()
162+
const first = createTransaction({
163+
id: `first`,
164+
createdAt: new Date(now.getTime()),
165+
nextAttemptAt: now.getTime(),
166+
})
167+
const second = createTransaction({
168+
id: `second`,
169+
createdAt: new Date(now.getTime() + 1),
170+
nextAttemptAt: now.getTime(),
171+
})
172+
173+
scheduler.schedule(first)
174+
scheduler.schedule(second)
175+
176+
scheduler.markStarted(first)
177+
scheduler.markFailed(first)
178+
scheduler.updateTransaction({
179+
...first,
180+
retryCount: 1,
181+
nextAttemptAt: now.getTime() + 5000,
182+
})
183+
184+
expect(scheduler.getNext()).toBeUndefined()
185+
186+
vi.advanceTimersByTime(5000)
187+
188+
const retryTx = scheduler.getNext()
189+
expect(retryTx?.id).toBe(`first`)
190+
191+
scheduler.markStarted(retryTx!)
192+
scheduler.markCompleted(retryTx!)
193+
194+
const finalTx = scheduler.getNext()
195+
expect(finalTx?.id).toBe(`second`)
196+
})
197+
198+
it(`maintains FIFO order regardless of scheduling order`, () => {
199+
vi.useFakeTimers()
200+
201+
const now = new Date(`2026-01-01T00:00:00.000Z`)
202+
vi.setSystemTime(now)
203+
204+
const scheduler = new KeyScheduler()
205+
const older = createTransaction({
206+
id: `older`,
207+
createdAt: new Date(now.getTime()),
208+
nextAttemptAt: now.getTime(),
209+
})
210+
const newer = createTransaction({
211+
id: `newer`,
212+
createdAt: new Date(now.getTime() + 1),
213+
nextAttemptAt: now.getTime(),
214+
})
215+
216+
scheduler.schedule(newer)
217+
scheduler.schedule(older)
218+
219+
const next = scheduler.getNext()
220+
expect(next?.id).toBe(`older`)
221+
})
222+
223+
it(`preserves FIFO order after updateTransaction`, () => {
224+
vi.useFakeTimers()
225+
226+
const now = new Date(`2026-01-01T00:00:00.000Z`)
227+
vi.setSystemTime(now)
228+
229+
const scheduler = new KeyScheduler()
230+
const first = createTransaction({
231+
id: `first`,
232+
createdAt: new Date(now.getTime()),
233+
nextAttemptAt: now.getTime(),
234+
})
235+
const second = createTransaction({
236+
id: `second`,
237+
createdAt: new Date(now.getTime() + 1),
238+
nextAttemptAt: now.getTime(),
239+
})
240+
241+
scheduler.schedule(first)
242+
scheduler.schedule(second)
243+
244+
scheduler.updateTransaction({
245+
...first,
246+
retryCount: 1,
247+
nextAttemptAt: now.getTime() + 5000,
248+
})
249+
250+
vi.advanceTimersByTime(5000)
251+
252+
const next = scheduler.getNext()
253+
expect(next?.id).toBe(`first`)
254+
})
255+
256+
it(`returns undefined when no transactions are scheduled`, () => {
257+
const scheduler = new KeyScheduler()
258+
expect(scheduler.getNext()).toBeUndefined()
259+
})
260+
261+
it(`processes transactions with identical createdAt in scheduling order`, () => {
262+
vi.useFakeTimers()
263+
264+
const now = new Date(`2026-01-01T00:00:00.000Z`)
265+
vi.setSystemTime(now)
266+
267+
const scheduler = new KeyScheduler()
268+
const first = createTransaction({
269+
id: `first`,
270+
createdAt: new Date(now.getTime()),
271+
nextAttemptAt: now.getTime(),
272+
})
273+
const second = createTransaction({
274+
id: `second`,
275+
createdAt: new Date(now.getTime()),
276+
nextAttemptAt: now.getTime(),
277+
})
278+
279+
scheduler.schedule(first)
280+
scheduler.schedule(second)
281+
282+
const tx1 = scheduler.getNext()
283+
expect(tx1?.id).toBe(`first`)
284+
285+
scheduler.markStarted(first)
286+
scheduler.markCompleted(first)
287+
288+
const tx2 = scheduler.getNext()
289+
expect(tx2?.id).toBe(`second`)
290+
})
291+
})

0 commit comments

Comments
 (0)