From 10e81025dce76e8b03c38898c8bace04115c8bd8 Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Mon, 22 Sep 2025 16:48:39 +0200 Subject: [PATCH 1/2] Ensure errors of mutation callbacks are reported asynchronously --- .../src/__tests__/mutationObserver.test.tsx | 87 +++++++++++++++++++ packages/query-core/src/mutationObserver.ts | 68 +++++++++------ 2 files changed, 129 insertions(+), 26 deletions(-) diff --git a/packages/query-core/src/__tests__/mutationObserver.test.tsx b/packages/query-core/src/__tests__/mutationObserver.test.tsx index 995156be41..888eadf5e9 100644 --- a/packages/query-core/src/__tests__/mutationObserver.test.tsx +++ b/packages/query-core/src/__tests__/mutationObserver.test.tsx @@ -383,4 +383,91 @@ describe('mutationObserver', () => { unsubscribe() }) + + describe('erroneous mutation callback', () => { + afterEach(() => { + process.removeAllListeners('unhandledRejection') + }) + + test('onSuccess and onSettled is transferred to different execution context where it is reported', async () => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + + const onSuccessError = new Error('onSuccess-error') + const onSuccess = vi.fn(() => { + throw onSuccessError + }) + const onSettledError = new Error('onSettled-error') + const onSettled = vi.fn(() => { + throw onSettledError + }) + + const mutationObserver = new MutationObserver(queryClient, { + mutationFn: (text: string) => Promise.resolve(text.toUpperCase()), + }) + + const subscriptionHandler = vi.fn() + const unsubscribe = mutationObserver.subscribe(subscriptionHandler) + + mutationObserver.mutate('success', { + onSuccess, + onSettled, + }) + + await vi.advanceTimersByTimeAsync(0) + + expect(onSuccess).toHaveBeenCalledTimes(1) + expect(onSettled).toHaveBeenCalledTimes(1) + + expect(unhandledRejectionFn).toHaveBeenCalledTimes(2) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(1, onSuccessError) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(2, onSettledError) + + expect(subscriptionHandler).toHaveBeenCalledTimes(2) + + unsubscribe() + }) + + test('onError and onSettled is transferred to different execution context where it is reported', async () => { + const unhandledRejectionFn = vi.fn() + process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + + const onErrorError = new Error('onError-error') + const onError = vi.fn(() => { + throw onErrorError + }) + const onSettledError = new Error('onSettled-error') + const onSettled = vi.fn(() => { + throw onSettledError + }) + + const error = new Error('error') + const mutationObserver = new MutationObserver(queryClient, { + mutationFn: (_: string) => Promise.reject(error), + }) + + const subscriptionHandler = vi.fn() + const unsubscribe = mutationObserver.subscribe(subscriptionHandler) + + mutationObserver + .mutate('error', { + onError, + onSettled, + }) + .catch(() => {}) + + await vi.advanceTimersByTimeAsync(0) + + expect(onError).toHaveBeenCalledTimes(1) + expect(onSettled).toHaveBeenCalledTimes(1) + + expect(unhandledRejectionFn).toHaveBeenCalledTimes(2) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(1, onErrorError) + expect(unhandledRejectionFn).toHaveBeenNthCalledWith(2, onSettledError) + + expect(subscriptionHandler).toHaveBeenCalledTimes(2) + + unsubscribe() + }) + }) }) diff --git a/packages/query-core/src/mutationObserver.ts b/packages/query-core/src/mutationObserver.ts index abb2a5389c..21523963ce 100644 --- a/packages/query-core/src/mutationObserver.ts +++ b/packages/query-core/src/mutationObserver.ts @@ -172,33 +172,49 @@ export class MutationObserver< } satisfies MutationFunctionContext if (action?.type === 'success') { - this.#mutateOptions.onSuccess?.( - action.data, - variables, - onMutateResult, - context, - ) - this.#mutateOptions.onSettled?.( - action.data, - null, - variables, - onMutateResult, - context, - ) + try { + this.#mutateOptions.onSuccess?.( + action.data, + variables, + onMutateResult, + context, + ) + } catch (e) { + void Promise.reject(e) + } + try { + this.#mutateOptions.onSettled?.( + action.data, + null, + variables, + onMutateResult, + context, + ) + } catch (e) { + void Promise.reject(e) + } } else if (action?.type === 'error') { - this.#mutateOptions.onError?.( - action.error, - variables, - onMutateResult, - context, - ) - this.#mutateOptions.onSettled?.( - undefined, - action.error, - variables, - onMutateResult, - context, - ) + try { + this.#mutateOptions.onError?.( + action.error, + variables, + onMutateResult, + context, + ) + } catch (e) { + void Promise.reject(e) + } + try { + this.#mutateOptions.onSettled?.( + undefined, + action.error, + variables, + onMutateResult, + context, + ) + } catch (e) { + void Promise.reject(e) + } } } From 7fbb7acb0696e6bba3f6586d2aca0e350b2fa5ba Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 23 Sep 2025 15:15:59 +0200 Subject: [PATCH 2/2] Fixed accidental removal of vitests unhandledRejection handler --- .../src/__tests__/mutationObserver.test.tsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/query-core/src/__tests__/mutationObserver.test.tsx b/packages/query-core/src/__tests__/mutationObserver.test.tsx index 888eadf5e9..abbcfeac42 100644 --- a/packages/query-core/src/__tests__/mutationObserver.test.tsx +++ b/packages/query-core/src/__tests__/mutationObserver.test.tsx @@ -385,13 +385,14 @@ describe('mutationObserver', () => { }) describe('erroneous mutation callback', () => { - afterEach(() => { - process.removeAllListeners('unhandledRejection') - }) - - test('onSuccess and onSettled is transferred to different execution context where it is reported', async () => { + test('onSuccess and onSettled is transferred to different execution context where it is reported', async ({ + onTestFinished, + }) => { const unhandledRejectionFn = vi.fn() process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + onTestFinished(() => { + process.off('unhandledRejection', unhandledRejectionFn) + }) const onSuccessError = new Error('onSuccess-error') const onSuccess = vi.fn(() => { @@ -428,9 +429,14 @@ describe('mutationObserver', () => { unsubscribe() }) - test('onError and onSettled is transferred to different execution context where it is reported', async () => { + test('onError and onSettled is transferred to different execution context where it is reported', async ({ + onTestFinished, + }) => { const unhandledRejectionFn = vi.fn() process.on('unhandledRejection', (error) => unhandledRejectionFn(error)) + onTestFinished(() => { + process.off('unhandledRejection', unhandledRejectionFn) + }) const onErrorError = new Error('onError-error') const onError = vi.fn(() => {