Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failType not triggered for rejected promise #147

Open
ak99372 opened this issue Jun 3, 2019 · 3 comments
Open

failType not triggered for rejected promise #147

ak99372 opened this issue Jun 3, 2019 · 3 comments

Comments

@ak99372
Copy link

ak99372 commented Jun 3, 2019

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
@cesarp
Copy link

cesarp commented Aug 13, 2019

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.

@jeffbski
Copy link
Owner

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
  }
}); 

@ak99372
Copy link
Author

ak99372 commented Aug 15, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants