fix(subscriber): don't let a raising callback kill event dispatch - #629
Merged
Conversation
`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
force-pushed
the
fix/subscriber-callback-errors
branch
from
September 2, 2026 17:22
3474f0a to
83489c6
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
Client::Subscriberdispatches every CDP event on a single thread, and the loop had no rescue: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:
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: aFerrum::TimeoutErrorraised inside a Ferrum thread, followed by every later test failing withFerrum::NoSuchTargetErrorfromContext#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.Contextsalready rescuesBrowserError/TimeoutErrorinconnect_workerand 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:
The snippet above then runs to completion: both later
create_pagecalls succeed and the existing page still navigates.Verification
spec/unit/client/subscriber_spec.rb: a raising callback doesn't stop the events that follow, and the error is reported. Both examples fail withTimeout::Errorwithout the fix, since nothing is dispatched after the raise.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-driveraccess), which also explains whyflatten: falsehelped 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.