Apollo-ios: How to handle errors correctly

[odanu]

Hi,
I am currently on the stage of migrating from 0.33 -> 0.36. Gone through all the provided documentation, but I am still confused on what would be the correct way of handling errors(also read the suggested article: https://medium.com/@sachee/200-ok-error-handling-in-graphql-7ec869aec9bc).

Before going to write this question, I just wanted to give it a try myself.
One of the most important use cases related to error handling is that we want to cover the case when a auth token expires on the backend, which results in an error when performing the next app query or mutation. At this stage, we want to catch the error, perform automatically a relogin(call login mutation), which basically will return a new auth token and after all that re-execute the failed request(query/mutation).
As far as I managed to understand, the best way to do this is through ApolloErrorInterceptor, which I basically did. I created a custom ApolloErrorInterceptor, returned it in InterceptorProvider.additionalErrorInterceptor.
The first problem I met is that it seems that not absolutely all errors get through this interceptor. Which left me without ideas on how to continue, but since this new apollo sdk approach is pretty fresh, I also left some space to consider it might be a bug, full stop.

Now my question is: what is the correct way of handling errors? Can we improve the documentation to provide more examples about it?

[RolandasRazma]

Interesting, I thought it was me but maybe InterceptorProvider.additionalErrorInterceptor is not what we expect.
I just went through same update and had same problem as you do. what worked for me was:

/// InterceptorProvider
    func interceptors<Operation: GraphQLOperation>(for operation: Operation) -> [ApolloInterceptor] {
        return [
            MaxRetryInterceptor(),
            LegacyCacheReadInterceptor(store: self.store),
            TokenAddingInterceptor(...),
            NetworkFetchInterceptor(client: self.client),
            TokenErrorInterceptor(...),
            ResponseCodeInterceptor(),
            LegacyParsingInterceptor(cacheKeyForObject: self.store.cacheKeyForObject),
            AutomaticPersistedQueryInterceptor(),
            LegacyCacheWriteInterceptor(store: self.store),
        ]
    }

    func additionalErrorInterceptor<Operation: GraphQLOperation>(for operation: Operation) -> ApolloErrorInterceptor? {
        return nil
    }

/// TokenErrorInterceptor
    func interceptAsync<Operation: GraphQLOperation>(chain: RequestChain, request: HTTPRequest<Operation>, response: HTTPResponse<Operation>?, completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) {
        guard
            let responseStatusCode = response?.httpResponse.statusCode,
            responseStatusCode == 401
        else {
            chain.proceedAsync(request: request, response: response, completion: completion)
            return
        }

        tokenService.renewAccessToken { [weak self] (error: OAuthSwiftError?) in
            guard self != nil else { return }

            if let error = error {
                chain.handleErrorAsync(error, request: request, response: response, completion: completion)
            } else {
                chain.retry(request: request, completion: completion)
            }
        }
    }

neither additionalErrorInterceptor nor having TokenErrorInterceptor after ResponseCodeInterceptor worked

[designatednerd]

additionalErrorInterceptor is generally intended for error handling after something has been dealt with by the request chain. In this case, you want to deal with it within the request chain, so that you can call retry.

@RolandasRazma's example is a great one if the token error is coming back from middleware and causing a 401 Unauthorized to be returned.

If there's something within GraphQL that's returning the error, you'll get a 200 OK and an error on GraphQLResult's errors property. In that case, you would want to insert your TokenErrorInterceptor after the LegacyParsingInterceptor, since that's where those would actually be parsed.

[odanu]

additionalErrorInterceptor is generally intended for error handling _after_ something has been dealt with by the request chain.

Just for the sake of understanding it better, what does it mean and why in some particular conditions it is not called.? I guess some of the docs needs to be more explicit about the use cases when it should be used.

[designatednerd]

It only wouldn't be called if it doesn't exist. It's designed to be a place to have all errors, no matter what the origin, pass through before being handed back to the caller. This allows for things like error logging that are hard to do without a centralized error handler.

I'll work on clarifying this in the docs.

[designatednerd]

@odanu @RolandasRazma Added some docs on the additional error interceptor to #1484

[designatednerd]

OK, that's merged - is there anything else here I can help clarify, or do you mind if we close this out?

[odanu]

I think you can close this ticket. Though you have to maybe give it a try for different use cases around additionalErrorInterceptor. As written above, it looks like it doesn't catch all the errors.

[designatednerd]

nor having TokenErrorInterceptor after ResponseCodeInterceptor worked

Here the issue would be that the 401 response code would be caught by the ResponseCodeInterceptor and returned as an error through handleErrorAsync (which in turn calls the additionalErrorInterceptor if it exists) and the chain would not proceed.

That would mean TokenErrorInterceptor wouldn't get called because an error has already been caught. That's why you have to put it before the ResponseCodeInterceptor.

Does that help explain why that wouldn't work? Or is there another place where things aren't showing up where you expect them to?

[odanu]

No Ellen. I was not referring this part. Let's sum up again the thoughts.
The documentation and you, assures that additionalErrorInterceptor gets called, having the error which was collected in any of the interceptors. Which basically means that no matter at which level of the request/response/parsing/etc an error occurs, it will arrive to the additionalErrorInterceptor.
Now as mentioned in the initial question, it seems that not all of the errors reach the additionalErrorInterceptor. That was my initial test. I just thought it would be the right place to handle all the errors, but got blocked because some of the errors didn't get to it. Check the question again please :).

[designatednerd]

Right, and my question is, what are the errors that are not passed through in your testing?

Note that if there is an _expected_ error, which is handled by the interceptor (for example, the auto-persisted-query-specific error which is handled by the AutomaticPersistedQueryInterceptor), then that is not passed on since that is an error that was expected and handled.

[odanu]

I will try to collect the information tomorrow;)

[odanu]

So I can find 2 cases when additionalErrorInterceptor is not used:

1. when an interceptor caught the error triggered retry... the example with expired token
2. GraphQLError objects which arrive through GraphQLResults.errors array.

But I guess both of these cases is expected behaviour.

[designatednerd]

You're correct - because a GraphQLResult can contain partial results with an error explaining what's missing, simply having errors be non-empty doesn't indicate that a request failed.

You can see an example of where we're looking for _specific_ errors to handle in that array in the AutomaticPersistedQueryInterceptor, and if you know your backend will be returning _specific_ errors you need to handle in that array (for example, token expiration errors there instead of as a 401), you can add your own interceptors to handle those.

Otherwise, GraphQLErrors should be returned as part of the GraphQLResult.

[designatednerd]

@odanu Anything else here or can I close this out?

[odanu]

Oh sorry. All good. I had to close it earlier.

[designatednerd]

👍 Thank you!