Treat all transport errors as recoverable

This is a cherry-pick of 86b7cc7, to resolve #1823 on the
fix/socketrocket-fix branch which one of our customers is using.

Source/ARTRealtime.m

@@ -1519,16 +1519,8 @@ - (void)realtimeTransportFailed:(id<ARTRealtimeTransport>)transport withError:(A
         }
     }
 
-    switch (transportError.type) {
-        case ARTRealtimeTransportErrorTypeBadResponse:
-        case ARTRealtimeTransportErrorTypeOther:
-            [self transition:ARTRealtimeFailed withErrorInfo:[ARTErrorInfo createFromNSError:transportError.error]];
-            break;
-        default: {
-            ARTErrorInfo *error = [ARTErrorInfo createFromNSError:transportError.error];
-            [self transitionToDisconnectedOrSuspendedWithError:error];
-        }
-    }
+    ARTErrorInfo *error = [ARTErrorInfo createFromNSError:transportError.error];
+    [self transitionToDisconnectedOrSuspendedWithError:error];
 }
 
 - (void)realtimeTransportNeverConnected:(id<ARTRealtimeTransport>)transport {
@@ -1537,7 +1529,7 @@ - (void)realtimeTransportNeverConnected:(id<ARTRealtimeTransport>)transport {
         return;
     }
 
-    [self transition:ARTRealtimeFailed withErrorInfo:[ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:@"Transport never connected"]];
+    [self transitionToDisconnectedOrSuspendedWithError:[ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:@"Transport never connected"]];
 }
 
 - (void)realtimeTransportRefused:(id<ARTRealtimeTransport>)transport withError:(ARTRealtimeTransportError *)error {
@@ -1547,13 +1539,13 @@ - (void)realtimeTransportRefused:(id<ARTRealtimeTransport>)transport withError:(
     }
 
     if (error && error.type == ARTRealtimeTransportErrorTypeRefused) {
-        [self transition:ARTRealtimeFailed withErrorInfo:[ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:[NSString stringWithFormat:@"Connection refused using %@", error.url]]];
+        [self transitionToDisconnectedOrSuspendedWithError:[ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:[NSString stringWithFormat:@"Connection refused using %@", error.url]]];
     }
     else if (error) {
-        [self transition:ARTRealtimeFailed withErrorInfo:[ARTErrorInfo createFromNSError:error.error]];
+        [self transitionToDisconnectedOrSuspendedWithError:[ARTErrorInfo createFromNSError:error.error]];
     }
     else {
-        [self transition:ARTRealtimeFailed];
+        [self transitionToDisconnectedOrSuspendedWithError:nil];
     }
 }
 
@@ -1563,7 +1555,7 @@ - (void)realtimeTransportTooBig:(id<ARTRealtimeTransport>)transport {
         return;
     }
 
-    [self transition:ARTRealtimeFailed withErrorInfo:[ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:@"Transport too big"]];
+    [self transitionToDisconnectedOrSuspendedWithError:[ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:@"Transport too big"]];
 }
 
 - (void)realtimeTransportSetMsgSerial:(id<ARTRealtimeTransport>)transport msgSerial:(int64_t)msgSerial {

Spec/Test Utilities/TestUtilities.swift

@@ -744,6 +744,7 @@ enum FakeNetworkResponse {
     case requestTimeout(timeout: TimeInterval)
     case hostInternalError(code: Int)
     case host400BadRequest
+    case arbitraryError
 
     var error: NSError {
         switch self {
@@ -757,6 +758,8 @@ enum FakeNetworkResponse {
             return NSError(domain: AblyTestsErrorDomain, code: code, userInfo: [NSLocalizedDescriptionKey: "internal error", NSLocalizedFailureReasonErrorKey: AblyTestsErrorDomain + ".FakeNetworkResponse"])
         case .host400BadRequest:
             return NSError(domain: AblyTestsErrorDomain, code: 400, userInfo: [NSLocalizedDescriptionKey: "bad request", NSLocalizedFailureReasonErrorKey: AblyTestsErrorDomain + ".FakeNetworkResponse"])
+        case .arbitraryError:
+            return NSError(domain: AblyTestsErrorDomain, code: 1, userInfo: [NSLocalizedDescriptionKey: "error from FakeNetworkResponse.arbitraryError"])
         }
     }
 
@@ -772,6 +775,8 @@ enum FakeNetworkResponse {
             return ARTRealtimeTransportError(error: error, badResponseCode: code, url: url)
         case .host400BadRequest:
             return ARTRealtimeTransportError(error: error, badResponseCode: 400, url: url)
+        case .arbitraryError:
+            return ARTRealtimeTransportError(error: error, type: .other, url: url)
         }
     }
 }
@@ -857,6 +862,8 @@ class MockHTTP: ARTHttp {
             requestCallback?(HTTPURLResponse(url: URL(string: "http://cocoa.test.suite")!, statusCode: code, httpVersion: nil, headerFields: nil), nil, nil)
         case .host400BadRequest:
             requestCallback?(HTTPURLResponse(url: URL(string: "http://cocoa.test.suite")!, statusCode: 400, httpVersion: nil, headerFields: nil), nil, nil)
+        case .arbitraryError:
+            requestCallback?(nil, nil, NSError(domain: AblyTestsErrorDomain, code: 1, userInfo: [NSLocalizedDescriptionKey: "error from FakeNetworkResponse.arbitraryError"]))
         }
     }
 
