Skip to content

feat: say why a browser start failed instead of just timing out - #83

Merged
nathanfallet merged 3 commits into
mainfrom
feat/surface-why-the-browser-failed-to-start
Aug 27, 2026
Merged

feat: say why a browser start failed instead of just timing out#83
nathanfallet merged 3 commits into
mainfrom
feat/surface-why-the-browser-failed-to-start

Conversation

@nathanfallet

Copy link
Copy Markdown
Member

Problem

When a browser never opens its debug port, all you get is a 30s wait and a generic FailedToConnectToBrowserException. There is no way to tell why it failed.

The reason is in fact already known — testConnection() catches the error on every attempt — but it is logged at debug level, so it never reaches production logs:

} catch (e: Exception) {
    logger.debug("Could not start: ${e.message}")
    false
}

And the info branch that gives up says it is "reading error" while the block that would actually read it is commented out.

Changes

1. Report the failure. start() now logs at error level: the port it waited on, how long, the process id, whether that process is still alive, and the last connection error.

Browser never opened its debug port on 127.0.0.1:52475 after 30500ms
  (pid=6220, alive=true). Last connection error: ConnectException: Connection refused

That last field is the useful one — it separates two causes that were indistinguishable:

  • connection refused → nothing ever listened, the browser did not open the port;
  • timeout / unexpected response → something answers, so another process holds it.

And alive=false would say the browser process died outright rather than hanging.

2. Discard the browser's stdio instead of piping it. Nothing reads those pipes today, 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 opens its debug port, the port never opens and the start times out with no visible cause. This is also why the "read the error stream" block was commented out with "seems to block indefinitely on CI": reading a pipe that was never drained can hang.

Notes

No API change; both are internal. :core:jvmTest passes.

:core:build fails on kotlinStoreYarnLock here, as it does on main — unrelated.

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 6.66667% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../kotlin/dev/kdriver/core/browser/DefaultBrowser.kt 12.50% 7 Missing ⚠️
...ain/kotlin/dev/kdriver/core/browser/Process.jvm.kt 0.00% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.
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.
@nathanfallet
nathanfallet force-pushed the feat/surface-why-the-browser-failed-to-start branch from be9ab33 to d548142 Compare August 26, 2026 18:19
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.
@nathanfallet
nathanfallet merged commit dc7f7ec into main Aug 27, 2026
3 of 5 checks passed
@nathanfallet
nathanfallet deleted the feat/surface-why-the-browser-failed-to-start branch August 27, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant