From e1f5e605e5c4db3966e07d5b3320a71785bf425c Mon Sep 17 00:00:00 2001 From: Tarek Loubani Date: Fri, 21 Aug 2026 15:06:26 -0400 Subject: [PATCH] fix(call): detect dead signaling WebSocket connections via ping interval The signaling WebSocket was created from the shared OkHttpClient without a ping interval. In OkHttp 4.x this means no protocol-level pings are sent and the WebSocket read timeout is infinite, so a half-open connection (e.g. after switching from WiFi to cellular without a TCP reset) is never detected: onFailure never fires, the existing reconnect logic never runs, and the call goes silently mute/deaf while participants still appear present. Derive a dedicated client for the signaling WebSocket that pings every 10 seconds. OkHttp now fails the socket when a pong is not received in time, which triggers the existing onFailure -> restartWebSocket path. Regular HTTP calls keep using the unchanged shared client. Assisted-by: opencode:ox-alpha Signed-off-by: Tarek Loubani --- .../talk/webrtc/WebSocketInstance.kt | 12 +++- .../WebSocketInstanceSignalingClientTest.kt | 55 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/com/nextcloud/talk/webrtc/WebSocketInstanceSignalingClientTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt b/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt index 2664a021daf..0739652750c 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt +++ b/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt @@ -43,6 +43,7 @@ import org.greenrobot.eventbus.Subscribe import org.greenrobot.eventbus.ThreadMode import java.io.IOException import java.lang.Thread.sleep +import java.util.concurrent.TimeUnit import javax.inject.Inject @AutoInjector(NextcloudTalkApplication::class) @@ -80,6 +81,7 @@ class WebSocketInstance internal constructor(conversationUser: User, connectionU private var messagesQueue: MutableList = ArrayList() private val signalingMessageReceiver = ExternalSignalingMessageReceiver() val signalingMessageSender = ExternalSignalingMessageSender() + private val signalingHttpClient: OkHttpClient by lazy { createSignalingHttpClient(okHttpClient!!) } init { sharedApplication!!.componentApplication.inject(this) @@ -140,7 +142,7 @@ class WebSocketInstance internal constructor(conversationUser: User, connectionU reconnecting = true Log.d(TAG, "restartWebSocket: $connectionUrl") val request = Request.Builder().url(connectionUrl).build() - okHttpClient!!.newWebSocket(request, this) + signalingHttpClient.newWebSocket(request, this) } override fun onMessage(webSocket: WebSocket, text: String) { @@ -524,5 +526,13 @@ class WebSocketInstance internal constructor(conversationUser: User, connectionU private const val TAG = "WebSocketInstance" private const val NORMAL_CLOSURE = 1000 private const val ONE_SECOND: Long = 1000 + private const val PING_INTERVAL_SECONDS: Long = 10 + + // Dedicated client with pings, so half-open WebSocket connections + // (e.g. after a WiFi to cellular switch without TCP reset) fail and trigger the reconnect path. + internal fun createSignalingHttpClient(baseClient: OkHttpClient): OkHttpClient = + baseClient.newBuilder() + .pingInterval(PING_INTERVAL_SECONDS, TimeUnit.SECONDS) + .build() } } diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/WebSocketInstanceSignalingClientTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/WebSocketInstanceSignalingClientTest.kt new file mode 100644 index 00000000000..3617eed4802 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/webrtc/WebSocketInstanceSignalingClientTest.kt @@ -0,0 +1,55 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.webrtc + +import okhttp3.OkHttpClient +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotSame +import org.junit.Test +import java.util.concurrent.TimeUnit + +/** + * Tests for the signaling WebSocket client configuration ([WebSocketInstance.createSignalingHttpClient]). + * + * Without a ping interval OkHttp sends no protocol-level pings on WebSockets and uses an infinite read timeout, + * so a half-open connection (e.g. after a WiFi to cellular switch without a TCP reset) is never detected: the + * reconnect path via "onFailure" never runs and the call goes silently mute/deaf. These tests pin the ping + * configuration that makes dead connections fail fast. + */ +class WebSocketInstanceSignalingClientTest { + + @Test + fun signalingClientHasPingIntervalConfigured() { + val signalingClient = WebSocketInstance.createSignalingHttpClient(OkHttpClient()) + + // hardcoded on purpose: fails if the ping interval in WebSocketInstance changes or is removed + assertEquals( + "signaling WebSocket client must send pings to detect half-open connections", + 10_000, + signalingClient.pingIntervalMillis + ) + } + + @Test + fun signalingClientIsDerivedFromBaseClientWithoutMutatingIt() { + val baseClient = OkHttpClient.Builder() + .connectTimeout(45, TimeUnit.SECONDS) + .readTimeout(45, TimeUnit.SECONDS) + .build() + + val signalingClient = WebSocketInstance.createSignalingHttpClient(baseClient) + + assertNotSame("a dedicated client instance must be used", baseClient, signalingClient) + assertEquals( + "base client (shared with regular HTTP calls) must stay unchanged", + 0, + baseClient.pingIntervalMillis + ) + assertEquals(baseClient.connectTimeoutMillis, signalingClient.connectTimeoutMillis) + assertEquals(baseClient.readTimeoutMillis, signalingClient.readTimeoutMillis) + } +}