@@ -1232,7 +1239,8 @@ class TestProxyTransport: ARTWebSocketTransport {
             case .noInternet,
                  .hostUnreachable,
                  .hostInternalError,
-                 .host400BadRequest:
+                 .host400BadRequest,
+                 .arbitraryError:
                 performFakeConnectionError(0.1, error: networkResponse.transportError(for: url))
             case .requestTimeout(let timeout):
                 performFakeConnectionError(0.1 + timeout, error: networkResponse.transportError(for: url))
@@ -1613,9 +1621,11 @@ extension ARTWebSocketTransport {
     }
 
     func simulateIncomingError() {
-        let error = NSError(domain: ARTAblyErrorDomain, code: 0, userInfo: [NSLocalizedDescriptionKey:"Fail test"])
-        let webSocketDelegate = self as ARTWebSocketDelegate
-        webSocketDelegate.webSocket(self.websocket!, didFailWithError: error)
+        // Simulate receiving an ERROR ProtocolMessage, which should put a client into the FAILED state (per RTN15i)
+        let protocolMessage = ARTProtocolMessage()
+        protocolMessage.action = .error
+        protocolMessage.error = ARTErrorInfo.create(withCode: 50000 /* arbitrarily chosen */, message: "Fail test")
+        receive(protocolMessage)
     }
 }
 

Spec/Tests/RealtimeClientConnectionTests.swift

@@ -2208,7 +2208,7 @@ class RealtimeClientConnectionTests: XCTestCase {
     }
 
     // RTN14d
