Skip to content

Commit fcb5e9d

Browse files
committed
Merge branch 'master' of github.com:reduxjs/redux-toolkit
2 parents 3ba2510 + 381d675 commit fcb5e9d

File tree

3 files changed

+107
-6
lines changed

3 files changed

+107
-6
lines changed

packages/action-listener-middleware/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ Current options are:
100100

101101
- `extra`: an optional "extra argument" that will be injected into the `listenerApi` parameter of each listener. Equivalent to [the "extra argument" in the Redux Thunk middleware](https://redux.js.org/usage/writing-logic-thunks#injecting-config-values-into-thunks).
102102

103+
- `onError`: an optional error handler that gets called with synchronous errors raised by `listener` and `predicate`.
104+
103105
### `listenerMiddleware.addListener(predicate, listener, options?) : Unsubscribe`
104106

105107
Statically adds a new listener callback to the middleware.

packages/action-listener-middleware/src/index.ts

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ export interface HasMatchFunction<T> {
3838
match: MatchFunction<T>
3939
}
4040

41+
function assertFunction(
42+
func: unknown,
43+
expected: string
44+
): asserts func is (...args: unknown[]) => unknown {
45+
if (typeof func !== 'function') {
46+
throw new TypeError(`${expected} in not a function`)
47+
}
48+
}
49+
4150
export const hasMatchFunction = <T>(
4251
v: Matcher<T>
4352
): v is HasMatchFunction<T> => {
@@ -94,6 +103,10 @@ export type ActionListener<
94103
O extends ActionListenerOptions
95104
> = (action: A, api: ActionListenerMiddlewareAPI<S, D, O>) => void
96105

106+
export interface ListenerErrorHandler {
107+
(error: unknown): void
108+
}
109+
97110
export interface ActionListenerOptions {
98111
/**
99112
* Determines if the listener runs 'before' or 'after' the reducers have been called.
@@ -105,6 +118,10 @@ export interface ActionListenerOptions {
105118

106119
export interface CreateListenerMiddlewareOptions<ExtraArgument = unknown> {
107120
extra?: ExtraArgument
121+
/**
122+
* Receives synchronous errors that are raised by `listener` and `listenerOption.predicate`.
123+
*/
124+
onError?: ListenerErrorHandler
108125
}
109126

110127
export interface AddListenerAction<
@@ -121,6 +138,28 @@ export interface AddListenerAction<
121138
}
122139
}
123140

