Skip to content

Commit a572853

Browse files
committed
fix(core:engine): surface TorState.Error on silent startup failures and add 90s timeout
Three bugs caused the Tor loading overlay to freeze the UI indefinitely: 1. registerStatusReceiver() assigned torReceiver before registerReceiver() succeeded. If registration threw, the field was non-null but unregistered. Future calls returned early on the null-guard so no STATUS broadcasts ever arrived and _torState stayed Connecting forever. 2. ensureStarted() only logged startService() failures. _torState remained Connecting with no way to transition to Error, blocking the UI. 3. onAppReady() used torState.filter().first() with no timeout. Any silent failure (SELinux denial, native crash with no onServiceDisconnected, broadcast registration failure) suspended the coroutine and froze the overlay forever. Fixes: - registerStatusReceiver: assign torReceiver only after successful registration; emit TorState.Error on failure so future ensureStarted() calls can retry. - ensureStarted: emit TorState.Error in the onFailure handler. - onAppReady: wrap filter.first() in withTimeoutOrNull(90s); treat null as Error. Tests added: - TorManagerTest T6: ensureStarted emits Error when startService throws. - WebViewViewModelTorTest T7: onAppReady surfaces error after 90s timeout. - Fixed Test 2: replaced advanceUntilIdle() with runCurrent() between onAppReady() and the Ready emission to avoid accidentally firing the new timeout delay.
1 parent 93bbcf1 commit a572853

5 files changed

Lines changed: 100 additions & 9 deletions

File tree

core/engine/build.gradle.kts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ plugins {
55
android {
66
namespace = "io.shellify.core.engine"
77

8+
testOptions {
9+
unitTests {
10+
// Prevents android.util.Log.* from throwing "not mocked" in JVM unit tests.
11+
isReturnDefaultValues = true
12+
}
13+
}
14+
815
packaging {
916
jniLibs {
1017
excludes += "**/libxul.so"

core/engine/src/main/java/io/shellify/app/core/engine/TorManager.kt

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,11 @@ class TorManager(
146146
val bound = context.bindService(intent, torServiceConnection, Context.BIND_AUTO_CREATE)
147147
if (!bound) Log.w(TAG, "bindService returned false — process-death detection unavailable")
148148
}
149-
}.onFailure { Log.e(TAG, "Failed to start TorService", it) }
149+
}.onFailure {
150+
// Surface the failure so the UI can offer a retry instead of freezing on Connecting.
151+
Log.e(TAG, "Failed to start TorService", it)
152+
_torState.value = TorState.Error("Failed to start Tor: ${it.message}")
153+
}
150154
}
151155
}
152156

