apollo-ios icon indicating copy to clipboard operation
apollo-ios copied to clipboard

Support per-call timeouts

Open RamblinWreck77 opened this issue 1 year ago • 2 comments

Use case

Our app requires different timeouts for different network operations, and the (seemingly) current system of setting a global timeout on the URLSessionConfiguration that the entire ApolloClient instance uses does not work for our use-case.

Our app queues network calls with async-await, and currently the only way we could find to implement per-call timeouts in Apollo iOS was rather messy: (this isn't exactly right/code that can compile, I've stripped it down to make it more readable/show the highlights)

func ourAPIManagerQuery(request: GraphQLQuery, timeoutSeconds: Int) async -> Result {

   return await withTaskGroup { taskGroup in
   
      // Add timeout
      taskGroup.addTask {
         try? await Task.sleep(nanoseconds: SecondsToNanoseconds(timeoutSeconds))
         
         return .failure(OurCustomTimeoutError)
      }
      
      // Apollo Network Task
      taskGroup.addTask {
      
         return await withCheckedContinuation { continuation in
            
            GraphQLNetwork.shared.apollo.fetch(...) { [continuation] in

                 // result switch here that calls continuation with .resume()
            
            }    
         }
      }
      
      // Result storage
      var firstResult: Result = <default result>
      
      // Fun for-await loop that gets the first task to respond, sets to result, then cancels all others
      // So if the timeout returns first we will cancel the in-flight network task, and of course not error if
      // if we get a result in-time
      for await firstTaskResult in taskGroup.prefix(1) {
         firstResult = firstTaskResult
         taskGroup.cancelAll()
      }
      
      return firstResult
   }
}

The above will execute a timeout in parallel with the ApolloClient API call, and whichever completes first will have its result passed back to our internal API system. This seems to work, but it's super messy and far from ideal, we would prefer to just pass in a timeout value and have a custom error thrown when triggered.

Describe the solution you'd like

.fetch() and .perform() have an optional input param that could be something like:

.perform(mutation: mutation, ...., timeoutOption: TimeoutOption = .global) (default value so you can override)

enum TimeoutOption {
   case global                                                // use the URLSessionConfig (default)
   case seconds(seconds: Int, errorType: any Error)          // define timeout in seconds, return custom error
   case miliseconds(miliseconds: Int, errorType: any Error) // define timeout in milliseconds, return custom error
}

I'm not sure if any Error would work here, but the idea is to allow the developer to specify a custom error type (that conforms to the Error protocol) that the requestor can handle.

RamblinWreck77 avatar Aug 24 '23 22:08 RamblinWreck77

This feature request has been added to the back log of features for the 2.0 release, which is planned to reimagine the entire networking layer.

AnthonyMDev avatar Sep 15 '23 18:09 AnthonyMDev

This can be implemented a little differently today with the context: parameter that was recently added, and with a custom fetch interceptor. Still not ideal, but maybe an option with the existing library that is a little lower level.

jimisaacs avatar Sep 19 '23 00:09 jimisaacs