From 7458176c18480ece56c96542da7b71855ba88c58 Mon Sep 17 00:00:00 2001 From: Aubrey Taylor Date: Fri, 30 Oct 2026 15:27:47 +1100 Subject: [PATCH 1/2] fix(web): retry JS when Chrome drops the DevTools socket before replying --- .../main/java/maestro/drivers/CdpWebDriver.kt | 34 ++++-- .../drivers/CdpWebDriverLostReplyTest.kt | 101 ++++++++++++++++++ .../main/kotlin/maestro/web/cdp/CdpClient.kt | 23 +++- maestro-web/src/test/kotlin/CdpClientTest.kt | 83 ++++++++++++++ 4 files changed, 230 insertions(+), 11 deletions(-) create mode 100644 maestro-client/src/test/java/maestro/drivers/CdpWebDriverLostReplyTest.kt create mode 100644 maestro-web/src/test/kotlin/CdpClientTest.kt diff --git a/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt b/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt index 717ce63808..03b59d6847 100644 --- a/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt +++ b/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 @@ -81,6 +82,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 +189,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 +205,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 +218,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 +248,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 +258,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 +267,7 @@ class CdpWebDriver( override fun close() { injectedArguments = emptyMap() + lastDeviceInfo = null try { seleniumDriver?.quit() @@ -275,8 +284,13 @@ 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, @@ -284,7 +298,7 @@ class CdpWebDriver( heightPixels = height, widthGrid = width, heightGrid = height, - ) + ).also { lastDeviceInfo = it } } override fun launchApp( @@ -502,7 +516,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 +608,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 +666,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) { diff --git a/maestro-client/src/test/java/maestro/drivers/CdpWebDriverLostReplyTest.kt b/maestro-client/src/test/java/maestro/drivers/CdpWebDriverLostReplyTest.kt new file mode 100644 index 0000000000..3203c9ac5a --- /dev/null +++ b/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 { 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 + } +} diff --git a/maestro-web/src/main/kotlin/maestro/web/cdp/CdpClient.kt b/maestro-web/src/main/kotlin/maestro/web/cdp/CdpClient.kt index 66ba96d3cc..2279881fa4 100644 --- a/maestro-web/src/main/kotlin/maestro/web/cdp/CdpClient.kt +++ b/maestro-web/src/main/kotlin/maestro/web/cdp/CdpClient.kt @@ -16,12 +16,14 @@ 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 @@ -29,6 +31,14 @@ import java.io.File import java.util.* import java.util.concurrent.atomic.AtomicInteger +class CdpReplyLostException( + messageId: Int, + closeReason: CloseReason?, + otherEvents: List, +) : 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() 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 DefaultClientWebSocketSession.use(block: suspend (DefaultClientWebSocketSession) -> R): R { return try { block(this) diff --git a/maestro-web/src/test/kotlin/CdpClientTest.kt b/maestro-web/src/test/kotlin/CdpClientTest.kt new file mode 100644 index 0000000000..145093de00 --- /dev/null +++ b/maestro-web/src/test/kotlin/CdpClientTest.kt @@ -0,0 +1,83 @@ +import com.google.common.truth.Truth.assertThat +import io.ktor.server.application.install +import io.ktor.server.cio.CIO +import io.ktor.server.engine.embeddedServer +import io.ktor.server.routing.routing +import io.ktor.server.websocket.WebSockets +import io.ktor.server.websocket.webSocket +import io.ktor.websocket.CloseReason +import io.ktor.websocket.Frame +import io.ktor.websocket.close +import io.ktor.websocket.readText +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.int +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import org.junit.jupiter.api.Test +import java.net.ServerSocket + +class CdpClientTest { + + @Test + fun `reports Chrome's close reason, but no page data, when the socket closes before the reply`() { + val port = freePort() + val server = embeddedServer(CIO, port = port) { + install(WebSockets) + routing { + webSocket("/devtools/page/A") { + incoming.receive() + send(Frame.Text("""{"method":"Runtime.consoleAPICalled","params":{"args":[{"value":"secret-token"}]}}""")) + send(Frame.Text("""{"method":"Inspector.detached","params":{"reason":"target_closed"}}""")) + close(CloseReason(CloseReason.Codes.GOING_AWAY, "target closed")) + } + } + }.start(wait = false) + + val result = try { + runCatching { runBlocking { CdpClient().evaluate("1", target(port)) } } + } finally { + server.stop(0, 0) + } + + val error = result.exceptionOrNull() + assertThat(error).isInstanceOf(CdpReplyLostException::class.java) + assertThat(error).hasMessageThat().contains("target closed") + assertThat(error).hasMessageThat().contains("Inspector.detached (target_closed)") + assertThat(error).hasMessageThat().contains("Runtime.consoleAPICalled") + assertThat(error).hasMessageThat().doesNotContain("secret-token") + } + + @Test + fun `returns the reply to its own command`() { + val port = freePort() + val server = embeddedServer(CIO, port = port) { + install(WebSockets) + routing { + webSocket("/devtools/page/A") { + val request = (incoming.receive() as Frame.Text).readText() + val id = Json.parseToJsonElement(request).jsonObject["id"]!!.jsonPrimitive.int + send(Frame.Text("""{"method":"Runtime.consoleAPICalled","params":{}}""")) + send(Frame.Text("""{"id":$id,"result":{"result":{"type":"string","value":"2"}}}""")) + } + } + }.start(wait = false) + + val result = try { + runBlocking { CdpClient().evaluate("1+1", target(port)) } + } finally { + server.stop(0, 0) + } + + assertThat(result).isEqualTo("2") + } + + private fun target(port: Int) = CdpTarget( + id = "A", + title = "", + url = "", + webSocketDebuggerUrl = "ws://127.0.0.1:$port/devtools/page/A", + ) + + private fun freePort(): Int = ServerSocket(0).use { it.localPort } +} From cae3aec9d66c18e524ec69325fdcdd5cd27368cb Mon Sep 17 00:00:00 2001 From: Aubrey Taylor Date: Fri, 30 Oct 2026 15:28:43 +1100 Subject: [PATCH 2/2] fix(web): send keys to Selenium's active element instead of re-finding it by xpath --- .../src/main/java/maestro/drivers/CdpWebDriver.kt | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt b/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt index 03b59d6847..8ea5823557 100644 --- a/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt +++ b/maestro-client/src/main/java/maestro/drivers/CdpWebDriver.kt @@ -25,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 @@ -1028,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()) } }