From 62e3a77b785bd7417506a76f3435de78546a04e2 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Wed, 2 Sep 2026 21:58:56 +0300 Subject: [PATCH] fix(client): keep Ferrum's threads from taking the host process down `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 --- CHANGELOG.md | 3 +++ lib/ferrum/browser/process.rb | 2 +- lib/ferrum/client.rb | 7 ++++++- lib/ferrum/client/subscriber.rb | 4 ++-- lib/ferrum/client/web_socket.rb | 4 ++++ lib/ferrum/utils/thread.rb | 4 +++- spec/client_spec.rb | 10 ++++++++++ 7 files changed, 29 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5eb27d3..9754673e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,9 @@ the browser; a failed `IO.close` is raised to the caller. ### Fixed +- An error in the client or websocket thread was re-raised in the main thread wherever it happened to be, or took + the process down with it, since `Utils::Thread.spawn` defaulted to `abort_on_exception: true` and neither thread + rescued. Ferrum's threads no longer abort, they report to stderr and mark the connection dead [#470] - Commands in flight when the browser died waited out `:protocol_timeout` each before raising, so a dead browser turned into a long parade of slow failures. They're released as soon as the socket closes, and a command sent after that raises `Ferrum::DeadBrowserError` right away [#470] diff --git a/lib/ferrum/browser/process.rb b/lib/ferrum/browser/process.rb index a278dcc9..5b9e1c7e 100644 --- a/lib/ferrum/browser/process.rb +++ b/lib/ferrum/browser/process.rb @@ -177,7 +177,7 @@ def async_stop user_data_dir = @user_data_dir @pid = @user_data_dir = nil - Utils::Thread.spawn(abort_on_exception: false) do + Utils::Thread.spawn do Killer.kill(pid) if pid Killer.kill(xvfb_pid) if xvfb_pid Killer.remove_directory(user_data_dir) if user_data_dir diff --git a/lib/ferrum/client.rb b/lib/ferrum/client.rb index 1ccd32cc..4321476e 100644 --- a/lib/ferrum/client.rb +++ b/lib/ferrum/client.rb @@ -342,7 +342,12 @@ def start @pendings[message["id"]]&.set(message) end end - + rescue StandardError => e + # Nothing delivers responses once this thread is gone, so the connection + # is done for. Close it rather than leave every command hanging. + warn("Ferrum: dispatch stopped, #{e.class}: #{e.message}\n #{e.backtrace&.first}") + @ws.messages.close + ensure release_pendings end end diff --git a/lib/ferrum/client/subscriber.rb b/lib/ferrum/client/subscriber.rb index 4e973f70..bda895d0 100644 --- a/lib/ferrum/client/subscriber.rb +++ b/lib/ferrum/client/subscriber.rb @@ -106,7 +106,7 @@ def clear(session_id:) private def start - @regular_thread = Utils::Thread.spawn(abort_on_exception: false) do + @regular_thread = Utils::Thread.spawn do loop do message = @regular.pop break unless message @@ -115,7 +115,7 @@ def start end end - @priority_thread = Utils::Thread.spawn(abort_on_exception: false) do + @priority_thread = Utils::Thread.spawn do loop do message = @priority.pop break unless message diff --git a/lib/ferrum/client/web_socket.rb b/lib/ferrum/client/web_socket.rb index 13b694f7..cdbca237 100644 --- a/lib/ferrum/client/web_socket.rb +++ b/lib/ferrum/client/web_socket.rb @@ -152,6 +152,10 @@ def start @driver_mutex.synchronize { @driver.parse(data) } end rescue EOFError, Errno::ECONNRESET, Errno::EPIPE, IOError # rubocop:disable Lint/ShadowedException + # The browser went away, nothing to report. + rescue StandardError => e + warn("Ferrum: websocket reader stopped, #{e.class}: #{e.message}\n #{e.backtrace&.first}") + ensure @messages.close end end diff --git a/lib/ferrum/utils/thread.rb b/lib/ferrum/utils/thread.rb index 899fd186..d793da14 100644 --- a/lib/ferrum/utils/thread.rb +++ b/lib/ferrum/utils/thread.rb @@ -14,10 +14,12 @@ module Thread # # @param [Boolean] abort_on_exception # Whether the thread aborts the process if it raises an unhandled exception. + # Off by default: an exception in one of Ferrum's own threads reaches the + # main thread wherever it happens to be, or kills the process outright. # # @return [Thread] # - def spawn(abort_on_exception: true) + def spawn(abort_on_exception: false) ::Thread.new(abort_on_exception) do |whether_abort_on_exception| ::Thread.current.abort_on_exception = whether_abort_on_exception ::Thread.current.report_on_exception = true if ::Thread.current.respond_to?(:report_on_exception=) diff --git a/spec/client_spec.rb b/spec/client_spec.rb index c64c0c3b..b1585041 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -37,6 +37,16 @@ def messages expect { page.go_to("/") }.to raise_error(Ferrum::DeadBrowserError) expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 1 end + + it "reports an unexpected reader error instead of raising in the main thread" do + page = remote.create_page + driver = remote.client.instance_variable_get(:@ws).instance_variable_get(:@driver) + allow(driver).to receive(:parse).and_raise(Errno::ETIMEDOUT) + + expect do + expect { page.go_to("/") }.to raise_error(Ferrum::DeadBrowserError) + end.to output(/websocket reader stopped, Errno::ETIMEDOUT/).to_stderr + end end describe "event callbacks" do