Skip to content

Commit

Permalink
Fix WebSocketTransport not kicking off reconnect strategy (#49)
Browse files Browse the repository at this point in the history
* Fixed failures not triggering reconnect strategy

* Added tests for transport reconnect
  • Loading branch information
dsrees authored May 14, 2019
1 parent e7cbc79 commit 3179fae
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/main/kotlin/org/phoenixframework/Socket.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ internal data class StateChangeCallbacks(
/** The code used when the socket was closed without error */
const val WS_CLOSE_NORMAL = 1000

/** The socket was closed due to a SocketException. Likely the client lost connectivity */
const val WS_CLOSE_SOCKET_EXCEPTION = 4000

/** The socket was closed due to an EOFException. Likely the server abruptly closed */
const val WS_CLOSE_EOF_EXCEPTION = 4001

/**
* Connects to a Phoenix Server
*/
Expand Down
13 changes: 13 additions & 0 deletions src/main/kotlin/org/phoenixframework/Transport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ import okhttp3.Request
import okhttp3.Response
import okhttp3.WebSocket
import okhttp3.WebSocketListener
import java.io.EOFException
import java.net.ConnectException
import java.net.SocketException
import java.net.URL

/**
Expand Down Expand Up @@ -131,6 +134,16 @@ class WebSocketTransport(
override fun onFailure(webSocket: WebSocket, t: Throwable, response: Response?) {
this.readyState = Transport.ReadyState.CLOSED
this.onError?.invoke(t, response)

// Do not attempt to recover if the initial connection was refused
if (t is ConnectException) return

// Check if the socket was closed for some recoverable reason
if (t is SocketException) {
this.onClosed(webSocket, WS_CLOSE_SOCKET_EXCEPTION, "Socket Exception")
} else if (t is EOFException) {
this.onClosed(webSocket, WS_CLOSE_EOF_EXCEPTION, "EOF Exception")
}
}

override fun onClosing(webSocket: WebSocket, code: Int, reason: String) {
Expand Down
33 changes: 32 additions & 1 deletion src/test/kotlin/org/phoenixframework/WebSocketTransportTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.google.common.truth.Truth.assertThat
import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.verifyZeroInteractions
import com.nhaarman.mockitokotlin2.whenever
import okhttp3.OkHttpClient
import okhttp3.Response
Expand All @@ -12,13 +13,14 @@ import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.MockitoAnnotations
import java.io.EOFException
import java.net.SocketException
import java.net.URL

class WebSocketTransportTest {

@Mock lateinit var mockClient: OkHttpClient
@Mock lateinit var mockWebSocket: WebSocket
@Mock lateinit var mockChannel: Channel
@Mock lateinit var mockResponse: Response

lateinit var transport: WebSocketTransport
Expand Down Expand Up @@ -72,6 +74,8 @@ class WebSocketTransportTest {
@Test
fun `onFailure sets ready state to CLOSED and invokes onError callback`() {
val mockClosure = mock<(Throwable, Response?) -> Unit>()
val mockOnClose = mock<(Int) -> Unit>()
transport.onClose = mockOnClose
transport.onError = mockClosure

transport.readyState = Transport.ReadyState.CONNECTING
Expand All @@ -80,6 +84,33 @@ class WebSocketTransportTest {
transport.onFailure(mockWebSocket, throwable, mockResponse)
assertThat(transport.readyState).isEqualTo(Transport.ReadyState.CLOSED)
verify(mockClosure).invoke(throwable, mockResponse)
verifyZeroInteractions(mockOnClose)
}

@Test
fun `onFailure also triggers onClose for SocketException`() {
val mockOnError = mock<(Throwable, Response?) -> Unit>()
val mockOnClose = mock<(Int) -> Unit>()
transport.onClose = mockOnClose
transport.onError = mockOnError

val throwable = SocketException()
transport.onFailure(mockWebSocket, throwable, mockResponse)
verify(mockOnError).invoke(throwable, mockResponse)
verify(mockOnClose).invoke(4000)
}

@Test
fun `onFailure also triggers onClose for EOFException`() {
val mockOnError = mock<(Throwable, Response?) -> Unit>()
val mockOnClose = mock<(Int) -> Unit>()
transport.onClose = mockOnClose
transport.onError = mockOnError

val throwable = EOFException()
transport.onFailure(mockWebSocket, throwable, mockResponse)
verify(mockOnError).invoke(throwable, mockResponse)
verify(mockOnClose).invoke(4001)
}

@Test
Expand Down

0 comments on commit 3179fae

Please sign in to comment.