-
Notifications
You must be signed in to change notification settings - Fork 120
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
Reject after dispatching error action. #13
Reject after dispatching error action. #13
Conversation
Is this going to be merged? |
error => { | ||
dispatch({ ...action, payload: error, error: true }); | ||
return Promise.reject(error); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's one simpler way that is returning the original promise (action.payload
) instead of returning the result of action.payload.then()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't that result in a race condition where the user-attached .then
can run before the dispatch
call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kovensky No, javascript is single threaded. Promise handlers are executed in the order they are added.
I agree with @Qrysto that I think we need to branch off the original promise to dispatch the actions, but ultimately return the original promise from I think what's happening here is that the middleware is essentially inserting a There's a related issue here: #23 |
I also stumbled upon this issue, and would really appreciate to see this resolved. Any plans on merging this? |
Is @acdlite monitoring this pull request? We are running into the same issues the OP has and this would definitely solve those issues. |
Please create a new npm version with this code! :-) |
This is a big breaking change and I'm not sure why it was needed - the reducer or next middleware in the chain can easily throw back all error-actions. With this change code that relies on rejections being transformed to error-actions and dispatched onward is broken and needs to add an additional handler for the new rejection in addition to the error-action handling. It also takes away from the next middleware the ability to affect the return value of the dispatch. Is it possible to at least add a configuration option to keep it the way it was? |
The current behaviour might be intentional but I ran into an issue this evening where calling
then
on a dispatched promise would always result in being fulfilled:This PR will reject the error after dispatching the action.