fix(client): keep Ferrum's threads from taking the host process down - #632
Merged
Conversation
`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
force-pushed
the
fix/threads-cant-kill-the-host-process
branch
from
September 3, 2026 11:36
4d8494c to
62e3a77
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #470.
The problem
Utils::Thread.spawndefaulted toabort_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 onlyEOFError, 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:
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
ensurenever getting tobrowser.quit.The change
Utils::Thread.spawndefaults toabort_on_exception: false. Every call site already passedfalseexcept 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 usesThread.main.raisedirectly.@messagesin anensurerather than only on expected disconnects.Ferrum::DeadBrowserErrorrather than an error from a place they never called.The snippet above becomes:
Verification
spec/client_spec.rb: an unexpected error in the reader is reported and the next command raisesDeadBrowserError. Without the fix the example fails withexpected Ferrum::DeadBrowserError, got #<Errno::ETIMEDOUT>.Overlaps #630 slightly: there the pendings are released when the queue closes, so with both merged the
DeadBrowserErrorhere arrives immediately rather than after:protocol_timeout. Trivial conflict inClient#startif they land in the same window.