perf(#1170): remove runBlocking network IO from token refresh and WS setup (#1185)

app/src/main/java/com/m57/hermescontrol/data/remote/DashboardSessionTokenRefresher.kt

@@ -2,26 +2,64 @@ package com.m57.hermescontrol.data.remote
 
 import com.m57.hermescontrol.data.local.AuthManager
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
+import kotlinx.coroutines.withContext
 import okhttp3.OkHttpClient
 import okhttp3.Request
+import okhttp3.Response
 import okio.Buffer
 
 internal object DashboardSessionTokenRefresher {
     private val tokenPattern = Regex("""__HERMES_SESSION_TOKEN__\s*=\s*"([^"]+)"""")
+    private val refreshMutex = Mutex()
 
     // Synchronized so concurrent 401s / reconnect storms don't each fire a
     // redundant GET at the dashboard SPA (thundering-herd guard).
     @Synchronized
     fun refresh(): String? =
         try {
-            val token = fetch(AuthManager.baseUrl(), OkHttpProvider.probe) ?: return null
+            val token = fetchSync(AuthManager.baseUrl(), OkHttpProvider.probe) ?: return null
             AuthManager.setToken(token)
             token
         } catch (_: Exception) {
             null
         }
 
-    internal fun fetch(
+    suspend fun refreshAsync(): String? =
+        refreshMutex.withLock {
+            try {
+                val token = fetch(AuthManager.baseUrl(), OkHttpProvider.probe) ?: return@withLock null
+                AuthManager.setToken(token)
+                token
+            } catch (_: Exception) {
+                null
+            }
+        }
+
+    internal suspend fun fetch(
+        baseUrl: String,
+        client: OkHttpClient,
+    ): String? =
+        withContext(Dispatchers.IO) {
+            try {
+                val request =
+                    Request
+                        .Builder()
+                        .url(baseUrl)
+                        .get()
+                        .build()
+
+                client.newCall(request).await().use { response ->
+                    if (!response.isSuccessful) return@withContext null
+                    extractToken(response)
+                }
+            } catch (_: Exception) {
+                null
+            }
+        }
+
+    internal fun fetchSync(
         baseUrl: String,
         client: OkHttpClient,
     ): String? =
@@ -32,13 +70,16 @@ internal object DashboardSessionTokenRefresher {
                     .url(baseUrl)
                     .get()
                     .build()
-            // execute() + body read on Dispatchers.IO: the body read touches
+
-            // the socket on the CALLING thread otherwise — when that is main
-            // (loopback ticket mint from the kanban events connect), it throws
-            // NetworkOnMainThreadException (same bug class as requestWsTicket).
-            kotlinx.coroutines.runBlocking(Dispatchers.IO) {
             client.newCall(request).execute().use { response ->
-                    if (!response.isSuccessful) return@runBlocking null
+                if (!response.isSuccessful) return null
+                extractToken(response)
+            }
+        } catch (_: Exception) {
+            null
+        }
+
+    private fun extractToken(response: Response): String? {
         // Cap the read — the SPA HTML can be large, and we only need the
         // injected token near the top of the document. Reading the whole
         // body into a String risks OOM on low-end devices.
@@ -48,16 +89,12 @@ internal object DashboardSessionTokenRefresher {
                 source.read(buffer, MAX_BODY_BYTES)
                 buffer.readUtf8()
             }
-                    tokenPattern
+        return tokenPattern
             .find(html)
             ?.groupValues
             ?.getOrNull(1)
             ?.takeIf { it.isNotBlank() }
     }
-            }
-        } catch (_: Exception) {
-            null
-        }
 
     private const val MAX_BODY_BYTES: Long = 64 * 1024
 }

app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt

@@ -8,6 +8,7 @@ import com.m57.hermescontrol.data.remote.CookieManager
 import com.m57.hermescontrol.data.remote.DashboardSessionTokenRefresher
 import com.m57.hermescontrol.data.remote.NetworkMonitor
 import com.m57.hermescontrol.data.remote.OkHttpProvider
+import com.m57.hermescontrol.data.remote.await
 import com.m57.hermescontrol.data.session.ActiveSessionHolder
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CompletableDeferred
@@ -547,7 +548,7 @@ object HermesWsClient {
      * or because ticket refresh succeeded). Returns false if we are in gated mode
      * and ticket refresh failed or cannot be performed.
      */
-    internal fun refreshWsTicketIfNeeded(generation: Int): Boolean {
+    internal suspend fun refreshWsTicketIfNeeded(generation: Int): Boolean {
         val isGated =
             try {
                 AuthManager.serverStore.getLatestState().wsAuthParam == "ticket"
@@ -563,11 +564,9 @@ object HermesWsClient {
             // restart. Refresh it before each WebSocket handshake so automatic
             // reconnect does not get stuck in AUTH_EXPIRED with a stale token.
             val token =
-                synchronized(DashboardSessionTokenRefresher) {
                 runCatching {
-                        DashboardSessionTokenRefresher.fetch(AuthManager.baseUrl(), OkHttpProvider.probe)
+                    DashboardSessionTokenRefresher.refreshAsync()
                 }.getOrNull()
-                }
             synchronized(outboundLock) {
                 if (isCurrentGeneration(generation) && token != null) {
                     runCatching { AuthManager.setToken(token) }
@@ -610,7 +609,7 @@ object HermesWsClient {
      * ticket. Loopback mode: refresh the dashboard token and return it.
      * Returns null when a ticket could not be obtained.
      */
-    internal fun mintWsTicket(): String? {
+    internal suspend fun mintWsTicket(): String? {
         val isGated =
             try {
                 AuthManager.serverStore.getLatestState().wsAuthParam == "ticket"
@@ -618,7 +617,7 @@ object HermesWsClient {
                 false
             }
         if (!isGated) {
-            DashboardSessionTokenRefresher.refresh()
+            DashboardSessionTokenRefresher.refreshAsync()
             return AuthManager.getToken()
         }
         return requestWsTicket().ticket
@@ -631,7 +630,7 @@ object HermesWsClient {
     )
 
     /** POST /api/auth/ws-ticket (cookie-auth'd via the shared CookieJar) and parse the ticket. */
-    private fun requestWsTicket(): TicketRequestResult {
+    private suspend fun requestWsTicket(): TicketRequestResult {
         try {
             val client = OkHttpProvider.probe
             val request =
@@ -641,16 +640,12 @@ object HermesWsClient {
                     .post("{}".toRequestBody())
                     .build()
 
-            // Run the ENTIRE call on Dispatchers.IO — execute() already hops,
-            // but ResponseBody.string() reads the socket on the CALLING
-            // thread. When the caller is main (kanban events connect), that
-            // read throws NetworkOnMainThreadException and the mint fails.
             val (code, body) =
-                kotlinx.coroutines.runBlocking(Dispatchers.IO) {
+                withContext(Dispatchers.IO) {
                     if (CookieManager.isInitialized()) {
                         CookieManager.useStore(CookieManager.cookieJar.currentServer())
                     }
-                    client.newCall(request).execute().use { resp ->
+                    client.newCall(request).await().use { resp ->
                         resp.code to resp.body.string()
                     }
                 }
@@ -1184,11 +1179,12 @@ object HermesWsClient {
                 if (intentionalClose.get()) return
                 connectionGeneration.incrementAndGet()
             }
+        wsScope.launch(Dispatchers.IO) {
             if (!refreshWsTicketIfNeeded(generation)) {
                 Log.w(TAG, "Aborting openSocket: WS ticket refresh failed")
-            return
+                return@launch
             }
-        if (!isCurrentGeneration(generation)) return
+            if (!isCurrentGeneration(generation)) return@launch
             val url = AuthManager.wsUrl()
             val safeUrl = url.replace(Regex("token=[^&]+"), "token=REDACTED")
             if (BuildConfig.DEBUG) Log.d(TAG, "Connecting to $safeUrl")
@@ -1203,6 +1199,7 @@ object HermesWsClient {
                 }
             }
         }
+    }
 
     private fun isCurrentGeneration(generation: Int): Boolean =
         connectionGeneration.get() == generation && !intentionalClose.get()

app/src/main/java/com/m57/hermescontrol/data/ws/KanbanEventsClient.kt

@@ -5,6 +5,7 @@ import com.m57.hermescontrol.BuildConfig
 import com.m57.hermescontrol.data.local.AuthManager
 import com.m57.hermescontrol.data.remote.OkHttpProvider
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
@@ -163,10 +164,12 @@ class KanbanEventsClient {
     ) {
         if (closed || gen != generation) return
 
+        scope.launch(Dispatchers.IO) {
             val credential = HermesWsClient.mintWsTicket()
+            if (closed || gen != generation) return@launch
             if (credential.isNullOrBlank()) {
-            onStatus(KanbanLiveStatus.AUTH_FAILED)
+                scope.launch { onStatus(KanbanLiveStatus.AUTH_FAILED) }
-            return
+                return@launch
             }
 
             val rawAuthParam = AuthManager.serverStore.getLatestState().wsAuthParam
@@ -244,6 +247,7 @@ class KanbanEventsClient {
                     },
                 )
         }
+    }
 
     private fun scheduleReconnect(
         scope: CoroutineScope,

app/src/test/java/com/m57/hermescontrol/data/remote/DashboardSessionTokenRefresherTest.kt

@@ -5,6 +5,7 @@ import io.mockk.every
 import io.mockk.mockkObject
 import io.mockk.unmockkAll
 import io.mockk.verify
+import kotlinx.coroutines.runBlocking
 import okhttp3.OkHttpClient
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
@@ -32,7 +33,8 @@ class DashboardSessionTokenRefresherTest {
     }
 
     @Test
-    fun fetchExtractsInjectedDashboardToken() {
+    fun fetchExtractsInjectedDashboardToken() =
+        runBlocking {
             server.enqueue(
                 MockResponse()
                     .setResponseCode(200)
@@ -46,7 +48,8 @@ class DashboardSessionTokenRefresherTest {
         }
 
     @Test
-    fun fetchReturnsNullWhenDashboardDoesNotInjectToken() {
+    fun fetchReturnsNullWhenDashboardDoesNotInjectToken() =
+        runBlocking {
             server.enqueue(MockResponse().setResponseCode(200).setBody("<html></html>"))
 
             val token = DashboardSessionTokenRefresher.fetch(server.url("/").toString(), OkHttpClient())
@@ -55,7 +58,8 @@ class DashboardSessionTokenRefresherTest {
         }
 
     @Test
-    fun fetchReturnsNullForFailedResponse() {
+    fun fetchReturnsNullForFailedResponse() =
+        runBlocking {
             server.enqueue(MockResponse().setResponseCode(500))
 
             val token = DashboardSessionTokenRefresher.fetch(server.url("/").toString(), OkHttpClient())
@@ -64,7 +68,8 @@ class DashboardSessionTokenRefresherTest {
         }
 
     @Test
-    fun fetchReturnsNullWhenTokenIsBlank() {
+    fun fetchReturnsNullWhenTokenIsBlank() =
+        runBlocking {
             server.enqueue(
                 MockResponse()
                     .setResponseCode(200)
@@ -76,6 +81,20 @@ class DashboardSessionTokenRefresherTest {
             assertNull(token)
         }
 
+    @Test
+    fun fetchSyncExtractsInjectedDashboardToken() {
+        server.enqueue(
+            MockResponse()
+                .setResponseCode(200)
+                .setBody("""<script>window.__HERMES_SESSION_TOKEN__ = "sync-token";</script>"""),
+        )
+
+        val token = DashboardSessionTokenRefresher.fetchSync(server.url("/").toString(), OkHttpClient())
+
+        assertEquals("sync-token", token)
+        assertEquals("/", server.takeRequest().path)
+    }
+
     @Test
     fun refreshUpdatesAuthManagerOnSuccess() {
         mockkObject(AuthManager)
@@ -94,6 +113,25 @@ class DashboardSessionTokenRefresherTest {
         verify { AuthManager.setToken("mocked-token") }
     }
 
+    @Test
+    fun refreshAsyncUpdatesAuthManagerOnSuccess() =
+        runBlocking {
+            mockkObject(AuthManager)
+            every { AuthManager.baseUrl() } returns server.url("/").toString()
+            every { AuthManager.setToken(any()) } returns Unit
+
+            server.enqueue(
+                MockResponse()
+                    .setResponseCode(200)
+                    .setBody("""<script>window.__HERMES_SESSION_TOKEN__ = "async-token";</script>"""),
+            )
+
+            val token = DashboardSessionTokenRefresher.refreshAsync()
+
+            assertEquals("async-token", token)
+            verify { AuthManager.setToken("async-token") }
+        }
+
     @Test
     fun refreshReturnsNullOnException() {
         mockkObject(AuthManager)

app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt

@@ -10,6 +10,7 @@ import com.m57.hermescontrol.data.remote.OkHttpProvider
 import com.m57.hermescontrol.data.remote.ServerEndpoint
 import com.m57.hermescontrol.data.remote.buildFakePersistentCookieJar
 import com.m57.hermescontrol.data.session.ActiveSessionHolder
+import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkObject
@@ -84,6 +85,10 @@ class HermesWsClientTest {
         // the WS stack can build its OkHttp clients without app context.
         CookieManager.setJarForTest(buildFakePersistentCookieJar())
 
+        mockkObject(DashboardSessionTokenRefresher)
+        coEvery { DashboardSessionTokenRefresher.refreshAsync() } returns null
+        every { DashboardSessionTokenRefresher.refresh() } returns null
+
         // Reset state
         val connectedField = HermesWsClient::class.java.getDeclaredField("connected")
         connectedField.isAccessible = true
@@ -850,12 +855,11 @@ class HermesWsClientTest {
     @Test
     fun testDisconnectThenConnectSupersedesBlockedSocketOpen() =
         runBlocking {
-            mockkObject(DashboardSessionTokenRefresher)
             every { AuthManager.baseUrl() } returns mockWebServer.url("/").toString()
             val refreshStarted = CountDownLatch(1)
             val releaseRefresh = CountDownLatch(1)
             val refreshCalls = AtomicInteger(0)
-            every { DashboardSessionTokenRefresher.fetch(any(), any()) } answers {
+            coEvery { DashboardSessionTokenRefresher.refreshAsync() } answers {
                 if (refreshCalls.getAndIncrement() == 0) {
                     refreshStarted.countDown()
                     releaseRefresh.await(5, TimeUnit.SECONDS)
@@ -1181,10 +1185,8 @@ class HermesWsClientTest {
         // 5-6s latch windows routinely overshoot under parallel load).
         HermesWsClient.setReconnectBackoffForTest(0L)
         // The reconnect path refreshes the WS token over the network before
-        // opening the socket. Stub it so no real HTTP call sits inside the
+        // opening the socket. Stubbed in setUp() so no real HTTP call sits inside the
         // test's timing window (CI network latency was the dominant flake).
-        mockkObject(DashboardSessionTokenRefresher)
-        every { DashboardSessionTokenRefresher.fetch(any(), any()) } returns null
 
         var serverSocket1: WebSocket? = null
         var serverSocket2: WebSocket? = null
@@ -1523,6 +1525,11 @@ class HermesWsClientTest {
 
         HermesWsClient.connect()
 
+        runBlocking {
+            withTimeout(5000) {
+                HermesWsClient.connectionStatus.first { it == ConnectionStatus.DISCONNECTED }
+            }
+        }
         assertEquals(ConnectionStatus.DISCONNECTED, HermesWsClient.connectionStatus.value)
         ticketServer.shutdown()
     }
@@ -1549,6 +1556,11 @@ class HermesWsClientTest {
 
         HermesWsClient.connect()
 
+        runBlocking {
+            withTimeout(5000) {
+                HermesWsClient.connectionStatus.first { it == ConnectionStatus.RECONNECTING }
+            }
+        }
         assertEquals(ConnectionStatus.RECONNECTING, HermesWsClient.connectionStatus.value)
     }