Skip to content

Fix WebSocketTransport not kicking off reconnect strategy #49

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 2 commits into from
May 14, 2019
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
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