Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/wide-camels-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@tanstack/vue-query': patch
---

fix(vue-query/useBaseQuery): prevent dual error propagation when 'suspense()' and error watcher both handle the same error

86 changes: 86 additions & 0 deletions packages/vue-query/src/__tests__/useInfiniteQuery.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { getCurrentInstance } from 'vue-demi'
import { queryKey, sleep } from '@tanstack/query-test-utils'
import { useInfiniteQuery } from '../useInfiniteQuery'
import { infiniteQueryOptions } from '../infiniteQueryOptions'
import type { Mock } from 'vitest'

vi.mock('../useQueryClient')
vi.mock('../useBaseQuery')

describe('useInfiniteQuery', () => {
beforeEach(() => {
Expand Down Expand Up @@ -78,4 +81,87 @@ describe('useInfiniteQuery', () => {
})
expect(status.value).toStrictEqual('success')
})

describe('throwOnError', () => {
it('should throw from error watcher when throwOnError is true and suspense is not used', async () => {
const throwOnErrorFn = vi.fn().mockReturnValue(true)
useInfiniteQuery({
queryKey: ['infiniteThrowOnErrorWithoutSuspense'],
queryFn: () =>
sleep(10).then(() => Promise.reject(new Error('Some error'))),
initialPageParam: 0,
getNextPageParam: () => 12,
retry: false,
throwOnError: throwOnErrorFn,
})

// Capture the Unhandled Rejection caused by the watcher throw in Vue 3.
const rejectionHandler = vi.fn()
process.on('unhandledRejection', rejectionHandler)

await vi.advanceTimersByTimeAsync(10)

process.off('unhandledRejection', rejectionHandler)

// throwOnError is evaluated and throw is attempted (not suppressed by suspense)
expect(throwOnErrorFn).toHaveBeenCalledTimes(1)
expect(throwOnErrorFn).toHaveBeenCalledWith(
Error('Some error'),
expect.objectContaining({
state: expect.objectContaining({ status: 'error' }),
}),
)
// The watcher rethrows, so an unhandled rejection is observed.
expect(rejectionHandler).toHaveBeenCalledTimes(1)
expect(rejectionHandler).toHaveBeenCalledWith(
Error('Some error'),
expect.anything(),
)
})
})

describe('suspense', () => {
it('should not throw from error watcher when suspense is handling the error with throwOnError: true', async () => {
const getCurrentInstanceSpy = getCurrentInstance as Mock
getCurrentInstanceSpy.mockImplementation(() => ({ suspense: {} }))

// Spy on unhandled rejections so we can assert the watcher does not rethrow.
const rejectionHandler = vi.fn()
process.on('unhandledRejection', rejectionHandler)

const throwOnErrorFn = vi.fn().mockReturnValue(true)
const query = useInfiniteQuery({
queryKey: ['infiniteSuspenseThrowOnError'],
queryFn: () =>
sleep(10).then(() => Promise.reject(new Error('Some error'))),
initialPageParam: 0,
getNextPageParam: () => 12,
retry: false,
throwOnError: throwOnErrorFn,
})

let rejectedError: unknown
const promise = query.suspense().catch((error) => {
rejectedError = error
})

await vi.advanceTimersByTimeAsync(10)

await promise

process.off('unhandledRejection', rejectionHandler)

expect(rejectedError).toBeInstanceOf(Error)
expect((rejectedError as Error).message).toBe('Some error')
// throwOnError is evaluated in both suspense() and the error watcher
expect(throwOnErrorFn).toHaveBeenCalledTimes(2)
// The error watcher must not rethrow when suspense is active, so no
// unhandled rejection should be observed.
expect(rejectionHandler).not.toHaveBeenCalled()
expect(query).toMatchObject({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually does not assert, what comment suggests.

Should there be a spy on unhandledRejection with assertion that it was not caled?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DamianOsipiuk You're right β€” the previous assertion did not match the comment's intent. In 9e2924c I added a vi.fn() spy on unhandledRejection and asserted not.toHaveBeenCalled() so the watcher's non-rethrow is directly verified.

status: { value: 'error' },
isError: { value: true },
})
})
})
})
75 changes: 75 additions & 0 deletions packages/vue-query/src/__tests__/useQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,40 @@ describe('useQuery', () => {
}),
)
})

