Skip to content

fix(client): keep Ferrum's threads from taking the host process down - #632

Merged
route merged 1 commit into
mainfrom
fix/threads-cant-kill-the-host-process
Sep 3, 2026
Merged

fix(client): keep Ferrum's threads from taking the host process down#632
route merged 1 commit into
mainfrom
fix/threads-cant-kill-the-host-process

Conversation

@route

@route route commented Sep 2, 2026

Copy link
Copy Markdown
Member

Refs #470.

The problem

Utils::Thread.spawn defaulted to abort_on_exception: true, and the two transport threads used that default. The client dispatch thread has no rescue at all, and the websocket reader rescues only EOFError, ECONNRESET, EPIPE, IOError. Anything else is re-raised in the main thread wherever it happens to be.

Forcing one on main, an error the reader doesn't expect:

b = Ferrum::Browser.new
page = b.create_page
driver = b.client.instance_variable_get(:@ws).instance_variable_get(:@driver)
def driver.parse(_data) = raise(Errno::ETIMEDOUT)

page.go_to("data:text/html,<h1>hi</h1>")   # Errno::ETIMEDOUT, out of a navigation

In a suite that's a failure attributed to whatever call was unlucky enough to be running. In a server it's process death, with the caller's ensure never getting to browser.quit.

The change

  • Utils::Thread.spawn defaults to abort_on_exception: false. Every call site already passed false except these two, so the explicit arguments are gone and the intent lives in one place. The deliberate main-thread raise for JS errors is untouched: it uses Thread.main.raise directly.
  • Both transport threads rescue and report to stderr, and the websocket reader closes @messages in an ensure rather than only on expected disconnects.
  • Either thread dying now means the connection is dead, so callers get Ferrum::DeadBrowserError rather than an error from a place they never called.

The snippet above becomes:

Ferrum: websocket reader stopped, Errno::ETIMEDOUT: Operation timed out
  ...
Ferrum::DeadBrowserError

Verification

  • New example in spec/client_spec.rb: an unexpected error in the reader is reported and the next command raises DeadBrowserError. Without the fix the example fails with expected Ferrum::DeadBrowserError, got #<Errno::ETIMEDOUT>.
  • Full suite: 616 examples, 0 failures.

Overlaps #630 slightly: there the pendings are released when the queue closes, so with both merged the DeadBrowserError here arrives immediately rather than after :protocol_timeout. Trivial conflict in Client#start if they land in the same window.

`Utils::Thread.spawn` defaulted to `abort_on_exception: true`, and neither the client dispatch thread nor the
websocket reader rescued anything beyond four expected disconnects. Any other error, a socket error outside that
list for instance, was re-raised in the main thread wherever it happened to be: a failure blamed on whatever call
was running in a suite, a dead process in a server, with the caller's `ensure` never reaching the browser.

Threads now default to not aborting, both transport threads rescue and report to stderr, and the connection is
marked dead so callers get `DeadBrowserError` instead of an error from a place they never called.

Refs #470
@route
route force-pushed the fix/threads-cant-kill-the-host-process branch from 4d8494c to 62e3a77 Compare September 3, 2026 11:36
@route
route merged commit e3918c7 into main Sep 3, 2026
7 checks passed
@route
route deleted the fix/threads-cant-kill-the-host-process branch September 3, 2026 12:18
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