Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(effects): limit retries to 10 by default #2376

Merged
merged 1 commit into from
Feb 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions modules/effects/spec/effects_error_handler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
import { ErrorHandler, Provider } from '@angular/core';
import { ErrorHandler, Provider, Type } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Action, Store } from '@ngrx/store';
import { Observable, of } from 'rxjs';
import { Observable, of, throwError } from 'rxjs';
import { catchError } from 'rxjs/operators';
import { createEffect, EFFECTS_ERROR_HANDLER, EffectsModule } from '..';
import * as effectsSrc from '../src/effects_error_handler';

describe('Effects Error Handler', () => {
let subscriptionCount: number;
let globalErrorHandler: jasmine.Spy;
let storeNext: jasmine.Spy;

function makeEffectTestBed(...providers: Provider[]) {
function makeEffectTestBed(effect: Type<any>, ...providers: Provider[]) {
subscriptionCount = 0;

TestBed.configureTestingModule({
imports: [EffectsModule.forRoot([ErrorEffect])],
imports: [EffectsModule.forRoot([effect])],
providers: [
{
provide: Store,
Expand All @@ -38,8 +39,14 @@ describe('Effects Error Handler', () => {
storeNext = store.next;
}

it('should retry on infinite error up to 10 times', () => {
makeEffectTestBed(AlwaysErrorEffect);

expect(globalErrorHandler.calls.count()).toBe(10);
});

it('should retry and notify error handler when effect error handler is not provided', () => {
makeEffectTestBed();
makeEffectTestBed(ErrorEffect);

// two subscriptions expected:
// 1. Initial subscription to the effect (this will error)
Expand All @@ -62,7 +69,7 @@ describe('Effects Error Handler', () => {
);
});

makeEffectTestBed({
makeEffectTestBed(ErrorEffect, {
provide: EFFECTS_ERROR_HANDLER,
useValue: effectsErrorHandlerSpy,
});
Expand All @@ -84,6 +91,10 @@ describe('Effects Error Handler', () => {
});
}

class AlwaysErrorEffect {
effect$ = createEffect(() => throwError('always an error'));
}

/**
* This observable factory returns an observable that will never emit, but the first subscriber will get an immediate
* error. All subsequent subscribers will just get an observable that does not emit.
Expand Down
22 changes: 15 additions & 7 deletions modules/effects/src/effects_error_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,25 @@ export type EffectsErrorHandler = <T extends Action>(
errorHandler: ErrorHandler
) => Observable<T>;

export const defaultEffectsErrorHandler: EffectsErrorHandler = <
T extends Action
>(
const MAX_NUMBER_OF_RETRY_ATTEMPTS = 10;

export function defaultEffectsErrorHandler<T extends Action>(
observable$: Observable<T>,
errorHandler: ErrorHandler
): Observable<T> => {
errorHandler: ErrorHandler,
retryAttemptLeft: number = MAX_NUMBER_OF_RETRY_ATTEMPTS
): Observable<T> {
return observable$.pipe(
catchError(error => {
if (errorHandler) errorHandler.handleError(error);
if (retryAttemptLeft <= 1) {
return observable$; // last attempt
}
// Return observable that produces this particular effect
return defaultEffectsErrorHandler(observable$, errorHandler);
return defaultEffectsErrorHandler(
observable$,
errorHandler,
retryAttemptLeft - 1
);
})
);
};
}
5 changes: 4 additions & 1 deletion modules/effects/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ export { EffectConfig } from './models';
export { Effect } from './effect_decorator';
export { getEffectsMetadata } from './effects_metadata';
export { mergeEffects } from './effects_resolver';
export { EffectsErrorHandler } from './effects_error_handler';
export {
EffectsErrorHandler,
defaultEffectsErrorHandler,
} from './effects_error_handler';
export { EffectsMetadata, CreateEffectMetadata } from './models';
export { Actions, ofType } from './actions';
export { EffectsModule } from './effects_module';
Expand Down
6 changes: 4 additions & 2 deletions projects/ngrx.io/content/guide/effects/lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export class LogEffects {
Starting with version 8, when an error happens in the effect's main stream it is
reported using Angular's `ErrorHandler`, and the source effect is
**automatically** resubscribed to (instead of completing), so it continues to
listen to all dispatched Actions.
listen to all dispatched Actions. By default, effects are resubscribed up to 10
errors.

Generally, errors should be handled by users. However, for the cases where errors were missed,
this new behavior adds an additional safety net.
Expand Down Expand Up @@ -96,7 +97,8 @@ The behavior of the default resubscription handler can be customized
by providing a custom handler using the `EFFECTS_ERROR_HANDLER` injection token.

This allows you to provide a custom behavior, such as only retrying on
certain "retryable" errors, or with maximum number of retries.
certain "retryable" errors, or change the maximum number of retries (it's set to
10 by default).

<code-example header="customise-error-handler.effects.ts">
import { ErrorHandler, NgModule } from '@angular/core';
Expand Down