Merge pull request #898 from synonymdev/chore/keychain-error-logs

jvsena42 · web-flow · commit cb141bc7c71a · 2026-04-21T08:53:36.000-03:00
chore: log cause of keychain load failures
diff --git a/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt b/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt
@@ -87,4 +87,6 @@ class AndroidKeyStore(
         }
         generateKey()
     }
+
+    fun containsAlias(): Boolean = keyStore.containsAlias(alias)
 }
diff --git a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt
@@ -19,8 +19,11 @@ import to.bitkit.ext.fromBase64
 import to.bitkit.ext.toBase64
 import to.bitkit.utils.AppError
 import to.bitkit.utils.Logger
+import java.util.Collections
+import java.util.concurrent.ConcurrentHashMap
 import javax.inject.Inject
 import javax.inject.Singleton
+import kotlin.coroutines.cancellation.CancellationException
 
 private val Context.keychainDataStore: DataStore<Preferences> by preferencesDataStore(
     name = "keychain"
@@ -32,59 +35,76 @@ class Keychain @Inject constructor(
     @ApplicationContext private val context: Context,
     @IoDispatcher private val dispatcher: CoroutineDispatcher,
 ) : BaseCoroutineScope(dispatcher) {
+    companion object {
+        private const val TAG = "Keychain"
+        private const val CAUSE_CHAIN_DEPTH = 4
+    }
+
     private val keyStore by lazy { AndroidKeyStore(alias = "keychain") }
 
     @Suppress("MemberNameEqualsClassName")
     private val keychain = context.keychainDataStore
 
+    private val loadDiagnosticsEmitted: MutableSet<String> =
+        Collections.newSetFromMap(ConcurrentHashMap<String, Boolean>())
+
     val snapshot get() = runBlocking(this.coroutineContext) { keychain.data.first() }
 
     fun loadString(key: String): String? = load(key)?.decodeToString()
 
-    @Suppress("TooGenericExceptionCaught", "SwallowedException")
+    @Suppress("TooGenericExceptionCaught")
     fun load(key: String): ByteArray? {
         try {
             return snapshot[key.indexed]?.fromBase64()?.let {
                 keyStore.decrypt(it)
             }
-        } catch (_: Exception) {
-            throw KeychainError.FailedToLoad(key)
+        } catch (c: CancellationException) {
+            throw c
+        } catch (t: Throwable) {
+            emitLoadDiagnosticsOnce(key, t)
+            throw KeychainError.FailedToLoad(key, cause = t)
         }
     }
 
     suspend fun saveString(key: String, value: String) = save(key, value.toByteArray())
 
-    @Suppress("TooGenericExceptionCaught", "SwallowedException")
+    @Suppress("TooGenericExceptionCaught", "ThrowsCount")
     suspend fun save(key: String, value: ByteArray) {
         if (exists(key)) throw KeychainError.FailedToSaveAlreadyExists(key)
 
         try {
             val encryptedValue = keyStore.encrypt(value)
             keychain.edit { it[key.indexed] = encryptedValue.toBase64() }
-        } catch (_: Exception) {
-            throw KeychainError.FailedToSave(key)
+        } catch (c: CancellationException) {
+            throw c
+        } catch (t: Throwable) {
+            throw KeychainError.FailedToSave(key, cause = t)
         }
         Logger.info("Saved to keychain: $key")
     }
 
     /** Inserts or replaces a string value associated with a given key in the keychain. */
-    @Suppress("TooGenericExceptionCaught", "SwallowedException")
+    @Suppress("TooGenericExceptionCaught")
     suspend fun upsertString(key: String, value: String) {
         try {
             val encryptedValue = keyStore.encrypt(value.toByteArray())
             keychain.edit { it[key.indexed] = encryptedValue.toBase64() }
-        } catch (_: Exception) {
-            throw KeychainError.FailedToSave(key)
+        } catch (c: CancellationException) {
+            throw c
+        } catch (t: Throwable) {
+            throw KeychainError.FailedToSave(key, cause = t)
         }
         Logger.info("Upsert in keychain: $key")
     }
 
-    @Suppress("TooGenericExceptionCaught", "SwallowedException")
+    @Suppress("TooGenericExceptionCaught")
     suspend fun delete(key: String) {
         try {
             keychain.edit { it.remove(key.indexed) }
-        } catch (_: Exception) {
-            throw KeychainError.FailedToDelete(key)
+        } catch (c: CancellationException) {
+            throw c
+        } catch (t: Throwable) {
+            throw KeychainError.FailedToDelete(key, cause = t)
         }
         Logger.debug("Deleted from keychain: $key")
     }
@@ -120,6 +140,32 @@ class Keychain @Inject constructor(
             .map { string -> string?.toIntOrNull() }
     }
 
+    private fun emitLoadDiagnosticsOnce(key: String, cause: Throwable) {
+        if (!loadDiagnosticsEmitted.add(key)) return
+
+        val aliasPresent = probe { keyStore.containsAlias() }
+        val entryPresent = probe { snapshot.contains(key.indexed) }
+        val walletIndex = probe {
+            runBlocking { db.configDao().getAll().first() }.firstOrNull()?.walletIndex ?: 0L
+        }
+        val causeChain = generateSequence(cause) { it.cause }
+            .take(CAUSE_CHAIN_DEPTH)
+            .joinToString(separator = " <- ") { it.javaClass.simpleName }
+
+        Logger.warn(
+            "Decrypt failed for key='$key' walletIndex='$walletIndex' " +
+                "aliasPresent='$aliasPresent' entryPresent='$entryPresent' " +
+                "causeChain='$causeChain'",
+            context = TAG,
+        )
+    }
+
+    private inline fun <T> probe(block: () -> T): String =
+        runCatching(block).fold(
+            onSuccess = { it.toString() },
+            onFailure = { "error:${it.javaClass.simpleName}" },
+        )
+
     enum class Key {
         PUSH_NOTIFICATION_TOKEN,
         PUSH_NOTIFICATION_PRIVATE_KEY,
@@ -130,10 +176,19 @@ class Keychain @Inject constructor(
     }
 }
 
-sealed class KeychainError(message: String) : AppError(message) {
-    class FailedToDelete(key: String) : KeychainError("Failed to delete $key from keychain.")
-    class FailedToLoad(key: String) : KeychainError("Failed to load $key from keychain.")
-    class FailedToSave(key: String) : KeychainError("Failed to save to $key keychain.")
+sealed class KeychainError(message: String, cause: Throwable? = null) : AppError(message, cause) {
+    class FailedToDelete(key: String, cause: Throwable? = null) :
+        KeychainError("Failed to delete $key from keychain${cause.causeSuffix()}", cause)
+
+    class FailedToLoad(val key: String, cause: Throwable? = null) :
+        KeychainError("Failed to load $key from keychain${cause.causeSuffix()}", cause)
+
+    class FailedToSave(key: String, cause: Throwable? = null) :
+        KeychainError("Failed to save to $key keychain${cause.causeSuffix()}", cause)
+
     class FailedToSaveAlreadyExists(key: String) :
         KeychainError("Key $key already exists in keychain. Explicitly delete key before attempting to update value.")
 }
+
+private fun Throwable?.causeSuffix(): String =
+    this?.let { " (cause='${it.javaClass.simpleName}')" } ?: "."
diff --git a/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt b/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt
@@ -0,0 +1,96 @@
+package to.bitkit.data.keychain
+
+import org.junit.Test
+import java.io.IOException
+import javax.crypto.AEADBadTagException
+import javax.crypto.BadPaddingException
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNull
+import kotlin.test.assertSame
+
+class KeychainErrorTest {
+
+    @Test
+    fun `FailedToLoad without cause renders key and terminating period`() {
+        val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC")
+
+        assertEquals("Failed to load BIP39_MNEMONIC from keychain.", error.message)
+        assertNull(error.cause)
+    }
+
+    @Test
+    fun `FailedToLoad preserves cause and embeds its simple class name`() {
+        val cause = AEADBadTagException("leak-probe-9f3a")
+        val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC", cause = cause)
+
+        assertSame(cause, error.cause)
+        assertEquals(
+            "Failed to load BIP39_MNEMONIC from keychain (cause='AEADBadTagException')",
+            error.message,
+        )
+    }
+
+    @Test
+    fun `FailedToLoad does not leak underlying cause message`() {
+        val probe = "leak-probe-9f3a"
+        val causes = listOf(
+            AEADBadTagException(probe),
+            BadPaddingException(probe),
+            ClassCastException(probe),
+            IllegalArgumentException(probe),
+            IOException(probe),
+        )
+
+        causes.forEach { cause ->
+            val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC", cause = cause)
+            assertFalse(
+                actual = error.message.orEmpty().contains(probe),
+                message = "message leaked cause.message for ${cause.javaClass.simpleName}",
+            )
+        }
+    }
+
+    @Test
+    fun `FailedToLoad exposes the failing key`() {
+        val error = KeychainError.FailedToLoad(key = "BIP39_PASSPHRASE", cause = IOException())
+
+        assertEquals("BIP39_PASSPHRASE", error.key)
+    }
+
+    @Test
+    fun `FailedToSave preserves cause and renders simple class name`() {
+        val cause = IOException("probe")
+        val error = KeychainError.FailedToSave(key = "BIP39_MNEMONIC", cause = cause)
+
+        assertSame(cause, error.cause)
+        assertEquals(
+            "Failed to save to BIP39_MNEMONIC keychain (cause='IOException')",
+            error.message,
+        )
+    }
+
+    @Test
+    fun `FailedToDelete preserves cause and renders simple class name`() {
+        val cause = IOException("probe")
+        val error = KeychainError.FailedToDelete(key = "BIP39_MNEMONIC", cause = cause)
+
+        assertSame(cause, error.cause)
+        assertEquals(
+            "Failed to delete BIP39_MNEMONIC from keychain (cause='IOException')",
+            error.message,
+        )
+    }
+
+    @Test
+    fun `FailedToSaveAlreadyExists has no cause and static message`() {
+        val error = KeychainError.FailedToSaveAlreadyExists(key = "BIP39_MNEMONIC")
+
+        assertNull(error.cause)
+        assertEquals(
+            "Key BIP39_MNEMONIC already exists in keychain. " +
+                "Explicitly delete key before attempting to update value.",
+            error.message,
+        )
+    }
+}
diff --git a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt
@@ -155,6 +155,17 @@ class WalletRepoTest : BaseUnitTest() {
         assertFalse(result)
     }
 
+    @Test
+    fun `walletExists relies on exists and does not touch load paths`() = test {
+        whenever(keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(true)
+
+        val result = sut.walletExists()
+
+        assertTrue(result)
+        verify(keychain, never()).loadString(Keychain.Key.BIP39_MNEMONIC.name)
+        verify(keychain, never()).load(Keychain.Key.BIP39_MNEMONIC.name)
+    }
+
     @Test
     fun `setWalletExistsState should update walletState with current existence status`() = test {
         whenever(keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(true)

Original file line number	Diff line number	Diff line change
`@@ -87,4 +87,6 @@ class AndroidKeyStore(`
`87`	`87`	`}`
`88`	`88`	`generateKey()`
`89`	`89`	`}`
	`90`	`+`
	`91`	`+ fun containsAlias(): Boolean = keyStore.containsAlias(alias)`
`90`	`92`	`}`