fix(web): survive a dropped DevTools reply and stop re-finding the focused element by xpath

maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt

@@ -1,6 +1,7 @@
 package maestro.drivers
 
 import CdpClient
+import CdpReplyLostException
 import CdpTarget
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import kotlinx.coroutines.runBlocking
@@ -24,7 +25,6 @@ import maestro.web.record.JcodecVideoEncoder
 import maestro.web.record.WebScreenRecorder
 import okio.Sink
 import okio.buffer
-import org.openqa.selenium.By
 import org.openqa.selenium.JavascriptExecutor
 import org.openqa.selenium.Keys
 import org.openqa.selenium.WebDriver
@@ -81,6 +81,8 @@ class CdpWebDriver(
     // URL hint used to resolve the target when no explicit id is given (the flow's APP_ID).
     private var appIdHint: String? = null
 
+    private var lastDeviceInfo: DeviceInfo? = null
+
     init {
         Maestro::class.java.getResourceAsStream("/maestro-web.js")?.let {
             it.bufferedReader().use { br ->
@@ -186,9 +188,11 @@ class CdpWebDriver(
         return target
     }
 
-    private fun executeJS(js: String): Any? {
+    // A lost reply to non-idempotent JS isn't retried: Chrome may already have run it.
+    private fun executeJS(js: String, idempotent: Boolean = true): Any? {
         return runBlocking {
             repeat(JS_EXECUTION_MAX_ATTEMPTS) { attempt ->
+                var sentJs = false
                 try {
                     val target = resolveTarget()
 
@@ -200,6 +204,7 @@ class CdpWebDriver(
 
                     Thread.sleep(100)
 
+                    sentJs = true
                     val resultStr = cdpClient.evaluate(js, target)
 
                     // CDP returns an empty value for expressions that evaluate to undefined (e.g.
@@ -212,7 +217,9 @@ class CdpWebDriver(
                     // The page can navigate or reload out from under us (e.g. right after
                     // launchApp/clearState), invalidating the CDP target mid-evaluation. Wait
                     // briefly for it to settle and retry against a freshly listed target.
-                    if (isRetryableJsError(e) && attempt < JS_EXECUTION_MAX_ATTEMPTS - 1) {
+                    val retryable = isRetryableJsError(e) ||
+                        (e is CdpReplyLostException && (idempotent || !sentJs))
+                    if (retryable && attempt < JS_EXECUTION_MAX_ATTEMPTS - 1) {
                         LOGGER.warn("Transient error executing JS, retrying (attempt ${attempt + 1})", e)
                         // The target may have been invalidated by navigation; force a re-resolve.
                         resolvedTarget = null
@@ -240,7 +247,7 @@ class CdpWebDriver(
         if (point.y >= 0 && point.y.toLong() <= windowHeight) return 0L
 
         val scrolledPixels =
-            executeJS("(() => {const delta = ${point.y} - Math.floor(window.innerHeight / 2); window.scrollBy({ top: delta, left: 0, behavior: 'smooth' }); return delta})()") as Int
+            executeJS("(() => {const delta = ${point.y} - Math.floor(window.innerHeight / 2); window.scrollBy({ top: delta, left: 0, behavior: 'smooth' }); return delta})()", idempotent = false) as Int
         sleep(3000L)
         return scrolledPixels.toLong()
     }
@@ -250,7 +257,7 @@ class CdpWebDriver(
     }
 
     private fun scroll(top: String, left: String) {
-        executeJS("window.scroll({ top: $top, left: $left, behavior: 'smooth' })")
+        executeJS("window.scroll({ top: $top, left: $left, behavior: 'smooth' })", idempotent = false)
     }
 
     private fun random(start: Int, end: Int): Int {
@@ -259,6 +266,7 @@ class CdpWebDriver(
 
     override fun close() {
         injectedArguments = emptyMap()
+        lastDeviceInfo = null
 
         try {
             seleniumDriver?.quit()
@@ -275,16 +283,21 @@ class CdpWebDriver(
     }
 
     override fun deviceInfo(): DeviceInfo {
-        val width = executeJS("window.innerWidth") as Int
-        val height = executeJS("window.innerHeight") as Int
+        val size = executeJS("[window.innerWidth, window.innerHeight]") as? List<*>
+        val width = (size?.getOrNull(0) as? Number)?.toInt()
+        val height = (size?.getOrNull(1) as? Number)?.toInt()
+
+        if (width == null || height == null) {
+            return lastDeviceInfo ?: error("Could not read the window size (got $size)")
+        }
 
         return DeviceInfo(
             platform = Platform.WEB,
             widthPixels = width,
             heightPixels = height,
             widthGrid = width,
             heightGrid = height,
-        )
+        ).also { lastDeviceInfo = it }
     }
 
     override fun launchApp(
@@ -502,7 +515,7 @@ class CdpWebDriver(
             return
         }
 
-        executeJS("window.maestro.tapOnSyntheticElement(${point.x}, ${point.y})")
+        executeJS("window.maestro.tapOnSyntheticElement(${point.x}, ${point.y})", idempotent = false)
     }
 
     override fun longPress(point: Point) {
@@ -594,7 +607,7 @@ class CdpWebDriver(
 
         if (isFlutter) {
             // Use Flutter-specific smooth animated scrolling
-            executeJS("window.maestro.smoothScrollFlutter('UP', 500)")
+            executeJS("window.maestro.smoothScrollFlutter('UP', 500)", idempotent = false)
         } else {
             // Use standard scroll for regular web pages
             scroll("window.scrollY + Math.round(window.innerHeight / 2)", "window.scrollX")
@@ -652,7 +665,7 @@ class CdpWebDriver(
 
         if (isFlutter) {
             // Flutter web: Use smooth animated scrolling with easing
-            executeJS("window.maestro.smoothScrollFlutter('${swipeDirection.name}', $durationMs)")
+            executeJS("window.maestro.smoothScrollFlutter('${swipeDirection.name}', $durationMs)", idempotent = false)
         } else {
             // HTML web: Use standard window scrolling
             when (swipeDirection) {
@@ -1014,20 +1027,13 @@ class CdpWebDriver(
             val iframe = jsExecutor.executeScript("return document.activeElement") as WebElement
             driver.switchTo().frame(iframe)
             try {
-                jsExecutor.executeScript("$maestroWebScript")
-                val xPath = jsExecutor.executeScript(
-                    "return window.maestro.createXPathFromElement(document.activeElement)"
-                ) as String
-                val element = driver.findElement(By.ByXPath(xPath))
-                action(element)
+                action(driver.switchTo().activeElement())
             } finally {
                 try { driver.switchTo().defaultContent() }
                 catch (e: Exception) { LOGGER.warn("Failed to switch back to default content", e) }
             }
         } else {
-            val xPath = executeJS("window.maestro.createXPathFromElement(document.activeElement)") as String
-            val element = driver.findElement(By.ByXPath(xPath))
-            action(element)
+            action(driver.switchTo().activeElement())
         }
     }
 

maestro-client/src/test/java/maestro/drivers/CdpWebDriverLostReplyTest.kt

@@ -0,0 +1,101 @@
+package maestro.drivers
+
+import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+import com.google.common.truth.Truth.assertThat
+import okhttp3.Response
+import okhttp3.WebSocket
+import okhttp3.WebSocketListener
+import okhttp3.mockwebserver.Dispatcher
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
+import okhttp3.mockwebserver.RecordedRequest
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import java.util.concurrent.atomic.AtomicInteger
+
+class CdpWebDriverLostReplyTest {
+
+    private val server = MockWebServer()
+    private val evaluations = AtomicInteger()
+
+    private var dropped: (evaluation: Int) -> Boolean = { false }
+
+    @AfterEach
+    fun tearDown() {
+        server.shutdown()
+    }
+
+    @Test
+    fun `deviceInfo retries an evaluation whose reply was lost`() {
+        dropped = { it == 1 }
+        val driver = openDriver()
+
+        val info = driver.deviceInfo()
+
+        assertThat(info.widthPixels).isEqualTo(1024)
+        assertThat(info.heightPixels).isEqualTo(768)
+    }
+
+    @Test
+    fun `deviceInfo keeps the last window size when every reply is lost`() {
+        val driver = openDriver()
+        driver.deviceInfo()
+        dropped = { true }
+
+        val info = driver.deviceInfo()
+
+        assertThat(info.widthPixels).isEqualTo(1024)
+        assertThat(info.heightPixels).isEqualTo(768)
+    }
+
+    @Test
+    fun `deviceInfo fails with a clear error when it has never read the window size`() {
+        dropped = { true }
+        val driver = openDriver()
+
+        val error = assertThrows<IllegalStateException> { driver.deviceInfo() }
+
+        assertThat(error).hasMessageThat().contains("Could not read the window size")
+    }
+
+    private fun openDriver(): CdpWebDriver {
+        server.dispatcher = object : Dispatcher() {
+            override fun dispatch(request: RecordedRequest): MockResponse = when (request.path) {
+                "/json" -> MockResponse().setBody(
+                    """[{"id":"A","title":"","url":"http://app","type":"page",""" +
+                        """"webSocketDebuggerUrl":"ws://${server.hostName}:${server.port}/devtools/page/A"}]"""
+                )
+                "/devtools/page/A" -> MockResponse().withWebSocketUpgrade(DevToolsPage())
+                else -> MockResponse().setResponseCode(404)
+            }
+        }
+        server.start()
+
+        return CdpWebDriver(
+            isStudio = false,
+            screenSize = null,
+            cdpUrl = "http://${server.hostName}:${server.port}",
+        ).apply { open() }
+    }
+
+    private inner class DevToolsPage : WebSocketListener() {
+        override fun onMessage(webSocket: WebSocket, text: String) {
+            if (dropped(evaluations.incrementAndGet())) {
+                webSocket.cancel()
+                return
+            }
+
+            val request = jacksonObjectMapper().readTree(text)
+            val expression = request["params"]["expression"].asText()
+            val value = if ("[window.innerWidth, window.innerHeight]" in expression) "[1024,768]" else "null"
+            webSocket.send("""{"id":${request["id"].asInt()},"result":{"result":{"type":"string","value":"$value"}}}""")
+        }
+
+        override fun onClosing(webSocket: WebSocket, code: Int, reason: String) {
+            webSocket.close(code, null)
+        }
+
+        override fun onFailure(webSocket: WebSocket, t: Throwable, response: Response?) = Unit
+    }
+}

maestro-web/src/main/kotlin/maestro/web/cdp/CdpClient.kt

@@ -16,19 +16,29 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
+import kotlinx.coroutines.withTimeoutOrNull
 import kotlinx.serialization.Serializable
 import kotlinx.serialization.encodeToString
 import kotlinx.serialization.json.Json
 import kotlinx.serialization.json.JsonObject
 import kotlinx.serialization.json.JsonPrimitive
 import kotlinx.serialization.json.buildJsonObject
+import kotlinx.serialization.json.contentOrNull
 import kotlinx.serialization.json.jsonObject
 import kotlinx.serialization.json.jsonPrimitive
 import kotlinx.serialization.json.put
 import java.io.File
 import java.util.*
 import java.util.concurrent.atomic.AtomicInteger
 
+class CdpReplyLostException(
+    messageId: Int,
+    closeReason: CloseReason?,
+    otherEvents: List<String>,
+) : IllegalStateException(
+    "No message with id $messageId received (close reason: $closeReason, other events: $otherEvents)"
+)
+
 /**
  * Descriptor for a CDP target (an open tab/page).
  */
@@ -374,17 +384,28 @@ class CdpClient(
     }
 
     private suspend fun DefaultClientWebSocketSession.waitForMessage(messageId: Int): String {
+        val unmatched = mutableListOf<String>()
         for (frame in incoming) {
             if (frame is Frame.Text) {
                 val text = frame.readText()
                 if (text.contains("\"id\":$messageId")) {
                     return text
                 }
+                unmatched += describeEvent(text)
             }
         }
-        error("No message with id $messageId received")
+        val reason = withTimeoutOrNull(1000) { closeReason.await() }
+        throw CdpReplyLostException(messageId, reason, unmatched)
     }
 
+    // Page data (URLs, console output) can carry credentials, so keep only Chrome's own fields.
+    private fun describeEvent(text: String): String = runCatching {
+        val root = json.parseToJsonElement(text).jsonObject
+        val method = root["method"]?.jsonPrimitive?.contentOrNull ?: "reply ${root["id"]}"
+        val reason = root["params"]?.jsonObject?.get("reason")?.jsonPrimitive?.contentOrNull
+        if (reason != null) "$method ($reason)" else method
+    }.getOrDefault("unparseable frame")
+
     private suspend fun <R> DefaultClientWebSocketSession.use(block: suspend (DefaultClientWebSocketSession) -> R): R {
         return try {
             block(this)