Redesign error model: typed transport exceptions + operation result types (breaking)

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Changelog
 
+### 2.0.0
+
+* **Breaking:** redesigned error model. Transport failures now throw a typed `AdbException` hierarchy (`AdbConnectException`, `AdbAuthException`, `AdbStreamOpenException`, `AdbConnectionClosedException`, `AdbTimeoutException`, `AdbProtocolException`), all extending `IOException`.
+* **Breaking:** operation outcomes are now returned instead of thrown: `install`/`installMultiple` return `InstallResult`, `uninstall` returns `UninstallResult`, `push`/`pull` return `SyncResult`, `root`/`unroot` return `RootResult`.
+* **Breaking:** a connection dropped mid-stream now throws `AdbConnectionClosedException` instead of presenting as a clean end-of-stream.
+* **Breaking:** a read or write that exceeds its timeout now throws `AdbTimeoutException` instead of a raw `SocketTimeoutException`.
+* `pmInstall` (the non-`cmd` install path) now checks the `pm install` result instead of ignoring it.
+* Added opt-in result helpers — `onSuccess`/`onFailure`/`orThrow` on each `…Result` type. `orThrow` raises `AdbOperationFailedException`, which is **not** an `AdbException`, so a transport-level `catch (AdbException)` won't catch an opted-in operation-failure throw.
+
 ### 1.2.10
 
 * Creating configuration for enabling keep alive on dadb socket

README.md

@@ -20,12 +20,26 @@ Connect to `emulator-5554` and install `apkFile`:
 
 ```kotlin
 Dadb.create("localhost", 5555).use { dadb ->
-    dadb.install(apkFile)
+    when (val result = dadb.install(apkFile)) {
+        is InstallResult.Success -> println("installed")
+        is InstallResult.Failure -> println("install failed: ${result.reason}")
+    }
 }
 ```
 
 *Note: Connect to the odd adb daemon port (5555), not the even emulator console port (5554)*
 
+### Error handling
+
+dadb distinguishes *transport* failures from *operation* failures:
+
+- **Transport failures throw** an `AdbException` (an `IOException`): `AdbConnectException`,
+  `AdbAuthException`, `AdbStreamOpenException`, `AdbConnectionClosedException`, `AdbProtocolException`.
+  Catch `AdbException` to decide whether to reconnect.
+- **Operation outcomes are returned**, never thrown: `install`/`installMultiple` → `InstallResult`,
+  `uninstall` → `UninstallResult`, `push`/`pull` → `SyncResult`, `root`/`unroot` → `RootResult`.
+  A non-zero `shell` exit code stays a value on `AdbShellResponse`.
+
 ### Discover a Device
 
 The following discovers and returns a connected device or emulator. If there are multiple it returns the first one found.

dadb/src/main/kotlin/dadb/AdbConnection.kt

@@ -25,6 +25,7 @@ import org.jetbrains.annotations.TestOnly
 import java.io.Closeable
 import java.io.IOException
 import java.net.Socket
+import java.net.SocketTimeoutException
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicInteger
 
