Merge pull request #665 from algolia/feat/unreachable-hosts-exception

Fix retry strategy behaviour if all the hosts are unreachable

Author: VladislavFitz

Sources/AlgoliaSearchClient/Transport/HTTP/HTTPRequest.swift

@@ -14,12 +14,13 @@ class HTTPRequest<ResponseType: Decodable, Output>: AsyncOperation, ResultContai
   typealias Result = Swift.Result<Output, Swift.Error>
 
   let requester: HTTPRequester
-  let hostIterator: HostIterator
   let retryStrategy: RetryStrategy
   let timeout: TimeInterval
   let request: URLRequest
+  let callType: CallType
   let transform: (ResponseType) -> Output
   let completion: (Result) -> Void
+  let hostIterator: HostIterator
   var didUpdateProgress: ((ProgressReporting) -> Void)?
 
   var underlyingTask: (TransportTask)? {
@@ -42,19 +43,20 @@ class HTTPRequest<ResponseType: Decodable, Output>: AsyncOperation, ResultContai
 
   init(requester: HTTPRequester,
        retryStrategy: RetryStrategy,
-       hostIterator: HostIterator,
        request: URLRequest,
+       callType: CallType,
        timeout: TimeInterval,
        transform: @escaping Transform,
        completion: @escaping (Result) -> Void) {
     self.requester = requester
     self.retryStrategy = retryStrategy
-    self.hostIterator = hostIterator
     self.request = request
+    self.callType = callType
     self.timeout = timeout
     self.transform = transform
     self.completion = completion
     self.underlyingTask = nil
+    self.hostIterator = retryStrategy.retryableHosts(for: callType)
   }
 
   override func main() {
@@ -151,12 +153,13 @@ extension HTTPRequest where ResponseType == Output {
                    retryStrategy: RetryStrategy,
                    hostIterator: HostIterator,
                    request: URLRequest,
+                   callType: CallType,
                    timeout: TimeInterval,
                    completion: @escaping (Result) -> Void) {
     self.init(requester: requester,
               retryStrategy: retryStrategy,
-              hostIterator: hostIterator,
               request: request,
+              callType: callType,
               timeout: timeout,
               transform: { $0 },
               completion: completion)

Sources/AlgoliaSearchClient/Transport/HTTP/HTTPRequestBuilder.swift

@@ -27,15 +27,22 @@ class HTTPRequestBuilder {
   func build<Response: Decodable, Output>(for command: AlgoliaCommand, transform: @escaping (Response) -> Output, with completion: @escaping (HTTPRequest<Response, Output>.Result) -> Void) -> HTTPRequest<Response, Output> {
 
     let timeout = command.requestOptions?.timeout(for: command.callType) ?? configuration.timeout(for: command.callType)
-    let hostIterator = HostIterator(retryStrategy: retryStrategy, callType: command.callType)
     var request = command.urlRequest.setIfNotNil(\.credentials, to: credentials)
     let userAgentsValue = UserAgentController.userAgents.map(\.description).joined(separator: "; ")
     request.addValue(userAgentsValue, forHTTPHeaderField: HTTPHeaderKey.userAgent.rawValue)
-    return HTTPRequest(requester: requester, retryStrategy: retryStrategy, hostIterator: hostIterator, request: request, timeout: timeout, transform: transform, completion: completion)
+    return HTTPRequest(requester: requester,
+                       retryStrategy: retryStrategy,
+                       request: request,
+                       callType: command.callType,
+                       timeout: timeout,
+                       transform: transform,
+                       completion: completion)
   }
 
   func build<Response: Decodable, Output>(for command: AlgoliaCommand, transform: @escaping (Response) -> Output, responseType: Output.Type) -> HTTPRequest<Response, Output> {
-    return build(for: command, transform: transform, with: { (_:HTTPRequest<Response, Output>.Result) in })
+    return build(for: command,
+                 transform: transform,
+                 with: { (_:HTTPRequest<Response, Output>.Result) in })
   }
 
 }

Sources/AlgoliaSearchClient/Transport/HTTP/HTTPTransport+Error.swift

@@ -9,9 +9,21 @@ import Foundation
 
 extension HTTPTransport {
 
-  enum Error: Swift.Error {
+  enum Error: Swift.Error, LocalizedError {
     case noReachableHosts
     case missingData
+    case decodingFailure(Swift.Error)
+    
+    var localizedDescription: String {
+      switch self {
+      case .noReachableHosts:
+        return "All hosts are unreachable"
+      case .missingData:
+        return "Missing response data"
+      case .decodingFailure:
+        return "Response decoding failed"
+      }
+    }
   }
 
 }

Sources/AlgoliaSearchClient/Transport/HTTP/HTTPTransport+Result.swift

@@ -32,7 +32,7 @@ extension Result where Success: Decodable, Failure == Error {
       let object = try jsonDecoder.decode(Success.self, from: data)
       self = .success(object)
     } catch let error {
-      self = .failure(error)
+      self = .failure(HTTPTransport.Error.decodingFailure(error))
     }
 
   }

Sources/AlgoliaSearchClient/Transport/RetryStrategy/AlgoliaRetryStrategy.swift

@@ -25,23 +25,21 @@ class AlgoliaRetryStrategy: RetryStrategy {
   convenience init(configuration: Configuration) {
     self.init(hosts: configuration.hosts)
   }
-
-  func host(for callType: CallType) -> RetryableHost? {
+  
+  func retryableHosts(for callType: CallType) -> HostIterator {
     return queue.sync {
+      // Reset expired hosts
       hosts.resetExpired(expirationDelay: hostsExpirationDelay)
-
-      func firstUpHostForCallType() -> RetryableHost? {
-        return hosts.first { $0.supports(callType) && $0.isUp }
-      }
-
-      guard let host = firstUpHostForCallType() else {
+      
+      // If all hosts of the required type are down, reset them all
+      if !hosts.contains(where: { $0.supports(callType) && $0.isUp }) {
         hosts.resetAll(for: callType)
-        return firstUpHostForCallType()
       }
-
-      return host
+      
+      return HostIterator { [weak self] in
+        self?.hosts.first { $0.supports(callType) && $0.isUp }
+      }
     }
-
   }
 
   func notify<T>(host: RetryableHost, result: Result<T, Swift.Error>) -> RetryOutcome<T> {

Sources/AlgoliaSearchClient/Transport/RetryStrategy/HostIterator.swift

@@ -9,16 +9,14 @@ import Foundation
 
 class HostIterator: IteratorProtocol {
 
-  var retryStrategy: RetryStrategy
-  let callType: CallType
-
-  init(retryStrategy: RetryStrategy, callType: CallType) {
-    self.retryStrategy = retryStrategy
-    self.callType = callType
+  var getHost: () -> RetryableHost?
+  
+  init(getHost: @escaping () -> RetryableHost?) {
+    self.getHost = getHost
   }
 
   func next() -> RetryableHost? {
-    return retryStrategy.host(for: callType)
+    return getHost()
   }
 
 }

Sources/AlgoliaSearchClient/Transport/RetryStrategy/RetryStrategy.swift

@@ -7,13 +7,9 @@
 
 import Foundation
 
-protocol HostResultNotifiable {
-
-}
-
 protocol RetryStrategy: class {
 
-  func host(for callType: CallType) -> RetryableHost?
+  func retryableHosts(for callType: CallType) -> HostIterator
   func notify<T>(host: RetryableHost, result: Result<T, Swift.Error>) -> RetryOutcome<T>
 
 }

Tests/AlgoliaSearchClientTests/Unit/AlgoliaRetryStrategyTests.swift

@@ -24,49 +24,92 @@ class AlgoliaRetryStrategyTests: XCTestCase {
   func testHostsRotation() {
 
     let strategy = AlgoliaRetryStrategy(hosts: [host1, host2, host3, host4, host5])
-
-    XCTAssertEqual(strategy.host(for: .write)?.url.absoluteString, "algolia1.com")
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia3.com")
-
+    
+    let writeHosts = strategy.retryableHosts(for: .write)
+    let readHosts = strategy.retryableHosts(for: .read)
+    
+    // Fresh state, first read and write hosts are available
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, "algolia1.com")
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, "algolia3.com")
+
+    // First write host failed
     XCTAssertNoThrow(strategy.notify(host: host1, result: retryableErrorResult))
-    XCTAssertEqual(strategy.host(for: .write)?.url.absoluteString, "algolia2.com")
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia3.com")
+    // Second write host is available now
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, "algolia2.com")
+    // First read host is still available
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, "algolia3.com")
 
+    // Second write host failed
     _ = strategy.notify(host: host2, result: retryableErrorResult)
-    XCTAssertEqual(strategy.host(for: .write)?.url.absoluteString, "algolia1.com")
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia3.com")
+    // No more writable hosts available
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, nil)
+    // First read host is still available
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, "algolia3.com")
 
+    // First read host failed
     _ = strategy.notify(host: host3, result: retryableErrorResult)
-    XCTAssertEqual(strategy.host(for: .write)?.url.absoluteString, "algolia1.com")
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia4.com")
+    // No more writable hosts available
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, nil)
+    // Second read host is available
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, "algolia4.com")
 
+    // Second read host failed
     _ = strategy.notify(host: host4, result: retryableErrorResult)
-    XCTAssertEqual(strategy.host(for: .write)?.url.absoluteString, "algolia1.com")
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia5.com")
+    // No more writable hosts available
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, nil)
+    // Third read host is available
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, "algolia5.com")
 