it('should throw from error watcher when throwOnError is true and suspense is not used', async () => {
const throwOnErrorFn = vi.fn().mockReturnValue(true)
useQuery({
queryKey: ['throwOnErrorWithoutSuspense'],
queryFn: () =>
sleep(10).then(() => Promise.reject(new Error('Some error'))),
retry: false,
throwOnError: throwOnErrorFn,
})

// Capture the Unhandled Rejection caused by the watcher throw in Vue 3.
const rejectionHandler = vi.fn()
process.on('unhandledRejection', rejectionHandler)

await vi.advanceTimersByTimeAsync(10)

process.off('unhandledRejection', rejectionHandler)

// throwOnError is evaluated and throw is attempted (not suppressed by suspense)
expect(throwOnErrorFn).toHaveBeenCalledTimes(1)
expect(throwOnErrorFn).toHaveBeenCalledWith(
Error('Some error'),
expect.objectContaining({
state: expect.objectContaining({ status: 'error' }),
}),
)
// The watcher rethrows, so an unhandled rejection is observed.
expect(rejectionHandler).toHaveBeenCalledTimes(1)
expect(rejectionHandler).toHaveBeenCalledWith(
Error('Some error'),
expect.anything(),
)
})
})

describe('outside scope warning', () => {
Expand Down Expand Up @@ -613,5 +647,46 @@ describe('useQuery', () => {
}),
)
})

it('should not throw from error watcher when suspense is handling the error with throwOnError: true', async () => {
const getCurrentInstanceSpy = getCurrentInstance as Mock
getCurrentInstanceSpy.mockImplementation(() => ({ suspense: {} }))

// Spy on unhandled rejections so we can assert the watcher does not rethrow.
const rejectionHandler = vi.fn()
process.on('unhandledRejection', rejectionHandler)

const throwOnErrorFn = vi.fn().mockReturnValue(true)
const query = useQuery({
queryKey: ['suspense6'],
queryFn: () =>
sleep(10).then(() => Promise.reject(new Error('Some error'))),
retry: false,
throwOnError: throwOnErrorFn,
})

let rejectedError: unknown
const promise = query.suspense().catch((error) => {
rejectedError = error
})

await vi.advanceTimersByTimeAsync(10)

await promise

process.off('unhandledRejection', rejectionHandler)

expect(rejectedError).toBeInstanceOf(Error)
expect((rejectedError as Error).message).toBe('Some error')
// throwOnError is evaluated in both suspense() and the error watcher
expect(throwOnErrorFn).toHaveBeenCalledTimes(2)
// The error watcher must not rethrow when suspense is active, so no
// unhandled rejection should be observed.
expect(rejectionHandler).not.toHaveBeenCalled()
expect(query).toMatchObject({
status: { value: 'error' },
isError: { value: true },
})
})
})
})
34 changes: 21 additions & 13 deletions packages/vue-query/src/useBaseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ export function useBaseQuery<
return state.refetch(...args)
}

let suspenseFetchCount = 0

const suspense = () => {
return new Promise<QueryObserverResult<TData, TError>>(
(resolve, reject) => {
Expand All @@ -164,9 +166,14 @@ export function useBaseQuery<
)
if (optimisticResult.isStale) {
stopWatch()
observer
.fetchOptimistic(defaultedOptions.value)
.then(resolve, (error: TError) => {
suspenseFetchCount += 1
observer.fetchOptimistic(defaultedOptions.value).then(
(result) => {
suspenseFetchCount -= 1
resolve(result)
},
(error: TError) => {
suspenseFetchCount -= 1
if (
shouldThrowError(defaultedOptions.value.throwOnError, [
error,
Expand All @@ -177,7 +184,8 @@ export function useBaseQuery<
} else {
resolve(observer.getCurrentResult())
}
})
},
)
} else {
stopWatch()
resolve(optimisticResult)
Expand All @@ -196,15 +204,15 @@ export function useBaseQuery<
watch(
() => state.error,
(error) => {
if (
state.isError &&
!state.isFetching &&
shouldThrowError(defaultedOptions.value.throwOnError, [
error as TError,
observer.getCurrentQuery(),
])
) {
throw error
if (state.isError && !state.isFetching) {
const shouldThrow = shouldThrowError(
defaultedOptions.value.throwOnError,
[error as TError, observer.getCurrentQuery()],
)

if (shouldThrow && suspenseFetchCount === 0) {
throw error
}
}
},
)
Expand Down
Loading