Merge pull request #1703 from ably/fix/1626-handle-reachability-events

Fix/1626 handle reachability events
ably · May 16, 2023 · 0bbcfad · 0bbcfad
2 parents 0a97ac3 + c40bdad
commit 0bbcfad
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 0 deletions.
diff --git a/Source/ARTRealtime.m b/Source/ARTRealtime.m
@@ -180,12 +180,14 @@ @interface ARTRealtimeInternal ()
 NS_ASSUME_NONNULL_END
 
 const NSTimeInterval _immediateReconnectionDelay = 0.1;
+const NSTimeInterval _reachabilityReconnectionAttemptThreshold = 0.1;
 
 @implementation ARTRealtimeInternal {
     BOOL _resuming;
     BOOL _renewingToken;
     BOOL _shouldImmediatelyReconnect;
     ARTEventEmitter<ARTEvent *, ARTErrorInfo *> *_pingEventEmitter;
+    NSDate *_reachabilityActivatedAt;
     NSDate *_connectionLostAt;
     NSDate *_lastActivity;
     Class _reachabilityClass;
@@ -540,6 +542,16 @@ - (void)updateWithErrorInfo:(nullable ARTErrorInfo *)errorInfo {
 - (void)handleNetworkIsReachable:(BOOL)reachable {
     if (reachable) {
         switch (_connection.state_nosync) {
+            case ARTRealtimeConnecting: {
+                // There is no certain way to detect whether reachability callback was called because you've just set it up,
+                // or because it really has detected reachability of the host. So we rely on a short timeout here - if the callback was called
+                // shortly after the setting up reachability, then reconnection shouldn't be done. Whereas if it was called after the certain delay (a threshold) - than it means you are in the middle of the dead-end connection attemt and it should be restarted.
+                // See https://github.com/ably/ably-cocoa/issues/1713 for more details.
+                if ([self shouldRestartPendingConnectionAttempt]) {
+                    [self transportReconnectWithExistingParameters];
+                }
+                break;
+            }
             case ARTRealtimeDisconnected:
             case ARTRealtimeSuspended:
                 [self transition:ARTRealtimeConnecting withMetadata:[[ARTConnectionStateChangeMetadata alloc] init]];
@@ -567,6 +579,7 @@ - (void)setReachabilityActive:(BOOL)active {
         _reachability = [[_reachabilityClass alloc] initWithLogger:self.logger queue:_queue];
     }
     if (active) {
+        _reachabilityActivatedAt = [NSDate date];
         __weak ARTRealtimeInternal *weakSelf = self;
         [_reachability listenForHost:[_transport host] callback:^(BOOL reachable) {
             ARTRealtimeInternal *strongSelf = weakSelf;
@@ -577,6 +590,7 @@ - (void)setReachabilityActive:(BOOL)active {
     }
     else {
         [_reachability off];
+        _reachabilityActivatedAt = nil;
     }
 }
 
@@ -1036,6 +1050,15 @@ - (BOOL)isTokenError:(nullable ARTErrorInfo *)error {
     return error != nil && [[[ARTDefaultErrorChecker alloc] init] isTokenError:error];
 }
 
+- (void)transportReconnectWithExistingParameters {
+    [self resetTransportWithResumeKey:_transport.resumeKey connectionSerial:_transport.connectionSerial];
+    NSString *host = [self getClientOptions].testOptions.reconnectionRealtimeHost; // for tests purposes only, always `nil` in production
+    if (host != nil) {
+        [self.transport setHost:host];
+    }
+    [self transportConnectForcingNewToken:false newConnection:true];
+}
+
 - (void)transportReconnectWithHost:(NSString *)host {
     [self resetTransportWithResumeKey:_transport.resumeKey connectionSerial:_transport.connectionSerial];
     [self.transport setHost:host];
@@ -1208,6 +1231,13 @@ - (BOOL)shouldQueueEvents {
     }
 }
 
+- (BOOL)shouldRestartPendingConnectionAttempt {
+    if (!_reachabilityActivatedAt)
+        return NO;
+    NSDate *currentTime = [NSDate date];
+    return [currentTime timeIntervalSinceDate:_reachabilityActivatedAt] > _reachabilityReconnectionAttemptThreshold;
+}
+
 - (BOOL)isActive {
     return [self shouldQueueEvents] || [self shouldSendEvents];
 }

diff --git a/Source/ARTTestClientOptions.m b/Source/ARTTestClientOptions.m
@@ -21,6 +21,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone {
     copied.realtimeRequestTimeout = self.realtimeRequestTimeout;
     copied.shuffleArray = self.shuffleArray;
     copied.transportFactory = self.transportFactory;
+    copied.reconnectionRealtimeHost = self.reconnectionRealtimeHost;
 
     return copied;
 }

diff --git a/Source/PrivateHeaders/Ably/ARTTestClientOptions.h b/Source/PrivateHeaders/Ably/ARTTestClientOptions.h
@@ -31,6 +31,12 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nonatomic) id<ARTRealtimeTransportFactory> transportFactory;
 
+/**
+ RTN20c helper.
+ This property is used to provide a way for the test code to simulate the case where a reconnection attempt results in a different outcome to the original connection attempt. Initial value is `nil`.
+ */
+@property (readwrite, nonatomic) NSString *reconnectionRealtimeHost;
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Test/Tests/RealtimeClientConnectionTests.swift b/Test/Tests/RealtimeClientConnectionTests.swift
@@ -4238,6 +4238,40 @@ class RealtimeClientConnectionTests: XCTestCase {
             client.connect()
         }
     }
+
+    // RTN20c
+    func test__106_b__Connection__Operating_System_events_for_network_internet_connectivity_changes__should_restart_the_pending_connection_attempt_if_the_operating_system_indicates_that_the_underlying_internet_connection_is_now_available_when_CONNECTING() throws {
+        let test = Test()
+        let options = try AblyTests.commonAppSetup(for: test)
+        let realtimeHost = options.realtimeHost
+        options.realtimeHost = "10.255.255.1" // non-routable IP address
+        options.autoConnect = false
+        options.testOptions.reconnectionRealtimeHost = realtimeHost
+        let client = ARTRealtime(options: options)
+        client.internal.setReachabilityClass(TestReachability.self)
+        defer { client.dispose(); client.close() }
+
+        waitUntil(timeout: testTimeout) { done in
+            client.connection.on { stateChange in
+                switch stateChange.current {
+                case .connecting:
+                    guard let reachability = client.internal.reachability as? TestReachability else {
+                        XCTFail("expected test reachability"); return
+                    }
+                    delay(0.2) { // delay more than `_reachabilityReconnectionAttemptThreshold` constant (0.1)
+                        reachability.simulate(true)
+                    }
+                case .connected:
+                    done()
+                case .disconnected, .suspended:
+                    XCTFail("Should never reach these states in this test.")
+                default:
+                    break
+                }
+            }
+            client.connect()
+        }
+    }
 
     // RTN22
     func test__107__Connection__Operating_System_events_for_network_internet_connectivity_changes__Ably_can_request_that_a_connected_client_re_authenticates_by_sending_the_client_an_AUTH_ProtocolMessage() throws {