From 913558e65c3e45a38acdfe6f073534720f985c70 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 7 Dec 2017 00:10:32 -0800 Subject: [PATCH] feat(ErrorHandler): use the angular ErrorHandler for reporting errors Fixes #626 BREAKING CHANGE: Errors are now reported to the @angular/core ErrorHandler. Errors are no longer reported to the console. --- modules/effects/spec/effect_sources.spec.ts | 18 +++------ modules/effects/src/effect_notification.ts | 43 ++++++--------------- modules/effects/src/effect_sources.ts | 7 ++-- modules/effects/src/effects_module.ts | 12 +----- modules/effects/src/error_reporter.ts | 39 ------------------- modules/effects/src/index.ts | 1 - modules/effects/src/tokens.ts | 1 - 7 files changed, 21 insertions(+), 100 deletions(-) delete mode 100644 modules/effects/src/error_reporter.ts diff --git a/modules/effects/spec/effect_sources.spec.ts b/modules/effects/spec/effect_sources.spec.ts index 77912301a5..9ca476baf9 100644 --- a/modules/effects/spec/effect_sources.spec.ts +++ b/modules/effects/spec/effect_sources.spec.ts @@ -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'); @@ -118,7 +110,7 @@ describe('EffectSources', () => { }); function toActions(source: any): Observable { - source['errorReporter'] = mockErrorReporter; + source['errorHandler'] = mockErrorReporter; return effectSources.toActions.call(source); } }); diff --git a/modules/effects/src/effect_notification.ts b/modules/effects/src/effect_notification.ts index ee71cc315d..ce7067a6a4 100644 --- a/modules/effects/src/effect_notification.ts +++ b/modules/effects/src/effect_notification.ts @@ -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 | (() => Observable); @@ -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 + }` + ) + ); } } } diff --git a/modules/effects/src/effect_sources.ts b/modules/effects/src/effect_sources.ts index 1e68d23491..26f76f7f72 100644 --- a/modules/effects/src/effect_sources.ts +++ b/modules/effects/src/effect_sources.ts @@ -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 { - constructor(private errorReporter: ErrorReporter) { + constructor(private errorHandler: ErrorHandler) { super(); } @@ -37,7 +36,7 @@ export class EffectSources extends Subject { map.call( exhaustMap.call(source$, resolveEffectSource), (output: EffectNotification) => { - verifyOutput(output, this.errorReporter); + verifyOutput(output, this.errorHandler); return output.notification; } diff --git a/modules/effects/src/effects_module.ts b/modules/effects/src/effects_module.ts index 90d09e1b9e..32de116bc4 100644 --- a/modules/effects/src/effects_module.ts +++ b/modules/effects/src/effects_module.ts @@ -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 { @@ -30,7 +29,6 @@ export class EffectsModule { providers: [ EffectsRunner, EffectSources, - ErrorReporter, Actions, rootEffects, { @@ -38,10 +36,6 @@ export class EffectsModule { deps: rootEffects, useFactory: createSourceInstances, }, - { - provide: CONSOLE, - useFactory: getConsole, - }, ], }; } @@ -50,7 +44,3 @@ export class EffectsModule { export function createSourceInstances(...instances: any[]) { return instances; } - -export function getConsole() { - return console; -} diff --git a/modules/effects/src/error_reporter.ts b/modules/effects/src/error_reporter.ts deleted file mode 100644 index da974906b0..0000000000 --- a/modules/effects/src/error_reporter.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { - ErrorHandler, - Injectable, - InjectionToken, - Inject, -} from '@angular/core'; -import { Observable } from 'rxjs/Observable'; -import { Notification } from 'rxjs/Notification'; -import { Action } from '@ngrx/store'; -import { CONSOLE } from './tokens'; - -export interface EffectError extends Error { - Source: any; - Effect: Observable | (() => Observable); - Error: any; - Notification: Notification; -} - -export interface InvalidActionError extends Error { - Source: any; - Effect: Observable | (() => Observable); - Dispatched: Action | null | undefined; - Notification: Notification; -} - -@Injectable() -export class ErrorReporter implements ErrorHandler { - constructor(@Inject(CONSOLE) private console: any) {} - - handleError(error: any): void { - this.console.group(error.message); - - for (let key in error) { - this.console.error(`${key}:`, error[key]); - } - - this.console.groupEnd(); - } -} diff --git a/modules/effects/src/index.ts b/modules/effects/src/index.ts index ed6868bd77..7f833ae971 100644 --- a/modules/effects/src/index.ts +++ b/modules/effects/src/index.ts @@ -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'; diff --git a/modules/effects/src/tokens.ts b/modules/effects/src/tokens.ts index 330fd8da68..b4bc66d18d 100644 --- a/modules/effects/src/tokens.ts +++ b/modules/effects/src/tokens.ts @@ -9,4 +9,3 @@ export const ROOT_EFFECTS = new InjectionToken[]>( export const FEATURE_EFFECTS = new InjectionToken( 'ngrx/effects: Feature Effects' ); -export const CONSOLE = new InjectionToken('Browser Console');