+    // Third read host failed
     _ = strategy.notify(host: host5, result: retryableErrorResult)
-    XCTAssertEqual(strategy.host(for: .write)?.url.absoluteString, "algolia1.com")
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia3.com")
+    // No more writable hosts available
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, nil)
+    // No more writable hosts available
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, nil)
+    
+    // When got new write hosts, all the write hosts reseted
+    let newWriteHosts = strategy.retryableHosts(for: .write)
+    XCTAssertEqual(newWriteHosts.next()?.url.absoluteString, "algolia1.com")
+    
+    // Previous write iterator is updated as well as it uses the weak reference to the same retry strategy
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, "algolia1.com")
+    
+    // Still no read host available
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, nil)
+
+    
+    // When got new read hosts, all the read hosts reseted
+    let newReadHosts = strategy.retryableHosts(for: .read)
+    XCTAssertEqual(newReadHosts.next()?.url.absoluteString, "algolia3.com")
+
+    // Previous read iterator is updated as well as it uses the weak reference to the same retry strategy
+    XCTAssertEqual(readHosts.next()?.url.absoluteString, "algolia3.com")
+    
+    // Same state kept for write hosts iterators
+    XCTAssertEqual(writeHosts.next()?.url.absoluteString, "algolia1.com")
+    XCTAssertEqual(newWriteHosts.next()?.url.absoluteString, "algolia1.com")
+
 
   }
 
   func testTimeout() {
 
     let strategy = AlgoliaRetryStrategy(hosts: [host3, host4, host5])
-
+    let readHosts = strategy.retryableHosts(for: .read)
+    
     var host: RetryableHost?
-    var lastUpdate: Date? = strategy.host(for: .read)?.lastUpdated
+    var lastUpdate: Date? = readHosts.next()?.lastUpdated
 
     _ = strategy.notify(host: host3, result: timeoutErrorResult)
-    host = strategy.host(for: .read)
+    host = readHosts.next()
     XCTAssertGreaterThan(host!.lastUpdated.timeIntervalSince1970, lastUpdate!.timeIntervalSince1970)
     lastUpdate = host?.lastUpdated
     XCTAssertNotNil(host)
     XCTAssertTrue(host!.isUp)
     XCTAssertEqual(host!.retryCount, 1)
 
     _ = strategy.notify(host: host3, result: timeoutErrorResult)
-    host = strategy.host(for: .read)
+    host = readHosts.next()
     XCTAssertGreaterThan(host!.lastUpdated.timeIntervalSince1970, lastUpdate!.timeIntervalSince1970)
     XCTAssertNotNil(host)
     XCTAssertTrue(host!.isUp)
@@ -77,14 +120,17 @@ class AlgoliaRetryStrategyTests: XCTestCase {
   func testResetExpired() {
     let expirationDelay: TimeInterval = 3
     let strategy = AlgoliaRetryStrategy(hosts: [host3, host4, host5], hostsExpirationDelay: expirationDelay)
-
+    _ = strategy.retryableHosts(for: .read)
+    
     _ = strategy.notify(host: host3, result: retryableErrorResult)
     _ = strategy.notify(host: host4, result: retryableErrorResult)
     _ = strategy.notify(host: host5, result: retryableErrorResult)
 
     sleep(UInt32(expirationDelay))
+    
+    let newReadHosts = strategy.retryableHosts(for: .read)
 
-    XCTAssertEqual(strategy.host(for: .read)?.url.absoluteString, "algolia3.com")
+    XCTAssertEqual(newReadHosts.next()?.url.absoluteString, "algolia3.com")
   }
 
 }