Skip to content

Commit

Permalink
fix: all Promise rejections must be Errors (#4843)
Browse files Browse the repository at this point in the history
* fix: make sure all promise rejections are Errors

because our default TError type is now Error

* fix: make sure all promise rejections are Errors

because our default TError type is now Error

* remove string promise rejection

I can't say I fully understand this code, but whatever we reject here does not land up in our reducer because we have already rejected with a CancelledError

the rejection here is just necessary to stop further pages from fetching after the AbortSignal has been consumed

* fix another test

* fix types

* fix createQuery tests
  • Loading branch information
TkDodo committed Jan 20, 2023
1 parent 63943c6 commit 8c373d2
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 117 deletions.
5 changes: 3 additions & 2 deletions packages/query-core/src/infiniteQueryBehavior.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ export function infiniteQueryBehavior<

// Get query function
const queryFn =
context.options.queryFn || (() => Promise.reject('Missing queryFn'))
context.options.queryFn ||
(() => Promise.reject(new Error('Missing queryFn')))

const buildNewPages = (
pages: unknown[],
Expand All @@ -66,7 +67,7 @@ export function infiniteQueryBehavior<
previous?: boolean,
): Promise<unknown[]> => {
if (cancelled) {
return Promise.reject('Cancelled')
return Promise.reject()
}

if (typeof param === 'undefined' && !manual && pages.length) {
Expand Down
2 changes: 1 addition & 1 deletion packages/query-core/src/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export class Mutation<
this.#retryer = createRetryer({
fn: () => {
if (!this.options.mutationFn) {
return Promise.reject('No mutationFn found')
return Promise.reject(new Error('No mutationFn found'))
}
return this.options.mutationFn(this.state.variables!)
},
Expand Down
2 changes: 1 addition & 1 deletion packages/query-core/src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ export class Query<
// Create fetch function
const fetchFn = () => {
if (!this.options.queryFn) {
return Promise.reject('Missing queryFn')
return Promise.reject(new Error('Missing queryFn'))
}
this.#abortSignalConsumed = false
return this.options.queryFn(
Expand Down
6 changes: 3 additions & 3 deletions packages/query-core/src/tests/hydration.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ describe('dehydration and rehydration', () => {

const serverAddTodo = jest
.fn()
.mockImplementation(() => Promise.reject('offline'))
.mockImplementation(() => Promise.reject(new Error('offline')))
const serverOnMutate = jest.fn().mockImplementation((variables) => {
const optimisticTodo = { id: 1, text: variables.text }
return { optimisticTodo }
Expand Down Expand Up @@ -424,7 +424,7 @@ describe('dehydration and rehydration', () => {

const serverAddTodo = jest
.fn()
.mockImplementation(() => Promise.reject('offline'))
.mockImplementation(() => Promise.reject(new Error('offline')))

const queryClient = createQueryClient()

Expand Down Expand Up @@ -453,7 +453,7 @@ describe('dehydration and rehydration', () => {

const serverAddTodo = jest
.fn()
.mockImplementation(() => Promise.reject('offline'))
.mockImplementation(() => Promise.reject(new Error('offline')))

const queryClient = createQueryClient()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('InfiniteQueryBehavior', () => {
await waitFor(() => {
return expect(observerResult).toMatchObject({
isError: true,
error: 'Missing queryFn',
error: new Error('Missing queryFn'),
})
})

Expand Down
11 changes: 8 additions & 3 deletions packages/query-core/src/tests/mutationCache.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@ describe('mutationCache', () => {
await executeMutation(testClient, {
mutationKey: key,
variables: 'vars',
mutationFn: () => Promise.reject('error'),
mutationFn: () => Promise.reject(new Error('error')),
onMutate: () => 'context',
})
} catch {}

const mutation = testCache.getAll()[0]
expect(onError).toHaveBeenCalledWith('error', 'vars', 'context', mutation)
expect(onError).toHaveBeenCalledWith(
new Error('error'),
'vars',
'context',
mutation,
)
})

test('should be awaited', async () => {
Expand All @@ -38,7 +43,7 @@ describe('mutationCache', () => {
await executeMutation(testClient, {
mutationKey: key,
variables: 'vars',
mutationFn: () => Promise.reject('error'),
mutationFn: () => Promise.reject(new Error('error')),
onError: async () => {
states.push(3)
await sleep(1)
Expand Down
10 changes: 5 additions & 5 deletions packages/query-core/src/tests/mutations.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ describe('mutations', () => {
const mutation = new MutationObserver(queryClient, {
mutationFn: async () => {
await sleep(20)
return Promise.reject('err')
return Promise.reject(new Error('err'))
},
onMutate: (text) => text,
variables: 'todo',
Expand Down Expand Up @@ -214,7 +214,7 @@ describe('mutations', () => {
data: undefined,
error: null,
failureCount: 1,
failureReason: 'err',
failureReason: new Error('err'),
isError: false,
isIdle: false,
isLoading: true,
Expand All @@ -231,9 +231,9 @@ describe('mutations', () => {
expect(states[3]).toEqual({
context: 'todo',
data: undefined,
error: 'err',
error: new Error('err'),
failureCount: 2,
failureReason: 'err',
failureReason: new Error('err'),
isError: true,
isIdle: false,
isLoading: false,
Expand Down Expand Up @@ -340,6 +340,6 @@ describe('mutations', () => {
} catch (err) {
error = err
}
expect(error).toEqual('No mutationFn found')
expect(error).toEqual(new Error('No mutationFn found'))
})
})
2 changes: 1 addition & 1 deletion packages/query-core/src/tests/query.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ describe('query', () => {

const unsubscribe = observer.subscribe(() => undefined)
await sleep(10)
expect(mockLogger.error).toHaveBeenCalledWith('Missing queryFn')
expect(mockLogger.error).toHaveBeenCalledWith(new Error('Missing queryFn'))

unsubscribe()
})
Expand Down
28 changes: 14 additions & 14 deletions packages/react-query/src/__tests__/useMutation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ describe('useMutation', () => {
const mutateFn = jest.fn<Promise<Value>, [value: Value]>()

mutateFn.mockImplementationOnce(() => {
return Promise.reject('Error test Jonas')
return Promise.reject(new Error('Error test Jonas'))
})

mutateFn.mockImplementation(async (value) => {
Expand All @@ -163,18 +163,16 @@ describe('useMutation', () => {
})

function Page() {
const { mutate, failureCount, failureReason, data, status } = useMutation<
Value,
string,
Value
>({ mutationFn: mutateFn })
const { mutate, failureCount, failureReason, data, status } = useMutation(
{ mutationFn: mutateFn },
)

return (
<div>
<h1>Data {data?.count}</h1>
<h2>Status {status}</h2>
<h2>Failed {failureCount} times</h2>
<h2>Failed because {failureReason ?? 'null'}</h2>
<h2>Failed because {failureReason?.message ?? 'null'}</h2>
<button onClick={() => mutate({ count: ++count })}>mutate</button>
</div>
)
Expand Down Expand Up @@ -318,7 +316,7 @@ describe('useMutation', () => {

function Page() {
const { mutateAsync } = useMutation({
mutationFn: async (_text: string) => Promise.reject('oops'),
mutationFn: async (_text: string) => Promise.reject(new Error('oops')),
onError: async () => {
callbacks.push('useMutation.onError')
},
Expand All @@ -339,7 +337,7 @@ describe('useMutation', () => {
},
})
} catch (error) {
callbacks.push(`mutateAsync.error:${error}`)
callbacks.push(`mutateAsync.error:${(error as Error).message}`)
}
}, 10)
}, [mutateAsync])
Expand Down Expand Up @@ -405,7 +403,7 @@ describe('useMutation', () => {
const { mutate } = useMutation({
mutationFn: (_text: string) => {
count++
return Promise.reject('oops')
return Promise.reject(new Error('oops'))
},
retry: 1,
retryDelay: 5,
Expand Down Expand Up @@ -594,7 +592,9 @@ describe('useMutation', () => {
mutationFn: async (_text: string) => {
await sleep(1)
count++
return count > 1 ? Promise.resolve('data') : Promise.reject('oops')
return count > 1
? Promise.resolve('data')
: Promise.reject(new Error('oops'))
},
retry: 1,
retryDelay: 5,
Expand Down Expand Up @@ -635,13 +635,13 @@ describe('useMutation', () => {
isLoading: true,
isPaused: false,
failureCount: 1,
failureReason: 'oops',
failureReason: new Error('oops'),
})
expect(states[3]).toMatchObject({
isLoading: true,
isPaused: true,
failureCount: 1,
failureReason: 'oops',
failureReason: new Error('oops'),
})

onlineMock.mockReturnValue(true)
Expand All @@ -654,7 +654,7 @@ describe('useMutation', () => {
isLoading: true,
isPaused: false,
failureCount: 1,
failureReason: 'oops',
failureReason: new Error('oops'),
})
expect(states[5]).toMatchObject({
isLoading: false,
Expand Down
Loading

0 comments on commit 8c373d2

Please sign in to comment.