Errors thrown reducing the RECEIVE_TYPE action using useFetcher are swallowed.

[praxxis]

React version (e.g., 16.8.5) 16/17

Concurrent mode no

Rest Hooks version (e.g., 4.5.9) 5.0.0-k.*

Describe the bug
If an error occurs while processing the RECEIVE_TYPE action type the promise returned by useFetcher is not rejected. An example of how this could occur would be an updater function that throws an error.

Interestingly it looks like there's already a question in the code on whether this bug occurs. Errors are caught, but they're returned, not thrown, by the reducer.

To Reproduce

1. Load up https://codesandbox.io/s/modest-cache-m2frd?file=/src/App.js
2. Click the "add" button and note that a new entry is added to the list.
3. Uncomment the error on line 8 (// throw new Error("oh no");) and reload.
4. Click the "add" button and note that the server request succeeds, but nothing is appended to the list, and no error is thrown.

Expected behavior
Some sort of error should be observable, ideally by having the useFetcher promise reject.

Additional context
We discovered this bug as a cross browser issue (object.fromEntities does not exist in Safari 11 and we were baffled by items not being added to our lists along with there being no errors), but were able to create a more generic reproduction that could occur with any code base.

[ntucker]

Thanks for the report and detailed description!

[ntucker]

The design here seems related to #458. So we can probably resolve these together.

The handling of errors on reducer that you linked to places them inside the state. (You can observe this by inspecting the state.) For declarative binds, this means the components would find the error (https://github.com/coinbase/rest-hooks/blob/master/packages/core/src/react-integration/hooks/useError.ts).

So essentially there is currently a distinction between fully processing on the promise returned from these promises. In #458 this means we didn't run all the denormalization stuff, and in this case we didn't handle errors from the reducer.

It looks like probably the best design is for the promises instead of focusing on resolving the fetch, they resolve the entire update into the store.

In this case, we would want to await the promise returned from dispatch() (which means react updated), then look for errors (to handle this issue), and also resolve if none are found to handle #458.

@praxxis thumbs up if this design seems like a good resolution

[praxxis]

Sounds good to me. We've also seen the behaviour described in #458 (returning a POJO) and assumed it was by design, so I'll be following along there too.

[ntucker]

Tracking solution in #760