fix (avoid) handling of websocket delegate methods for disconnected websocket; improve logging

Author: lavenj

Source/SocketIO/Client/SocketIOClient.swift

@@ -182,7 +182,7 @@ open class SocketIOClient: NSObject, SocketIOClientSpec {
     /// Called when the client has disconnected from socket.io.
     ///
     /// - parameter reason: The reason for the disconnection.
-    open func didDisconnect(reason: String) {
+    open func didDisconnect(reason: SocketConnectionChangeReason) {
         guard status != .disconnected else { return }
 
         DefaultSocketLogger.Logger.log("Disconnected: \(reason)", type: logType)
@@ -307,7 +307,7 @@ open class SocketIOClient: NSObject, SocketIOClientSpec {
 
         guard status == .connected else {
             wrappedCompletion?()
-            handleClientEvent(.error, data: ["Tried emitting when not connected"])
+            handleClientEvent(.error, data: [SocketError.triedEmittingWhenNotConnected])
             return
         }
 
@@ -382,7 +382,7 @@ open class SocketIOClient: NSObject, SocketIOClientSpec {
         case .connect:
             didConnect(toNamespace: nsp, payload: packet.data.isEmpty ? nil : packet.data[0] as? [String: Any])
         case .disconnect:
-            didDisconnect(reason: "Got Disconnect")
+            didDisconnect(reason: .gotDisconnectPacket)
         case .error:
             handleEvent("error", data: packet.data, isInternalMessage: true, withAck: packet.id)
         }
@@ -522,7 +522,7 @@ open class SocketIOClient: NSObject, SocketIOClientSpec {
     /// Called when the manager detects a broken connection, or when a manual reconnect is triggered.
     ///
     /// - parameter reason: The reason this socket is reconnecting.
-    open func setReconnecting(reason: String) {
+    open func setReconnecting(reason: SocketConnectionChangeReason) {
         status = .connecting
 
         handleClientEvent(.reconnect, data: [reason])

Source/SocketIO/Client/SocketIOClientSpec.swift

@@ -88,12 +88,12 @@ public protocol SocketIOClientSpec : AnyObject {
     /// Called when the client has disconnected from socket.io.
     ///
     /// - parameter reason: The reason for the disconnection.
-    func didDisconnect(reason: String)
+    func didDisconnect(reason: SocketConnectionChangeReason)
 
     /// Called when the client encounters an error.
     ///
     /// - parameter reason: The reason for the disconnection.
-    func didError(reason: String)
+    func didError(error: SocketError)
 
     /// Disconnects the socket.
     func disconnect()
@@ -271,15 +271,15 @@ public protocol SocketIOClientSpec : AnyObject {
     /// Called when the manager detects a broken connection, or when a manual reconnect is triggered.
     ///
     /// parameter reason: The reason this socket is going reconnecting.
-    func setReconnecting(reason: String)
+    func setReconnecting(reason: SocketConnectionChangeReason)
 }
 
 public extension SocketIOClientSpec {
     /// Default implementation.
-    func didError(reason: String) {
-        DefaultSocketLogger.Logger.error("\(reason)", type: "SocketIOClient")
+    func didError(error: SocketError) {
+        DefaultSocketLogger.Logger.error("\(error)", type: "SocketIOClient")
 
-        handleClientEvent(.error, data: [reason])
+        handleClientEvent(.error, data: [error])
     }
 }
 

Source/SocketIO/Engine/SocketEngine.swift

@@ -198,9 +198,9 @@ open class SocketEngine:
              2: Bad handshake request
              3: Bad request
              */
-            didError(reason: error)
+            didError(error: .engineErrorMessage(error))
         } catch {
-            client?.engineDidError(reason: "Got unknown error from server \(msg)")
+            client?.engineDidError(error: .engineUnknownMessage(msg))
         }
     }
 
@@ -214,12 +214,13 @@ open class SocketEngine:
         }
     }
 
-    private func closeOutEngine(reason: String) {
+    private func closeOutEngine(reason: SocketConnectionChangeReason) {
         sid = ""
         closed = true
         invalidated = true
         connected = false
-
+        // nil out the delegate here; we're getting callbacks from the old socket when closing and recreating the WS
+        ws?.delegate = nil
         ws?.disconnect()
         stopPolling()
         client?.engineDidClose(reason: reason)
@@ -236,7 +237,7 @@ open class SocketEngine:
         if connected {
             DefaultSocketLogger.Logger.error("Engine tried opening while connected. Assuming this was a reconnect",
                                              type: SocketEngine.logType)
-            _disconnect(reason: "reconnect")
+            _disconnect(reason: .socketError(.triedOpeningWhileConnected))
         }
 
         DefaultSocketLogger.Logger.log("Starting engine. Server: \(url)", type: SocketEngine.logType)
@@ -315,25 +316,25 @@ open class SocketEngine:
     }
 
     /// Called when an error happens during execution. Causes a disconnection.
-    open func didError(reason: String) {
-        DefaultSocketLogger.Logger.error("\(reason)", type: SocketEngine.logType)
-        client?.engineDidError(reason: reason)
-        disconnect(reason: reason)
+	open func didError(error: SocketError) {
+        DefaultSocketLogger.Logger.error("\(error)", type: SocketEngine.logType)
+        client?.engineDidError(error: error)
+        disconnect(reason: .socketError(error))
     }
 
     /// Disconnects from the server.
     ///
     /// - parameter reason: The reason for the disconnection. This is communicated up to the client.
-    open func disconnect(reason: String) {
+    open func disconnect(reason: SocketConnectionChangeReason) {
         engineQueue.async {
             self._disconnect(reason: reason)
         }
     }
 
-    private func _disconnect(reason: String) {
+    private func _disconnect(reason: SocketConnectionChangeReason) {
         guard connected && !closed else { return closeOutEngine(reason: reason) }
 
-        DefaultSocketLogger.Logger.log("Engine is being closed.", type: SocketEngine.logType)
+        DefaultSocketLogger.Logger.log("Engine is being closed. \(reason)", type: SocketEngine.logType)
 
         if polling {
             disconnectPolling(reason: reason)
@@ -345,7 +346,7 @@ open class SocketEngine:
 
     // We need to take special care when we're polling that we send it ASAP
     // Also make sure we're on the emitQueue since we're touching postWait
-    private func disconnectPolling(reason: String) {
+    private func disconnectPolling(reason: SocketConnectionChangeReason) {
         postWait.append((String(SocketEnginePacketType.close.rawValue), {}))
 
         doRequest(for: createRequestForPostWithPostWait()) {_, _, _ in }
@@ -402,7 +403,7 @@ open class SocketEngine:
         postWait.removeAll(keepingCapacity: false)
     }
 
-    private func handleClose(_ reason: String) {
+    private func handleClose(_ reason: SocketConnectionChangeReason) {
         client?.engineDidClose(reason: reason)
     }
 
@@ -416,13 +417,13 @@ open class SocketEngine:
 
     private func handleOpen(openData: String) {
         guard let json = try? openData.toDictionary() else {
-            didError(reason: "Error parsing open packet")
+            didError(error: .openPacketUnparseable)
 
             return
         }
 
         guard let sid = json["sid"] as? String else {
-            didError(reason: "Open packet contained no sid")
+            didError(error: .openPacketMissingSID)
 
             return
         }
@@ -458,7 +459,7 @@ open class SocketEngine:
             doPoll()
         }
 
-        client?.engineDidOpen(reason: "Connect")
+        client?.engineDidOpen(reason: .engineOpen)
     }
 
     private func handlePong(with message: String) {
@@ -492,8 +493,9 @@ open class SocketEngine:
             // Make sure not to ping old connections
             guard let this = self, this.sid == id else { return }
 
-            if abs(this.lastCommunication?.timeIntervalSinceNow ?? deadlineMs) >= deadlineMs {
-                this.closeOutEngine(reason: "Ping timeout")
+            let actualTime = this.lastCommunication?.timeIntervalSinceNow ?? deadlineMs
+            if abs(actualTime) >= deadlineMs {
+                this.closeOutEngine(reason: .socketError(.pingTimeout(actualTime)))
             } else {
                 this.checkPings()
             }
@@ -541,7 +543,7 @@ open class SocketEngine:
         case .open:
             handleOpen(openData: String(message.dropFirst()))
         case .close:
-            handleClose(message)
+            handleClose(.engineCloseMessage(message))
         default:
             DefaultSocketLogger.Logger.log("Got unknown packet type", type: SocketEngine.logType)
         }
@@ -571,7 +573,7 @@ open class SocketEngine:
 
         // Server is not responding
         if pongsMissed > pongsMissedMax {
-            closeOutEngine(reason: "Ping timeout")
+            closeOutEngine(reason: .socketError(.pongsMissed(pongsMissed)))
             return
         }
 
@@ -687,12 +689,11 @@ open class SocketEngine:
         }
     }
 
-    private func websocketDidDisconnect(error: Error?) {
+    private func websocketDidDisconnect(reason: SocketConnectionChangeReason) {
         probing = false
 
         if closed {
-            client?.engineDidClose(reason: "Disconnect")
-
+            client?.engineDidClose(reason: reason)
             return
         }
 
@@ -705,12 +706,13 @@ open class SocketEngine:
         connected = false
         polling = true
 
-        if let error = error as? WSError {
-            didError(reason: "\(error.message). code=\(error.code), type=\(error.type)")
-        } else if let reason = error?.localizedDescription {
-            didError(reason: reason)
-        } else {
-            client?.engineDidClose(reason: "Socket Disconnected")
+        //following existing patterns for these, just cleaning up a bit
+        switch(reason) {
+        case .socketError(let error):
+            didError(error: error)
+        default:
+            //following existing pattern, we close ourselves out here
+            client?.engineDidClose(reason: reason)
         }
     }
 
@@ -728,37 +730,47 @@ extension SocketEngine {
     public func URLSession(session: URLSession, didBecomeInvalidWithError error: NSError?) {
         DefaultSocketLogger.Logger.error("Engine URLSession became invalid", type: "SocketEngine")
 
-        didError(reason: "Engine URLSession became invalid")
+        didError(error: .urlSessionBecameInvalid(error))
     }
 }
 
-enum EngineError: Error {
-    case canceled
-}
+//enum EngineError: Error {
+//    case canceled
+//}
 
 extension SocketEngine {
     /// Delegate method for WebSocketDelegate.
     ///
     /// - Parameters:
     ///   - event: WS Event
     ///   - _:
-    public func didReceive(event: WebSocketEvent, client _: WebSocket) {
+    public func didReceive(event: WebSocketEvent, client websocket: WebSocket) {
+        guard websocket === self.ws else {
+            DefaultSocketLogger.Logger.log("Ignoring websocket event from wrong client: \(event)", type: SocketEngine.logType)
+            return
+        }
         switch event {
         case let .connected(headers):
             wsConnected = true
             client?.engineDidWebsocketUpgrade(headers: headers)
             websocketDidConnect()
         case .cancelled:
             wsConnected = false
-            websocketDidDisconnect(error: EngineError.canceled)
+            websocketDidDisconnect(reason: .websocketEngineCanceled)
         case let .disconnected(reason, code):
             wsConnected = false
-            websocketDidDisconnect(error: nil)
+            websocketDidDisconnect(reason: .socketError(.websocketEngineDisconnected(reason, Int(code))))
         case let .text(msg):
             parseEngineMessage(msg)
         case let .binary(data):
             parseEngineData(data)
+        case let .error(error):
+            DefaultSocketLogger.Logger.error("didReceive WebSocket error \(error as Any)", type: "SocketEngine")
+            //share the error with the clients.
+            client?.engineDidError(error: .websocketEngineError(error))
+
         case _:
+            //TODO: Handle or log other cases?
             break
         }
     }

Source/SocketIO/Engine/SocketEngineClient.swift

@@ -26,23 +26,23 @@
 import Foundation
 
 /// Declares that a type will be a delegate to an engine.
-@objc public protocol SocketEngineClient {
+public protocol SocketEngineClient: AnyObject {
     // MARK: Methods
 
     /// Called when the engine errors.
     ///
     /// - parameter reason: The reason the engine errored.
-    func engineDidError(reason: String)
+    func engineDidError(error: SocketError)
 
     /// Called when the engine closes.
     ///
     /// - parameter reason: The reason that the engine closed.
-    func engineDidClose(reason: String)
+    func engineDidClose(reason: SocketConnectionChangeReason)
 
     /// Called when the engine opens.
     ///
     /// - parameter reason: The reason the engine opened.
-    func engineDidOpen(reason: String)
+    func engineDidOpen(reason: SocketConnectionChangeReason)
 
     /// Called when the engine receives a ping message. Only called in socket.io >3.
     func engineDidReceivePing()

Source/SocketIO/Engine/SocketEnginePollable.swift

@@ -137,7 +137,7 @@ extension SocketEnginePollable {
                 }
 
                 if this.polling {
-                    this.didError(reason: err?.localizedDescription ?? "Error")
+                    this.didError(error: .urlSessionError(err))
                 }
 
                 return
@@ -173,17 +173,17 @@ extension SocketEnginePollable {
 
         DefaultSocketLogger.Logger.log("POSTing", type: "SocketEnginePolling")
 
-        doRequest(for: req) {[weak self] _, res, err in
+        doRequest(for: req) {[weak self] _, responseOpt, errorOpt in
             guard let this = self else { return }
-            guard let res = res as? HTTPURLResponse, res.statusCode == 200 else {
-                if let err = err {
-                    DefaultSocketLogger.Logger.error(err.localizedDescription, type: "SocketEnginePolling")
+            guard let response = responseOpt as? HTTPURLResponse, response.statusCode == 200 else {
+                if let error = errorOpt {
+                    DefaultSocketLogger.Logger.error(error.localizedDescription, type: "SocketEnginePolling")
                 } else {
-                    DefaultSocketLogger.Logger.error("Error flushing waiting posts", type: "SocketEnginePolling")
+                    DefaultSocketLogger.Logger.error("Error flushing waiting posts: \((responseOpt as? HTTPURLResponse)?.statusCode ?? -1)", type: "SocketEnginePolling")
                 }
 
                 if this.polling {
-                    this.didError(reason: err?.localizedDescription ?? "Error")
+                    this.didError(error: .urlSessionError(errorOpt))
                 }
 
                 return

Source/SocketIO/Engine/SocketEngineSpec.swift

@@ -106,12 +106,12 @@ public protocol SocketEngineSpec: class {
     func connect()
 
     /// Called when an error happens during execution. Causes a disconnection.
-    func didError(reason: String)
+    func didError(error: SocketError)
 
     /// Disconnects from the server.
     ///
     /// - parameter reason: The reason for the disconnection. This is communicated up to the client.
-    func disconnect(reason: String)
+    func disconnect(reason: SocketConnectionChangeReason)
 
     /// Called to switch from HTTP long-polling to WebSockets. After calling this method the engine will be in
     /// WebSocket mode.