Skip to content

Commit

Permalink
feat(effects): make resubscription handler overridable (#2295)
Browse files Browse the repository at this point in the history
Closes #2294

BREAKING CHANGE:

`resubscribeOnError` renamed to `useEffectsErrorHandler` in `createEffect` metadata

BEFORE:

```ts
  class MyEffects {
    effect$ = createEffect(() => stream$, {
      resubscribeOnError: true, // default
    });
  }
```

AFTER:

```ts
  class MyEffects {
    effect$ = createEffect(() => stream$, {
      useEffectsErrorHandler: true, // default
    });
  }
```
  • Loading branch information
zak-cloudnc authored and brandonroberts committed Jan 27, 2020
1 parent 900bf75 commit 3a9ad63
Show file tree
Hide file tree
Showing 17 changed files with 290 additions and 91 deletions.
24 changes: 13 additions & 11 deletions modules/effects/spec/effect_creator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,30 +52,32 @@ describe('createEffect()', () => {
a = createEffect(() => of({ type: 'a' }));
b = createEffect(() => of({ type: 'b' }), { dispatch: true });
c = createEffect(() => of({ type: 'c' }), { dispatch: false });
d = createEffect(() => of({ type: 'd' }), { resubscribeOnError: true });
d = createEffect(() => of({ type: 'd' }), {
useEffectsErrorHandler: true,
});
e = createEffect(() => of({ type: 'd' }), {
resubscribeOnError: false,
useEffectsErrorHandler: false,
});
f = createEffect(() => of({ type: 'e' }), {
dispatch: false,
resubscribeOnError: false,
useEffectsErrorHandler: false,
});
g = createEffect(() => of({ type: 'e' }), {
dispatch: true,
resubscribeOnError: false,
useEffectsErrorHandler: false,
});
}

const mock = new Fixture();

expect(getCreateEffectMetadata(mock)).toEqual([
{ propertyName: 'a', dispatch: true, resubscribeOnError: true },
{ propertyName: 'b', dispatch: true, resubscribeOnError: true },
{ propertyName: 'c', dispatch: false, resubscribeOnError: true },
{ propertyName: 'd', dispatch: true, resubscribeOnError: true },
{ propertyName: 'e', dispatch: true, resubscribeOnError: false },
{ propertyName: 'f', dispatch: false, resubscribeOnError: false },
{ propertyName: 'g', dispatch: true, resubscribeOnError: false },
{ propertyName: 'a', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'b', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'c', dispatch: false, useEffectsErrorHandler: true },
{ propertyName: 'd', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'e', dispatch: true, useEffectsErrorHandler: false },
{ propertyName: 'f', dispatch: false, useEffectsErrorHandler: false },
{ propertyName: 'g', dispatch: true, useEffectsErrorHandler: false },
]);
});

Expand Down
22 changes: 11 additions & 11 deletions modules/effects/spec/effect_decorator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,26 @@ describe('@Effect()', () => {
b: any;
@Effect({ dispatch: false })
c: any;
@Effect({ resubscribeOnError: true })
@Effect({ useEffectsErrorHandler: true })
d: any;
@Effect({ resubscribeOnError: false })
@Effect({ useEffectsErrorHandler: false })
e: any;
@Effect({ dispatch: false, resubscribeOnError: false })
@Effect({ dispatch: false, useEffectsErrorHandler: false })
f: any;
@Effect({ dispatch: true, resubscribeOnError: false })
@Effect({ dispatch: true, useEffectsErrorHandler: false })
g: any;
}

const mock = new Fixture();

expect(getEffectDecoratorMetadata(mock)).toEqual([
{ propertyName: 'a', dispatch: true, resubscribeOnError: true },
{ propertyName: 'b', dispatch: true, resubscribeOnError: true },
{ propertyName: 'c', dispatch: false, resubscribeOnError: true },
{ propertyName: 'd', dispatch: true, resubscribeOnError: true },
{ propertyName: 'e', dispatch: true, resubscribeOnError: false },
{ propertyName: 'f', dispatch: false, resubscribeOnError: false },
{ propertyName: 'g', dispatch: true, resubscribeOnError: false },
{ propertyName: 'a', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'b', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'c', dispatch: false, useEffectsErrorHandler: true },
{ propertyName: 'd', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'e', dispatch: true, useEffectsErrorHandler: false },
{ propertyName: 'f', dispatch: false, useEffectsErrorHandler: false },
{ propertyName: 'g', dispatch: true, useEffectsErrorHandler: false },
]);
});

