apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.88k stars 726 forks source link

Error Policies #205

Open mdnjustin opened 6 years ago

mdnjustin commented 6 years ago

I was wondering if anyone has had this issue, where error responses from GraphQL come back and get saved into cache. I would like to use the option "none" from below Unless it's already there and I don't know where it is.

none: This is the default policy to match how Apollo Client 1.0 worked. Any GraphQL Errors are treated the same as network errors and any data is ignored from the response. ignore: Ignore allows you to read any data that is returned alongside GraphQL Errors, but doesn’t save the errors or report them to your UI. all: Using the all policy is the best way to notify your users of potential issues while still showing as much data as possible from your server. It saves both data and errors into the Apollo Cache so your UI can use them.

I tried working around by trying to subclass ApolloClient, GraphQLResponse but neither of these are subclass-able.

samlandfried commented 6 years ago

@mdnjustin I'm running into this same issue. Were you able to find a resolution or workaround?

These are the three workarounds I'm considering:

  1. Make the query response non-nullable, so if an error occurs, the data field is null. This is unfortunate since it would hide the responses for other successful queries.

  2. Don't use caching. Also unfortunate for performance reasons.

  3. Modifying the server to hide the field that encountered the error. I think this might be doable with directives, but it feels like a strong departure from GraphQL conventions.

sebastiannicklas commented 5 years ago

Hi, is there any update on this topic?

designatednerd commented 5 years ago

Yeah this was mentioned in a Big Nerd Ranch article about GraphQL before I got hired and I agree, allowing some granularity on this would probably be a much better idea.

joshuarobs commented 4 years ago

Hi, any updates on this feature?

designatednerd commented 4 years ago

This is going to wind up being part of a bigger project around improving caching that will come at the end of the Swift Codegen project. Wish I had something more concrete for you, I agree this is dumb, but there's a lot of stuff wrapped up in this that I'm still in the process of trying to pull apart.

GyroJoe commented 3 years ago

It's now possible to implement this relatively easily with the new Interceptor chain infrastructure.

Just wrap the LegacyCacheWriteInterceptor and skip calling it for responses you don't want to cache.

Something like this:

class NoCachedErrorsWrapperInterceptor {
  let cacheWriteInterceptor: LegacyCacheWriteInterceptor

  init(wrapping cacheWriteInterceptor: LegacyCacheWriteInterceptor) {
    self.cacheWriteInterceptor = cacheWriteInterceptor
  }
}

extension NoCachedErrorsWrapperInterceptor: ApolloInterceptor {
  func interceptAsync<Operation>(chain: RequestChain, request: HTTPRequest<Operation>, response: HTTPResponse<Operation>?, completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) where Operation : GraphQLOperation {
    guard let errors = response?.parsedResponse?.errors else {
      // No errors, or response not parsed yet - just continue
      cacheWriteInterceptor.interceptAsync(chain: chain, request: request, response: response, completion: completion)
      return
    }

    if errors.count == 0 {
      cacheWriteInterceptor.interceptAsync(chain: chain, request: request, response: response, completion: completion)
    }
    else {
      // Don't cache any response with errors
      chain.proceedAsync(request: request, response: response, completion: completion)
    }
  }
}

Maybe this could be integrated into LegacyCacheWriteInterceptor with a constructor parameter to customize?

designatednerd commented 3 years ago

Hi, thanks for sharing this! I would not recommend holding on to a reference to another interceptor because that could cause some retain cycle weirdness, but in general yes, you can do this.

I'd strongly recommend using your own interceptor to do this - the main reason being that different apps are going to have different policies on precisely what errors should cause something not to be written to the cache at all, since errors can contain a partial error with a location where you can determine what exact sub-object the error was on, and write everything else to the cache. Deciding when to do this can be considerably more complex than a simple on/off, depending on your application.

sahilsharma-toast commented 1 year ago

You can create your own CacheWriteInterceptor and add it to the RequestChain.

Just adding a few lines to the default CacheWriteInterceptor do the trick.

        // If we found some errors, just continue without writing to cache
        if let errors = createdResponse.parsedResponse?.errors,
           !errors.isEmpty {
            chain.proceedAsync(
                request: request,
                response: createdResponse,
                completion: completion
            )
            return
        }