Proper error handling with useLazyQuery

Client SDKs
1

Hello all.
First, apologies if there is already a topic for this. I searched and did not find anything similar or with a viable answer.

The setup: in our web app, it’s pretty common for us to query data based on user interaction, like doing a search. For this, it seemed to make the most sense to employ useLazyQuery. Our general use case is pretty straight forward:

  • Page/containing component is already fully loaded
  • User interacts, say by clicking a button
  • Button handler calls a function that calls the query set up in useLazyQuery
  • Response is handled in .then(…)

No real issues with that flow, except error handling, which is the reason for this post.

Our AGQL server is, for the most part, just a passthrough that aggregates different backend services. The queries just pass along the params to some REST endpoint, package the response, and send it back as the result of the query. (not worried about mutations in this case)
For the sake of discussion, I have classified three different scenarios where we do not get back expected data in our response:

  1. An error occurred in the backend service and that error is packaged as part of a normal response to the query, per our api spec.
  2. An error occurred somewhere in the AGQL server while making a call to the backend service(e.g. bad url, response structure changed, etc…). In this case we throw a new GraphQLError and let the AGQL framework handle the response to the query. It is thrown from the catch(…) block of a try/catch wrapping the code that calls the backend service and handles the http response.
  3. An error occurred somewhere in our client code, unrelated to the query itself.

For #1, we check the response for an error property, as specified in our own api spec.
For #3, we rely on the .catch(…) block of the promise and that works pretty well.
However, #2 is tripping us up. With useLazyQuery specifically, the .then(…) block is executed instead of the .catch(…) block, as was my first assumption, but it looks like that’s not the case. Please correct me if I am wrong.
So we next looked at the error and errors object in the response to the query, as we were seeing the error response in the network data (and that data’s structure was specific to an error being returned, unrelated to our spec). However, both error and errors properties of the response were undefined. We couldn’t locate the error info in the response at all.
We found that we had to include errorPolicy: ‘all’ (or similar) when calling the query to get data back in either error or errors.
The error property would contain error information if, for example, the query we tried to execute was malformed.
If, however, the error info was from, say, GraphQLError being thrown by the server, it would show up in errors. Problem there is the errors property is deprecated.

So this all raises a few questions:

  1. If errors is deprecated, how are errors thrown by the server expected to be handled?
  2. Why does useLazyQuery not behave it the expected way, where errors would be handled in the .catch(…) block first?
  3. Is there a better way to implement useLazyQuery? Or should we not be using it at all?
  4. Is our approach for error handling in the client incorrect in the context of Apollo?
  5. Is our AGQL server implementation bad (e.g. should we never throw an error, or GraphQLError error)?

Thank you in advance for any help. I will respond to follow-ups as quickly as possible.

2

Hey @Branden_Boucher :waving_hand:

I believe you’re seeing a bug with the error handling. Unless you’re using errorPolicy: 'ignore', there is no reason that both error and errors should be undefined. Any chance you could create a reproduction of this? For your questions:

  1. If errors is deprecated, how are errors thrown by the server expected to be handled?

That deprecation is meant to alert everyone of the changes coming for 4.0. In our 4.0 branch, we’ve reworked errors quite a bit and errors are much more consistent. The errors property is gone entirely in favor of error. It was too confusing when you should use one or the other (especially because your component has no idea whether a GraphQL error or “network” error caused it!). Promise resolution/rejection is also a lot more predictable in 4.0 since we have unified error handling between GraphQL errors and “network” errors (any non-GraphQL error) so that errorPolicy is applied to both.

  1. Why does useLazyQuery not behave it the expected way, where errors would be handled in the .catch(…) block first?

I think this is a bug. It should reject the promise if errorPolicy is set to none and only resolve if set to all. Again, a reproduction here would be useful. We might be able to fix this in a patch release.

  1. Is there a better way to implement useLazyQuery? Or should we not be using it at all?

I’m curious, are you reading the data, error, etc. properties from the hook? If you’re just relying on the returned promise from the execute function, you might consider just using client.query directly, otherwise you might be rerendering your component unnecessarily since both useLazyQuery and your own setState functions could cause rerenders. I’d be curious though if you’re seeing the same issue with client.query or not.

I will mention that our 4.0 branch has completely rewritten useLazyQuery. Currently it uses useQuery under the hood so that after you call the execute function for the first time, it behaves like useQuery does (meaning changes in options, such as variables, could cause network fetches, even if you haven’t called the execute function). We’ve changed this to behave more as expected, which is that network fetches are only performed as a result of calling execute. I believe this separation from useQuery will also fix a few issues related to how that execute function works.

  1. Is our approach for error handling in the client incorrect in the context of Apollo?

I suppose I’m making assumptions on what your server response looks like. Could you perhaps give me some examples for your scenarios mentioned above? I assumed 1 and 2 returned GraphQL errors via the errors property in the GraphQL response, but perhaps that’s incorrect? And am I safe to assume that 3 means that an error is thrown somewhere in the link chain? I’d be able to answer this more thoroughly if you could help me understand what each of these look like in practice :slightly_smiling_face:

  1. Is our AGQL server implementation bad (e.g. should we never throw an error, or GraphQLError error)?

I think its fine. In fact it looks like Apollo Server encourages that. Check out the error handling docs. The main thing Apollo Client cares about is whether those errors end up on the errors property of the GraphQL response, but it doesn’t know or care how those errors got there.

3

Thank you for your thorough response, Jerel.

I will try to get some examples out in a response soon, especially for future readers.

For right now, I can try to clarify. If a GraphQLError is thrown from the server, a useMutation or useQuery hook will catch it in the .catch block of the promise (I am not sure at the moment the effects of different errorPolicy settings has on this). useLazyQuery will only show the error(s) in the errors property of the response, never in the .catch block, and only if errorPolicy is set to all.
And we are not using the data or error properties from the hook. Just the arguments passed into the callbacks for the .then and .catch blocks. Though, I have tested with the data and error properties and there was no difference, at least for useLazyQuery (no errors ever in the error property and only in the data property if we set the errorPolicy to all).
Again, this is all in the case of an error being thrown from the AGQL server.

I have since started testing with creating an ApolloClient instance and just using the .query and .mutation methods on it. This not only works exactly as expected but is almost a drop-in replacement for our hooks. Additionally, most of our use-cases are for calling queries based on user interaction, and often multiple times. So being able to call .query with different variables, is perfect. Another reason we will probably switch to this approach is we can move some of the query logic outside of the components since we don’t need to follow hook usage rules.

Thanks again for the feedback.