From 9428da380d89cc5b2c586d1362e8e20e816eb4e8 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Thu, 2 May 2024 17:11:48 +0100 Subject: [PATCH 1/5] rethrow errors from dispatching createAsyncThunk --- packages/toolkit/src/createAsyncThunk.ts | 20 +++++++++- .../src/tests/createAsyncThunk.test.ts | 39 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/createAsyncThunk.ts b/packages/toolkit/src/createAsyncThunk.ts index f384ab9af2..9503e05088 100644 --- a/packages/toolkit/src/createAsyncThunk.ts +++ b/packages/toolkit/src/createAsyncThunk.ts @@ -1,4 +1,4 @@ -import type { Dispatch, UnknownAction } from 'redux' +import type { Action, Dispatch, UnknownAction } from 'redux' import type { PayloadAction, ActionCreatorWithPreparedPayload, @@ -114,6 +114,18 @@ export const miniSerializeError = (value: any): SerializedError => { return { message: String(value) } } +export class DispatchError { + constructor(public readonly cause: unknown) {} +} + +function tryDispatch(dispatch: Dispatch, action: Action) { + try { + return dispatch(action) + } catch (err) { + throw new DispatchError(err) + } +} + export type AsyncThunkConfig = { state?: unknown dispatch?: Dispatch @@ -617,7 +629,8 @@ export const createAsyncThunk = /* @__PURE__ */ (() => { } abortController.signal.addEventListener('abort', abortHandler) }) - dispatch( + tryDispatch( + dispatch, pending( requestId, arg, @@ -658,6 +671,9 @@ export const createAsyncThunk = /* @__PURE__ */ (() => { }), ]) } catch (err) { + if (err instanceof DispatchError) { + throw err.cause + } finalAction = err instanceof RejectWithValue ? rejected(null, requestId, arg, err.payload, err.meta) diff --git a/packages/toolkit/src/tests/createAsyncThunk.test.ts b/packages/toolkit/src/tests/createAsyncThunk.test.ts index ef6ae71838..e21a8ee198 100644 --- a/packages/toolkit/src/tests/createAsyncThunk.test.ts +++ b/packages/toolkit/src/tests/createAsyncThunk.test.ts @@ -991,3 +991,42 @@ describe('meta', () => { expect(thunk.fulfilled.type).toBe('a/fulfilled') }) }) + +describe('reducer errors will be rethrown', () => { + const asyncThunk = createAsyncThunk('test', async (shouldThrow: boolean) => { + await delay(100) + if (shouldThrow) { + throw new Error('Error in reducer') + } + return 'Success' + }) + const faultStoreForType = (type: 'pending' | 'fulfilled' | 'rejected') => + configureStore({ + reducer(state = null, action) { + // obviously reducers shouldn't ever throw - but legitimate errors do happen, with typos for example + if (asyncThunk[type].match(action)) { + throw new Error('Error in reducer') + } + return state + }, + }) + // TODO: this breaks our promise that not unwrapping will never throw - do we care? + it('rethrows errors found when dispatching pending action', () => { + const store = faultStoreForType('pending') + expect(store.dispatch(asyncThunk(false))).rejects.toThrowError( + 'Error in reducer', + ) + }) + it('rethrows errors found when dispatching fulfilled action', async () => { + const store = faultStoreForType('fulfilled') + await expect(store.dispatch(asyncThunk(false))).rejects.toThrowError( + 'Error in reducer', + ) + }) + it('rethrows errors found when dispatching rejected action', async () => { + const store = faultStoreForType('rejected') + await expect(store.dispatch(asyncThunk(true))).rejects.toThrowError( + 'Error in reducer', + ) + }) +}) From dd340e3ad3471e1f13966c9e5f341028d339531a Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Thu, 2 May 2024 17:24:51 +0100 Subject: [PATCH 2/5] remove util only used once --- packages/toolkit/src/createAsyncThunk.ts | 25 +++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/toolkit/src/createAsyncThunk.ts b/packages/toolkit/src/createAsyncThunk.ts index 9503e05088..e964c032b6 100644 --- a/packages/toolkit/src/createAsyncThunk.ts +++ b/packages/toolkit/src/createAsyncThunk.ts @@ -629,17 +629,20 @@ export const createAsyncThunk = /* @__PURE__ */ (() => { } abortController.signal.addEventListener('abort', abortHandler) }) - tryDispatch( - dispatch, - pending( - requestId, - arg, - options?.getPendingMeta?.( - { requestId, arg }, - { getState, extra }, - ), - ) as any, - ) + try { + dispatch( + pending( + requestId, + arg, + options?.getPendingMeta?.( + { requestId, arg }, + { getState, extra }, + ), + ) as any, + ) + } catch (err) { + throw new DispatchError(err) + } finalAction = await Promise.race([ abortedPromise, Promise.resolve( From 255d339bd0e5bb4b97e6029e774681fc1f47fc42 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Thu, 2 May 2024 17:25:51 +0100 Subject: [PATCH 3/5] actually remove it --- packages/toolkit/src/createAsyncThunk.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/toolkit/src/createAsyncThunk.ts b/packages/toolkit/src/createAsyncThunk.ts index e964c032b6..7f93fe2792 100644 --- a/packages/toolkit/src/createAsyncThunk.ts +++ b/packages/toolkit/src/createAsyncThunk.ts @@ -118,14 +118,6 @@ export class DispatchError { constructor(public readonly cause: unknown) {} } -function tryDispatch(dispatch: Dispatch, action: Action) { - try { - return dispatch(action) - } catch (err) { - throw new DispatchError(err) - } -} - export type AsyncThunkConfig = { state?: unknown dispatch?: Dispatch From 13be620320eaae49b499e7d39e29ed9003beefb7 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Thu, 2 May 2024 17:34:21 +0100 Subject: [PATCH 4/5] add missing await --- packages/toolkit/src/tests/createAsyncThunk.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/toolkit/src/tests/createAsyncThunk.test.ts b/packages/toolkit/src/tests/createAsyncThunk.test.ts index e21a8ee198..c344e99878 100644 --- a/packages/toolkit/src/tests/createAsyncThunk.test.ts +++ b/packages/toolkit/src/tests/createAsyncThunk.test.ts @@ -1011,9 +1011,9 @@ describe('reducer errors will be rethrown', () => { }, }) // TODO: this breaks our promise that not unwrapping will never throw - do we care? - it('rethrows errors found when dispatching pending action', () => { + it('rethrows errors found when dispatching pending action', async () => { const store = faultStoreForType('pending') - expect(store.dispatch(asyncThunk(false))).rejects.toThrowError( + await expect(store.dispatch(asyncThunk(false))).rejects.toThrowError( 'Error in reducer', ) }) From e268789f3dcf65404593a0f3927d354a0ad5b018 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Thu, 2 May 2024 17:38:26 +0100 Subject: [PATCH 5/5] add (currently failing) RTKQ test --- .../toolkit/src/query/tests/matchers.test.tsx | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/toolkit/src/query/tests/matchers.test.tsx b/packages/toolkit/src/query/tests/matchers.test.tsx index 5b26740e98..1c0b6c6432 100644 --- a/packages/toolkit/src/query/tests/matchers.test.tsx +++ b/packages/toolkit/src/query/tests/matchers.test.tsx @@ -3,6 +3,7 @@ import { hookWaitFor, setupApiStore, } from '@internal/tests/utils/helpers' +import type { UnknownAction } from '@reduxjs/toolkit' import { createSlice } from '@reduxjs/toolkit' import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react' import { act, renderHook } from '@testing-library/react' @@ -239,3 +240,34 @@ test('inferred types', () => { }, }) }) + +describe('errors in reducers are not swallowed', () => { + const faultyStoreFor = (matcher: (action: UnknownAction) => boolean) => + setupApiStore(api, { + ...actionsReducer, + faultyReducer(state = null, action: UnknownAction) { + if (matcher(action)) { + throw new Error('reducer error') + } + return state + }, + }) + test('pending action reducer errors should be thrown', async () => { + const storeRef = faultyStoreFor(api.endpoints.querySuccess.matchPending) + await expect( + storeRef.store.dispatch(querySuccess2.initiate({} as any)), + ).rejects.toThrow('reducer error') + }) + test('fulfilled action reducer errors should be thrown', async () => { + const storeRef = faultyStoreFor(api.endpoints.querySuccess.matchFulfilled) + await expect( + storeRef.store.dispatch(querySuccess2.initiate({} as any)), + ).rejects.toThrow('reducer error') + }) + test('rejected action reducer errors should be thrown', async () => { + const storeRef = faultyStoreFor(api.endpoints.queryFail.matchRejected) + await expect( + storeRef.store.dispatch(queryFail.initiate({} as any)), + ).rejects.toThrow('reducer error') + }) +})