react-apollo-hooks: Rethrow apollo errors and recommend error boundaries
Right now errors are returned as a part of the useQuery result. Maybe a better idea would be to throw them and recommend to use Error Boundaries instead (at least in the suspense mode).
So instead of this code:
const Hello = () => {
const { data, error } = useQuery(GET_HELLO);
if (error) return `Error! ${error.message}`;
return <p>{data.hello.world}</p>;
};
we could write:
const Hello = () => {
// we can be sure here that the error hasn't occurred
return <p>{data.hello.world}</p>;
};
and use an error boundary at the root of the react tree (or anywhere else above the component) - similar to how we use the <Suspense /> component.
About this issue
- Original URL
- State: open
- Created 6 years ago
- Reactions: 18
- Comments: 24 (21 by maintainers)
I’d make it optional (but enabled by default) so if you wanted the old behavior you could use it like e. g.
const { data, error } = useQuery(MY_QUERY, { throw: false });.But the general idea is to make error management similar to what suspense does for data loading. So in most cases, it should be enough to put a single error boundary at the top of the React tree, but if you need it, you can also put it at the bottom, just above a component which requires a special behavior. Please compare:
and:
Of course, you can mix suspense and error boundaries. Please also note that you have access to the error instance inside of the error boundaries.
So I’d like support for error boundaries enabled by default, but not yet. As I said in https://github.com/trojanowski/react-apollo-hooks/issues/28#issuecomment-467191957 I’d like to have two hooks - the high-level one recommended for most cases (which will use suspense and error boundaries) and the second one - when you prefer to manage everything by yourself.
@FredyC - Would you mind sharing the
ApolloDefendercomponent?Well, generally you are better without actually throwing errors 😃 It’s a quick & dirty way, but also kinda error-prone (pun intended). I would say the ErrorBoundary should be considered more like a safety net, not the general way of handling errors.
A much better way, in my opinion, is to utilize the Context. You can simply expose yourself a
onErrorcallback to which you will pass any error you have. TheProvidercan similarly decide to render fallback UI, with the help ofReact.cloneElementit can even simulate the behavior of ErrorBoundary to remount if it’s desired. It definitely gives much better control. I made myself an abstraction that in essence looks like this.Note: ApolloDefender is proprietary for now
This is also a reason why I wanted to wrap the errors with additional information in #93, but @trojanowski rejected without giving it much of the thought seemingly. I have a custom fork that has it and it’s amazing. In the ApolloDefender I can see all information about a failed operation without any awkwardness.
FYI, I am having mixed luck with throwing an error synchronously and having the
ErrorBoundaryhandle it.useEffect()to be run again and throw a second error. The first error shows the error boundary fallback UI for a second and then the app crashes - not being able to handle the second error.It seems that the interactions between thrown errors and error boundaries are very finicky. So far my workaround is not to throw synchronous errors, but expose them as part of the API. The client component can check for these errors explicitly and show error messages. This seems to be much more stable.
Hm, but throwing errors are not related to Suspense, it’s about error boundaries which are stable. So I would stick to the original plan and make it default 😃 And btw, I tried to use
throwwith a newuseMutationand had a problem related it is reserved word. Something likeshouldThrowwould be more appropriate.Otherwise, I agree with high/low hooks, I already got something like that implemented because I was annoyed by checking for data and if it contains actual data. It works quite nicely.