Wrap Android connect permission failures (#1167)

Author: himu-gupta

kable-core/src/androidHostTest/kotlin/com/juul/kable/BluetoothDeviceTests.kt

@@ -0,0 +1,77 @@
+package com.juul.kable
+
+import android.bluetooth.BluetoothDevice
+import android.bluetooth.BluetoothGattCallback
+import android.content.Context
+import android.os.Build
+import android.os.Handler
+import com.juul.kable.logs.Logging
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.verify
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.MutableStateFlow
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import org.robolectric.annotation.Config
+import kotlin.coroutines.EmptyCoroutineContext
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.assertSame
+import kotlin.time.Duration.Companion.seconds
+
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [Build.VERSION_CODES.S])
+class BluetoothDeviceTests {
+
+    @Test
+    fun connect_securityException_wrapsInIllegalStateExceptionAndReleasesThreading() {
+        val securityException = SecurityException("Missing BLUETOOTH_CONNECT permission")
+        val device = mockk<BluetoothDevice> {
+            every { address } returns "00:11:22:AA:BB:CC"
+            every {
+                connectGatt(
+                    any<Context>(),
+                    false,
+                    any<BluetoothGattCallback>(),
+                    any<Int>(),
+                    any<Int>(),
+                    any<Handler>(),
+                )
+            } throws securityException
+        }
+        val threading = mockk<Threading.Handler> {
+            every { handler } returns mockk()
+        }
+        val threadingStrategy = mockk<ThreadingStrategy> {
+            every { acquire() } returns threading
+            every { release(threading) } just Runs
+        }
+        every { threading.strategy } returns threadingStrategy
+
+        val exception = assertFailsWith<IllegalStateException> {
+            device.connect(
+                coroutineContext = EmptyCoroutineContext,
+                context = RuntimeEnvironment.getApplication(),
+                autoConnect = false,
+                transport = Transport.Auto,
+                phy = Phy.Le1M,
+                state = MutableStateFlow(State.Disconnected()),
+                services = MutableStateFlow<List<PlatformDiscoveredService>?>(null),
+                mtu = MutableStateFlow<Int?>(null),
+                onCharacteristicChanged = MutableSharedFlow<ObservationEvent<ByteArray>>(),
+                logging = Logging(),
+                threadingStrategy = threadingStrategy,
+                disconnectTimeout = 1.seconds,
+            )
+        }
+
+        assertEquals(securityException.message, exception.message)
+        assertSame(securityException, exception.cause)
+        verify(exactly = 1) { threadingStrategy.release(threading) }
+    }
+}

kable-core/src/androidMain/kotlin/BluetoothDevice.kt

@@ -54,6 +54,9 @@ internal fun BluetoothDevice.connect(
 
             else -> connectGattCompat(context, autoConnect, callback, transport)
         } ?: throw IOException("Binder remote-invocation error")
+    } catch (e: SecurityException) {
+        threading.release()
+        throw IllegalStateException(e.message, e)
     } catch (t: Throwable) {
         threading.release()
         throw t

kable-core/src/commonMain/kotlin/SharedRepeatableAction.awaitConnect.kt

@@ -3,15 +3,18 @@ package com.juul.kable
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.ensureActive
+import kotlin.coroutines.cancellation.CancellationException
 
 internal suspend fun SharedRepeatableAction<CoroutineScope>.awaitConnect(): CoroutineScope =
     try {
         await()
-    } catch (e: IllegalStateException) {
-        // If calling code cancels we'd unintentionally swallow the `CancellationException`, as it
-        // is a subclass of `IllegalStateException`. `ensureActive()` honors caller cancellation.
+    } catch (e: CancellationException) {
+        // A disconnect cancels the shared action, while caller cancellation must remain a
+        // CancellationException. `ensureActive()` distinguishes between the two cases.
         // https://github.com/JuulLabs/kable/issues/1136
         currentCoroutineContext().ensureActive()
 
+        throw IllegalStateException("Cannot connect peripheral that has been cancelled", e)
+    } catch (e: InactiveScopeException) {
         throw IllegalStateException("Cannot connect peripheral that has been cancelled", e)
     }

kable-core/src/commonMain/kotlin/SharedRepeatableAction.kt

@@ -13,6 +13,8 @@ internal fun <T> CoroutineScope.sharedRepeatableAction(
     action: suspend (scope: CoroutineScope) -> T,
 ) = SharedRepeatableAction(this, action)
 
+internal class InactiveScopeException : IllegalStateException("Scope is not active")
+
 /**
  * A mechanism for launching and awaiting a shared [action][State.action] ([Deferred]) repeatedly.
  *
@@ -80,7 +82,7 @@ internal class SharedRepeatableAction<T>(
     }
 
     private fun create(): State<T> {
-        check(scope.coroutineContext.job.isActive) { "Scope is not active" }
+        if (!scope.coroutineContext.job.isActive) throw InactiveScopeException()
         val root = Job(scope.coroutineContext.job)
         val scope = CoroutineScope(scope.coroutineContext + root)
         val action = scope.async { action(scope) }

kable-core/src/commonTest/kotlin/SharedRepeatableActionAwaitConnectTests.kt

@@ -0,0 +1,68 @@
+package com.juul.kable
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.CoroutineStart.UNDISPATCHED
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitCancellation
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.supervisorScope
+import kotlinx.coroutines.test.runTest
+import kotlin.coroutines.cancellation.CancellationException
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+
+class SharedRepeatableActionAwaitConnectTests {
+
+    @Test
+    fun illegalStateExceptionFromAction_isPreserved() = runTest {
+        val expected = IllegalStateException("Missing BLUETOOTH_CONNECT permission")
+        val actionScope = CoroutineScope(SupervisorJob())
+        val action: SharedRepeatableAction<CoroutineScope> = actionScope.sharedRepeatableAction {
+            throw expected
+        }
+
+        val actual = assertFailsWith<IllegalStateException> {
+            action.awaitConnect()
+        }
+
+        assertEquals(expected.message, actual.message)
+        actionScope.cancel()
+    }
+
+    @Test
+    fun inactiveParentScope_isReportedAsCancelledPeripheral() = runTest {
+        val parentScope = CoroutineScope(SupervisorJob())
+        val action = parentScope.sharedRepeatableAction { it }
+        parentScope.cancel()
+
+        val exception = assertFailsWith<IllegalStateException> {
+            action.awaitConnect()
+        }
+
+        assertEquals("Cannot connect peripheral that has been cancelled", exception.message)
+    }
+
+    @Test
+    fun cancelledAction_isReportedAsCancelledPeripheral() = runTest {
+        val actionScope = CoroutineScope(coroutineContext + SupervisorJob())
+        val action: SharedRepeatableAction<CoroutineScope> = actionScope.sharedRepeatableAction {
+            awaitCancellation()
+        }
+
+        supervisorScope {
+            val deferred = async(start = UNDISPATCHED) {
+                action.awaitConnect()
+            }
+            action.cancelAndJoin(CancellationException("Disconnect requested"))
+
+            val exception = assertFailsWith<IllegalStateException> {
+                deferred.await()
+            }
+
+            assertEquals("Cannot connect peripheral that has been cancelled", exception.message)
+        }
+        actionScope.cancel()
+    }
+}