Expand Down
33 changes: 19 additions & 14 deletions modules/effects/spec/effect_sources.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,27 @@ import {
OnIdentifyEffects,
OnInitEffects,
createEffect,
EFFECTS_ERROR_HANDLER,
EffectsErrorHandler,
Actions,
} from '../';
import { defaultEffectsErrorHandler } from '../src/effects_error_handler';
import { EffectsRunner } from '../src/effects_runner';
import { Store } from '@ngrx/store';
import { ofType } from '../src';

describe('EffectSources', () => {
let mockErrorReporter: ErrorHandler;
let effectSources: EffectSources;
let effectsErrorHandler: EffectsErrorHandler;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [
{
provide: EFFECTS_ERROR_HANDLER,
useValue: defaultEffectsErrorHandler,
},
EffectSources,
EffectsRunner,
{
Expand All @@ -47,6 +55,7 @@ describe('EffectSources', () => {

mockErrorReporter = TestBed.get(ErrorHandler);
effectSources = TestBed.get(EffectSources);
effectsErrorHandler = TestBed.get(EFFECTS_ERROR_HANDLER);

spyOn(mockErrorReporter, 'handleError');
});
Expand Down Expand Up @@ -144,6 +153,12 @@ describe('EffectSources', () => {
});

describe('toActions() Operator', () => {
function toActions(source: any): Observable<any> {
source['errorHandler'] = mockErrorReporter;
source['effectsErrorHandler'] = effectsErrorHandler;
return (effectSources as any)['toActions'].call(source);
}

describe('with @Effect()', () => {
const a = { type: 'From Source A' };
const b = { type: 'From Source B' };
Expand Down Expand Up @@ -346,9 +361,9 @@ describe('EffectSources', () => {
expect(toActions(sources$)).toBeObservable(expected);
});

it('should not resubscribe on error when resubscribeOnError is false', () => {
it('should not resubscribe on error when useEffectsErrorHandler is false', () => {
class Eff {
@Effect({ resubscribeOnError: false })
@Effect({ useEffectsErrorHandler: false })
b$ = hot('a--b--c--d').pipe(
map(v => {
if (v == 'b') throw new Error('An Error');
Expand Down Expand Up @@ -387,11 +402,6 @@ describe('EffectSources', () => {

expect(output).toBeObservable(expected);
});

function toActions(source: any): Observable<any> {
source['errorHandler'] = mockErrorReporter;
return (effectSources as any)['toActions'].call(source);
}
});

describe('with createEffect()', () => {
Expand Down Expand Up @@ -635,7 +645,7 @@ describe('EffectSources', () => {
expect(toActions(sources$)).toBeObservable(expected);
});

it('should not resubscribe on error when resubscribeOnError is false', () => {
it('should not resubscribe on error when useEffectsErrorHandler is false', () => {
const sources$ = of(
new class {
b$ = createEffect(
Expand All @@ -646,7 +656,7 @@ describe('EffectSources', () => {
return v;
})
),
{ dispatch: false, resubscribeOnError: false }
{ dispatch: false, useEffectsErrorHandler: false }
);
}()
);
Expand Down Expand Up @@ -678,11 +688,6 @@ describe('EffectSources', () => {

expect(output).toBeObservable(expected);
});

function toActions(source: any): Observable<any> {
source['errorHandler'] = mockErrorReporter;
return (effectSources as any)['toActions'].call(source);
}
});
});

Expand Down
100 changes: 100 additions & 0 deletions modules/effects/spec/effects_error_handler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { ErrorHandler, Provider } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Action, Store } from '@ngrx/store';
import { Observable, of } from 'rxjs';
import { catchError } from 'rxjs/operators';
import { createEffect, EFFECTS_ERROR_HANDLER, EffectsModule } from '..';

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

function makeEffectTestBed(...providers: Provider[]) {
subscriptionCount = 0;

TestBed.configureTestingModule({
imports: [EffectsModule.forRoot([ErrorEffect])],
providers: [
{
provide: Store,
useValue: {
next: jasmine.createSpy('storeNext'),
dispatch: jasmine.createSpy('dispatch'),
},
},
{
provide: ErrorHandler,
useValue: {
handleError: jasmine.createSpy('globalErrorHandler'),
},
},
...providers,
],
});

globalErrorHandler = TestBed.get(ErrorHandler).handleError;
const store = TestBed.get(Store);
storeNext = store.next;
}

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

// two subscriptions expected:
// 1. Initial subscription to the effect (this will error)
// 2. Resubscription to the effect after error (this will not error)
expect(subscriptionCount).toBe(2);
expect(globalErrorHandler).toHaveBeenCalledWith(new Error('effectError'));
});

it('should use custom error behavior when EFFECTS_ERROR_HANDLER is provided', () => {
const effectsErrorHandlerSpy = jasmine
.createSpy()
.and.callFake((effect$: Observable<any>, errorHandler: ErrorHandler) => {
return effect$.pipe(
catchError(err => {
errorHandler.handleError(
new Error('inside custom handler: ' + err.message)
);
return of({ type: 'custom action' });
})
);
});

makeEffectTestBed({
provide: EFFECTS_ERROR_HANDLER,
useValue: effectsErrorHandlerSpy,
});

expect(effectsErrorHandlerSpy).toHaveBeenCalledWith(
jasmine.any(Observable),
TestBed.get(ErrorHandler)
);
expect(globalErrorHandler).toHaveBeenCalledWith(
new Error('inside custom handler: effectError')
);
expect(subscriptionCount).toBe(1);
expect(storeNext).toHaveBeenCalledWith({ type: 'custom action' });
});

class ErrorEffect {
effect$ = createEffect(errorFirstSubscriber, {
useEffectsErrorHandler: true,
});
}

/**
* 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.
*/
function errorFirstSubscriber(): Observable<Action> {
return new Observable(observer => {
subscriptionCount++;

if (subscriptionCount === 1) {
observer.error(new Error('effectError'));
}
});
}
});
28 changes: 14 additions & 14 deletions modules/effects/spec/effects_metadata.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ describe('Effects metadata', () => {
@Effect({ dispatch: false })
c: any;
d = createEffect(() => of({ type: 'a' }), { dispatch: false });
@Effect({ dispatch: false, resubscribeOnError: false })
@Effect({ dispatch: false, useEffectsErrorHandler: false })
e: any;
z: any;
}

const mock = new Fixture();
const expected: EffectMetadata<Fixture>[] = [
{ propertyName: 'a', dispatch: true, resubscribeOnError: true },
{ propertyName: 'c', dispatch: false, resubscribeOnError: true },
{ propertyName: 'b', dispatch: true, resubscribeOnError: true },
{ propertyName: 'd', dispatch: false, resubscribeOnError: true },
{ propertyName: 'e', dispatch: false, resubscribeOnError: false },
{ propertyName: 'a', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'c', dispatch: false, useEffectsErrorHandler: true },
{ propertyName: 'b', dispatch: true, useEffectsErrorHandler: true },
{ propertyName: 'd', dispatch: false, useEffectsErrorHandler: true },
{ propertyName: 'e', dispatch: false, useEffectsErrorHandler: false },
];

expect(getSourceMetadata(mock)).toEqual(
Expand All @@ -45,20 +45,20 @@ describe('Effects metadata', () => {
e: any;
f = createEffect(() => of({ type: 'f' }), { dispatch: false });
g = createEffect(() => of({ type: 'g' }), {
resubscribeOnError: false,
useEffectsErrorHandler: false,
});
}

const mock = new Fixture();

expect(getEffectsMetadata(mock)).toEqual({
a: { dispatch: true, resubscribeOnError: true },
c: { dispatch: true, resubscribeOnError: true },
e: { dispatch: false, resubscribeOnError: true },
b: { dispatch: true, resubscribeOnError: true },
d: { dispatch: true, resubscribeOnError: true },
f: { dispatch: false, resubscribeOnError: true },
g: { dispatch: true, resubscribeOnError: false },
a: { dispatch: true, useEffectsErrorHandler: true },
c: { dispatch: true, useEffectsErrorHandler: true },
e: { dispatch: false, useEffectsErrorHandler: true },
b: { dispatch: true, useEffectsErrorHandler: true },
d: { dispatch: true, useEffectsErrorHandler: true },
f: { dispatch: false, useEffectsErrorHandler: true },
g: { dispatch: true, useEffectsErrorHandler: false },
});
});

Expand Down
2 changes: 1 addition & 1 deletion modules/effects/src/effect_creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type ObservableType<T, OriginalType> = T extends false ? OriginalType : Action;
* Creates an effect from an `Observable` and an `EffectConfig`.
*
* @param source A function which returns an `Observable`.
* @param config A `Partial<EffectConfig>` to configure the effect. By default, `dispatch` is true and `resubscribeOnError` is true.
* @param config A `Partial<EffectConfig>` to configure the effect. By default, `dispatch` is true and `useEffectsErrorHandler` is true.
* @returns If `EffectConfig`#`dispatch` is true, returns `Observable<Action>`. Else, returns `Observable<unknown>`.
*
* @usageNotes
Expand Down
Loading

0 comments on commit 3a9ad63

Please sign in to comment.