@@ -40,7 +41,7 @@ internal class AdbConnection internal constructor(
     private val nextLocalId = AtomicInteger(0)
     private val messageQueue = AdbMessageQueue(adbReader)
 
-    @Throws(IOException::class)
+    @Throws(AdbException::class)
     fun open(destination: String): AdbStream {
         val localId = newId()
         messageQueue.startListening(localId)
@@ -49,6 +50,19 @@ internal class AdbConnection internal constructor(
             val message = messageQueue.take(localId, Constants.CMD_OKAY)
             val remoteId = message.arg0
             return AdbStreamImpl(messageQueue, adbWriter, maxPayloadSize, localId, remoteId)
+        } catch (e: AdbStreamClosed) {
+            // adbd answered A_OPEN with A_CLSE: it refused this service. Connection is still alive.
+            messageQueue.stopListening(localId)
+            throw AdbStreamOpenException(destination, "adbd refused to open stream: $destination", e)
+        } catch (e: AdbException) {
+            // Already a typed transport fault — don't re-wrap.
+            messageQueue.stopListening(localId)
+            throw e
+        } catch (e: IOException) {
+            // Raw socket fault while opening: a timeout (adbd unresponsive) vs the connection dying.
+            messageQueue.stopListening(localId)
+            throw if (e is SocketTimeoutException) AdbTimeoutException("Timed out opening stream: $destination", e)
+                  else AdbConnectionClosedException("Connection lost while opening stream: $destination", e)
         } catch (e: Throwable) {
             messageQueue.stopListening(localId)
             throw e
@@ -101,12 +115,20 @@ internal class AdbConnection internal constructor(
             return connect(source, sink, keyPair, socket)
         }
 
-        private fun connect(source: Source, sink: Sink, keyPair: AdbKeyPair? = null, closeable: Closeable? = null): AdbConnection {
+        internal fun connect(source: Source, sink: Sink, keyPair: AdbKeyPair? = null, closeable: Closeable? = null): AdbConnection {
             val adbReader = AdbReader(source)
             val adbWriter = AdbWriter(sink)
 
             try {
                 return connect(adbReader, adbWriter, keyPair, closeable)
+            } catch (e: AdbException) {
+                adbReader.close()
+                adbWriter.close()
+                throw e
+            } catch (e: IOException) {
+                adbReader.close()
+                adbWriter.close()
+                throw AdbConnectException("Connection handshake failed", e)
             } catch (t: Throwable) {
                 adbReader.close()
                 adbWriter.close()
@@ -120,8 +142,8 @@ internal class AdbConnection internal constructor(
             var message = adbReader.readMessage()
 
             if (message.command == Constants.CMD_AUTH) {
-                checkNotNull(keyPair) { "Authentication required but no KeyPair provided" }
-                check(message.arg0 == Constants.AUTH_TYPE_TOKEN) { "Unsupported auth type: $message" }
+                if (keyPair == null) throw AdbAuthException("Authentication required but no key pair was provided")
+                if (message.arg0 != Constants.AUTH_TYPE_TOKEN) throw AdbProtocolException("Unsupported auth type: $message")
 
                 val signature = keyPair.signPayload(message)
                 adbWriter.writeAuth(Constants.AUTH_TYPE_SIGNATURE, signature)
@@ -133,7 +155,11 @@ internal class AdbConnection internal constructor(
                 }
             }
 
-            if (message.command != Constants.CMD_CNXN) throw IOException("Connection failed: $message")
+            if (message.command != Constants.CMD_CNXN) {
+                // A trailing AUTH means the device rejected our key / stayed unauthorized.
+                if (message.command == Constants.CMD_AUTH) throw AdbAuthException("Device rejected authentication (unauthorized)")
+                throw AdbConnectException("Connection failed: $message")
+            }
 
             val connectionString = parseConnectionString(String(message.payload))
             val version = message.arg0
@@ -149,7 +175,7 @@ internal class AdbConnection internal constructor(
                     .map { it.split("=") }
                     .mapNotNull { if (it.size != 2) null else it[0] to it[1] }
                     .toMap()
-            if ("features" !in keyValues) throw IOException("Failed to parse features from connection string: $connectionString")
+            if ("features" !in keyValues) throw AdbConnectException("Failed to parse features from connection string: $connectionString")
             val features = keyValues.getValue("features").split(",").toSet()
             return ConnectionString(features)
         }

dadb/src/main/kotlin/dadb/AdbException.kt

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2021 mobile.dev inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package dadb
+
+import java.io.IOException
+
+/**
+ * Root of all dadb transport/connection failures. Extends [IOException] because these are genuine,
+ * recoverable socket I/O faults, consistent with the okio stream surface dadb exposes. Every
+ * transport fault thrown from a public method is one of these subtypes — no bare IOException leaks.
+ */
+sealed class AdbException(message: String, cause: Throwable? = null) : IOException(message, cause)
+
+/** Could not establish the connection: TCP connect/timeout, or the A_CNXN handshake failed.
+ *  Nothing ran — safe to retry by reconnecting. */
+class AdbConnectException(message: String, cause: Throwable? = null) : AdbException(message, cause)
+
+/** A_AUTH was rejected (no key, or device left UNAUTHORIZED). Reconnecting with the same key will
+ *  not help — fix or accept the key. */
+class AdbAuthException(message: String, cause: Throwable? = null) : AdbException(message, cause)
+
+/** adbd refused to OPEN the stream (A_OPEN answered with A_CLSE): unknown/forbidden service string,
+ *  or device offline. The underlying connection is still usable. */
+class AdbStreamOpenException(
+    val destination: String,
+    message: String,
+    cause: Throwable? = null
+) : AdbException(message, cause)
+
+/** An established connection/stream died unexpectedly (socket EOF/RST mid-operation). Reconnect —
+ *  but the operation MAY have partially executed; re-check state before retrying non-idempotent work. */
+class AdbConnectionClosedException(message: String, cause: Throwable? = null) : AdbException(message, cause)
+
+/** An operation exceeded its deadline with adbd unresponsive: a read past socketTimeout, or a write
+ *  past the write timeout (okio closes the socket on expiry). Distinct from a peer-driven EOF/RST.
+ *  Reconnect; like a dropped connection the operation may have partially executed. */
+class AdbTimeoutException(message: String, cause: Throwable? = null) : AdbException(message, cause)
+
+/** The peer sent a malformed or unexpected apacket (desync). No reliable way to re-sync — the
+ *  connection must be closed and re-established. */
+class AdbProtocolException(message: String, cause: Throwable? = null) : AdbException(message, cause)

dadb/src/main/kotlin/dadb/AdbOperationFailedException.kt

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2021 mobile.dev inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package dadb
+
+/**
+ * Thrown by the opt-in `orThrow()` helpers when an operation returned a `Failure`. This is an
+ * *operation outcome* surfaced as an exception by caller choice — the transport is healthy.
+ *
+ * It is deliberately **not** an [AdbException]: a `catch (AdbException)` that exists to reconnect on
+ * transport death will not catch this, so the two axes (transport died vs. operation rejected) stay
+ * distinct even for callers who opt into the throwing convenience. Catch it explicitly when you use
+ * `orThrow()`.
+ *
+ * [exitCode] is populated only for operations whose failure carries one (e.g. `uninstall`).
+ */
+class AdbOperationFailedException(
+    val reason: String,
+    val exitCode: Int? = null,
+) : RuntimeException(
+    if (exitCode != null) "Operation failed (exit $exitCode): $reason" else "Operation failed: $reason"
+)

dadb/src/main/kotlin/dadb/AdbStream.kt

@@ -19,6 +19,7 @@ package dadb
 
 import okio.*
 import java.lang.Integer.min
+import java.net.SocketTimeoutException
 import java.nio.ByteBuffer
 
 interface AdbStream : AutoCloseable {
@@ -117,9 +118,16 @@ internal class AdbStreamImpl internal constructor(
     private fun nextMessage(command: Int): AdbMessage? {
         return try {
             messageQueue.take(localId, command)
+        } catch (e: AdbStreamClosed) {
+            // Peer sent A_CLSE: normal end-of-stream (e.g. shell/exec output finished). EOF is correct.
+            close()
+            null
         } catch (e: IOException) {
+            // Mid-stream fault: a timeout (adbd unresponsive) vs the transport dying. Either way the
+            // stream is done.
             close()
-            return null
+            throw if (e is SocketTimeoutException) AdbTimeoutException("Read timed out on stream $localId; device unresponsive", e)
+                  else AdbConnectionClosedException("Connection lost while reading stream $localId", e)
         }
     }
 

dadb/src/main/kotlin/dadb/AdbSync.kt

@@ -22,6 +22,11 @@ import java.io.IOException
 import java.nio.charset.StandardCharsets
 import kotlin.jvm.Throws
 
+/** Internal signal that an adbd sync operation returned FAIL (e.g. file not found / permission
+ *  denied). Caught by Dadb.push/pull and folded into a [SyncResult.Failure]; never surfaced raw
+ *  from the high-level API. Not an AdbException — the transport is healthy. */
+internal class AdbSyncFailException(val reason: String) : IOException(reason)
+
 internal const val LIST = "LIST"
 internal const val RECV = "RECV"
 internal const val SEND = "SEND"
@@ -67,7 +72,11 @@ class AdbSyncStream(
         stream.sink.flush()
 
         val packet = readPacket()
-        if (packet.id != OKAY) throw IOException("Unexpected sync packet id: ${packet.id}")
+        if (packet.id == FAIL) {
+            val message = stream.source.readString(packet.arg.toLong(), StandardCharsets.UTF_8)
+            throw AdbSyncFailException(message)
+        }
+        if (packet.id != OKAY) throw AdbProtocolException("Unexpected sync packet id: ${packet.id}")
     }
 
     @Throws(IOException::class)
@@ -85,9 +94,9 @@ class AdbSyncStream(
             if (packet.id == DONE) break
             if (packet.id == FAIL) {
                 val message = stream.source.readString(packet.arg.toLong(), StandardCharsets.UTF_8)
-                throw IOException("Sync failed: $message")
+                throw AdbSyncFailException(message)
             }
-            if (packet.id != DATA) throw IOException("Unexpected sync packet id: ${packet.id}")
+            if (packet.id != DATA) throw AdbProtocolException("Unexpected sync packet id: ${packet.id}")
             val chunkSize = packet.arg
             stream.source.readFully(buffer, chunkSize.toLong())
             buffer.readAll(sink)

dadb/src/main/kotlin/dadb/AdbWriter.kt

@@ -19,6 +19,8 @@ package dadb
 
 import okio.Sink
 import okio.buffer
+import java.io.IOException
+import java.net.SocketTimeoutException
 import java.nio.ByteBuffer
 
 internal class AdbWriter(sink: Sink) : AutoCloseable {
@@ -73,24 +75,32 @@ internal class AdbWriter(sink: Sink) : AutoCloseable {
             length: Int
     ) {
         log { "(${Thread.currentThread().name}) > ${AdbMessage(command, arg0, arg1, length, 0, 0, payload ?: ByteArray(0))}" }
-        synchronized(bufferedSink) {
-            bufferedSink.apply {
-                writeIntLe(command)
-                writeIntLe(arg0)
-                writeIntLe(arg1)
-                if (payload == null) {
-                    writeIntLe(0)
-                    writeIntLe(0)
-                } else {
-                    writeIntLe(length)
-                    writeIntLe(payloadChecksum(payload))
+        try {
+            synchronized(bufferedSink) {
+                bufferedSink.apply {
+                    writeIntLe(command)
+                    writeIntLe(arg0)
+                    writeIntLe(arg1)
+                    if (payload == null) {
+                        writeIntLe(0)
+                        writeIntLe(0)
+                    } else {
+                        writeIntLe(length)
+                        writeIntLe(payloadChecksum(payload))
+                    }
+                    writeIntLe(command xor -0x1)
+                    if (payload != null) {
+                        write(payload, offset, length)
+                    }
+                    flush()
                 }
-                writeIntLe(command xor -0x1)
-                if (payload != null) {
-                    write(payload, offset, length)
-                }
-                flush()
             }
+        } catch (e: IOException) {
+            // A failed socket write means the transport is gone. A timeout is a stall (adbd stopped
+            // draining); anything else is a dropped connection. This is the single funnel for every
+            // write, so no raw IOException leaks out either way.
+            throw if (e is SocketTimeoutException) AdbTimeoutException("Write timed out; device unresponsive", e)
+                  else AdbConnectionClosedException("Connection lost while writing to device", e)
         }
     }