@@ -227,7 +231,11 @@ class TorManager(
227231
private fun registerStatusReceiver() {
228232
if (torReceiver != null) return
229233

230-
torReceiver = object : BroadcastReceiver() {
234+
// Build the receiver locally so it is only assigned to torReceiver after successful
235+
// registration. If registerReceiver() throws and we assigned torReceiver first, the
236+
// null-guard at the top of this function would skip all future registration attempts,
237+
// leaving _torState stuck at Connecting forever (no broadcasts would ever arrive).
238+
val receiver = object : BroadcastReceiver() {
231239
override fun onReceive(ctx: Context, intent: Intent) {
232240
val status = intent.getStringExtra(TorService.EXTRA_STATUS) ?: return
233241
when (status) {
@@ -243,11 +251,17 @@ class TorManager(
243251
runCatching {
244252
ContextCompat.registerReceiver(
245253
context,
246-
torReceiver,
254+
receiver,
247255
IntentFilter(TorService.ACTION_STATUS),
248256
ContextCompat.RECEIVER_NOT_EXPORTED,
249257
)
250-
}.onFailure { Log.w(TAG, "Failed to register TorService receiver", it) }
258+
// Assign only on success so a failed registration leaves torReceiver null
259+
// and the next ensureStarted() call can attempt registration again.
260+
torReceiver = receiver
261+
}.onFailure {
262+
Log.e(TAG, "Failed to register TorService receiver", it)
263+
_torState.value = TorState.Error("Failed to listen for Tor status: ${it.message}")
264+
}
251265
}
252266

253267
private fun stop() {

core/engine/src/test/java/io/shellify/app/core/engine/TorManagerTest.kt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package io.shellify.app.core.engine
22

33
import android.content.ComponentName
4+
import android.content.Context
5+
import io.mockk.every
46
import io.mockk.mockk
57
import io.mockk.verify
68
import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -92,4 +94,21 @@ class TorManagerTest {
9294
val state = manager.torState.value
9395
assertTrue("Expected TorState.Error but got $state", state is TorState.Error)
9496
}
97+
98+
@Test
99+
fun `T6 - ensureStarted emits TorState Error when startService throws`() = testScope.runTest {
100+
// Arrange: context.startService throws (e.g. IllegalStateException from background start).
101+
val context = mockk<Context>(relaxed = true) {
102+
every { startService(any()) } throws IllegalStateException("Not allowed in background")
103+
}
104+
val manager = TorManager(context = context, testScope = testScope)
105+
106+
// Act
107+
manager.ensureStarted(appId = 1L, preserveIdentity = false)
108+
testDispatcher.scheduler.advanceUntilIdle()
109+
110+
// Assert: error is surfaced so the UI can show Retry instead of freezing on Connecting.
111+
val state = manager.torState.value
112+
assertTrue("Expected TorState.Error after startService failure but got $state", state is TorState.Error)
113+
}
95114
}

feature/webview/src/main/java/io/shellify/app/presentation/webview/WebViewViewModel.kt

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import kotlinx.coroutines.flow.filter
2525
import kotlinx.coroutines.flow.first
2626
import kotlinx.coroutines.flow.update
2727
import kotlinx.coroutines.launch
28+
import kotlinx.coroutines.withTimeoutOrNull
2829

2930
class WebViewViewModel(
3031
private val initialApp: WebApp,
@@ -221,7 +222,12 @@ class WebViewViewModel(
221222
// This is the critical gate that prevents DNS and traffic leaks (T-02-23).
222223
// Also handles TorState.Error so the coroutine is not suspended indefinitely
223224
// when TorService fails to start (CR-02).
224-
val state = tm.torState.filter { it is TorState.Ready || it is TorState.Error }.first()
225+
// withTimeoutOrNull guards against TorService hanging silently (e.g. native
226+
// binary crash with no onServiceDisconnected, or registerReceiver failure) —
227+
// without it the overlay freezes the UI forever (CR-07).
228+
val state = withTimeoutOrNull(TOR_STARTUP_TIMEOUT_MS) {
229+
tm.torState.filter { it is TorState.Ready || it is TorState.Error }.first()
230+
} ?: TorState.Error("Tor startup timed out after ${TOR_STARTUP_TIMEOUT_MS / 1_000}s")
225231
if (state is TorState.Error) {
226232
_uiState.update { it.copy(error = WebLoadError.from(-1, state.message)) }
227233
return@launch
@@ -367,6 +373,15 @@ class WebViewViewModel(
367373

368374
private fun currentApp(): WebApp = _uiState.value.app ?: initialApp
369375

376+
companion object {
377+
/**
378+
* Maximum time to wait for [TorState.Ready] or [TorState.Error] before aborting.
379+
* Prevents the Tor connecting overlay from freezing the UI indefinitely when
380+
* TorService hangs or crashes without emitting a status broadcast (CR-07).
381+
*/
382+
internal const val TOR_STARTUP_TIMEOUT_MS = 90_000L
383+
}
384+
370385
class Factory(
371386
private val app: WebApp,
372387
private val services: WebViewServiceProvider,

feature/webview/src/test/java/io/shellify/app/presentation/webview/WebViewViewModelTorTest.kt

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ import kotlinx.coroutines.flow.first
2020
import kotlinx.coroutines.launch
2121
import kotlinx.coroutines.test.StandardTestDispatcher
2222
import kotlinx.coroutines.test.TestScope
23+
import kotlinx.coroutines.test.advanceTimeBy
2324
import kotlinx.coroutines.test.advanceUntilIdle
2425
import kotlinx.coroutines.test.resetMain
26+
import kotlinx.coroutines.test.runCurrent
2527
import kotlinx.coroutines.test.runTest
2628
import kotlinx.coroutines.test.setMain
2729
import org.junit.After
2830
import org.junit.Assert.assertEquals
2931
import org.junit.Assert.assertFalse
32+
import org.junit.Assert.assertTrue
3033
import org.junit.Before
3134
import org.junit.Test
3235

@@ -130,20 +133,26 @@ class WebViewViewModelTorTest {
130133
fun `when useTor true, onAppReady gates LoadUrl on TorState Ready`() = testScope.runTest {
131134
torStateFlow.value = TorState.Connecting
132135
val vm = createViewModel(torApp)
133-
advanceUntilIdle()
136+
advanceUntilIdle() // Safe: no withTimeoutOrNull in play yet during vm init.
134137

135138
vm.onAppReady(torApp)
136-
advanceUntilIdle()
139+
// Use runCurrent() instead of advanceUntilIdle() here and below.
140+
// advanceUntilIdle() advances virtual time through ALL pending delays, including
141+
// the TOR_STARTUP_TIMEOUT_MS delay, which would fire the timeout and emit an error
142+
// before TorState.Ready ever arrives — causing a false failure.
143+
runCurrent()
137144

138145
// Should NOT have emitted LoadUrl yet while Connecting
139146
val commands = mutableListOf<WebViewCommand>()
140147
val job = launch { vm.commands.collect { commands += it } }
141148

142149
// No LoadUrl should be emitted while Connecting
143-
advanceUntilIdle()
150+
runCurrent()
144151
assertFalse("No LoadUrl while Connecting", commands.any { it is WebViewCommand.LoadUrl })
145152

146-
// Emit Ready — now LoadUrl should be dispatched
153+
// Emit Ready — now LoadUrl should be dispatched.
154+
// advanceUntilIdle() is safe here because withTimeoutOrNull cancels its internal timer
155+
// the moment filter.first() returns TorState.Ready, leaving no residual delayed tasks.
147156
torStateFlow.value = TorState.Ready
148157
advanceUntilIdle()
149158

@@ -218,4 +227,31 @@ class WebViewViewModelTorTest {
218227
advanceUntilIdle()
219228
assertEquals(TorState.Ready, vm.uiState.value.torState)
220229
}
230+
231+
/**
232+
* Test 7: When TorService never emits Ready or Error within TOR_STARTUP_TIMEOUT_MS,
233+
* onAppReady must surface a WebLoadError instead of freezing the UI forever (CR-07).
234+
*/
235+
@Test
236+
fun `when tor startup exceeds timeout, onAppReady surfaces error instead of freezing`() =
237+
testScope.runTest {
238+
// TorService is stuck at Connecting — no Ready/Error broadcast ever arrives.
239+
torStateFlow.value = TorState.Connecting
240+
val vm = createViewModel(torApp)
241+
runCurrent() // Run vm init without advancing virtual time.
242+
243+
vm.onAppReady(torApp)
244+
runCurrent() // Run the coroutine to its filter.first() suspension point.
245+
246+
// Just before the deadline: no error yet.
247+
// advanceTimeBy runs all tasks that become due at the new virtual time, so
248+
// no explicit runCurrent() is needed after it.
249+
advanceTimeBy(WebViewViewModel.TOR_STARTUP_TIMEOUT_MS - 1_000L)
250+
assertEquals(null, vm.uiState.value.error)
251+
252+
// Cross the deadline: timeout fires, withTimeoutOrNull returns null, error is set.
253+
advanceTimeBy(2_000L)
254+
val error = vm.uiState.value.error
255+
assertTrue("Expected a WebLoadError after timeout but got $error", error != null)
256+
}
221257
}

0 commit comments

Comments
 (0)