onError prop does not fire when errorPolicy is set to all

[glynkwei]

See https://github.com/apollographql/apollo-client/blob/905001e40cfadf841aa5a78aa428ab09f8cdf108/packages/apollo-client/src/core/ObservableQuery.ts#L215.

If fetchPolicy is set to all, then the errors property is set instead of the error property. This should be accounted for below:

react-apollo/src/Query.tsx

Line 241 in 1e0bdf7

const { loading, error, data } = currentResult;

[joelgetaction]

I am also seeing this and I just spent hours debugging this. The Mutation.onError method was NOT being called even though the render prop was getting the error. Very frustrating. For now I have set the error policy to none to get this working, but I'd prefer to use all but can't until this bug is fixed.

Hey @peggyrayzis is there an ETA for a fix on this? Thanks!

[t3dotgg]

Also in the boat of "hours debugging this". If nobody's picking it up I might give it a go myself

[hwillson]

Happy to review a PR for this - thanks!

[astorije]

Hey @theobr, did you happen to make progress on this?

Otherwise, any suggested alternative to this in the meantime?
We are using onError all over the place, and recently switching to errorPolicy="all" has broken error management. We can't use error within the render function either, because the function it calls sets some state, which upsets React.

Thanks!

[astorije]

Related: #2374

[amacleay]

FWIW, this behavior is entirely my fault (well, I guess @hwillson may share a little bit of the blame).

It seems to me as though this requires a little bit of thought:

• The if you don't pass an onError, the default behavior is to raise an exception (in other words, the default onError is throw e)
• As currently implemented, onError is mutually exclusive with the errors render prop: one or the other is going to get the data

My opinion (stated in that original PR) is that it feels like a bad idea to call onError (which defaults to raising an unhandled exception) when receiving the errors prop because the idea is that you are handling the errors through the render prop.

@astorije has also pointed out that this is problematic if you need to set some state based on errors, because mutating state in a render prop is a no-no, so the current behavior is unsatisfactory for cases where you are setting state based on errors.

Is there another approach we're missing? Maybe a teeError optional callback to uncondiitonally receive errors, even if they are in the render props?

[astorije]

@amacleay, an alternative could be to always call onError (even with the errors prop), but not raise an exception when errorPolicy="all".

[hwillson]

React Apollo has been refactored to use React Hooks behind the scenes for everything, which means a lot has changed since this issue was opened (and a lot of outstanding issues have been resolved). We'll close this issue off, but please let us know if you're still encountering this problem using React Apollo >= 3.1.0. Thanks!

[audiolion]

@hwillson this problem still persists with version >= 3.1.0

To recap with an example:

const result = useQuery<TData, TVariables>(query, {
    fetchPolicy: 'cache-and-network',
    errorPolicy: 'all',
    ...options,
    onError: error => {
      const { graphQLErrors } = error;
      const isUnauthenticated = graphQLErrors.find(
        error => !!error.extensions && error.extensions.code === 'UNAUTHENTICATED'
      );
      if (isUnauthenticated) {
        navigation.navigate('AuthLoading');
      }
    },
  });

With an errorPolicy: 'all', the onError is never fired. So the solution would be to use the error returned as part of result in the useQuery, however, now we are in a situation where I am trying to set state or do a navigation transition in the render which causes issues.

I suppose for now I can do some sort of workaround like:

const result = useQuery<TData, TVariables>(query, {
    fetchPolicy: 'cache-and-network',
    errorPolicy: 'all',
    ...options,
});
const { error } = result;

React.useEffect(() => {
    const { graphQLErrors } = error;
    const isUnauthenticated = graphQLErrors.find(
      error => !!error.extensions && error.extensions.code === 'UNAUTHENTICATED'
    );
    if (isUnauthenticated) {
      navigation.navigate('AuthLoading');
    }
  },
}, [error])

but the documentation surrounding this is confusing

[zingerj]

Just following up on this: is @audiolion's workaround really the best way to handle errors with Apollo right now? Would really appreciate any helpful resources around error handling as it is still one of the most ambiguous parts of GraphQL.