redux-logic icon indicating copy to clipboard operation
redux-logic copied to clipboard

failType not triggered for rejected promise

Open ak99372 opened this issue 5 years ago • 3 comments

I went over the API documentation and the setup of failType seems straight forward yet I can't make it work with async logic...

const myLogic= createLogic({
 type: "FETCH",
 processOptions: {
    failType: "FETCH_ERROR"
  },
 process({ getState }, dispatch, done) {
    return new Promise((resolve, reject) => {
      throw new Error("This is error inside promise");
    });
  }
}); 

I also read that "The process hook is run asynchronously" so I should be able to write it like this (though the result is the same)

async process({ getState }, dispatch, done) {
      throw new Error("This is error inside promise");
  }

My issues:

  • the failType action never gets triggered
  • I get "Possible Unhandled Promise Rejection" warning and "forget to call done()?" error

ak99372 avatar Jun 03 '19 19:06 ak99372

If you want to automatically dispatch then you need to set the dispatchReturn option to true, set a success action type too and remove dispatch and done from the process arguments.

cesarp avatar Aug 13 '19 01:08 cesarp

Thanks for responding @cesarp.

Yes, @ak99372 when using the original callback mode (with process(deps, dispatch, done) signature) then successType and failType decorate things that are dispatched, so dispatch(obj) would be decorated by successType if it was defined and dispatch(err) would be decorated by failType if it was specified. You still call done() when you are finished.

The newer async auto dispatch mode (using the process(deps) signature or by setting dispatchReturn to true) makes the result dispatched with the result being decorated by successType or failType depending on whether it was an error. This works with sync returns as well as returning a promise or observable. A reject would use the failType decorator if specified. You can also use async process(deps) signature to take advantage of async/await. Throwing from within an async function will be caught and rejected, so failType would also pick it up.

So you can switch into this mode by omitting the extra arguments (dispatch and done) or by specifying dispatchReturn option to true. I usually just omit the arguments so it is clear I don't need to do anything with them.

You code would look like this

const myLogic= createLogic({
 type: "FETCH",
 processOptions: {
    failType: "FETCH_ERROR"
  },
 process({ getState }) {
    const promise = fnThatReturnsPromise();
    return promise;
  }
}); 

OR using async/await style

const myLogic= createLogic({
 type: "FETCH",
 processOptions: {
    failType: "FETCH_ERROR"
  },
 async process({ getState }) {
    const result = await fnThatReturnsPromise();
    return result;
  }
}); 

And if you had thrown this would properly be decorated by failType

const myLogic= createLogic({
 type: "FETCH",
 processOptions: {
    failType: "FETCH_ERROR"
  },
 async process({ getState }) {
    throw new Error('myerror'); // will be decorated by failType
  }
}); 

jeffbski avatar Aug 14 '19 21:08 jeffbski

Thank you both for reply and indeed adding dispatchReturn to options now dispatches the failType action. Few related comments:

  • it's not very obvious that these two are even related and (without understanding how things work under the hood) I would expect exception inside process to trigger fail action (regardless whether return is dispatched or not). Maybe there is other reason (than implementation)?
  • other way around is it even possible (in my original example) for the failType to be triggered? (and if not maybe a warning in console when this option is set but can't occur would be helpful)
  • I read about the (3) modes in docs but the fact that (exception handling) behavior is different based on number of arguments is just trouble waiting to happen. We have a lot of logic that uses dispatch but as requirements change over the time it is possible that the same logic no longer needs dispatch (or to dispatch return) but I don't think any developer expects exception to be handled differently (when optional arguments are removed) especially when options is where people focus on to set the behavior.

Hope that helps with some usability feedback.

ak99372 avatar Aug 15 '19 07:08 ak99372