From 66c5e2174e0246170c277700c0211b7930f1755c Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Mon, 22 Sep 2025 18:46:56 +0200 Subject: [PATCH 1/2] Ensure errors of useMutation callbacks onError and onSettled are reported asynchronously --- .../src/__tests__/mutations.test.tsx | 320 +++++++++++++++++- packages/query-core/src/mutation.ts | 20 +- .../src/__tests__/useMutation.test.tsx | 18 +- .../src/__tests__/useMutation.test.tsx | 18 +- 4 files changed, 368 insertions(+), 8 deletions(-) diff --git a/packages/query-core/src/__tests__/mutations.test.tsx b/packages/query-core/src/__tests__/mutations.test.tsx index cb7cd79d3a..8f2cf357b6 100644 --- a/packages/query-core/src/__tests__/mutations.test.tsx +++ b/packages/query-core/src/__tests__/mutations.test.tsx @@ -1,6 +1,6 @@ import { queryKey, sleep } from '@tanstack/query-test-utils' import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' -import { QueryClient } from '..' +import { MutationCache, QueryClient } from '..' import { MutationObserver } from '../mutationObserver' import { executeMutation } from './utils' import type { MutationState } from '../mutation' @@ -842,4 +842,322 @@ describe('mutations', () => { expect(mutationError).toEqual(newMutationError) }) }) + + describe('erroneous mutation callback', () => { + afterEach(() => { + process.removeAllListeners('unhandledRejection') + }) + + test('error by global onSuccess triggers onError callback', async () => { + const newMutationError = new Error('mutation-error') + + queryClient = new QueryClient({ + mutationCache: new MutationCache({ + onSuccess: () => { + throw newMutationError + }, + }), + }) + queryClient.mount() + + const key = queryKey() + const results: Array = [] + + let mutationError: Error | undefined + executeMutation( + queryClient, + { + mutationKey: key, + mutationFn: () => Promise.resolve('success'), + onMutate: async () => { + results.push('onMutate-async') + await sleep(10) + return { backup: 'async-data' } + }, + onSuccess: async () => { + results.push('onSuccess-async-start') + await sleep(10) + throw newMutationError + }, + onError: async () => { + results.push('onError-async-start') + await sleep(10) + results.push('onError-async-end') + }, + onSettled: () => { + results.push('onSettled-promise') + return Promise.resolve('also-ignored') // Promise (should be ignored) + }, + }, + 'vars', + ).catch((error) => { + mutationError = error + }) + + await vi.advanceTimersByTimeAsync(30) + + expect(results).toEqual([ + 'onMutate-async', + 'onError-async-start', + 'onError-async-end', + 'onSettled-promise', + ]) + + expect(mutationError).toEqual(newMutationError) + }) + + test('error by mutations onSuccess triggers onError callback', async () => { + const key = queryKey() + const results: Array = [] + + const newMutationError = new Error('mutation-error') + + let mutationError: Error | undefined + executeMutation( + queryClient, + { + mutationKey: key, + mutationFn: () => Promise.resolve('success'), + onMutate: async () => { + results.push('onMutate-async') + await sleep(10) + return { backup: 'async-data' } + }, + onSuccess: async () => { + results.push('onSuccess-async-start') + await sleep(10) + throw newMutationError + }, + onError: async () => { + results.push('onError-async-start') + await sleep(10) + results.push('onError-async-end') + }, + onSettled: () => { + results.push('onSettled-promise') + return Promise.resolve('also-ignored') // Promise (should be ignored) + }, + }, + 'vars', + ).catch((error) => { + mutationError = error + }) + + await vi.advanceTimersByTimeAsync(30) + + expect(results).toEqual([ + 'onMutate-async', + 'onSuccess-async-start', + 'onError-async-start', + 'onError-async-end', + 'onSettled-promise', + ]) + + expect(mutationError).toEqual(newMutationError) + }) + + test('error by global onSettled triggers onError callback, calling global onSettled callback twice', async () => { + const newMutationError = new Error('mutation-error') + + queryClient = new QueryClient({ + mutationCache: new MutationCache({ + onSettled: async () => { + results.push('global-onSettled') + await sleep(10) + throw newMutationError + }, + }), + }) + queryClient.mount() + + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + + const key = queryKey() + const results: Array = [] + + let mutationError: Error | undefined + executeMutation( + queryClient, + { + mutationKey: key, + mutationFn: () => Promise.resolve('success'), + onMutate: async () => { + results.push('onMutate-async') + await sleep(10) + return { backup: 'async-data' } + }, + onSuccess: async () => { + results.push('onSuccess-async-start') + await sleep(10) + results.push('onSuccess-async-end') + }, + onError: async () => { + results.push('onError-async-start') + await sleep(10) + results.push('onError-async-end') + }, + onSettled: () => { + results.push('local-onSettled') + }, + }, + 'vars', + ).catch((error) => { + mutationError = error + }) + + await vi.advanceTimersByTimeAsync(50) + + expect(results).toEqual([ + 'onMutate-async', + 'onSuccess-async-start', + 'onSuccess-async-end', + 'global-onSettled', + 'onError-async-start', + 'onError-async-end', + 'global-onSettled', + 'local-onSettled', + ]) + + expect(unhandledRejectionFn).toHaveBeenCalledTimes(1) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(1, newMutationError) + + expect(mutationError).toEqual(newMutationError) + }) + + test('error by mutations onSettled triggers onError callback, calling both onSettled callbacks twice', async () => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + + const key = queryKey() + const results: Array = [] + + const newMutationError = new Error('mutation-error') + + let mutationError: Error | undefined + executeMutation( + queryClient, + { + mutationKey: key, + mutationFn: () => Promise.resolve('success'), + onMutate: async () => { + results.push('onMutate-async') + await sleep(10) + return { backup: 'async-data' } + }, + onSuccess: async () => { + results.push('onSuccess-async-start') + await sleep(10) + results.push('onSuccess-async-end') + }, + onError: async () => { + results.push('onError-async-start') + await sleep(10) + results.push('onError-async-end') + }, + onSettled: async () => { + results.push('onSettled-async-promise') + await sleep(10) + throw newMutationError + }, + }, + 'vars', + ).catch((error) => { + mutationError = error + }) + + await vi.advanceTimersByTimeAsync(50) + + expect(results).toEqual([ + 'onMutate-async', + 'onSuccess-async-start', + 'onSuccess-async-end', + 'onSettled-async-promise', + 'onError-async-start', + 'onError-async-end', + 'onSettled-async-promise', + ]) + + expect(unhandledRejectionFn).toHaveBeenCalledTimes(1) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(1, newMutationError) + + expect(mutationError).toEqual(newMutationError) + }) + + test('errors by onError and consecutive onSettled callbacks are transferred to different execution context where it are reported', async () => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + + const globalErrorError = new Error('global-error-error') + const globalSettledError = new Error('global-settled-error') + + queryClient = new QueryClient({ + mutationCache: new MutationCache({ + onError: () => { + throw globalErrorError + }, + onSettled: () => { + throw globalSettledError + }, + }), + }) + queryClient.mount() + + const key = queryKey() + const results: Array = [] + + const newMutationError = new Error('mutation-error') + const newErrorError = new Error('error-error') + const newSettledError = new Error('settled-error') + + let mutationError: Error | undefined + executeMutation( + queryClient, + { + mutationKey: key, + mutationFn: () => Promise.resolve('success'), + onMutate: async () => { + results.push('onMutate-async') + await sleep(10) + throw newMutationError + }, + onSuccess: () => { + results.push('onSuccess-async-start') + }, + onError: async () => { + results.push('onError-async-start') + await sleep(10) + throw newErrorError + }, + onSettled: async () => { + results.push('onSettled-promise') + await sleep(10) + throw newSettledError + }, + }, + 'vars', + ).catch((error) => { + mutationError = error + }) + + await vi.advanceTimersByTimeAsync(30) + + expect(results).toEqual([ + 'onMutate-async', + 'onError-async-start', + 'onSettled-promise', + ]) + + expect(mutationError).toEqual(newMutationError) + + expect(unhandledRejectionFn).toHaveBeenCalledTimes(4) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(1, globalErrorError) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(2, newErrorError) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith( + 3, + globalSettledError, + ) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(4, newSettledError) + }) + }) }) diff --git a/packages/query-core/src/mutation.ts b/packages/query-core/src/mutation.ts index 4d04626088..e4968da8d7 100644 --- a/packages/query-core/src/mutation.ts +++ b/packages/query-core/src/mutation.ts @@ -278,14 +278,22 @@ export class Mutation< this as Mutation, mutationFnContext, ) + } catch (e) { + void Promise.reject(e) + } + try { await this.options.onError?.( error as TError, variables, this.state.context, mutationFnContext, ) + } catch (e) { + void Promise.reject(e) + } + try { // Notify cache callback await this.#mutationCache.config.onSettled?.( undefined, @@ -295,7 +303,11 @@ export class Mutation< this as Mutation, mutationFnContext, ) + } catch (e) { + void Promise.reject(e) + } + try { await this.options.onSettled?.( undefined, error as TError, @@ -303,10 +315,12 @@ export class Mutation< this.state.context, mutationFnContext, ) - throw error - } finally { - this.#dispatch({ type: 'error', error: error as TError }) + } catch (e) { + void Promise.reject(e) } + + this.#dispatch({ type: 'error', error: error as TError }) + throw error } finally { this.#mutationCache.runNext(this) } diff --git a/packages/react-query/src/__tests__/useMutation.test.tsx b/packages/react-query/src/__tests__/useMutation.test.tsx index 30800c9b08..622a080eac 100644 --- a/packages/react-query/src/__tests__/useMutation.test.tsx +++ b/packages/react-query/src/__tests__/useMutation.test.tsx @@ -1064,7 +1064,14 @@ describe('useMutation', () => { }) }) - it('should go to error state if onError callback errors', async () => { + it('should go to error state if onError callback errors', async ({ + onTestFinished, + }) => { + onTestFinished(() => { + process.removeAllListeners('unhandledRejection') + }) + process.on('unhandledRejection', vi.fn()) + const error = new Error('error from onError') const mutateFnError = new Error('mutateFnError') @@ -1101,7 +1108,14 @@ describe('useMutation', () => { ).toBeInTheDocument() }) - it('should go to error state if onSettled callback errors', async () => { + it('should go to error state if onSettled callback errors', async ({ + onTestFinished, + }) => { + onTestFinished(() => { + process.removeAllListeners('unhandledRejection') + }) + process.on('unhandledRejection', vi.fn()) + const error = new Error('error from onSettled') const mutateFnError = new Error('mutateFnError') const onError = vi.fn() diff --git a/packages/solid-query/src/__tests__/useMutation.test.tsx b/packages/solid-query/src/__tests__/useMutation.test.tsx index 2ad4008191..16cac8ea76 100644 --- a/packages/solid-query/src/__tests__/useMutation.test.tsx +++ b/packages/solid-query/src/__tests__/useMutation.test.tsx @@ -1118,7 +1118,14 @@ describe('useMutation', () => { }) }) - it('should go to error state if onError callback errors', async () => { + it('should go to error state if onError callback errors', async ({ + onTestFinished, + }) => { + onTestFinished(() => { + process.removeAllListeners('unhandledRejection') + }) + process.on('unhandledRejection', vi.fn()) + const error = new Error('error from onError') const mutateFnError = new Error('mutateFnError') @@ -1159,7 +1166,14 @@ describe('useMutation', () => { ).toBeInTheDocument() }) - it('should go to error state if onSettled callback errors', async () => { + it('should go to error state if onSettled callback errors', async ({ + onTestFinished, + }) => { + onTestFinished(() => { + process.removeAllListeners('unhandledRejection') + }) + process.on('unhandledRejection', vi.fn()) + const error = new Error('error from onSettled') const mutateFnError = new Error('mutateFnError') const onError = vi.fn() From ba4e95a9d921d55304a509a160c14cd91a2caa92 Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 23 Sep 2025 15:13:09 +0200 Subject: [PATCH 2/2] Fixed accidental removal of vitests unhandledRejection handler --- .../src/__tests__/mutations.test.tsx | 25 +++++++++++++------ .../src/__tests__/useMutation.test.tsx | 10 +++++--- .../src/__tests__/useMutation.test.tsx | 10 +++++--- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/packages/query-core/src/__tests__/mutations.test.tsx b/packages/query-core/src/__tests__/mutations.test.tsx index 8f2cf357b6..7e59e4446a 100644 --- a/packages/query-core/src/__tests__/mutations.test.tsx +++ b/packages/query-core/src/__tests__/mutations.test.tsx @@ -844,10 +844,6 @@ describe('mutations', () => { }) describe('erroneous mutation callback', () => { - afterEach(() => { - process.removeAllListeners('unhandledRejection') - }) - test('error by global onSuccess triggers onError callback', async () => { const newMutationError = new Error('mutation-error') @@ -956,7 +952,9 @@ describe('mutations', () => { expect(mutationError).toEqual(newMutationError) }) - test('error by global onSettled triggers onError callback, calling global onSettled callback twice', async () => { + test('error by global onSettled triggers onError callback, calling global onSettled callback twice', async ({ + onTestFinished, + }) => { const newMutationError = new Error('mutation-error') queryClient = new QueryClient({ @@ -972,6 +970,9 @@ describe('mutations', () => { const unhandledRejectionFn = vi.fn() process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + onTestFinished(() => { + process.off('unhandledRejection', unhandledRejectionFn) + }) const key = queryKey() const results: Array = [] @@ -1025,9 +1026,14 @@ describe('mutations', () => { expect(mutationError).toEqual(newMutationError) }) - test('error by mutations onSettled triggers onError callback, calling both onSettled callbacks twice', async () => { + test('error by mutations onSettled triggers onError callback, calling both onSettled callbacks twice', async ({ + onTestFinished, + }) => { const unhandledRejectionFn = vi.fn() process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + onTestFinished(() => { + process.off('unhandledRejection', unhandledRejectionFn) + }) const key = queryKey() const results: Array = [] @@ -1084,9 +1090,14 @@ describe('mutations', () => { expect(mutationError).toEqual(newMutationError) }) - test('errors by onError and consecutive onSettled callbacks are transferred to different execution context where it are reported', async () => { + test('errors by onError and consecutive onSettled callbacks are transferred to different execution context where it are reported', async ({ + onTestFinished, + }) => { const unhandledRejectionFn = vi.fn() process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + onTestFinished(() => { + process.off('unhandledRejection', unhandledRejectionFn) + }) const globalErrorError = new Error('global-error-error') const globalSettledError = new Error('global-settled-error') diff --git a/packages/react-query/src/__tests__/useMutation.test.tsx b/packages/react-query/src/__tests__/useMutation.test.tsx index 622a080eac..7b30546d58 100644 --- a/packages/react-query/src/__tests__/useMutation.test.tsx +++ b/packages/react-query/src/__tests__/useMutation.test.tsx @@ -1067,10 +1067,11 @@ describe('useMutation', () => { it('should go to error state if onError callback errors', async ({ onTestFinished, }) => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) onTestFinished(() => { - process.removeAllListeners('unhandledRejection') + process.off('unhandledRejection', unhandledRejectionFn) }) - process.on('unhandledRejection', vi.fn()) const error = new Error('error from onError') const mutateFnError = new Error('mutateFnError') @@ -1111,10 +1112,11 @@ describe('useMutation', () => { it('should go to error state if onSettled callback errors', async ({ onTestFinished, }) => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) onTestFinished(() => { - process.removeAllListeners('unhandledRejection') + process.off('unhandledRejection', unhandledRejectionFn) }) - process.on('unhandledRejection', vi.fn()) const error = new Error('error from onSettled') const mutateFnError = new Error('mutateFnError') diff --git a/packages/solid-query/src/__tests__/useMutation.test.tsx b/packages/solid-query/src/__tests__/useMutation.test.tsx index 16cac8ea76..66a1f76a8e 100644 --- a/packages/solid-query/src/__tests__/useMutation.test.tsx +++ b/packages/solid-query/src/__tests__/useMutation.test.tsx @@ -1121,10 +1121,11 @@ describe('useMutation', () => { it('should go to error state if onError callback errors', async ({ onTestFinished, }) => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) onTestFinished(() => { - process.removeAllListeners('unhandledRejection') + process.off('unhandledRejection', unhandledRejectionFn) }) - process.on('unhandledRejection', vi.fn()) const error = new Error('error from onError') const mutateFnError = new Error('mutateFnError') @@ -1169,10 +1170,11 @@ describe('useMutation', () => { it('should go to error state if onSettled callback errors', async ({ onTestFinished, }) => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) onTestFinished(() => { - process.removeAllListeners('unhandledRejection') + process.off('unhandledRejection', unhandledRejectionFn) }) - process.on('unhandledRejection', vi.fn()) const error = new Error('error from onSettled') const mutateFnError = new Error('mutateFnError')