Skip to content

Commit 9d36c04

Browse files
committed
fix(vue-query/useBaseQuery): prevent dual error propagation when 'suspense()' and error watcher both handle the same error
1 parent 8a59b2d commit 9d36c04

3 files changed

Lines changed: 164 additions & 21 deletions

File tree

packages/vue-query/src/__tests__/useInfiniteQuery.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
2+
import { getCurrentInstance } from 'vue-demi'
23
import { sleep } from '@tanstack/query-test-utils'
34
import { useInfiniteQuery } from '../useInfiniteQuery'
45
import { infiniteQueryOptions } from '../infiniteQueryOptions'
6+
import type { Mock } from 'vitest'
57

68
vi.mock('../useQueryClient')
9+
vi.mock('../useBaseQuery')
710

811
describe('useInfiniteQuery', () => {
912
beforeEach(() => {
@@ -76,4 +79,73 @@ describe('useInfiniteQuery', () => {
7679
})
7780
expect(status.value).toStrictEqual('success')
7881
})
82+
83+
describe('throwOnError', () => {
84+
test('should throw from error watcher when throwOnError is true and suspense is not used', async () => {
85+
const throwOnErrorFn = vi.fn().mockReturnValue(true)
86+
useInfiniteQuery({
87+
queryKey: ['infiniteThrowOnErrorWithoutSuspense'],
88+
queryFn: () =>
89+
sleep(10).then(() => Promise.reject(new Error('Some error'))),
90+
initialPageParam: 0,
91+
getNextPageParam: () => 12,
92+
retry: false,
93+
throwOnError: throwOnErrorFn,
94+
})
95+
96+
// Suppress the Unhandled Rejection caused by watcher throw in Vue 3
97+
const rejectionHandler = () => {}
98+
process.on('unhandledRejection', rejectionHandler)
99+
100+
await vi.advanceTimersByTimeAsync(10)
101+
102+
process.off('unhandledRejection', rejectionHandler)
103+
104+
// throwOnError is evaluated and throw is attempted (not suppressed by suspense)
105+
expect(throwOnErrorFn).toHaveBeenCalledTimes(1)
106+
expect(throwOnErrorFn).toHaveBeenCalledWith(
107+
Error('Some error'),
108+
expect.objectContaining({
109+
state: expect.objectContaining({ status: 'error' }),
110+
}),
111+
)
112+
})
113+
})
114+
115+
describe('suspense', () => {
116+
test('should not throw from error watcher when suspense is handling the error with throwOnError: true', async () => {
117+
const getCurrentInstanceSpy = getCurrentInstance as Mock
118+
getCurrentInstanceSpy.mockImplementation(() => ({ suspense: {} }))
119+
120+
const throwOnErrorFn = vi.fn().mockReturnValue(true)
121+
const query = useInfiniteQuery({
122+
queryKey: ['infiniteSuspenseThrowOnError'],
123+
queryFn: () =>
124+
sleep(10).then(() => Promise.reject(new Error('Some error'))),
125+
initialPageParam: 0,
126+
getNextPageParam: () => 12,
127+
retry: false,
128+
throwOnError: throwOnErrorFn,
129+
})
130+
131+
let rejectedError: unknown
132+
const promise = query.suspense().catch((error) => {
133+
rejectedError = error
134+
})
135+
136+
await vi.advanceTimersByTimeAsync(10)
137+
138+
await promise
139+
140+
expect(rejectedError).toBeInstanceOf(Error)
141+
expect((rejectedError as Error).message).toBe('Some error')
142+
// throwOnError is evaluated in both suspense() and the error watcher
143+
expect(throwOnErrorFn).toHaveBeenCalledTimes(2)
144+
// but the error watcher should not throw when suspense is active
145+
expect(query).toMatchObject({
146+
status: { value: 'error' },
147+
isError: { value: true },
148+
})
149+
})
150+
})
79151
})

