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..0dfe5ca3a 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,15 +269,19 @@ open class DefaultBrowser( logger.info("Connection to browser established") val info = info ?: run { - logger.info("Browser info not initialized, reading error") - /* - // This seems to block indefinitely on CI, so inspection is required - withTimeoutOrNull(1000) { - _process?.errorStream?.bufferedReader()?.use { - logger.info("Browser stderr: ${it.readText()}") - } - } - */ + // 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) + 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") + + ". Browser stderr: " + (stderr?.trim()?.takeIf { it.isNotEmpty() } ?: "") + ) stop() throw FailedToConnectToBrowserException() } @@ -344,6 +351,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/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/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/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt index 86184342b..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 @@ -73,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/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, 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