refactor(Effects): Implement ErrorReporter as an ErrorHandler (#635)

Closes #626
ngrx · Dec 13, 2017 · c3d46a8 · c3d46a8
1 parent c642a62
commit c3d46a8
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 25 deletions.
diff --git a/modules/effects/spec/effect_sources.spec.ts b/modules/effects/spec/effect_sources.spec.ts
@@ -32,7 +32,7 @@ describe('EffectSources', () => {
     mockErrorReporter = TestBed.get(ErrorReporter);
     effectSources = TestBed.get(EffectSources);
 
-    spyOn(mockErrorReporter, 'report');
+    spyOn(mockErrorReporter, 'handleError');
   });
 
   it('should have an "addEffects" method to push new source instances', () => {
@@ -103,7 +103,7 @@ describe('EffectSources', () => {
 
       toActions(sources$).subscribe();
 
-      expect(mockErrorReporter.report).toHaveBeenCalled();
+      expect(mockErrorReporter.handleError).toHaveBeenCalled();
     });
 
     it('should not complete the group if just one effect completes', () => {

diff --git a/modules/effects/src/effect_notification.ts b/modules/effects/src/effect_notification.ts
@@ -1,7 +1,11 @@
 import { Observable } from 'rxjs/Observable';
 import { Notification } from 'rxjs/Notification';
 import { Action } from '@ngrx/store';
-import { ErrorReporter } from './error_reporter';
+import {
+  ErrorReporter,
+  EffectError,
+  InvalidActionError,
+} from './error_reporter';
 
 export interface EffectNotification {
   effect: Observable<any> | (() => Observable<any>);
@@ -24,14 +28,16 @@ function reportErrorThrown(
   reporter: ErrorReporter
 ) {
   if (output.notification.kind === 'E') {
-    const errorReason = `Effect ${getEffectName(output)} threw an error`;
+    const errorReason = new Error(
+      `Effect ${getEffectName(output)} threw an error`
+    ) as EffectError;
 
-    reporter.report(errorReason, {
-      Source: output.sourceInstance,
-      Effect: output.effect,
-      Error: output.notification.error,
-      Notification: output.notification,
-    });
+    errorReason.Source = output.sourceInstance;
+    errorReason.Effect = output.effect;
+    errorReason.Error = output.notification.error;
+    errorReason.Notification = output.notification;
+
+    reporter.handleError(errorReason);
   }
 }
 
@@ -44,16 +50,16 @@ function reportInvalidActions(
     const isInvalidAction = !isAction(action);
 
     if (isInvalidAction) {
-      const errorReason = `Effect ${getEffectName(
-        output
-      )} dispatched an invalid action`;
+      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.report(errorReason, {
-        Source: output.sourceInstance,
-        Effect: output.effect,
-        Dispatched: action,
-        Notification: output.notification,
-      });
+      reporter.handleError(errorReason);
     }
   }
 }

diff --git a/modules/effects/src/error_reporter.ts b/modules/effects/src/error_reporter.ts
@@ -1,15 +1,37 @@
-import { Injectable, InjectionToken, Inject } from '@angular/core';
+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<any> | (() => Observable<any>);
+  Error: any;
+  Notification: Notification<Action | null | undefined>;
+}
+
+export interface InvalidActionError extends Error {
+  Source: any;
+  Effect: Observable<any> | (() => Observable<any>);
+  Dispatched: Action | null | undefined;
+  Notification: Notification<Action | null | undefined>;
+}
+
 @Injectable()
-export class ErrorReporter {
+export class ErrorReporter implements ErrorHandler {
   constructor(@Inject(CONSOLE) private console: any) {}
 
-  report(reason: string, details: any): void {
-    this.console.group(reason);
+  handleError(error: any): void {
+    this.console.group(error.message);
 
-    for (let key in details) {
-      this.console.error(`${key}:`, details[key]);
+    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
@@ -10,4 +10,5 @@ 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';