fix: tab reloads actually reload — kanban switch spam, plugins/skills refresh, ws mint (#897)
- ws: ticket-mint response body was read on the calling (main) thread —
the socket read threw NetworkOnMainThreadException, the mint failed,
and the kanban events stream never connected (board stuck Offline).
execute() + body read now run entirely on Dispatchers.IO; same fix in
DashboardSessionTokenRefresher.fetch (loopback mint path).
- kanban: re-selecting the already-active board reloads data and
reconnects events WITHOUT the redundant POST /boards/{id}/switch that
refresh fired on every tap.
- plugins: top-bar Refresh + pull-to-refresh wired through HermesScaffold
(previously no reload control at all).
- skills: Refresh icon now does a plain GET reload; the heavy hub-update
POST moved to the cloud icon so refresh is no longer disguised as update.
- test: mock the 3-arg Log.w overload (catch now logs the full stack).
Co-authored-by: M57 <hy4ri@users.noreply.github.com>
This commit is contained in:
+8
-1
@@ -1,6 +1,7 @@
|
|||||||
package com.m57.hermescontrol.data.remote
|
package com.m57.hermescontrol.data.remote
|
||||||
|
|
||||||
import com.m57.hermescontrol.data.local.AuthManager
|
import com.m57.hermescontrol.data.local.AuthManager
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
import okhttp3.OkHttpClient
|
import okhttp3.OkHttpClient
|
||||||
import okhttp3.Request
|
import okhttp3.Request
|
||||||
import okio.Buffer
|
import okio.Buffer
|
||||||
@@ -31,8 +32,13 @@ internal object DashboardSessionTokenRefresher {
|
|||||||
.url(baseUrl)
|
.url(baseUrl)
|
||||||
.get()
|
.get()
|
||||||
.build()
|
.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 ->
|
client.newCall(request).execute().use { response ->
|
||||||
if (!response.isSuccessful) return null
|
if (!response.isSuccessful) return@runBlocking null
|
||||||
// Cap the read — the SPA HTML can be large, and we only need the
|
// 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
|
// injected token near the top of the document. Reading the whole
|
||||||
// body into a String risks OOM on low-end devices.
|
// body into a String risks OOM on low-end devices.
|
||||||
@@ -48,6 +54,7 @@ internal object DashboardSessionTokenRefresher {
|
|||||||
?.getOrNull(1)
|
?.getOrNull(1)
|
||||||
?.takeIf { it.isNotBlank() }
|
?.takeIf { it.isNotBlank() }
|
||||||
}
|
}
|
||||||
|
}
|
||||||
} catch (_: Exception) {
|
} catch (_: Exception) {
|
||||||
null
|
null
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -450,15 +450,18 @@ object HermesWsClient {
|
|||||||
.post("{}".toRequestBody())
|
.post("{}".toRequestBody())
|
||||||
.build()
|
.build()
|
||||||
|
|
||||||
// Run network call on Dispatchers.IO to avoid NetworkOnMainThreadException
|
// Run the ENTIRE call on Dispatchers.IO — execute() already hops,
|
||||||
val response =
|
// 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) {
|
kotlinx.coroutines.runBlocking(Dispatchers.IO) {
|
||||||
client.newCall(request).execute()
|
client.newCall(request).execute().use { resp ->
|
||||||
|
resp.code to resp.body?.string()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
response.use {
|
if (body != null && code in 200..299) {
|
||||||
if (response.isSuccessful) {
|
|
||||||
val body = response.body.string()
|
|
||||||
val ticketMatch = Regex("""\"ticket\":\"([^\"]+)\"""").find(body)
|
val ticketMatch = Regex("""\"ticket\":\"([^\"]+)\"""").find(body)
|
||||||
val ticket = ticketMatch?.groupValues?.getOrNull(1)
|
val ticket = ticketMatch?.groupValues?.getOrNull(1)
|
||||||
if (!ticket.isNullOrBlank()) {
|
if (!ticket.isNullOrBlank()) {
|
||||||
@@ -466,12 +469,15 @@ object HermesWsClient {
|
|||||||
}
|
}
|
||||||
Log.w(TAG, "WS ticket mint failed: response body did not contain ticket")
|
Log.w(TAG, "WS ticket mint failed: response body did not contain ticket")
|
||||||
} else {
|
} else {
|
||||||
Log.w(TAG, "WS ticket mint failed: HTTP ${response.code}")
|
Log.w(TAG, "WS ticket mint failed: HTTP $code")
|
||||||
return TicketRequestResult(null, response.code)
|
return TicketRequestResult(null, code)
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.w(TAG, "WS ticket mint failed: ${e.javaClass.simpleName}")
|
// Include the stack: the exception class alone (e.g.
|
||||||
|
// NetworkOnMainThreadException) can't show WHICH caller on what
|
||||||
|
// thread tripped the mint (main-scope kanban events connect vs
|
||||||
|
// the WS reconnect path).
|
||||||
|
Log.w(TAG, "WS ticket mint failed: ${e.javaClass.simpleName}", e)
|
||||||
return TicketRequestResult(null, null, exception = true)
|
return TicketRequestResult(null, null, exception = true)
|
||||||
}
|
}
|
||||||
return TicketRequestResult(null, null)
|
return TicketRequestResult(null, null)
|
||||||
|
|||||||
@@ -121,6 +121,15 @@ class KanbanViewModel(
|
|||||||
}
|
}
|
||||||
|
|
||||||
fun selectBoard(board: KanbanBoard) {
|
fun selectBoard(board: KanbanBoard) {
|
||||||
|
val current = _uiState.value.selectedBoard
|
||||||
|
if (current?.id == board.id) {
|
||||||
|
// Already-active board: reload its data and (re)connect the events
|
||||||
|
// stream WITHOUT the /switch round-trip — refresh and createTask
|
||||||
|
// re-select the same board and must not POST a redundant switch.
|
||||||
|
viewModelScope.launch { loadBoardIntoState(board) }
|
||||||
|
connectEvents(board)
|
||||||
|
return
|
||||||
|
}
|
||||||
_uiState.update { it.copy(selectedBoard = board, isLoading = true, errorMessage = null) }
|
_uiState.update { it.copy(selectedBoard = board, isLoading = true, errorMessage = null) }
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
val switchResult =
|
val switchResult =
|
||||||
|
|||||||
@@ -32,7 +32,6 @@ import androidx.compose.material3.OutlinedTextField
|
|||||||
import androidx.compose.material3.Switch
|
import androidx.compose.material3.Switch
|
||||||
import androidx.compose.material3.Text
|
import androidx.compose.material3.Text
|
||||||
import androidx.compose.material3.TextButton
|
import androidx.compose.material3.TextButton
|
||||||
import androidx.compose.material3.pulltorefresh.PullToRefreshBox
|
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
import androidx.compose.runtime.LaunchedEffect
|
import androidx.compose.runtime.LaunchedEffect
|
||||||
import androidx.compose.runtime.getValue
|
import androidx.compose.runtime.getValue
|
||||||
@@ -121,12 +120,9 @@ fun PluginsScreen(
|
|||||||
HermesScaffold(
|
HermesScaffold(
|
||||||
title = { Text(stringResource(R.string.screen_plugins)) },
|
title = { Text(stringResource(R.string.screen_plugins)) },
|
||||||
navigationIcon = onOpenDrawer?.let { NavIcon.Menu(it) },
|
navigationIcon = onOpenDrawer?.let { NavIcon.Menu(it) },
|
||||||
) { paddingValues ->
|
|
||||||
PullToRefreshBox(
|
|
||||||
isRefreshing = state.isLoading,
|
isRefreshing = state.isLoading,
|
||||||
onRefresh = { viewModel.loadPlugins() },
|
onRefresh = { viewModel.loadPlugins() },
|
||||||
modifier = Modifier.fillMaxSize(),
|
) { paddingValues ->
|
||||||
) {
|
|
||||||
when {
|
when {
|
||||||
state.isLoading && state.plugins.isEmpty() -> {
|
state.isLoading && state.plugins.isEmpty() -> {
|
||||||
SkeletonListState(modifier = Modifier.padding(paddingValues))
|
SkeletonListState(modifier = Modifier.padding(paddingValues))
|
||||||
@@ -206,7 +202,6 @@ fun PluginsScreen(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
showDetail?.let { plugin ->
|
showDetail?.let { plugin ->
|
||||||
DetailDialog(
|
DetailDialog(
|
||||||
|
|||||||
@@ -28,7 +28,6 @@ import androidx.compose.material.icons.filled.CloudDownload
|
|||||||
import androidx.compose.material.icons.filled.CloudOff
|
import androidx.compose.material.icons.filled.CloudOff
|
||||||
import androidx.compose.material.icons.filled.Edit
|
import androidx.compose.material.icons.filled.Edit
|
||||||
import androidx.compose.material.icons.filled.Extension
|
import androidx.compose.material.icons.filled.Extension
|
||||||
import androidx.compose.material.icons.filled.Refresh
|
|
||||||
import androidx.compose.material.icons.filled.Save
|
import androidx.compose.material.icons.filled.Save
|
||||||
import androidx.compose.material.icons.filled.Search
|
import androidx.compose.material.icons.filled.Search
|
||||||
import androidx.compose.material.icons.filled.Shield
|
import androidx.compose.material.icons.filled.Shield
|
||||||
@@ -113,11 +112,19 @@ fun SkillsScreen(
|
|||||||
modifier = modifier,
|
modifier = modifier,
|
||||||
title = { Text(stringResource(R.string.skills_screen_title)) },
|
title = { Text(stringResource(R.string.skills_screen_title)) },
|
||||||
navigationIcon = NavIcon.Menu(onOpen = onOpenDrawer),
|
navigationIcon = NavIcon.Menu(onOpen = onOpenDrawer),
|
||||||
|
isRefreshing = state.isLoading,
|
||||||
|
onRefresh = {
|
||||||
|
if (state.viewMode == SkillsViewMode.INSTALLED) {
|
||||||
|
viewModel.loadSkills()
|
||||||
|
} else {
|
||||||
|
viewModel.loadHubSources()
|
||||||
|
}
|
||||||
|
},
|
||||||
actions = {
|
actions = {
|
||||||
if (state.viewMode == SkillsViewMode.INSTALLED) {
|
if (state.viewMode == SkillsViewMode.INSTALLED) {
|
||||||
IconButton(onClick = { viewModel.updateSkillsFromHub() }) {
|
IconButton(onClick = { viewModel.updateSkillsFromHub() }) {
|
||||||
Icon(
|
Icon(
|
||||||
imageVector = Icons.Filled.Refresh,
|
imageVector = Icons.Filled.CloudDownload,
|
||||||
contentDescription = stringResource(R.string.content_desc_update_skills_hub),
|
contentDescription = stringResource(R.string.content_desc_update_skills_hub),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ class HermesWsClientTest {
|
|||||||
every { Log.d(any<String>(), any<String>()) } returns 0
|
every { Log.d(any<String>(), any<String>()) } returns 0
|
||||||
every { Log.i(any<String>(), any<String>()) } returns 0
|
every { Log.i(any<String>(), any<String>()) } returns 0
|
||||||
every { Log.w(any<String>(), any<String>()) } returns 0
|
every { Log.w(any<String>(), any<String>()) } returns 0
|
||||||
|
every { Log.w(any<String>(), any<String>(), any<Throwable>()) } returns 0
|
||||||
every { Log.e(any<String>(), any<String>(), any()) } returns 0
|
every { Log.e(any<String>(), any<String>(), any()) } returns 0
|
||||||
|
|
||||||
mockWebServer = MockWebServer()
|
mockWebServer = MockWebServer()
|
||||||
|
|||||||
Reference in New Issue
Block a user