Skip to content

Commit

Permalink
Fix isConnected
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ostap0207 committed Feb 20, 2019
1 parent 49dc2a2 commit 06a20eb
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/main/kotlin/org/phoenixframework/PhxSocket.kt
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ open class PhxSocket(
/// Timer to use when attempting to reconnect
private var reconnectTimer: PhxTimer? = null


private val gson: Gson = GsonBuilder()
.setLenient()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
Expand Down Expand Up @@ -154,8 +153,8 @@ open class PhxSocket(
// Public
//------------------------------------------------------------------------------
/** True if the Socket is currently connected */
val isConnected: Boolean
get() = connection != null
var isConnected: Boolean = false
private set

/**
* Disconnects the Socket
Expand All @@ -173,7 +172,7 @@ open class PhxSocket(
*/
fun connect() {
// Do not attempt to reconnect if already connected
if (isConnected) return
if (connection != null) return
connection = client.newWebSocket(request, this)
}

Expand Down Expand Up @@ -440,6 +439,7 @@ open class PhxSocket(
// WebSocketListener
//------------------------------------------------------------------------------
override fun onOpen(webSocket: WebSocket?, response: Response?) {
isConnected = true
this.onConnectionOpened()

}
Expand All @@ -451,10 +451,12 @@ open class PhxSocket(
}

override fun onClosed(webSocket: WebSocket?, code: Int, reason: String?) {
isConnected = false
this.onConnectionClosed(code)
}

override fun onFailure(webSocket: WebSocket?, t: Throwable, response: Response?) {
isConnected = false
this.onConnectionError(t, response)
}
}
42 changes: 42 additions & 0 deletions src/test/kotlin/org/phoenixframework/PhxSocketTest.kt
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.phoenixframework

import com.google.common.truth.Truth.assertThat
import okhttp3.WebSocket
import org.junit.Test
import org.mockito.Mockito

class PhxSocketTest {

Expand Down Expand Up @@ -36,4 +38,44 @@ class PhxSocketTest {
assertThat(PhxSocket("wss://localhost:4000/socket/websocket", spacesParams).endpoint.toString())
.isEqualTo("https://localhost:4000/socket/websocket?user_id=1&token=abc%20123")
}

@Test
fun isConnected_isTrue_WhenSocketConnected() {
val mockSocket = Mockito.mock(WebSocket::class.java)
val socket = PhxSocket("http://localhost:4000/socket/websocket")

socket.onOpen(mockSocket, null)

assertThat(socket.isConnected).isTrue()
}

@Test
fun isConnected_isFalse_WhenSocketNotYetConnected() {
val socket = PhxSocket("http://localhost:4000/socket/websocket")


assertThat(socket.isConnected).isFalse()
}

@Test
fun isConnected_isFalse_WhenSocketFailed() {
val mockSocket = Mockito.mock(WebSocket::class.java)
val socket = PhxSocket("http://localhost:4000/socket/websocket")

socket.onFailure(mockSocket, RuntimeException(), null)


assertThat(socket.isConnected).isFalse()
}

@Test
fun isConnected_isFalse_WhenSocketClosed() {
val mockSocket = Mockito.mock(WebSocket::class.java)
val socket = PhxSocket("http://localhost:4000/socket/websocket")

socket.onClosed(mockSocket, 0, "closed")


assertThat(socket.isConnected).isFalse()
}
}

0 comments on commit 06a20eb

Please sign in to comment.