Skip to content

Commit 6d1ee99

Browse files
committed
Fix isConnected
Currently `isConnected` is true when `connection` exists. However, this is different from JS and Swift implementations. In [JS][1] and [Swift][2] implementations `isConnected` is true only when the socket is actually connected. One of the problems this causes was the channels were closed during the following scenario: 1. WebSocket connection established, one channel is joined.. 2. Internet/Wifi disabled -> connection fails. 3. We periodically call `phxSocket.connect()` to reconnect. 4. Channels [try to rejoin as `isConnected` is true][4], but `phx_join` messages are being queued instead as connection is still not established. 5. Internet/Wifi enabled -> multiple queued `phx_join` messages are sent, Phoniex Server relies `phx_close` to all join messages but last. On `phx_close` the channel is removed from the socket, even tho the logs show that the state is being received. `connect` method is changed to use `connection != null` check, the same way it's done in [JS version][3]. [1]: https://github.com/phoenixframework/phoenix/blob/93db5ff3adf4c91a1ff1996e819e7dd5dfbddf1a/assets/js/phoenix.js#L906 [2]: https://github.com/davidstump/SwiftPhoenixClient/blob/ade5c27051a96aeeedff1594cb3e176b57a02f96/Sources/Socket.swift#L180 [3]: https://github.com/phoenixframework/phoenix/blob/93db5ff3adf4c91a1ff1996e819e7dd5dfbddf1a/assets/js/phoenix.js#L782 [4]: https://github.com/dsrees/JavaPhoenixClient/blob/master/src/main/kotlin/org/phoenixframework/PhxChannel.kt#L74
1 parent 49dc2a2 commit 6d1ee99

File tree

2 files changed

+48
-4
lines changed

2 files changed

+48
-4
lines changed

src/main/kotlin/org/phoenixframework/PhxSocket.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ open class PhxSocket(
9999
/// Timer to use when attempting to reconnect
100100
private var reconnectTimer: PhxTimer? = null
101101

102-
103102
private val gson: Gson = GsonBuilder()
104103
.setLenient()
105104
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
@@ -154,8 +153,8 @@ open class PhxSocket(
154153
// Public
155154
//------------------------------------------------------------------------------
156155
/** True if the Socket is currently connected */
157-
val isConnected: Boolean
158-
get() = connection != null
156+
var isConnected: Boolean = false
157+
private set
159158

160159
/**
161160
* Disconnects the Socket
@@ -173,7 +172,7 @@ open class PhxSocket(
173172
*/
174173
fun connect() {
175174
// Do not attempt to reconnect if already connected
176-
if (isConnected) return
175+
if (connection != null) return
177176
connection = client.newWebSocket(request, this)
178177
}
179178

@@ -440,6 +439,7 @@ open class PhxSocket(
440439
// WebSocketListener
441440
//------------------------------------------------------------------------------
442441
override fun onOpen(webSocket: WebSocket?, response: Response?) {
442+
isConnected = true
443443
this.onConnectionOpened()
444444

445445
}
@@ -451,10 +451,12 @@ open class PhxSocket(
451451
}
452452

453453
override fun onClosed(webSocket: WebSocket?, code: Int, reason: String?) {
454+
isConnected = false
454455
this.onConnectionClosed(code)
455456
}
456457

457458
override fun onFailure(webSocket: WebSocket?, t: Throwable, response: Response?) {
459+
isConnected = false
458460
this.onConnectionError(t, response)
459461
}
460462
}

src/test/kotlin/org/phoenixframework/PhxSocketTest.kt

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
package org.phoenixframework
22

33
import com.google.common.truth.Truth.assertThat
4+
import junit.framework.Assert.assertFalse
5+
import junit.framework.Assert.assertTrue
6+
import okhttp3.Response
7+
import okhttp3.WebSocket
48
import org.junit.Test
9+
import org.mockito.Mockito
510

611
class PhxSocketTest {
712

@@ -36,4 +41,41 @@ class PhxSocketTest {
3641
assertThat(PhxSocket("wss://localhost:4000/socket/websocket", spacesParams).endpoint.toString())
3742
.isEqualTo("https://localhost:4000/socket/websocket?user_id=1&token=abc%20123")
3843
}
44+
45+
@Test
46+
fun isConnected_isTrue_WhenSocketConnected() {
47+
val mockSocket = Mockito.mock(WebSocket::class.java)
48+
val socket = PhxSocket("http://localhost:4000/socket/websocket")
49+
50+
socket.onOpen(mockSocket, null)
51+
52+
assertTrue(socket.isConnected)
53+
}
54+
55+
@Test
56+
fun isConnected_isFalse_WhenSocketNotYetConnected() {
57+
val socket = PhxSocket("http://localhost:4000/socket/websocket")
58+
59+
assertFalse(socket.isConnected)
60+
}
61+
62+
@Test
63+
fun isConnected_isFalse_WhenSocketFailed() {
64+
val mockSocket = Mockito.mock(WebSocket::class.java)
65+
val socket = PhxSocket("http://localhost:4000/socket/websocket")
66+
67+
socket.onFailure(mockSocket, RuntimeException(), null)
68+
69+
assertFalse(socket.isConnected)
70+
}
71+
72+
@Test
73+
fun isConnected_isFalse_WhenSocketClosed() {
74+
val mockSocket = Mockito.mock(WebSocket::class.java)
75+
val socket = PhxSocket("http://localhost:4000/socket/websocket")
76+
77+
socket.onClosed(mockSocket, 0, "closed")
78+
79+
assertFalse(socket.isConnected)
80+
}
3981
}

0 commit comments

Comments
 (0)