mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-04 20:17:09 +00:00
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.
This commit is contained in:
1 parent
2b773bbf5d
commit
4f99af7d90
3 files changed
+44
-9
No files matched your search
@@ -714,7 +714,13 @@ class IosDriverBackend(private val udid: String) : DriverBackend {
|
|||||||
warmupErr?.let { throw IllegalStateException("WDA warmup failed after 3 attempts: $it") }
|
warmupErr?.let { throw IllegalStateException("WDA warmup failed after 3 attempts: $it") }
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun <T> 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 <T> withReconnect(replay: Boolean = true, block: () -> T): T {
|
||||||
return try {
|
return try {
|
||||||
block()
|
block()
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
@@ -730,6 +736,11 @@ class IosDriverBackend(private val udid: String) : DriverBackend {
|
|||||||
} finally {
|
} finally {
|
||||||
reconnectLock.unlock()
|
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()
|
block()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -742,13 +753,13 @@ class IosDriverBackend(private val udid: String) : DriverBackend {
|
|||||||
|
|
||||||
override fun terminate(bundleId: String) = withReconnect { driver.stopApp(bundleId) }
|
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
|
// 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
|
// first executes, so the on-device gap collapses to the runner's
|
||||||
// turnaround instead of a full transport round trip. Sequential requests
|
// turnaround instead of a full transport round trip. Sequential requests
|
||||||
// leave a gap wide enough for the app to navigate between the taps.
|
// 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 point = maestro.Point(x, y)
|
||||||
val firstTap = java.util.concurrent.CompletableFuture.runAsync { driver.tap(point) }
|
val firstTap = java.util.concurrent.CompletableFuture.runAsync { driver.tap(point) }
|
||||||
Thread.sleep(40)
|
Thread.sleep(40)
|
||||||
@@ -757,23 +768,23 @@ class IosDriverBackend(private val udid: String) : DriverBackend {
|
|||||||
Unit
|
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 root = driver.contentDescriptor(false)
|
||||||
val bounds = findBoundsBySelector(root, selector) ?: return@withReconnect
|
val bounds = findBoundsBySelector(root, selector) ?: return@withReconnect
|
||||||
driver.tap(maestro.Point((bounds[0] + bounds[2]) / 2, (bounds[1] + bounds[3]) / 2))
|
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))
|
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 ->
|
StubDriverBackend.KEY_MAP[key]?.let { keyCode ->
|
||||||
keyCodeToMaestro(keyCode)?.let { driver.pressKey(it) }
|
keyCodeToMaestro(keyCode)?.let { driver.pressKey(it) }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -190,6 +190,11 @@ class DriverService(
|
|||||||
try {
|
try {
|
||||||
observer.onNext(block())
|
observer.onNext(block())
|
||||||
observer.onCompleted()
|
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) {
|
} catch (cause: Exception) {
|
||||||
observer.onError(io.grpc.Status.INTERNAL.withDescription(cause.toString())
|
observer.onError(io.grpc.Status.INTERNAL.withDescription(cause.toString())
|
||||||
.withCause(cause).asRuntimeException())
|
.withCause(cause).asRuntimeException())
|
||||||
|
|||||||
@@ -83,6 +83,25 @@ class DriverServiceTest {
|
|||||||
assertEquals("hello world", backend.lastInputText)
|
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<io.grpc.StatusRuntimeException> {
|
||||||
|
client.inputText(Text.newBuilder().setValue("hello").build())
|
||||||
|
}
|
||||||
|
assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code)
|
||||||
|
}
|
||||||
|
|
||||||
@Test fun doubleTapDefaultComposesTwoTaps() {
|
@Test fun doubleTapDefaultComposesTwoTaps() {
|
||||||
// Interface delegation would bind the default doubleTap to the
|
// Interface delegation would bind the default doubleTap to the
|
||||||
// delegate, bypassing the tap override, so implement the interface
|
// delegate, bypassing the tap override, so implement the interface
|
||||||
|
|||||||
Reference in new issue
Block a user