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)

Most upvoted comments

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:

render(
  <Suspense fallback={<GlobalLoadingFallback />}>
    <MyPageComponent>
      <PageDataComponent />
      <Suspense fallback={<ImageLoadingFallback />}>
        <MyImageComponent />
      </Suspense>
    </MyPageComponent>
  </Suspense>
);

and:

// a helper component but you can also easily implement it by yourself
import { ErrorBoundary } from 'react-error-boundary';

render(
  <ErrorBoundary FallbackComponent={GlobalError}>
    <MyPageComponent>
      <PageDataComponent />
      <ErrorBoundary FallbackComponent={ImageError}>
        <MyImageComponent />
      </ErrorBoundary>
    </MyPageComponent>
  </ErrorBoundary>
);

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 ApolloDefender component?

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 onError callback to which you will pass any error you have. The Provider can similarly decide to render fallback UI, with the help of React.cloneElement it 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.

    <ApolloDefender
      onNetworkError={onNetworkError}
      onUserErrors={onUserErrors}
      onOperationError={onOperationError}
    >
      <SillyErrorBoundary onError={onUnhandledError}>
        {render()}
      </SillyErrorBoundary>
    </ApolloDefender>

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 ErrorBoundary handle it.

  1. In this custom hook, throwing a synchronous error works perfectly. It is caught by the ErrorBoundary and I am seeing the fallback UI.
  2. When using a MobX store, if I check for an error in the store and throw it synchronously, I get “Reaction already disposed” from MobX followed by “Rendered fewer hooks than expected” from React. (see this commit where I reverted the code). I spent half a day on this - I am sure all my hooks are at the top level and not hiding in any conditionals. I suspect that in this case MobX is rerendering the component while ErrorBoundary is also trying to render the fallback UI.
  3. In another case (a private project), the component is being rerendered as soon as I throw a synchronous error - I have no idea why - its state has not changed! This causes 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.

In the short term, I’ll add a throw option to useQuery. It will be disabled by default - I think it makes the most sense if used together with suspense - which right now is marked as experimental in docs.

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 throw with a new useMutation and had a problem related it is reserved word. Something like shouldThrow would 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.

  const { data, loading, error, ...rest } = Hooks.useQuery<TData, TVariables>(query, options)
  if (error) {
    throw error
  }
  return {
    loading: loading || !data || isEmpty(data),
    data: data as TData,
    ...rest,
  }