Fix ACConnectionMonitor didn't reconnect

There were three main issues.

The first was that ACConnectionMonitor started polling onConnected, and stopped on client.disconnect(). This meant that it would never be restarted after its first disconnection, since it would never receive another onConnected event.

The second was that ACStarscreamWebSocket wasn't calling onDisconnected in all scenarios where it was disconnected. Specifically, it wasn't calling onDisconnected during cancelled events (i.e. if client rather than server called disconnect) or error events.

Finally, I don't think that Starscream sockets are meant to be reused after a disconnect, so it now creates a new socket per connect request.

Author: High5Apps

Examples/ACStarscreamWebSocket.swift

@@ -10,44 +10,54 @@ import ACActionCable
 import Starscream
 
 class ACStarscreamWebSocket: ACWebSocketProtocol, WebSocketDelegate {
-
+    
     var url: URL
 
-    private var socket: WebSocket
+    private var socket: WebSocket?
 
     init(stringURL: String) {
         url = URL(string: stringURL)!
-        socket = WebSocket(request: URLRequest(url: url))
-        socket.delegate = self
     }
 
     var onConnected: ACConnectionHandler?
     var onDisconnected: ACDisconnectionHandler?
     var onText: ACTextHandler?
 
     func connect(headers: ACRequestHeaders?) {
-        socket.request.allHTTPHeaderFields = headers
-        socket.connect()
+        guard socket == nil else { return }
+        socket = WebSocket(request: URLRequest(url: url))
+        socket?.delegate = self
+        socket?.request.allHTTPHeaderFields = headers
+        socket?.connect()
     }
 
     func disconnect() {
-        socket.disconnect()
+        socket?.disconnect()
     }
 
     func send(text: String, completion: ACEventHandler?) {
-        socket.write(string: text, completion: completion)
+        socket?.write(string: text, completion: completion)
     }
 
     func didReceive(event: WebSocketEvent, client: WebSocket) {
         switch event {
         case .connected(let headers):
             onConnected?(headers)
         case .disconnected(let reason, _):
-            onDisconnected?(reason)
+            onSocketDisconnected(reason)
         case .text(let text):
             onText?(text)
+        case .cancelled:
+            onSocketDisconnected()
+        case .error(_):
+            onSocketDisconnected()
         default:
             break
         }
     }
+    
+    private func onSocketDisconnected(_ reason: String? = nil) {
+        onDisconnected?(reason)
+        socket = nil
+    }
 }

Sources/ACActionCable/ACClient.swift

@@ -20,6 +20,8 @@ public final class ACClient {
 
     public var headers: ACRequestHeaders? = nil
 
+    static var reconnectDelay: UInt32 = 1
+    
     var connectionMonitor: ACConnectionMontior?
 
     private var socket: ACWebSocketProtocol
@@ -78,6 +80,7 @@ public final class ACClient {
     // MARK: Connections
 
     public func connect() {
+        connectionMonitor?.start()
         socket.connect(headers: headers)
     }
 
@@ -89,6 +92,12 @@ public final class ACClient {
         socket.disconnect()
     }
 
+    func reconnect() {
+        socket.disconnect()
+        sleep(Self.reconnectDelay)
+        socket.connect(headers: headers)
+    }
+    
     // MARK: Sending
 
     func send(_ command: ACCommand, completion: ACEventHandler? = nil) {

Sources/ACActionCable/ACConnectionMonitor.swift

@@ -14,7 +14,6 @@ class ACConnectionMontior {
     static var now: () -> Date = { Date() }
     static var pollIntervalRange: Range<UInt32> = 3..<30
     static var pollIntervalMultiplier = 5.0
-    static var reconnectDelay: UInt32 = 1
 
     var startedAt: Date?
     var stoppedAt: Date?
@@ -78,9 +77,7 @@ class ACConnectionMontior {
     func reconnectIfStale() {
         guard isConnectionStale && !disconnectedRecently else { return }
         reconnectAttempts += 1
-        client?.disconnect(allowReconnect: true)
-        sleep(Self.reconnectDelay)
-        client?.connect()
+        client?.reconnect()
     }
 
     func stop() {
@@ -98,7 +95,6 @@ class ACConnectionMontior {
         reconnectAttempts = 0
         recordPing()
         disconnectedAt = nil
-        start()
     }
 
     private func onDisconnected(_ reason: String?) {

Tests/ACActionCableTests/ACClientTests.swift

@@ -47,7 +47,6 @@ final class ACClientTests: XCTestCase {
         client.connect()
         XCTAssertNotNil(client.connectionMonitor)
         XCTAssertEqual(expected, client.connectionMonitor!.staleThreshold)
-        socket.onConnected?(nil)
         XCTAssert(client.connectionMonitor!.isRunning)
     }
 

Tests/ACActionCableTests/ACConnectionMonitorTests.swift

@@ -18,7 +18,7 @@ class ACConnectionMonitorTests: XCTestCase {
         monitor = ACConnectionMontior(client: client, staleThreshold: 6)
         ACConnectionMontior.now = { Date() }
         ACConnectionMontior.pollIntervalRange = 3..<30
-        ACConnectionMontior.reconnectDelay = 0
+        ACClient.reconnectDelay = 0
     }
 
     func testStartStop() {
@@ -45,12 +45,6 @@ class ACConnectionMonitorTests: XCTestCase {
         XCTAssertEqual(stoppedAt, monitor.stoppedAt)
     }
 
-    func testShouldStartOnClientConnected() throws {
-        XCTAssert(!monitor.isRunning)
-        socket.onConnected?(nil)
-        XCTAssert(monitor.isRunning)
-    }
-    
     func testShouldRecordPingOnActionCablePing() throws {
         monitor.start()
         XCTAssertNil(monitor.pingedAt)