Skip to content

Commit 9e0ad5b

Browse files
authored
Merge pull request #6543 from nextcloud/fix/websocket-keepalive
fix(call): detect dead signaling WebSocket connections via ping interval
2 parents 419d6b4 + e1f5e60 commit 9e0ad5b

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

‎app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import org.greenrobot.eventbus.Subscribe
4343
import org.greenrobot.eventbus.ThreadMode
4444
import java.io.IOException
4545
import java.lang.Thread.sleep
46+
import java.util.concurrent.TimeUnit
4647
import javax.inject.Inject
4748

4849
@AutoInjector(NextcloudTalkApplication::class)
@@ -80,6 +81,7 @@ class WebSocketInstance internal constructor(conversationUser: User, connectionU
8081
private var messagesQueue: MutableList<String> = ArrayList()
8182
private val signalingMessageReceiver = ExternalSignalingMessageReceiver()
8283
val signalingMessageSender = ExternalSignalingMessageSender()
84+
private val signalingHttpClient: OkHttpClient by lazy { createSignalingHttpClient(okHttpClient!!) }
8385

8486
init {
8587
sharedApplication!!.componentApplication.inject(this)
@@ -140,7 +142,7 @@ class WebSocketInstance internal constructor(conversationUser: User, connectionU
140142
reconnecting = true
141143
Log.d(TAG, "restartWebSocket: $connectionUrl")
142144
val request = Request.Builder().url(connectionUrl).build()
143-
okHttpClient!!.newWebSocket(request, this)
145+
signalingHttpClient.newWebSocket(request, this)
144146
}
145147

146148
override fun onMessage(webSocket: WebSocket, text: String) {
@@ -524,5 +526,13 @@ class WebSocketInstance internal constructor(conversationUser: User, connectionU
524526
private const val TAG = "WebSocketInstance"
525527
private const val NORMAL_CLOSURE = 1000
526528
private const val ONE_SECOND: Long = 1000
529+
private const val PING_INTERVAL_SECONDS: Long = 10
530+
531+
// Dedicated client with pings, so half-open WebSocket connections
532+
// (e.g. after a WiFi to cellular switch without TCP reset) fail and trigger the reconnect path.
533+
internal fun createSignalingHttpClient(baseClient: OkHttpClient): OkHttpClient =
534+
baseClient.newBuilder()
535+
.pingInterval(PING_INTERVAL_SECONDS, TimeUnit.SECONDS)
536+
.build()
527537
}
528538
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Nextcloud Talk - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: GPL-3.0-or-later
6+
*/
7+
package com.nextcloud.talk.webrtc
8+
9+
import okhttp3.OkHttpClient
10+
import org.junit.Assert.assertEquals
11+
import org.junit.Assert.assertNotSame
12+
import org.junit.Test
13+
import java.util.concurrent.TimeUnit
14+
15+
/**
16+
* Tests for the signaling WebSocket client configuration ([WebSocketInstance.createSignalingHttpClient]).
17+
*
18+
* Without a ping interval OkHttp sends no protocol-level pings on WebSockets and uses an infinite read timeout,
19+
* so a half-open connection (e.g. after a WiFi to cellular switch without a TCP reset) is never detected: the
20+
* reconnect path via "onFailure" never runs and the call goes silently mute/deaf. These tests pin the ping
21+
* configuration that makes dead connections fail fast.
22+
*/
23+
class WebSocketInstanceSignalingClientTest {
24+
25+
@Test
26+
fun signalingClientHasPingIntervalConfigured() {
27+
val signalingClient = WebSocketInstance.createSignalingHttpClient(OkHttpClient())
28+
29+
// hardcoded on purpose: fails if the ping interval in WebSocketInstance changes or is removed
30+
assertEquals(
31+
"signaling WebSocket client must send pings to detect half-open connections",
32+
10_000,
33+
signalingClient.pingIntervalMillis
34+
)
35+
}
36+
37+
@Test
38+
fun signalingClientIsDerivedFromBaseClientWithoutMutatingIt() {
39+
val baseClient = OkHttpClient.Builder()
40+
.connectTimeout(45, TimeUnit.SECONDS)
41+
.readTimeout(45, TimeUnit.SECONDS)
42+
.build()
43+
44+
val signalingClient = WebSocketInstance.createSignalingHttpClient(baseClient)
45+
46+
assertNotSame("a dedicated client instance must be used", baseClient, signalingClient)
47+
assertEquals(
48+
"base client (shared with regular HTTP calls) must stay unchanged",
49+
0,
50+
baseClient.pingIntervalMillis
51+
)
52+
assertEquals(baseClient.connectTimeoutMillis, signalingClient.connectTimeoutMillis)
53+
assertEquals(baseClient.readTimeoutMillis, signalingClient.readTimeoutMillis)
54+
}
55+
}

0 commit comments

Comments
 (0)