fix error swallowing for cmd.run when effect has an error

redux-loop · Aug 26, 2018 · a0e7721 · a0e7721
1 parent b767df7
commit a0e7721
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 4 deletions.
diff --git a/src/cmd.js b/src/cmd.js
@@ -32,14 +32,15 @@ function handleRunCmd(cmd, dispatch, getState){
     let result = cmd.func(...getMappedCmdArgs(cmd.args, dispatch, getState))
 
     if (isPromiseLike(result) && !cmd.forceSync){
-      return result.then(onSuccess, onFail).then(action => {
-        return action ? [action] : [];
+      return result.then( onSuccess, (error) => {
+        console.error(error);
+        return onFail(error);
       })
+      .then(action => action ? [action] : [])
     }
     let resultAction = onSuccess(result);
     return resultAction ? Promise.resolve([resultAction]) : null;
-  }
-  catch(err){
+  } catch(err){
     if(!cmd.failActionCreator){
       console.error(err);
       throw err //don't swallow errors if they are not handling them

diff --git a/test/cmd.spec.js b/test/cmd.spec.js
@@ -45,11 +45,14 @@ describe('Cmds', () => {
         });
 
         it('resolves with an empty array if the function returns a rejected promise', async () => {
+          let consoleErr = jest.spyOn(console, 'error').mockImplementation(() => {});
           sideEffect.mockReturnValueOnce(Promise.reject(123));
           let cmd = Cmd.run(sideEffect);
           let result = executeCmd(cmd, dispatch, getState);
           expect(sideEffect.mock.calls.length).toBe(1);
           await expect(result).resolves.toEqual([]);
+          expect(consoleErr).toHaveBeenCalled();
+          consoleErr.mockRestore();
         });
 
         it('rethrows the thrown error if the function throws', function(){
@@ -58,6 +61,15 @@ describe('Cmds', () => {
           sideEffect.mockImplementationOnce(() => {throw err});
           let cmd = Cmd.run(sideEffect);
           expect(() => executeCmd(cmd, dispatch, getState)).toThrow(err);
+          consoleErr.mockRestore();
+        });
+
+        it('calls console.error if the sideEffect has an error instead of swallowing the error completely', async function(){
+          let consoleErr = jest.spyOn(console, 'error').mockImplementation(() => {});
+          let err = new Error("foo")
+          sideEffect.mockReturnValueOnce(Promise.reject(err));
+          let cmd = Cmd.run(sideEffect);
+          await executeCmd(cmd, dispatch, getState)
           expect(consoleErr).toHaveBeenCalledWith(err);
           consoleErr.mockRestore();
         });
@@ -131,6 +143,7 @@ describe('Cmds', () => {
         });
 
         it('runs the rejection value (for promises) through the fail handler and resolves with it in an array', async () => {
+          let consoleErr = jest.spyOn(console, 'error').mockImplementation(() => {});
           sideEffect.mockReturnValueOnce(Promise.reject(123));
           let cmd = Cmd.run(sideEffect, {
             successActionCreator: actionCreator1,
@@ -139,6 +152,7 @@ describe('Cmds', () => {
 
           let result = executeCmd(cmd, dispatch, getState);
           await expect(result).resolves.toEqual([actionCreator2(123)]);
+          consoleErr.mockRestore();
         });
       });
     });
@@ -153,12 +167,16 @@ describe('Cmds', () => {
     });
 
     describe('Cmd.list', () => {
+      let error;
+
       beforeEach(() => {
         jest.useFakeTimers();
+        error = jest.spyOn(console, 'error').mockImplementation(() => {});
       });
 
       afterEach(() => {
         jest.useRealTimers();
+        error.mockRestore();
       });
 
       describe('when sequence is false', () => {