why `failed(let error)` behaves different from `waiting(let error)`?

[wangjiejacques]

In the NWWebSocket.swift

from the following code, we will call reportErrorOrDisconnection for waiting(let error), which will call webSocketDidDisconnect for certain errors. On the other hand, we will call tearDownConnection for failed(let error), which will not call webSocketDidDisconnect even if it's disconnected. Why this?

    /// The handler for managing changes to the `connection.state` via the `stateUpdateHandler` on a `NWConnection`.
    /// - Parameter state: The new `NWConnection.State`
    private func stateDidChange(to state: NWConnection.State) {
        switch state {
        case .ready:
            delegate?.webSocketDidConnect(connection: self)
        case .waiting(let error):
            reportErrorOrDisconnection(error)
        case .failed(let error):
            tearDownConnection(error: error)
        case .setup, .preparing:
            break
        case .cancelled:
            tearDownConnection(error: nil)
        @unknown default:
            fatalError()
        }
    }

[danielrbrowne]

@wangjiejacques The reason why tearDownConnection(error:) is called in the failed case (vs reportErrorOrDisconnection(_ error:) in the waiting case) is because by the time the connection has transitioned to the failed state, the disconnection event will have already been reported where the error occurred, at one of the other call-sites of reportErrorOrDisconnection(_ error:) (e.g. sending or receiving data, or when expecting a Pong response to a Ping).

I verified this was the behaviour during my testing (i.e. previously I was also reporting the disconnection as a result of transitioning to the failed connection state, and seeing the disconnection being reported twice).

You've raised a good point however, in that I should make this behaviour clear via some comments in stateDidChange(to:).

[wangjiejacques]

Hi @danielrbrowne , thanks for you quick response, you are right, reportErrorOrDisconnection was called before failed, in this case, we will have bug of reconnect.

Here is my case,

1. Open app and connect socket.
2. Click home button to go to background.
3. Wait for some seconds
4. Back to foreground.

After all these 4 steps, 2 things will happed in the NWWebSocket. A. self.reportErrorOrDisconnection(error) will be called by connection?.receiveMessage { [weak self] (data, context, _, error) in inside the method public func listen(). (👍 you are right, reportErrorOrDisconnection is called.) Method trace: reportErrorOrDisconnection → webSocketDidDisconnect → attemptReconnect → connect

B. tearDownConnection(error: error) will be called inside the method private func stateDidChange(to state: NWConnection.State), and finally we will have this connection = nil. Method trace: tearDownConnection → connection = nil

The bug I have is that, when we try to reconnect in the A, sometimes the step B connection = nil is not called yet, which cause the method connect do nothing.(connection == nil is false), maybe we need to set connection = nil somewhere inside reportErrorOrDisconnection.

open func connect() {
        if connection == nil {
            connection = NWConnection(to: endpoint, using: parameters)
            connection?.stateUpdateHandler = stateDidChange(to:)
            connection?.betterPathUpdateHandler = betterPath(isAvailable:)
            connection?.viabilityUpdateHandler = viabilityDidChange(isViable:)
            listen()
            connection?.start(queue: connectionQueue)
        }
    }

some code maybe related to https://github.com/pusher/pusher-websocket-swift

here is the code about reonnect in the PusherWebsocketDelegate.swift

public func webSocketDidDisconnect(connection: WebSocketConnection,
                                       closeCode: NWProtocolWebSocket.CloseCode,
                                       reason: Data?) {
        // Handles setting channel subscriptions to unsubscribed wheter disconnection
        // is intentional or not
        if connectionState == .disconnecting || connectionState == .connected {
            for (_, channel) in self.channels.channels {
                channel.subscribed = false
            }
        }

        self.connectionEstablishedMessageReceived = false
        self.socketConnected = false

        updateConnectionState(to: .disconnected)

        guard !intentionalDisconnect else {
            self.delegate?.debugLog?(message: PusherLogger.debug(for: .intentionalDisconnection))
            return
        }

        // Log the disconnection

        logDisconnection(closeCode: closeCode, reason: reason)

        // Attempt reconnect if possible

        // `autoReconnect` option is ignored if the closure code is within the 4000-4999 range
        if case .privateCode = closeCode {} else {
            guard self.options.autoReconnect else {
                return
            }
        }

        guard reconnectAttemptsMax == nil || reconnectAttempts < reconnectAttemptsMax! else {
            self.delegate?.debugLog?(message: PusherLogger.debug(for: .maxReconnectAttemptsLimitReached))
            return
        }

        attemptReconnect(closeCode: closeCode)
    }

[danielrbrowne]

@wangjiejacques Thanks for the detailed description of the race condition issue. I will look into a solution for this.

[danielrbrowne]

@wangjiejacques I believe this PR should resolve the race condition you were seeing: #24

I'll include it in the next releases of the NWWebSocket library and the Pusher Channels SDK.

[wangjiejacques]

@danielrbrowne Thank you very much!! 👍 do you know when will you release Pusher Channels SDK ?

[danielrbrowne]

@wangjiejacques It will as soon as possible follow code review by my colleague.

[danielrbrowne]

The race condition fix has been released in v0.5.1 of NWWebSocket (which itself has been included in v9.1.1 of the Pusher Channels Swift SDK).