Skip to content

Commit

Permalink
feat(ErrorHandler): Use the Angular ErrorHandler for reporting errors (
Browse files Browse the repository at this point in the history
…#667)

Closes #626

BREAKING CHANGE: The ErrorReporter has been replaced with ErrorHandler
from angular/core.

BEFORE:

Errors were reported to the ngrx/effects ErrorReporter. The
ErrorReporter would log to the console by default.

AFTER:

Errors are now reported to the @angular/core ErrorHandler.
  • Loading branch information
jbedard authored and brandonroberts committed Jan 9, 2018
1 parent 77c832a commit 8f297d1
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 100 deletions.
18 changes: 5 additions & 13 deletions modules/effects/spec/effect_sources.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,19 @@ import { _throw } from 'rxjs/observable/throw';
import { never } from 'rxjs/observable/never';
import { empty } from 'rxjs/observable/empty';
import { TestBed } from '@angular/core/testing';
import { ErrorReporter } from '../src/error_reporter';
import { CONSOLE } from '../src/tokens';
import { ErrorHandler } from '@angular/core';
import { Effect, EffectSources } from '../';

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

beforeEach(() => {
TestBed.configureTestingModule({
providers: [
EffectSources,
ErrorReporter,
{
provide: CONSOLE,
useValue: console,
},
],
providers: [EffectSources],
});

mockErrorReporter = TestBed.get(ErrorReporter);
mockErrorReporter = TestBed.get(ErrorHandler);
effectSources = TestBed.get(EffectSources);

spyOn(mockErrorReporter, 'handleError');
Expand Down Expand Up @@ -118,7 +110,7 @@ describe('EffectSources', () => {
});

function toActions(source: any): Observable<any> {
source['errorReporter'] = mockErrorReporter;
source['errorHandler'] = mockErrorReporter;
return effectSources.toActions.call(source);
}
});
Expand Down
43 changes: 12 additions & 31 deletions modules/effects/src/effect_notification.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { Observable } from 'rxjs/Observable';
import { Notification } from 'rxjs/Notification';
import { Action } from '@ngrx/store';
import {
ErrorReporter,
EffectError,
InvalidActionError,
} from './error_reporter';
import { ErrorHandler } from '@angular/core';

export interface EffectNotification {
effect: Observable<any> | (() => Observable<any>);
Expand All @@ -17,49 +13,34 @@ export interface EffectNotification {

export function verifyOutput(
output: EffectNotification,
reporter: ErrorReporter
reporter: ErrorHandler
) {
reportErrorThrown(output, reporter);
reportInvalidActions(output, reporter);
}

function reportErrorThrown(
output: EffectNotification,
reporter: ErrorReporter
) {
function reportErrorThrown(output: EffectNotification, reporter: ErrorHandler) {
if (output.notification.kind === 'E') {
const errorReason = new Error(
`Effect ${getEffectName(output)} threw an error`
) as EffectError;

errorReason.Source = output.sourceInstance;
errorReason.Effect = output.effect;
errorReason.Error = output.notification.error;
errorReason.Notification = output.notification;

reporter.handleError(errorReason);
reporter.handleError(output.notification.error);
}
}

function reportInvalidActions(
output: EffectNotification,
reporter: ErrorReporter
reporter: ErrorHandler
) {
if (output.notification.kind === 'N') {
const action = output.notification.value;
const isInvalidAction = !isAction(action);

if (isInvalidAction) {
const errorReason = new Error(
`Effect ${getEffectName(output)} dispatched an invalid action`
) as InvalidActionError;

errorReason.Source = output.sourceInstance;
errorReason.Effect = output.effect;
errorReason.Dispatched = action;
errorReason.Notification = output.notification;

reporter.handleError(errorReason);
reporter.handleError(
new Error(
`Effect ${getEffectName(output)} dispatched an invalid action: ${
action
}`
)
);
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions modules/effects/src/effect_sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@ import { concat } from 'rxjs/observable/concat';
import { Observable } from 'rxjs/Observable';
import { Subject } from 'rxjs/Subject';
import { Notification } from 'rxjs/Notification';
import { Injectable } from '@angular/core';
import { ErrorHandler, Injectable } from '@angular/core';
import { Action } from '@ngrx/store';
import { EffectNotification, verifyOutput } from './effect_notification';
import { getSourceForInstance } from './effects_metadata';
import { resolveEffectSource } from './effects_resolver';
import { ErrorReporter } from './error_reporter';

@Injectable()
export class EffectSources extends Subject<any> {
constructor(private errorReporter: ErrorReporter) {
constructor(private errorHandler: ErrorHandler) {
super();
}

Expand All @@ -37,7 +36,7 @@ export class EffectSources extends Subject<any> {
map.call(
exhaustMap.call(source$, resolveEffectSource),
(output: EffectNotification) => {
verifyOutput(output, this.errorReporter);
verifyOutput(output, this.errorHandler);

return output.notification;
}
Expand Down
12 changes: 1 addition & 11 deletions modules/effects/src/effects_module.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { NgModule, ModuleWithProviders, Type } from '@angular/core';
import { EffectSources } from './effect_sources';
import { Actions } from './actions';
import { ROOT_EFFECTS, FEATURE_EFFECTS, CONSOLE } from './tokens';
import { ROOT_EFFECTS, FEATURE_EFFECTS } from './tokens';
import { EffectsFeatureModule } from './effects_feature_module';
import { EffectsRootModule } from './effects_root_module';
import { EffectsRunner } from './effects_runner';
import { ErrorReporter } from './error_reporter';

@NgModule({})
export class EffectsModule {
Expand All @@ -30,18 +29,13 @@ export class EffectsModule {
providers: [
EffectsRunner,
EffectSources,
ErrorReporter,
Actions,
rootEffects,
{
provide: ROOT_EFFECTS,
deps: rootEffects,
useFactory: createSourceInstances,
},
{
provide: CONSOLE,
useFactory: getConsole,
},
],
};
}
Expand All @@ -50,7 +44,3 @@ export class EffectsModule {
export function createSourceInstances(...instances: any[]) {
return instances;
}

export function getConsole() {
return console;
}
39 changes: 0 additions & 39 deletions modules/effects/src/error_reporter.ts

This file was deleted.

1 change: 0 additions & 1 deletion modules/effects/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@ export { EffectSources } from './effect_sources';
export { OnRunEffects } from './on_run_effects';
export { toPayload } from './util';
export { EffectNotification } from './effect_notification';
export { ErrorReporter } from './error_reporter';
export { ROOT_EFFECTS_INIT } from './effects_root_module';
1 change: 0 additions & 1 deletion modules/effects/src/tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,3 @@ export const ROOT_EFFECTS = new InjectionToken<Type<any>[]>(
export const FEATURE_EFFECTS = new InjectionToken<any[][]>(
'ngrx/effects: Feature Effects'
);
export const CONSOLE = new InjectionToken<Console>('Browser Console');

0 comments on commit 8f297d1

Please sign in to comment.