From 83dd92b8a844791e3d351fe35a75c0ebc2faee75 Mon Sep 17 00:00:00 2001 From: NathanFallet Date: Wed, 26 Aug 2026 19:11:37 +0200 Subject: [PATCH 1/3] feat: say why a browser start failed instead of just timing out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the browser never opens its debug port, the only visible symptom was a 30s wait followed by a generic FailedToConnectToBrowserException. The reason was already known inside testConnection but only logged at debug level, so it never reached production logs. start() now logs, at error level, the port waited on, how long, the process id, whether that process is still alive, and the last connection error. That last one is the useful part: 'connection refused' means nothing ever listened on the port, while a timeout or an unexpected response means something else holds it — two causes that were indistinguishable until now. Browser stdout/stderr are also discarded rather than piped. Nothing read those pipes, and a pipe nobody drains fills up: once the OS buffer is full the browser blocks on its next write, and if that happens before it opens its debug port, the port never opens at all. --- .../dev/kdriver/core/browser/DefaultBrowser.kt | 16 +++++++++++++++- .../dev/kdriver/core/browser/Process.jvm.kt | 8 ++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt index 64a3fff25..fa2979ae4 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt @@ -48,6 +48,9 @@ open class DefaultBrowser( override var info: ContraDict? = null + /** Last error from [testConnection], surfaced if the browser never opens its debug port. */ + private var lastConnectionError: Exception? = null + // The canonical registry: mutated only while holding [updateTargetInfoMutex]. After each // mutation an immutable copy is published to [targetsSnapshot] so the non-suspend getters // below can read a consistent view without the lock (ISSUE-5). @@ -266,7 +269,17 @@ open class DefaultBrowser( logger.info("Connection to browser established") val info = info ?: run { - logger.info("Browser info not initialized, reading error") + // Say what actually failed. Without this the only visible symptom is a 30s wait and a + // generic exception, which cannot distinguish "nothing is listening on that port" (the + // browser never opened it) from "something answers but not what we expect" (a stale or + // foreign process holds it) — two very different causes. + val waitedMs = config.browserConnectionTimeout * (config.browserConnectionMaxTries + 1) + logger.error( + "Browser never opened its debug port on ${config.host}:${config.port} after ${waitedMs}ms " + + "(pid=${process?.pid()}, alive=${process?.isAlive()}). " + + "Last connection error: " + + (lastConnectionError?.let { "${it::class.simpleName}: ${it.message}" } ?: "none") + ) /* // This seems to block indefinitely on CI, so inspection is required withTimeoutOrNull(1000) { @@ -344,6 +357,7 @@ open class DefaultBrowser( } catch (e: CancellationException) { throw e } catch (e: Exception) { + lastConnectionError = e logger.debug("Could not start: ${e.message}") false } diff --git a/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt index 86184342b..8e1878733 100644 --- a/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt +++ b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt @@ -24,8 +24,12 @@ actual suspend fun startProcess( val builder = ProcessBuilder(command) builder.redirectInput(ProcessBuilder.Redirect.PIPE) - builder.redirectOutput(ProcessBuilder.Redirect.PIPE) - builder.redirectError(ProcessBuilder.Redirect.PIPE) + // Discarded, not piped. Nobody reads these streams, and a pipe nobody drains fills up: once + // the OS buffer is full (a few KB on Windows) the browser blocks on its next write. If that + // happens before it has opened its debug port, the port never opens and the start times out + // for no visible reason. + builder.redirectOutput(ProcessBuilder.Redirect.DISCARD) + builder.redirectError(ProcessBuilder.Redirect.DISCARD) if (isPosix) builder.redirectErrorStream(false) val process = builder.start() From d5481427fc132163e4e32426d2071b3cdb77c185 Mon Sep 17 00:00:00 2001 From: NathanFallet Date: Wed, 26 Aug 2026 20:16:39 +0200 Subject: [PATCH 2/3] feat: read the browser's stderr instead of discarding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following review. The first version discarded stdout/stderr to avoid the pipe filling up, but that contradicted the point of this change: it removed the richest source of information about why a start failed. zendriver keeps the pipes and reads stderr with a bounded wait, which is exactly what the commented-out block here was missing — the reason it was disabled with 'seems to block indefinitely on CI' is that the stream stays open for the process's whole life, so an unbounded read waits for it to exit. readStderrSnapshot() reads at most 64KB and gives up after 250ms, and its output goes into the failure log next to the port, pid, liveness and last connection error. On Linux it returns null, since the child there inherits our stderr rather than being piped. --- .../kdriver/core/browser/DefaultBrowser.kt | 12 +++------ .../dev/kdriver/core/browser/Process.kt | 13 +++++++++ .../dev/kdriver/core/browser/Process.jvm.kt | 27 ++++++++++++++----- .../dev/kdriver/core/browser/Process.posix.kt | 2 ++ 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt index fa2979ae4..0dfe5ca3a 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt @@ -274,20 +274,14 @@ open class DefaultBrowser( // browser never opened it) from "something answers but not what we expect" (a stale or // foreign process holds it) — two very different causes. val waitedMs = config.browserConnectionTimeout * (config.browserConnectionMaxTries + 1) + val stderr = process?.readStderrSnapshot() logger.error( "Browser never opened its debug port on ${config.host}:${config.port} after ${waitedMs}ms " + "(pid=${process?.pid()}, alive=${process?.isAlive()}). " + "Last connection error: " + - (lastConnectionError?.let { "${it::class.simpleName}: ${it.message}" } ?: "none") + (lastConnectionError?.let { "${it::class.simpleName}: ${it.message}" } ?: "none") + + ". Browser stderr: " + (stderr?.trim()?.takeIf { it.isNotEmpty() } ?: "") ) - /* - // This seems to block indefinitely on CI, so inspection is required - withTimeoutOrNull(1000) { - _process?.errorStream?.bufferedReader()?.use { - logger.info("Browser stderr: ${it.readText()}") - } - } - */ stop() throw FailedToConnectToBrowserException() } diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt index 04fe39f92..3be6d121a 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt @@ -8,6 +8,19 @@ expect abstract class Process { abstract fun destroy() } +/** + * Reads whatever is currently buffered on the process's stderr, or null if unavailable. + * + * Bounded in size and time on purpose: the stream stays open for the process's whole life, so an + * unbounded read would block until it exits rather than returning what the browser has said so far. + * + * Returns null where stderr is not captured (Linux, where the child simply inherits ours). + */ +expect suspend fun Process.readStderrSnapshot( + maxBytes: Int = 64 * 1024, + timeoutMillis: Long = 250, +): String? + expect suspend fun startProcess(exe: Path, params: List): Process expect fun addShutdownHook(hook: suspend () -> Unit) expect fun isPosix(): Boolean diff --git a/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt index 8e1878733..d88df4231 100644 --- a/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt +++ b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt @@ -3,6 +3,7 @@ package dev.kdriver.core.browser import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull import kotlinx.io.files.Path import java.io.File import java.net.InetAddress @@ -24,12 +25,8 @@ actual suspend fun startProcess( val builder = ProcessBuilder(command) builder.redirectInput(ProcessBuilder.Redirect.PIPE) - // Discarded, not piped. Nobody reads these streams, and a pipe nobody drains fills up: once - // the OS buffer is full (a few KB on Windows) the browser blocks on its next write. If that - // happens before it has opened its debug port, the port never opens and the start times out - // for no visible reason. - builder.redirectOutput(ProcessBuilder.Redirect.DISCARD) - builder.redirectError(ProcessBuilder.Redirect.DISCARD) + builder.redirectOutput(ProcessBuilder.Redirect.PIPE) + builder.redirectError(ProcessBuilder.Redirect.PIPE) if (isPosix) builder.redirectErrorStream(false) val process = builder.start() @@ -77,6 +74,24 @@ actual fun getEnv(name: String): String? { return System.getenv(name) } +/** + * Reads whatever is currently buffered on the process's stderr, bounded in both size and time. + * + * Both bounds matter: the stream stays open for as long as the process lives, so an unbounded read + * would block until it exits. That is why the equivalent block used to be commented out with + * "seems to block indefinitely on CI". + */ +actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? = + withTimeoutOrNull(timeoutMillis) { + withContext(Dispatchers.IO) { + runCatching { + val buffer = ByteArray(maxBytes) + val read = errorStream.read(buffer) + if (read > 0) String(buffer, 0, read) else null + }.getOrNull() + } + } + actual fun freePort(): Int? { ServerSocket(0, 5, InetAddress.getByName("127.0.0.1")).use { socket -> return socket.localPort diff --git a/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt b/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt index 1a78a6726..0aa3fba46 100644 --- a/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt +++ b/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt @@ -20,6 +20,8 @@ actual abstract class Process { actual abstract fun destroy() } +actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? = null + actual fun addShutdownHook(hook: suspend () -> Unit) { // POSIX doesn't have a direct equivalent to Java shutdown hooks // Could use atexit() but it doesn't support suspend functions From edcccceb9bddb612e7c154d940377f42fe2162e2 Mon Sep 17 00:00:00 2001 From: NathanFallet Date: Wed, 26 Aug 2026 20:19:47 +0200 Subject: [PATCH 3/3] fix: provide readStderrSnapshot on the mingw and js targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same omission as killTree: only JVM and POSIX had an actual, so the Native compilation failed. On mingw it returns null, because startProcess there calls CreateProcessW without redirecting the child's standard streams — there is no pipe to read. On JS it throws, like every other process function on that target. --- .../jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt | 4 ++++ .../kotlin/dev/kdriver/core/browser/Process.mingw.kt | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt b/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt index c0d60ec43..cca0ef77c 100644 --- a/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt +++ b/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt @@ -14,6 +14,10 @@ actual abstract class Process { actual abstract fun destroy() } +actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? { + throw UnsupportedOperationException() +} + actual suspend fun startProcess( exe: Path, params: List, diff --git a/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt b/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt index 4c2ed7de7..f340d642d 100644 --- a/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt +++ b/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt @@ -47,6 +47,12 @@ private class WindowsProcess( } } +/** + * Not available on this target: [startProcess] here calls `CreateProcessW` without redirecting the + * child's standard streams, so there is no pipe to read from. + */ +actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? = null + @OptIn(ExperimentalForeignApi::class) actual suspend fun startProcess( exe: Path,