From a5e292236393e927b45562c963d10f3c7b3efc10 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Wed, 2 Sep 2026 21:00:12 +0300 Subject: [PATCH 1/2] fix(client): release in-flight commands when the connection dies `send_message` waited out `:protocol_timeout` before checking whether the socket was still there, so every command in flight when the browser died paid the full timeout, and so did every command issued afterwards. A dead browser turned into a long parade of slow failures instead of one fast one. The dispatch loop now releases the pending commands once the message queue closes, and a command sent after that raises `DeadBrowserError` without waiting at all. Refs #470 --- CHANGELOG.md | 3 +++ lib/ferrum/client.rb | 10 ++++++++++ sig/ferrum/client.rbs | 2 ++ spec/browser_spec.rb | 17 +++++++++++++++++ 4 files changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f477cd3..7545ea69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ the browser; a failed `IO.close` is raised to the caller. ### Fixed +- 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] - A raising callback killed `Client::Subscriber`'s dispatch thread and with it every event for the rest of the process: pages created later raised `NoSuchTargetError`, open ones lost their execution contexts. Callbacks are now rescued one by one and reported to stderr [#470] diff --git a/lib/ferrum/client.rb b/lib/ferrum/client.rb index f6dd0045..4e1c8716 100644 --- a/lib/ferrum/client.rb +++ b/lib/ferrum/client.rb @@ -219,6 +219,8 @@ def send_message(message, async:, timeout: nil) @ws.send_message(message) true else + raise DeadBrowserError if @ws.messages.closed? + pending = Concurrent::IVar.new @pendings[message[:id]] = pending @ws.send_message(message) @@ -340,9 +342,17 @@ def start @pendings[message["id"]]&.set(message) end end + + release_pendings end end + # Nothing is going to answer the commands still in flight once the socket + # is gone, release them instead of making each wait out its timeout. + def release_pendings + @pendings.each_value { |pending| pending.try_set(nil) } + end + # Locked so two concurrent commands never share an id and read each # other's responses from @pendings. def next_command_id diff --git a/sig/ferrum/client.rbs b/sig/ferrum/client.rbs index d75549a0..bc90a592 100644 --- a/sig/ferrum/client.rbs +++ b/sig/ferrum/client.rbs @@ -74,6 +74,8 @@ module Ferrum def start: () -> void + def release_pendings: () -> void + def next_command_id: () -> ::Integer def raise_browser_error: (Hash[String, untyped] error) -> void diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index dfea13fa..a2676b94 100644 --- a/spec/browser_spec.rb +++ b/spec/browser_spec.rb @@ -413,6 +413,23 @@ def proxy_tunnel(socket, request, browser_url) expect(browser.body).to include("Hello world") end + + it "raises without waiting for the command to time out" do + start = Ferrum::Utils::ElapsedTime.monotonic_time + + expect { browser.crash }.to raise_error(Ferrum::DeadBrowserError) + expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 2 + + start = Ferrum::Utils::ElapsedTime.monotonic_time + + expect { browser.go_to }.to raise_error(Ferrum::DeadBrowserError) + expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 2 + + browser.restart + browser.go_to + + expect(browser.body).to include("Hello world") + end end describe "#close" do From b9fd2c2aaaf364a4b55f2326942b3c4b08cacce6 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Wed, 2 Sep 2026 22:32:22 +0300 Subject: [PATCH 2/2] test(client): cover the dead connection without timing the browser's death The crash spec timed how long Chrome takes to die, which the dockerized browser on CI doesn't answer quickly enough, and it left the shared browser dead for every spec after it. The connection is now closed under a command directly, on its own browser. --- spec/browser_spec.rb | 17 ----------------- spec/client_spec.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index a2676b94..dfea13fa 100644 --- a/spec/browser_spec.rb +++ b/spec/browser_spec.rb @@ -413,23 +413,6 @@ def proxy_tunnel(socket, request, browser_url) expect(browser.body).to include("Hello world") end - - it "raises without waiting for the command to time out" do - start = Ferrum::Utils::ElapsedTime.monotonic_time - - expect { browser.crash }.to raise_error(Ferrum::DeadBrowserError) - expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 2 - - start = Ferrum::Utils::ElapsedTime.monotonic_time - - expect { browser.go_to }.to raise_error(Ferrum::DeadBrowserError) - expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 2 - - browser.restart - browser.go_to - - expect(browser.body).to include("Hello world") - end end describe "#close" do diff --git a/spec/client_spec.rb b/spec/client_spec.rb index 34e3de39..2788c647 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -1,6 +1,37 @@ # frozen_string_literal: true describe Ferrum::Client do + describe "a dead connection" do + let(:remote) { Ferrum::Browser.new(base_url: base_url, timeout: 10, protocol_timeout: 10) } + + after { remote.quit } + + def messages + remote.client.instance_variable_get(:@ws).messages + end + + it "releases the commands in flight" do + page = remote.create_page + Thread.new do + sleep(0.2) + messages.close + end + start = Ferrum::Utils::ElapsedTime.monotonic_time + + expect { page.go_to("/really_slow") }.to raise_error(Ferrum::DeadBrowserError) + expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 5 + end + + it "fails the commands that follow without waiting" do + page = remote.create_page + messages.close + start = Ferrum::Utils::ElapsedTime.monotonic_time + + expect { page.go_to("/") }.to raise_error(Ferrum::DeadBrowserError) + expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 1 + end + end + describe "event callbacks" do let(:remote) { Ferrum::Browser.new(base_url: base_url, timeout: 3, protocol_timeout: 3) }