search

galhavivi / cofi

JavaScript Form Solutions
https://galhavivi.github.io/cofi
Other
2 stars 1 forks source link

if side effects of action throw error, state of model will be wrong #7

Closed WangLarry closed 3 years ago

WangLarry commented 3 years ago

Describe the bug In processQueue function, catch error of actions, then run following codes. It means that side effects are not ran totally, state is not complete.

To Reproduce In custom validator function, throw error or timeout.

galhavivi commented 3 years ago

Thank you for sharing love ☺️

True. I guess it’s a normal javascript behavior. You get notified by error that the action has failed (usually means that you need to fix your handlers or side effects which you supplied to cofi), and its only normal that it won’t continue to the side effects like “afterChangeValue” if changeValue for example has failed.

I think that if we do run the effects if the action fails, it can cause unexpected behavior in the system that uses cofi, since it expects the afterChangeValue to run after the value has changed successfully.

Do you have an alternative way to handle it?

Perhaps we can add a note in the error which says that side effects didn’t complete? WDYT?

WangLarry commented 3 years ago

MDN web docs say: The Promise.all() method takes an iterable of promises as an input, and returns a single Promise that resolves to an array of the results of the input promises. This returned promise will resolve when all of the input's promises have resolved, or if the input iterable contains no promises. It rejects immediately upon any of the input promises rejecting or non-promises throwing an error, and will reject with this first rejection message / error.

In comparison, the Promise returned by Promise.all() may be more appropriate if the tasks are dependent on each other / if you'd like to immediately reject upon any of them rejecting.

It means that one of promise is failed, others promise will stop immediately. So the result is not just ignore this promise, the state is unexpected totally.

WangLarry commented 3 years ago

The common scene: async validator throw error or timeout.

Maybe we can just deal this scene at first.

WangLarry commented 3 years ago

One method: Before run action, store the snapshot of current state. Then run actions and optimistic update state. If throw. error in actions, restore the snapshot in catch block, report errors to user, continue run following actions.

galhavivi commented 3 years ago

@WangLarry Great solution, Thank you :) Please check out the open PR that is link to this issue, and let me know what do you think.