-    func test__059__Connection__connection_request_fails__connection_attempt_fails_for_any_recoverable_reason() {
+    func test__059__Connection__connection_request_fails__connection_attempt_fails_for_any_recoverable_reason__for_example_a_timeout() {
         let options = AblyTests.commonAppSetup()
         options.realtimeHost = "10.255.255.1" // non-routable IP address
         options.disconnectedRetryTimeout = 1.0
@@ -2264,6 +2264,58 @@ class RealtimeClientConnectionTests: XCTestCase {
         expect(totalRetry).to(equal(Int(expectedTime / options.disconnectedRetryTimeout)))
     }
 
+    // RTN14d
+    // This is a slightly-modified copy of test__059 above, based on the test changes introduced in 86b7cc7. Since this cherry-pick is being done on a branch we intend to eventually throw away, I was happy to just make a copy instead of cherry-picking the refactoring introduced in 30a0979.
+    func test__059b__Connection__connection_request_fails__connection_attempt_fails_for_any_recoverable_reason__for_example_an_arbitrary_transport_error() {
+        let options = AblyTests.commonAppSetup()
+        options.disconnectedRetryTimeout = 1.0
+        options.autoConnect = false
+        let expectedTime = 3.0
+
+        let previousConnectionStateTtl = ARTDefault.connectionStateTtl()
+        defer { ARTDefault.setConnectionStateTtl(previousConnectionStateTtl) }
+        ARTDefault.setConnectionStateTtl(expectedTime)
+
+        let client = ARTRealtime(options: options)
+        client.internal.setTransport(TestProxyTransport.self)
+        TestProxyTransport.fakeNetworkResponse = .arbitraryError
+        defer { TestProxyTransport.fakeNetworkResponse = nil }
+        client.internal.shouldImmediatelyReconnect = false
+        defer {
+            client.connection.off()
+            client.close()
+        }
+
+        var totalRetry = 0
+        waitUntil(timeout: testTimeout) { done in
+            let partialDone = AblyTests.splitDone(2, done: done)
+            var start: NSDate?
+
+            client.connection.once(.disconnected) { stateChange in
+                expect(stateChange.reason!.message).to(contain("error from FakeNetworkResponse.arbitraryError"))
+                expect(stateChange.previous).to(equal(ARTRealtimeConnectionState.connecting))
+                expect(stateChange.retryIn).to(beCloseTo(options.disconnectedRetryTimeout))
+                partialDone()
+                start = NSDate()
+            }
+
+            client.connection.on(.suspended) { _ in
+                let end = NSDate()
+                expect(end.timeIntervalSince(start! as Date)).to(beCloseTo(expectedTime, within: 0.9))
+                partialDone()
+            }
+
+            client.connect()
+
+            client.connection.on(.connecting) { stateChange in
+                expect(stateChange.previous).to(equal(ARTRealtimeConnectionState.disconnected))
+                totalRetry += 1
+            }
+        }
+
+        expect(totalRetry).to(equal(Int(expectedTime / options.disconnectedRetryTimeout)))
+    }
+
     // RTN14e
     func test__060__Connection__connection_request_fails__connection_state_has_been_in_the_DISCONNECTED_state_for_more_than_the_default_connectionStateTtl_should_change_the_state_to_SUSPENDED() {
         let options = AblyTests.commonAppSetup()
@@ -3600,13 +3652,13 @@ class RealtimeClientConnectionTests: XCTestCase {
         afterEach__Connection__Host_Fallback()
     }
 
-    func test__090__Connection__Host_Fallback__should_not_use_an_alternative_host_when_the_client_receives_a_bad_request() {
+    func test__090__Connection__Host_Fallback__should_not_use_an_alternative_host_when_the_client_receives_a_bad_request() throws {
         beforeEach__Connection__Host_Fallback()
 
         let options = ARTClientOptions(key: "xxxx:xxxx")
         options.autoConnect = false
+        options.disconnectedRetryTimeout = 1.0 // so that the test doesn't have to wait a long time to observe a retry
         let client = ARTRealtime(options: options)
-        let channel = client.channels.get(uniqueChannelName())
 
         let previousRealtimeRequestTimeout = ARTDefault.realtimeRequestTimeout()
         defer { ARTDefault.setRealtimeRequestTimeout(previousRealtimeRequestTimeout) }
@@ -3616,26 +3668,36 @@ class RealtimeClientConnectionTests: XCTestCase {
         TestProxyTransport.fakeNetworkResponse = .host400BadRequest
         defer { TestProxyTransport.fakeNetworkResponse = nil }
 
-        var urlConnections = [URL]()
-        TestProxyTransport.networkConnectEvent = { transport, url in
-            if client.internal.transport !== transport {
-                return
+        let dataGatherer = DataGatherer<(stateChanges: [ARTConnectionStateChange], urlConnections: [URL])>(description: "Observe emitted state changes and transport connection attempts") { submit in
+            var stateChanges: [ARTConnectionStateChange] = []
+            var urlConnections = [URL]()
+
+            client.connection.on { stateChange in
+                stateChanges.append(stateChange)
+                if (stateChanges.count == 3) {
+                    submit((stateChanges: stateChanges, urlConnections: urlConnections))
+                }
+            }
+
+            TestProxyTransport.networkConnectEvent = { transport, url in
+                if client.internal.transport !== transport {
+                    return
+                }
+                urlConnections.append(url)
             }
-            urlConnections.append(url)
         }
         defer { TestProxyTransport.networkConnectEvent = nil }
 
         client.connect()
         defer { client.dispose(); client.close() }
 
-        waitUntil(timeout: testTimeout) { done in
-            channel.publish(nil, data: "message") { _ in
-                done()
-            }
-        }
+        let data = try dataGatherer.waitForData(timeout: testTimeout)
 
-        expect(urlConnections).to(haveCount(1))
-        expect(NSRegularExpression.match(urlConnections[0].absoluteString, pattern: "//realtime.ably.io")).to(beTrue())
+        // We expect the first connection attempt to fail due to the .fakeNetworkResponse configured above. This error does not meet the criteria for trying a fallback host, and so should not provoke the use of a fallback host. Hence the connection should transition to DISCONNECTED, and then subsequently retry, transitioning back to CONNECTING. We should see that there were two connection attempts, both to the primary host.
+
+        XCTAssertEqual(data.stateChanges.map { $0.current }, [.connecting, .disconnected, .connecting])
+        XCTAssertEqual(data.urlConnections.count, 2)
+        XCTAssertTrue(data.urlConnections.allSatisfy { url in NSRegularExpression.match(url.absoluteString, pattern: "//realtime.ably.io") })
 
         afterEach__Connection__Host_Fallback()
     }