packages/vue-query/src/__tests__/useQuery.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,34 @@ describe('useQuery', () => {
458458
}),
459459
)
460460
})
461+
462+
test('should throw from error watcher when throwOnError is true and suspense is not used', async () => {
463+
const throwOnErrorFn = vi.fn().mockReturnValue(true)
464+
useQuery({
465+
queryKey: ['throwOnErrorWithoutSuspense'],
466+
queryFn: () =>
467+
sleep(10).then(() => Promise.reject(new Error('Some error'))),
468+
retry: false,
469+
throwOnError: throwOnErrorFn,
470+
})
471+
472+
// Suppress the Unhandled Rejection caused by watcher throw in Vue 3
473+
const rejectionHandler = () => {}
474+
process.on('unhandledRejection', rejectionHandler)
475+
476+
await vi.advanceTimersByTimeAsync(10)
477+
478+
process.off('unhandledRejection', rejectionHandler)
479+
480+
// throwOnError is evaluated and throw is attempted (not suppressed by suspense)
481+
expect(throwOnErrorFn).toHaveBeenCalledTimes(1)
482+
expect(throwOnErrorFn).toHaveBeenCalledWith(
483+
Error('Some error'),
484+
expect.objectContaining({
485+
state: expect.objectContaining({ status: 'error' }),
486+
}),
487+
)
488+
})
461489
})
462490

463491
describe('suspense', () => {
@@ -569,5 +597,38 @@ describe('useQuery', () => {
569597
}),
570598
)
571599
})
600+
601+
test('should not throw from error watcher when suspense is handling the error with throwOnError: true', async () => {
602+
const getCurrentInstanceSpy = getCurrentInstance as Mock
603+
getCurrentInstanceSpy.mockImplementation(() => ({ suspense: {} }))
604+
605+
const throwOnErrorFn = vi.fn().mockReturnValue(true)
606+
const query = useQuery({
607+
queryKey: ['suspense6'],
608+
queryFn: () =>
609+
sleep(10).then(() => Promise.reject(new Error('Some error'))),
610+
retry: false,
611+
throwOnError: throwOnErrorFn,
612+
})
613+
614+
let rejectedError: unknown
615+
const promise = query.suspense().catch((error) => {
616+
rejectedError = error
617+
})
618+
619+
await vi.advanceTimersByTimeAsync(10)
620+
621+
await promise
622+
623+
expect(rejectedError).toBeInstanceOf(Error)
624+
expect((rejectedError as Error).message).toBe('Some error')
625+
// throwOnError is evaluated in both suspense() and the error watcher
626+
expect(throwOnErrorFn).toHaveBeenCalledTimes(2)
627+
// but the error watcher should not throw when suspense is active
628+
expect(query).toMatchObject({
629+
status: { value: 'error' },
630+
isError: { value: true },
631+
})
632+
})
572633
})
573634
})

packages/vue-query/src/useBaseQuery.ts

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ export function useBaseQuery<
149149
return state.refetch(...args)
150150
}
151151

152+
let isSuspenseFetching = false
153+
152154
const suspense = () => {
153155
return new Promise<QueryObserverResult<TData, TError>>(
154156
(resolve, reject) => {
@@ -164,20 +166,28 @@ export function useBaseQuery<
164166
)
165167
if (optimisticResult.isStale) {
166168
stopWatch()
169+
isSuspenseFetching = true
167170
observer
168171
.fetchOptimistic(defaultedOptions.value)
169-
.then(resolve, (error: TError) => {
170-
if (
171-
shouldThrowError(defaultedOptions.value.throwOnError, [
172-
error,
173-
observer.getCurrentQuery(),
174-
])
175-
) {
176-
reject(error)
177-
} else {
178-
resolve(observer.getCurrentResult())
179-
}
180-
})
172+
.then(
173+
(result) => {
174+
isSuspenseFetching = false
175+
resolve(result)
176+
},
177+
(error: TError) => {
178+
isSuspenseFetching = false
179+
if (
180+
shouldThrowError(defaultedOptions.value.throwOnError, [
181+
error,
182+
observer.getCurrentQuery(),
183+
])
184+
) {
185+
reject(error)
186+
} else {
187+
resolve(observer.getCurrentResult())
188+
}
189+
},
190+
)
181191
} else {
182192
stopWatch()
183193
resolve(optimisticResult)
@@ -196,15 +206,15 @@ export function useBaseQuery<
196206
watch(
197207
() => state.error,
198208
(error) => {
199-
if (
200-
state.isError &&
201-
!state.isFetching &&
202-
shouldThrowError(defaultedOptions.value.throwOnError, [
203-
error as TError,
204-
observer.getCurrentQuery(),
205-
])
206-
) {
207-
throw error
209+
if (state.isError && !state.isFetching) {
210+
const shouldThrow = shouldThrowError(
211+
defaultedOptions.value.throwOnError,
212+
[error as TError, observer.getCurrentQuery()],
213+
)
214+
215+
if (shouldThrow && !isSuspenseFetching) {
216+
throw error
217+
}
208218
}
209219
},
210220
)

0 commit comments

Comments
 (0)