From 8d322d905b59839d5f6b9b1966963aab9f5d2584 Mon Sep 17 00:00:00 2001
From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Date: Wed, 1 Aug 2018 12:50:03 +0200
Subject: [PATCH] feat(Effects): stringify action on invalid action Message
 before: ERROR Error: Effect "AuthEffects.login$" dispatched an invalid
 action: [object Object] Message after: ERROR Error: Effect
 "AuthEffects.login$" dispatched an invalid action:
 [{"payload":{"user":{"name":"User"}},"type":"[Auth] Login Success"}]

---
 modules/effects/spec/effect_sources.spec.ts | 61 ++++++++++++++++++++-
 modules/effects/src/effect_notification.ts  | 10 +++-
 2 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/modules/effects/spec/effect_sources.spec.ts b/modules/effects/spec/effect_sources.spec.ts
index 9aad457d19..a30cac450b 100644
--- a/modules/effects/spec/effect_sources.spec.ts
+++ b/modules/effects/spec/effect_sources.spec.ts
@@ -35,6 +35,13 @@ describe('EffectSources', () => {
     const b = { type: 'From Source B' };
     const c = { type: 'From Source C that completes' };
     const d = { not: 'a valid action' };
+    const e = undefined;
+    const f = null;
+
+    let circularRef = {} as any;
+    circularRef.circularRef = circularRef;
+    const g = { circularRef };
+
     const error = new Error('An Error');
 
     class SourceA {
@@ -54,10 +61,22 @@ describe('EffectSources', () => {
     }
 
     class SourceE {
-      @Effect() e$ = throwError(error);
+      @Effect() e$ = alwaysOf(e);
+    }
+
+    class SourceF {
+      @Effect() f$ = alwaysOf(f);
     }
 
     class SourceG {
+      @Effect() g$ = alwaysOf(g);
+    }
+
+    class SourceError {
+      @Effect() e$ = throwError(error);
+    }
+
+    class SourceH {
       @Effect() empty = of('value');
       @Effect()
       never = timer(50, getTestScheduler() as any).pipe(map(() => 'update'));
@@ -90,12 +109,48 @@ describe('EffectSources', () => {
 
       toActions(sources$).subscribe();
 
-      expect(mockErrorReporter.handleError).toHaveBeenCalled();
+      expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
+        new Error(
+          'Effect "SourceD.d$" dispatched an invalid action: {"not":"a valid action"}'
+        )
+      );
+    });
+
+    it('should report an error if an effect dispatches an `undefined`', () => {
+      const sources$ = of(new SourceE());
+
+      toActions(sources$).subscribe();
+
+      expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
+        new Error('Effect "SourceE.e$" dispatched an invalid action: undefined')
+      );
+    });
+
+    it('should report an error if an effect dispatches a `null`', () => {
+      const sources$ = of(new SourceF());
+
+      toActions(sources$).subscribe();
+
+      expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
+        new Error('Effect "SourceF.f$" dispatched an invalid action: null')
+      );
+    });
+
+    it(`should not break when the action in the error message can't be stringified`, () => {
+      const sources$ = of(new SourceG());
+
+      toActions(sources$).subscribe();
+
+      expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
+        new Error(
+          'Effect "SourceG.g$" dispatched an invalid action: [object Object]'
+        )
+      );
     });
 
     it('should not complete the group if just one effect completes', () => {
       const sources$ = cold('g', {
-        g: new SourceG(),
+        g: new SourceH(),
       });
       const expected = cold('a----b-----', { a: 'value', b: 'update' });
 
diff --git a/modules/effects/src/effect_notification.ts b/modules/effects/src/effect_notification.ts
index 1d33bab58f..439166b9b6 100644
--- a/modules/effects/src/effect_notification.ts
+++ b/modules/effects/src/effect_notification.ts
@@ -37,7 +37,7 @@ function reportInvalidActions(
         new Error(
           `Effect ${getEffectName(
             output
-          )} dispatched an invalid action: ${action}`
+          )} dispatched an invalid action: ${stringify(action)}`
         )
       );
     }
@@ -57,3 +57,11 @@ function getEffectName({
 
   return `"${sourceName}.${propertyName}${isMethod ? '()' : ''}"`;
 }
+
+function stringify(action: Action | null | undefined) {
+  try {
+    return JSON.stringify(action);
+  } catch {
+    return action;
+  }
+}