141+
/**
142+
* Safely reports errors to the `errorHandler` provided.
143+
* Errors that occur inside `errorHandler` are notified in a new task.
144+
* Inspired by [rxjs reportUnhandledError](https://github.com/ReactiveX/rxjs/blob/6fafcf53dc9e557439b25debaeadfd224b245a66/src/internal/util/reportUnhandledError.ts)
145+
* @param errorHandler
146+
* @param errorToNotify
147+
*/
148+
const safelyNotifyError = (
149+
errorHandler: ListenerErrorHandler,
150+
errorToNotify: unknown
151+
): void => {
152+
try {
153+
errorHandler(errorToNotify)
154+
} catch (errorHandlerError) {
155+
// We cannot let an error raised here block the listener queue.
156+
// The error raised here will be picked up by `window.onerror`, `process.on('error')` etc...
157+
setTimeout(() => {
158+
throw errorHandlerError
159+
}, 0)
160+
}
161+
}
162+
124163
/**
125164
* @alpha
126165
*/
@@ -220,6 +259,9 @@ export const removeListenerAction = createAction(
220259

221260
const defaultWhen: MiddlewarePhase = 'afterReducer'
222261
const actualMiddlewarePhases = ['beforeReducer', 'afterReducer'] as const
262+
const defaultErrorHandler: ListenerErrorHandler = (...args: unknown[]) => {
263+
console.error('action-listener-middleware-error', ...args)
264+
}
223265

224266
/**
225267
* @alpha
@@ -239,7 +281,9 @@ export function createActionListenerMiddleware<
239281
}
240282

241283
const listenerMap = new Map<string, ListenerEntry>()
242-
const { extra } = middlewareOptions
284+
const { extra, onError = defaultErrorHandler } = middlewareOptions
285+
286+
assertFunction(onError, 'onError')
243287

244288
const middleware: Middleware<
245289
{
@@ -277,8 +321,18 @@ export function createActionListenerMiddleware<
277321
for (let entry of listenerMap.values()) {
278322
const runThisPhase =
279323
entry.when === 'both' || entry.when === currentPhase
280-
const runListener =
281-
runThisPhase && entry.predicate(action, currentState, originalState)
324+
325+
let runListener = runThisPhase
326+
327+
if (runListener) {
328+
try {
329+
runListener = entry.predicate(action, currentState, originalState)
330+
} catch (predicateError) {
331+
safelyNotifyError(onError, predicateError)
332+
runListener = false
333+
}
334+
}
335+
282336
if (!runListener) {
283337
continue
284338
}
@@ -293,8 +347,8 @@ export function createActionListenerMiddleware<
293347
extra,
294348
unsubscribe: entry.unsubscribe,
295349
})
296-
} catch (err) {
297-
// ignore errors deliberately
350+
} catch (listenerError) {
351+
safelyNotifyError(onError, listenerError)
298352
}
299353
}
300354
if (currentPhase === 'beforeReducer') {

packages/action-listener-middleware/src/tests/listenerMiddleware.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ describe('createActionListenerMiddleware', () => {
6060
const testAction3 = createAction<string>('testAction3')
6161
type TestAction3 = ReturnType<typeof testAction3>
6262

63+
beforeAll(() => {
64+
jest.spyOn(console, 'error').mockImplementation(noop)
65+
})
66+
6367
beforeEach(() => {
6468
middleware = createActionListenerMiddleware()
6569
reducer = jest.fn(() => ({}))
@@ -461,7 +465,7 @@ describe('createActionListenerMiddleware', () => {
461465
expect(reducer.mock.calls).toEqual([[{}, testAction1('a')]])
462466
})
463467

464-
test('Continues running other listeners if there is an error', () => {
468+
test('Continues running other listeners if one of them raises an error', () => {
465469
const matcher = (action: any) => true
466470

467471
middleware.addListener(matcher, () => {
@@ -475,6 +479,47 @@ describe('createActionListenerMiddleware', () => {
475479
expect(listener.mock.calls).toEqual([[testAction1('a'), middlewareApi]])
476480
})
477481

482+
test('Continues running other listeners if a predicate raises an error', () => {
483+
const matcher = (action: any) => true
484+
const firstListener = jest.fn(() => {})
485+
const secondListener = jest.fn(() => {})
486+
487+
middleware.addListener(() => {
488+
throw new Error('Predicate Panic!')
489+
}, firstListener);
490+
491+
middleware.addListener(matcher, secondListener)
492+
493+
store.dispatch(testAction1('a'))
494+
expect(firstListener).not.toHaveBeenCalled();
495+
expect(secondListener.mock.calls).toEqual([
496+
[testAction1('a'), middlewareApi],
497+
])
498+
})
499+
500+
test('Notifies listener errors to `onError`, if provided', () => {
501+
const onError = jest.fn();
502+
middleware = createActionListenerMiddleware({
503+
onError
504+
})
505+
reducer = jest.fn(() => ({}))
506+
store = configureStore({
507+
reducer,
508+
middleware: (gDM) => gDM().prepend(middleware),
509+
})
510+
511+
const listenerError = new Error('Boom!');
512+
513+
const matcher = (action: any) => true
514+
515+
middleware.addListener(matcher, () => {
516+
throw listenerError;
517+
});
518+
519+
store.dispatch(testAction1('a'))
520+
expect(onError).toBeCalledWith(listenerError);
521+
})
522+
478523
test('condition method resolves promise when the predicate succeeds', async () => {
479524
const store = configureStore({
480525
reducer: counterSlice.reducer,

0 commit comments

Comments
 (0)