Skip to content

Commit c532d81

Browse files
authored
fix(core): make sure batching finishes if exception is thrown (TanStack#3216)
incrementing and decrementing transactions need to happen in an atomic operation, no matter what happens in between; if we increment, but never decrement, we will get to a state where transactions can never become zero - we're basically "leaking" an open transaction. This leads to no observers being informed ever again because we think we still have an open transaction this can be fixed by using try/finally to always decrement the transactions
1 parent f5a0cec commit c532d81

File tree

2 files changed

+23
-4
lines changed

2 files changed

+23
-4
lines changed

src/core/notifyManager.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,15 @@ export class NotifyManager {
3030
}
3131

3232
batch<T>(callback: () => T): T {
33+
let result
3334
this.transactions++
34-
const result = callback()
35-
this.transactions--
36-
if (!this.transactions) {
37-
this.flush()
35+
try {
36+
result = callback()
37+
} finally {
38+
this.transactions--
39+
if (!this.transactions) {
40+
this.flush()
41+
}
3842
}
3943
return result
4044
}

src/core/tests/notifyManager.test.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,19 @@ describe('notifyManager', () => {
2929
expect(callbackBatchLevel2Spy).toHaveBeenCalledTimes(1)
3030
expect(callbackScheduleSpy).toHaveBeenCalledTimes(1)
3131
})
32+
33+
it('should flush if error is thrown', () => {
34+
const notifyManagerTest = new NotifyManager()
35+
const flushSpy = jest.fn()
36+
37+
notifyManagerTest.flush = flushSpy
38+
39+
try {
40+
notifyManagerTest.batch(() => {
41+
throw new Error('Foo')
42+
})
43+
} catch {}
44+
45+
expect(flushSpy).toHaveBeenCalledTimes(1)
46+
})
3247
})

0 commit comments

Comments
 (0)