Skip to content

fix(subscriber): don't let a raising callback kill event dispatch - #629

Merged
route merged 1 commit into
mainfrom
fix/subscriber-callback-errors
Sep 2, 2026
Merged

fix(subscriber): don't let a raising callback kill event dispatch#629
route merged 1 commit into
mainfrom
fix/subscriber-callback-errors

Conversation

@route

@route route commented Sep 2, 2026

Copy link
Copy Markdown
Member

Refs #470.

The problem

Client::Subscriber dispatches every CDP event on a single thread, and the loop had no rescue:

loop do
  message = @regular.pop
  break unless message
  call(message)          # a raising callback ends the thread, permanently
end

So the first exception out of any callback kills dispatch for the rest of the process. Every event after that is dropped: targets are never registered, pages created later raise NoSuchTargetError, and pages already open lose their execution contexts.

Reproduced on main:

b = Ferrum::Browser.new(protocol_timeout: 3, timeout: 3)
page = b.create_page                                                # ok
b.client.on("Target.targetCreated") { raise Ferrum::TimeoutError }  # e.g. a CDP call inside a handler
b.create_page                                                       # ok
b.create_page                                                       # Ferrum::NoSuchTargetError
page.go_to("data:text/html,<h1>hi</h1>")                            # Ferrum::NoExecutionContextError

It prints #<Thread:...utils/thread.rb...> terminated with exception (report_on_exception is true) and from then on the connection is deaf. That is the exact failure sequence reported in #470: a Ferrum::TimeoutError raised inside a Ferrum thread, followed by every later test failing with Ferrum::NoSuchTargetError from Context#create_target.

Callbacks that issue CDP commands make this reachable on any slow or loaded machine: the default on(:dialog) handler accepting a dialog, network interception blocks, or a driver's own handlers. Contexts already rescues BrowserError/TimeoutError in connect_worker and the detach paths, which is this same hazard patched one call site at a time.

The fix

Each callback is called with a rescue around it. The error goes to stderr, the other callbacks for that event still run, and the thread keeps dispatching:

Ferrum: Target.targetCreated callback raised Ferrum::TimeoutError: Timed out waiting for response...
  repro.rb:9:in 'block in <main>'

The snippet above then runs to completion: both later create_page calls succeed and the existing page still navigates.

Verification

  • New unit spec, spec/unit/client/subscriber_spec.rb: a raising callback doesn't stop the events that follow, and the error is reported. Both examples fail with Timeout::Error without the fix, since nothing is dispatched after the raise.
  • Full suite: 616 examples, 0 failures.

Note this is the amplifier, not the trigger, for #470. The timeouts that set it off there look like the two races fixed in #602 (unlocked command-id minting and unserialized websocket-driver access), which also explains why flatten: false helped in that thread: flatten funnels every page's commands through one client and one socket, maximising both races. This change makes a single such timeout survivable instead of fatal to the whole run.

`Client::Subscriber` dispatches every CDP event on one thread and its loop had no rescue, so the first exception
raised by a callback terminated that thread for the rest of the process. Nothing was dispatched after it: targets
stopped being registered and every page created later raised `NoSuchTargetError`, pages already open lost their
execution contexts. Callbacks that issue CDP commands make this reachable on any slow machine, a `TimeoutError` from
the default dialog handler or from a network interception block is enough.

Each callback is now called with a rescue around it, so the error is reported to stderr and the remaining callbacks
and events still run.

Refs #470
@route
route force-pushed the fix/subscriber-callback-errors branch from 3474f0a to 83489c6 Compare September 2, 2026 17:22
@route
route merged commit 5a19ecb into main Sep 2, 2026
7 checks passed
@route
route deleted the fix/subscriber-callback-errors branch September 2, 2026 17:33
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