From 4f99af7d90a7b7ac5b625227852bda8dd2f8dec0 Mon Sep 17 00:00:00 2001 From: PJ Date: Sat, 6 Jun 2026 00:56:59 +0530 Subject: [PATCH] fix(sidecar): never replay non-idempotent actions after reconnect A dropped connection mid-action (e.g. a read timeout while the device is still typing) re-ran the whole block after reconnecting, typing the text twice and double-firing taps. Non-idempotent actions now reconnect for the next RPC's benefit but surface UNAVAILABLE, which the runner already treats as transient; idempotent reads keep the replay. --- .../dev/sanderling/sidecar/DriverBackend.kt | 29 +++++++++++++------ .../dev/sanderling/sidecar/DriverService.kt | 5 ++++ .../sanderling/sidecar/DriverServiceTest.kt | 19 ++++++++++++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt index e21f5a5..635cb02 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt @@ -714,7 +714,13 @@ class IosDriverBackend(private val udid: String) : DriverBackend { warmupErr?.let { throw IllegalStateException("WDA warmup failed after 3 attempts: $it") } } - private fun withReconnect(block: () -> T): T { + // withReconnect recovers from a dropped WDA connection. replay re-runs + // the block after reconnecting and is only safe for idempotent reads: + // an action call can fail client-side (e.g. a read timeout mid-typing) + // after the device already applied it, so replaying types text or taps + // twice. Non-idempotent actions reconnect for the next RPC's benefit but + // surface UNAVAILABLE, which the runner treats as transient. + private fun withReconnect(replay: Boolean = true, block: () -> T): T { return try { block() } catch (e: Exception) { @@ -730,6 +736,11 @@ class IosDriverBackend(private val udid: String) : DriverBackend { } finally { reconnectLock.unlock() } + if (!replay) { + throw io.grpc.Status.UNAVAILABLE + .withDescription("WDA connection dropped mid-action; the action may have applied, reconnected: $e") + .withCause(e).asRuntimeException() + } block() } } @@ -742,13 +753,13 @@ class IosDriverBackend(private val udid: String) : DriverBackend { override fun terminate(bundleId: String) = withReconnect { driver.stopApp(bundleId) } - override fun tap(x: Int, y: Int) = withReconnect { driver.tap(maestro.Point(x, y)) } + override fun tap(x: Int, y: Int) = withReconnect(replay = false) { driver.tap(maestro.Point(x, y)) } // The second tap request is already queued at the XCTest runner while the // first executes, so the on-device gap collapses to the runner's // turnaround instead of a full transport round trip. Sequential requests // leave a gap wide enough for the app to navigate between the taps. - override fun doubleTap(x: Int, y: Int): Unit = withReconnect { + override fun doubleTap(x: Int, y: Int): Unit = withReconnect(replay = false) { val point = maestro.Point(x, y) val firstTap = java.util.concurrent.CompletableFuture.runAsync { driver.tap(point) } Thread.sleep(40) @@ -757,23 +768,23 @@ class IosDriverBackend(private val udid: String) : DriverBackend { Unit } - override fun longPress(x: Int, y: Int) = withReconnect { driver.longPress(maestro.Point(x, y)) } + override fun longPress(x: Int, y: Int) = withReconnect(replay = false) { driver.longPress(maestro.Point(x, y)) } - override fun tapSelector(selector: String) = withReconnect { + override fun tapSelector(selector: String) = withReconnect(replay = false) { val root = driver.contentDescriptor(false) val bounds = findBoundsBySelector(root, selector) ?: return@withReconnect driver.tap(maestro.Point((bounds[0] + bounds[2]) / 2, (bounds[1] + bounds[3]) / 2)) } - override fun inputText(text: String) = withReconnect { driver.inputText(text) } + override fun inputText(text: String) = withReconnect(replay = false) { driver.inputText(text) } - override fun eraseText(characterCount: Int) = withReconnect { driver.eraseText(characterCount) } + override fun eraseText(characterCount: Int) = withReconnect(replay = false) { driver.eraseText(characterCount) } - override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) = withReconnect { + override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) = withReconnect(replay = false) { driver.swipe(maestro.Point(fromX, fromY), maestro.Point(toX, toY), maxOf(durationMillis, 250L)) } - override fun pressKey(key: String) = withReconnect { + override fun pressKey(key: String) = withReconnect(replay = false) { StubDriverBackend.KEY_MAP[key]?.let { keyCode -> keyCodeToMaestro(keyCode)?.let { driver.pressKey(it) } } diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt index 3fc703d..5009d99 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt @@ -190,6 +190,11 @@ class DriverService( try { observer.onNext(block()) observer.onCompleted() + } catch (cause: io.grpc.StatusRuntimeException) { + // A backend that already chose a status code (e.g. UNAVAILABLE for + // a dropped-mid-action connection) keeps it, so the runner can + // tell transient failures from fatal ones. + observer.onError(cause) } catch (cause: Exception) { observer.onError(io.grpc.Status.INTERNAL.withDescription(cause.toString()) .withCause(cause).asRuntimeException()) diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt index ab52bab..a9a2eec 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt @@ -83,6 +83,25 @@ class DriverServiceTest { assertEquals("hello world", backend.lastInputText) } + // A backend that already chose a status code (the iOS backend surfaces + // UNAVAILABLE when the connection dropped mid-action) must keep it, so + // the runner can tell transient failures from fatal ones. + @Test fun backendStatusCodePassesThrough() { + val backend = object : DriverBackend by StubDriverBackend("android") { + override fun inputText(text: String) { + throw io.grpc.Status.UNAVAILABLE + .withDescription("connection dropped mid-action") + .asRuntimeException() + } + } + val client = newClient(backend) + + val thrown = kotlin.test.assertFailsWith { + client.inputText(Text.newBuilder().setValue("hello").build()) + } + assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) + } + @Test fun doubleTapDefaultComposesTwoTaps() { // Interface delegation would bind the default doubleTap to the // delegate, bypassing the tap override, so implement the interface