Skip to content

Commit f9320d1

Browse files
ovitrifclaude
andcommitted
fix: address PR review comments for mnemonic crash fix
- Remove unused assertTrue import from VssBackupClientTest - Remove doc comments and inline comments from VssBackupClient - Refactor setup() to return Result<Boolean> - Add DSL-style setupWithRetry() with SetupRetryLogger builder - Move retry logic from BackupRepo to VssBackupClient - Update tests for Result<Boolean> return type Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1950109 commit f9320d1

3 files changed

Lines changed: 64 additions & 52 deletions

File tree

app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import com.synonym.vssclient.vssNewClientWithLnurlAuth
1010
import com.synonym.vssclient.vssStore
1111
import kotlinx.coroutines.CompletableDeferred
1212
import kotlinx.coroutines.CoroutineDispatcher
13+
import kotlinx.coroutines.delay
1314
import kotlinx.coroutines.withContext
1415
import kotlinx.coroutines.withTimeout
1516
import to.bitkit.data.keychain.Keychain
@@ -28,24 +29,15 @@ class VssBackupClient @Inject constructor(
2829
) {
2930
private var isSetup = CompletableDeferred<Unit>()
3031

31-
/**
32-
* Sets up the VSS client. Returns true if setup succeeded, false if mnemonic not available.
33-
* Throws on other errors.
34-
*/
35-
suspend fun setup(walletIndex: Int = 0): Boolean = withContext(ioDispatcher) {
36-
// If already set up successfully, return immediately
37-
if (isSetup.isCompleted && !isSetup.isCancelled) {
38-
runCatching { isSetup.await() }.onSuccess { return@withContext true }
39-
}
32+
suspend fun setup(walletIndex: Int = 0): Result<Boolean> = withContext(ioDispatcher) {
33+
runCatching {
34+
if (isSetup.isCompleted && !isSetup.isCancelled) {
35+
runCatching { isSetup.await() }.onSuccess { return@runCatching true }
36+
}
4037

41-
// Check if mnemonic is available before proceeding
42-
val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)
43-
if (mnemonic == null) {
44-
Logger.warn("VSS client setup deferred: mnemonic not available yet", context = TAG)
45-
return@withContext false
46-
}
38+
val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)
39+
?: return@runCatching false
4740

48-
runCatching {
4941
withTimeout(30.seconds) {
5042
Logger.debug("VSS client setting up…", context = TAG)
5143
val vssUrl = Env.vssServerUrl
@@ -72,11 +64,44 @@ class VssBackupClient @Inject constructor(
7264
isSetup.complete(Unit)
7365
Logger.info("VSS client setup with server: '$vssUrl'", context = TAG)
7466
}
67+
true
7568
}.onFailure {
7669
isSetup.completeExceptionally(it)
77-
Logger.error("VSS client setup error", e = it, context = TAG)
70+
Logger.error("VSS client setup error", it, TAG)
71+
}
72+
}
73+
74+
class SetupRetryLogger {
75+
var onSuccess: (attempt: Int) -> Unit = {}
76+
var onRetry: (attempt: Int, maxAttempts: Int, delayMs: Long) -> Unit = { _, _, _ -> }
77+
var onExhausted: (maxAttempts: Int) -> Unit = {}
78+
}
79+
80+
suspend fun setupWithRetry(
81+
maxAttempts: Int = 10,
82+
baseDelayMs: Long = 1000L,
83+
logger: SetupRetryLogger.() -> Unit,
84+
): Result<Boolean> = withContext(ioDispatcher) {
85+
val log = SetupRetryLogger().apply(logger)
86+
var attempt = 0
87+
while (attempt < maxAttempts) {
88+
val result = setup()
89+
if (result.getOrDefault(false)) {
90+
log.onSuccess(attempt + 1)
91+
return@withContext Result.success(true)
92+
}
93+
if (result.isFailure) {
94+
return@withContext result
95+
}
96+
attempt++
97+
if (attempt < maxAttempts) {
98+
val delayMs = baseDelayMs * attempt
99+
log.onRetry(attempt, maxAttempts, delayMs)
100+
delay(delayMs)
101+
}
78102
}
79-
true
103+
log.onExhausted(maxAttempts)
104+
Result.success(false)
80105
}
81106

82107
fun reset() {

app/src/main/java/to/bitkit/repositories/BackupRepo.kt

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,22 @@ class BackupRepo @Inject constructor(
120120
isObserving = true
121121
Logger.debug("Start observing backup statuses and data store changes", context = TAG)
122122

123-
scope.launch { setupVssClientWithRetry() }
123+
scope.launch {
124+
vssBackupClient.setupWithRetry {
125+
onSuccess = { attempt ->
126+
Logger.debug("VSS client setup succeeded on attempt $attempt", context = TAG)
127+
}
128+
onRetry = { attempt, maxAttempts, delayMs ->
129+
Logger.debug(
130+
"VSS client setup deferred, retrying in ${delayMs}ms (attempt $attempt/$maxAttempts)",
131+
context = TAG,
132+
)
133+
}
134+
onExhausted = { maxAttempts ->
135+
Logger.warn("VSS client setup failed after $maxAttempts attempts", context = TAG)
136+
}
137+
}
138+
}
124139

125140
scope.launch {
126141
BackupCategory.entries.forEach { category ->
@@ -166,33 +181,6 @@ class BackupRepo @Inject constructor(
166181
Logger.debug("Stopped observing backup statuses and data store changes", context = TAG)
167182
}
168183

169-
private suspend fun setupVssClientWithRetry() {
170-
var attempt = 0
171-
val maxAttempts = 10
172-
val baseDelayMs = 1000L
173-
174-
while (attempt < maxAttempts && isObserving) {
175-
val success = runCatching { vssBackupClient.setup() }.getOrDefault(false)
176-
if (success) {
177-
Logger.debug("VSS client setup succeeded on attempt ${attempt + 1}", context = TAG)
178-
return
179-
}
180-
attempt++
181-
if (attempt < maxAttempts) {
182-
val delayMs = baseDelayMs * attempt // Linear backoff: 1s, 2s, 3s, ...
183-
Logger.debug(
184-
"VSS client setup deferred, retrying in ${delayMs}ms (attempt $attempt/$maxAttempts)",
185-
context = TAG,
186-
)
187-
delay(delayMs)
188-
}
189-
}
190-
191-
if (isObserving) {
192-
Logger.warn("VSS client setup failed after $maxAttempts attempts", context = TAG)
193-
}
194-
}
195-
196184
private fun startBackupStatusObservers() {
197185
// Observe backup status changes for each category
198186
BackupCategory.entries.forEach { category ->
@@ -570,7 +558,7 @@ class BackupRepo @Inject constructor(
570558
suspend fun getLatestBackupTime(): ULong? = withContext(ioDispatcher) {
571559
runCatching {
572560
withTimeout(VSS_TIMESTAMP_TIMEOUT) {
573-
vssBackupClient.setup()
561+
vssBackupClient.setup().getOrThrow()
574562
coroutineScope {
575563
BackupCategory.entries
576564
.filter { it != BackupCategory.LIGHTNING_CONNECTIONS }

app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import org.mockito.kotlin.whenever
1111
import to.bitkit.data.keychain.Keychain
1212
import to.bitkit.test.BaseUnitTest
1313
import kotlin.test.assertFalse
14-
import kotlin.test.assertTrue
1514

1615
class VssBackupClientTest : BaseUnitTest() {
1716

@@ -35,7 +34,7 @@ class VssBackupClientTest : BaseUnitTest() {
3534

3635
val result = sut.setup()
3736

38-
assertFalse(result)
37+
assertFalse(result.getOrThrow())
3938
}
4039

4140
@Test
@@ -65,8 +64,8 @@ class VssBackupClientTest : BaseUnitTest() {
6564
whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(null)
6665

6766
// Multiple calls should all return false without crashing
68-
assertFalse(sut.setup())
69-
assertFalse(sut.setup())
70-
assertFalse(sut.setup())
67+
assertFalse(sut.setup().getOrThrow())
68+
assertFalse(sut.setup().getOrThrow())
69+
assertFalse(sut.setup().getOrThrow())
7170
}
7271
}

0 commit comments

Comments
 (0)