Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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() } ?: "<none>")
)
stop()
throw FailedToConnectToBrowserException()
}
Expand Down Expand Up @@ -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
}
Expand Down
13 changes: 13 additions & 0 deletions core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@
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,

Check warning on line 20 in core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt

View check run for this annotation

codefactor.io / CodeFactor

core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt#L20

This expression contains a magic number. Consider defining it to a well named constant. (detekt.MagicNumber)

Check warning on line 20 in core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt

View check run for this annotation

codefactor.io / CodeFactor

core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt#L20

This expression contains a magic number. Consider defining it to a well named constant. (detekt.MagicNumber)
timeoutMillis: Long = 250,
): String?

expect suspend fun startProcess(exe: Path, params: List<String>): Process
expect fun addShutdownHook(hook: suspend () -> Unit)
expect fun isPosix(): Boolean
Expand Down
4 changes: 4 additions & 0 deletions core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
actual abstract fun destroy()
}

actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? {

Check warning on line 17 in core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt

View check run for this annotation

codefactor.io / CodeFactor

core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt#L17

The function readStderrSnapshot is missing documentation. (detekt.UndocumentedPublicFunction)
throw UnsupportedOperationException()
}

actual suspend fun startProcess(
exe: Path,
params: List<String>,
Expand Down
19 changes: 19 additions & 0 deletions core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
actual abstract fun destroy()
}

actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? = null

Check warning on line 23 in core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt

View check run for this annotation

codefactor.io / CodeFactor

core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt#L23

The function readStderrSnapshot is missing documentation. (detekt.UndocumentedPublicFunction)

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
Expand Down
Loading