Apollo-ios: chain.retry not call completion to fetch method

[vfumin]

Hi, i try to retry request from errorinterceptor, after refreshtoken, request sends, but i can't get completion event in fetch method

class RefreshTokenInterceptor: ApolloErrorInterceptor {

    private let requestManager: RequestManager
    private let bag = DisposeBag()

    init(requestManager: RequestManager) {
        self.requestManager = requestManager
    }

    /// Asynchronously handles the receipt of an error at any point in the chain.
    ///
    /// - Parameters:
    ///   - error: The received error
    ///   - chain: The chain the error was received on
    ///   - request: The request, as far as it was constructed
    ///   - response: [optional] The response, if one was received
    ///   - completion: The completion closure to fire when the operation has completed. Note that if you call `retry` on the chain, you will not want to call the completion block in this method.
    func handleErrorAsync<Operation: GraphQLOperation>(
        error: Error,
        chain: RequestChain,
        request: HTTPRequest<Operation>,
        response: HTTPResponse<Operation>?,
        completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) {
        guard response?.httpResponse.statusCode == 401 else {
            completion(.failure(error))
            return
        }
        requestManager.refreshToken()
            .subscribe(
                onSuccess: { [chain] in
                    chain.retry(request: request, completion: completion)
                },
                onError: { error in
                    completion(.failure(error))
                }
            )
            .disposed(by: bag)
    }
}

_Originally posted by @vfumin in https://github.com/apollographql/apollo-ios/issues/1530#issuecomment-734241073_

[designatednerd]

Do you hit onSuccess or onError in that code? I have a sneaking suspicion your disposeBag is getting emptied before your subscription hits either of those.

[designatednerd]

Also, you may want to consider making this an ApolloInterceptor and putting it in the chain right after the NetworkFetchInterceptor rather than routing every single error through it, which is what you're doing with an ApolloErrorInterceptor.

[vfumin]

chain.retry is working, but after complete retry request, not calling resultHandler in fetch(query:, resultHandler:).

i try debugging and not calling completion(.success(value)) in retrychain, but getting self.callbackQueue.async

public func returnValueAsync<Operation: GraphQLOperation>(
    for request: HTTPRequest<Operation>,
    value: GraphQLResult<Operation.Data>,
    completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) {

    guard self.isNotCancelled else {
      return
    }

    self.callbackQueue.async {
      completion(.success(value))
    }
  }

Main method for fetch data

 public func fetchUpcomings(userId: String) -> Single<[UpcomingEntity]> {
        let query = UserVisitsQuery(id: userId)
        return Single<[UpcomingEntity]>.create { [graphqlManager] observer in
            graphqlManager.client.fetch(query: query) { result in
                switch result {
                case .failure(let error):
                    observer(.error(error))

                case .success(let data):
                    let items = data.data?.userVisits?
                        .compactMap { visit -> UpcomingEntity? in
                            guard let visit = visit else {
                                return nil
                            }
                            return UpcomingEntity.from(entry: visit)
                        } ?? []
                    observer(.success(items))
                }
            }
            return Disposables.create()
        }
    }

[vfumin]

Even if remove refreshToken, completionHandler in fetch(query:) isn't called

 func handleErrorAsync<Operation: GraphQLOperation>(
        error: Error,
        chain: RequestChain,
        request: HTTPRequest<Operation>,
        response: HTTPResponse<Operation>?,
        completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) {
        guard response?.httpResponse.statusCode == 401 else {
            completion(.failure(error))
            return
        }
        chain.retry(request: request, completion: completion)
}

[designatednerd]

i try debugging and not calling completion(.success(value)) in retrychain, but getting self.callbackQueue.async

I'm not totally sure what you mean by this - can you explain in a bit more detail, please?

Even if remove refreshToken, completionHandler in fetch(query:) isn't called

Are you able via breakpointing to see where it stops being passed around? I don't see anything super obvious here in terms of why the completion wouldn't be called, unless maybe something has let go of the chain.

[vfumin]

Sorry for my bad english)
After chain.retry all interceptors of chains restart success, but i can't explain why not calling completion handler in the end of chain.
May be i can create test project for reproducing problem...

[vfumin]

Resolve problem with set parameter queue: DispatchQueue.global(qos: .userInteractive) to fetch method. With default value - DispatchQueue.main i can't get completion handler after retry.

[designatednerd]

That seems...really odd that that worked, but if at some point you are able to send me a project reproducing the problem, I'd happily take it. ellen at apollographql dot com.

[vfumin]

Yes, it's strange...problem not fixed

[yliu342]

I'm also experiencing the same issue. If you use chain.retry(request: request, completion: completion) in handleErrorAsync the completion of the original request is not called.

Using a different queue suggested by @vfumin worked but not sure why

[yliu342]

I also noticed there is a comment in the ApolloErrorInterceptor saying

- completion: The completion closure to fire when the operation has completed. Note that if you call `retry` on the chain, you will not want to call the completion block in this method.

Is that mean we should not even call completion when retry?

[designatednerd]

No, it means you should not call the completion block _in the error interceptor_ if you are retrying.

@yliu342 What version of the library are you on?

[yliu342]

@designatednerd I'm on 0.37.0 using SPM

[designatednerd]

Interesting. Could you send me a reproduction (or your project if you're comfortable with it)? ellen at apollographql dot com

[yliu342]

@designatednerd Sorry I can't sent the complete project to you but I will try to create a stripped down version and hopefully I can still reproduce the issue. I will keep you posted once I'm done.

[designatednerd]

Great, thank you!

[vfumin]

Fixing with RequestChain.swift with commenting queue.async

  /// Handles a resulting value by returning it on the appropriate queue.
  ///
  /// - Parameters:
  ///   - request: The request, as far as it has been constructed.
  ///   - value: The value to be returned
  ///   - completion: The completion closure to call when work is complete.
  public func returnValueAsync<Operation: GraphQLOperation>(
    for request: HTTPRequest<Operation>,
    value: GraphQLResult<Operation.Data>,
    completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) {

    guard self.isNotCancelled else {
      return
    }

//    self.callbackQueue.async {
      completion(.success(value))
//    }
  }

[designatednerd]

That is...very strange. The callback queue is passed in the initializer, so in theory unless self gets deallocated it shouldn't disappear. Also in theory if self had somehow been deallocated, self.isNotCancelled would have crashed, and the call to the callback queue also would have crashed rather than just not doing anything.

Are you using any non-default interceptors besides the RefreshTokenInterceptor you showed above?

Would definitely appreciate a repro project, something weird is going on here.

[yliu342]

@designatednerd I've send the sample project to you, let me know if you haven't receive it.

[designatednerd]

Got it! Thank you so much, poking it with a stick now.

[designatednerd]

AHA! So in requestChain when you call additionalErrorHandler and do your retry from there, the chain was getting released, which made this call fail after the call to completion on the retry.

Working on a fix!

[designatednerd]

Band-aid is up at #1563. Will publish a patch once I get that merged.

[designatednerd]

Fix is shipped as 0.38.3. @yliu342 thank you SO much for the repro, it was super-helpful.