Skip to content

Handling reconnect scenarios properly when socket is hung #1375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,5 @@ Socket.IO-Test-Server/node_modules/*
.idea/
docs/docsets/
docs/undocumented.json

.swiftpm
3 changes: 2 additions & 1 deletion Source/SocketIO/Client/SocketIOClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ open class SocketIOClient: NSObject, SocketIOClientSpec {

manager.handleQueue.asyncAfter(deadline: DispatchTime.now() + timeoutAfter) {[weak self] in
guard let this = self, this.status == .connecting || this.status == .notConnected else { return }

DefaultSocketLogger.Logger.log("Timeout: Socket not connected, so setting to disconnected", type: this.logType)

this.status = .disconnected
this.leaveNamespace()

Expand Down
5 changes: 2 additions & 3 deletions Source/SocketIO/Manager/SocketManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ open class SocketManager: NSObject, SocketManagerSpec, SocketParsable, SocketDat
private(set) var reconnectAttempts = -1

private var _config: SocketIOClientConfiguration
private var currentReconnectAttempt = 0
internal var currentReconnectAttempt = 0
private var reconnecting = false

// MARK: Initializers
Expand Down Expand Up @@ -186,9 +186,8 @@ open class SocketManager: NSObject, SocketManagerSpec, SocketParsable, SocketDat
///
/// Override if you wish to attach a custom `SocketEngineSpec`.
open func connect() {
guard !status.active else {
if status == .connected || (status == .connecting && currentReconnectAttempt == 0) {
DefaultSocketLogger.Logger.log("Tried connecting an already active socket", type: SocketManager.logType)

return
}

Expand Down
1 change: 0 additions & 1 deletion Tests/TestSocketIO/SocketAckManagerTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class SocketAckManagerTest : XCTestCase {

func testManagerTimeoutAck() {
let callbackExpection = expectation(description: "Manager should timeout ack with noAck status")
let itemsArray = ["Hi", "ho"]

func callback(_ items: [Any]) {
XCTAssertEqual(items.count, 1, "Timed out ack should have one value")
Expand Down
42 changes: 42 additions & 0 deletions Tests/TestSocketIO/SocketMangerTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,44 @@ class SocketMangerTest : XCTestCase {
waitForExpectations(timeout: 0.3)
}

func testManagerDoesNotCallConnectWhenConnectingWithLessThanOneReconnect() {
setUpSockets()

let expect = expectation(description: "The manager should not call connect on the engine")
expect.isInverted = true

let engine = TestEngine(client: manager, url: manager.socketURL, options: nil)

engine.onConnect = {
expect.fulfill()
}
manager.setTestStatus(.connecting)
manager.setCurrentReconnect(currentReconnect: 0)
manager.engine = engine

manager.connect()

waitForExpectations(timeout: 0.3)
}

func testManagerCallConnectWhenConnectingAndMoreThanOneReconnect() {
setUpSockets()

let expect = expectation(description: "The manager should call connect on the engine")
let engine = TestEngine(client: manager, url: manager.socketURL, options: nil)

engine.onConnect = {
expect.fulfill()
}
manager.setTestStatus(.connecting)
manager.setCurrentReconnect(currentReconnect: 1)
manager.engine = engine

manager.connect()

waitForExpectations(timeout: 0.8)
}

func testManagerCallsDisconnect() {
setUpSockets()

Expand Down Expand Up @@ -154,6 +192,10 @@ public enum ManagerExpectation: String {
}

public class TestManager: SocketManager {
public func setCurrentReconnect(currentReconnect: Int) {
self.currentReconnectAttempt = currentReconnect
}

public override func disconnect() {
setTestStatus(.disconnected)
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/TestSocketIO/SocketSideEffectTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ class TestEngine: SocketEngineSpec {
private(set) var ws: WebSocket? = nil
private(set) var version = SocketIOVersion.three

fileprivate var onConnect: (() -> ())?
internal var onConnect: (() -> ())?

required init(client: SocketEngineClient, url: URL, options: [String: Any]?) {
self.client = client
Expand Down