feat: add :protocol_timeout option for internal CDP calls and custom timeouts for #pdf/#screenshot

CHANGELOG.md

@@ -1,6 +1,12 @@
 ## [Unreleased](https://github.com/rubycdp/ferrum/compare/v0.17.2...main) ##
 
 ### Added
+- `Ferrum::Browser` option `:protocol_timeout` bounds individual internal CDP bookkeeping calls, e.g.
+  `Target.createTarget`, `Target.attachToTarget`. It's a separate setting from `:timeout`: its default (5s) happens
+  to match `:timeout`'s own default, but passing `timeout:` to `Browser.new` does not change it -- set
+  `:protocol_timeout` (or the `FERRUM_PROTOCOL_TIMEOUT` env var) explicitly for a different value.
+- `Ferrum::Page#pdf`/`#screenshot` accept a `timeout:` argument (default 60s) for the CDP call itself. Generating a
+  full-page screenshot or a large PDF is a known slow outlier among CDP commands.
 - `Ferrum::Frame#loader_id` provides a loader id when the frame navigates [#583]
 - `Ferrum::Frame#lifecycle_events` provides a list of frame's events like init, networkIdle, firstPaint, etc. [#583]
 - `Ferrum::Frame#idle?` whether frame was loaded [#583]

lib/ferrum/browser.rb

@@ -44,7 +44,8 @@ class Browser
 
     attr_reader :client, :process, :contexts, :options
 
-    delegate %i[timeout timeout= base_url base_url= default_user_agent default_user_agent= extensions] => :options
+    delegate %i[timeout timeout= protocol_timeout protocol_timeout=
+                base_url base_url= default_user_agent default_user_agent= extensions] => :options
     delegate %i[command] => :client
 
     #
@@ -80,13 +81,18 @@ class Browser
     #   When present, debug output is written to this object.
     #
     # @option options [Integer, Float] :slowmo
-    #   Set a delay in seconds to wait before sending command.
-    #   Useful companion of headless option, so that you have time to see
+    #   Set a delay in seconds to wait before sending a command.
+    #   Useful companion of a headless option, so that you have time to see
     #   changes.
     #
     # @option options [Numeric] :timeout (5)
-    #   The number of seconds we'll wait for a response when communicating with
-    #   browser.
+    #   The number of seconds we'll wait for a response when communicating
+    #   with the browser: navigations, JS evaluation, DOM queries, dispatching input, etc.
+    #
+    # @option options [Numeric] :protocol_timeout (5)
+    #   The number of seconds we'll wait for an individual internal CDP
+    #   bookkeeping call to respond, e.g. `Target.createTarget`,
+    #   `Target.attachToTarget`. These normally resolve in milliseconds.
     #
     # @option options [Boolean] :js_errors
     #   When true, JavaScript errors get re-raised in Ruby.

lib/ferrum/browser/options.rb

@@ -14,6 +14,7 @@ class Options
       WINDOW_SIZE = [1024, 768].freeze
       BASE_URL_SCHEMA = %w[http https].freeze
       DEFAULT_TIMEOUT = ENV.fetch("FERRUM_DEFAULT_TIMEOUT", 5).to_i
+      DEFAULT_PROTOCOL_TIMEOUT = ENV.fetch("FERRUM_PROTOCOL_TIMEOUT", DEFAULT_TIMEOUT).to_i
       PROCESS_TIMEOUT = ENV.fetch("FERRUM_PROCESS_TIMEOUT", 10).to_i
       DEBUG_MODE = !ENV.fetch("FERRUM_DEBUG", nil).nil?
 
@@ -22,14 +23,15 @@ class Options
                   :url, :ws_url, :env, :process_timeout, :browser_name, :browser_path,
                   :save_path, :proxy, :port, :host, :headless, :incognito, :dockerize, :browser_options,
                   :ignore_default_browser_options, :xvfb, :flatten
-      attr_accessor :timeout, :default_user_agent
+      attr_accessor :timeout, :protocol_timeout, :default_user_agent
 
       def initialize(options = nil)
         @options = Hash(options&.dup)
 
         @port = @options.fetch(:port, BROWSER_PORT)
         @host = @options.fetch(:host, BROWSER_HOST)
         @timeout = @options.fetch(:timeout, DEFAULT_TIMEOUT)
+        @protocol_timeout = @options.fetch(:protocol_timeout, DEFAULT_PROTOCOL_TIMEOUT)
         @window_size = @options.fetch(:window_size, WINDOW_SIZE)
         @js_errors = @options.fetch(:js_errors, false)
         @headless = @options.fetch(:headless, true)

lib/ferrum/client.rb

@@ -51,12 +51,16 @@ def initialize(client, session_id)
     # @param [Hash] params
     #   The command's parameters.
     #
+    # @param [Numeric, nil] timeout
+    #   How long to wait for this command's response, overriding
+    #   {Browser::Options#protocol_timeout}. See {Client#send_message}.
+    #
     # @return [Boolean, Hash]
     #   `true` when sent asynchronously, otherwise the command's result.
     #
-    def command(method, async: false, **params)
+    def command(method, async: false, timeout: nil, **params)
       message = build_message(method, params)
-      @client.send_message(message, async: async)
+      @client.send_message(message, async: async, timeout: timeout)
     end
 
     #
@@ -146,7 +150,7 @@ def event_name(event)
   class Client
     extend Forwardable
 
-    delegate %i[timeout timeout=] => :options
+    delegate %i[protocol_timeout protocol_timeout=] => :options
 
     attr_reader :ws_url, :options, :subscriber
 
@@ -174,38 +178,51 @@ def initialize(ws_url, options)
     # @param [Hash] params
     #   The command's parameters.
     #
+    # @param [Numeric, nil] timeout
+    #   How long to wait for this command's response, overriding
+    #   {Browser::Options#protocol_timeout}. See {#send_message}.
+    #
     # @return [Boolean, Hash]
     #   `true` when sent asynchronously, otherwise the command's result.
     #
-    def command(method, async: false, **params)
+    def command(method, async: false, timeout: nil, **params)
       message = build_message(method, params)
-      send_message(message, async: async)
+      send_message(message, async: async, timeout: timeout)
     end
 
     #
     # Sends a raw CDP message over the websocket. Synchronous calls block
-    # until a matching response arrives, or `timeout` (delegated to
-    # {Browser::Options#timeout}) elapses.
+    # until a matching response arrives, or `timeout` elapses, defaulting to
+    # `protocol_timeout` (delegated to {Browser::Options#protocol_timeout}).
+    # That default is the transport-level budget for internal CDP bookkeeping
+    # (e.g. `Target.createTarget`). {Page#command} overrides
+    # this back to `timeout`, or a caller-supplied budget (e.g. `#pdf`/
+    # `#screenshot`'s own `timeout:` argument), for the user-facing commands
+    # it issues -- some of which (e.g. `Page.navigate`, `Page.printToPDF`)
+    # rely on their own response latency to detect a stuck operation.
     #
     # @param [Hash] message
     #   The message to send, must include an `:id` key.
     #
     # @param [Boolean] async
     #   Whether to return immediately instead of waiting for a response.
     #
+    # @param [Numeric, nil] timeout
+    #   How long to wait for the response. Defaults to `protocol_timeout`.
+    #
     # @return [Boolean, Hash]
     #   `true` when sent asynchronously, otherwise the parsed `"result"`
     #   from the response.
     #
-    def send_message(message, async:)
+    def send_message(message, async:, timeout: nil)
       if async
         @ws.send_message(message)
         true
       else
         pending = Concurrent::IVar.new
         @pendings[message[:id]] = pending
         @ws.send_message(message)
-        data = pending.value!(timeout)
+        data = pending.value!(timeout || protocol_timeout)
         @pendings.delete(message[:id])
 
         raise DeadBrowserError if data.nil? && @ws.messages.closed?

lib/ferrum/context.rb

@@ -96,7 +96,7 @@ def create_target
 
       new_pending = Concurrent::IVar.new
       pending = @pendings.put_if_absent(target_id, new_pending) || new_pending
-      resolved = pending.value(@client.timeout)
+      resolved = pending.value(@client.protocol_timeout)
       raise NoSuchTargetError unless resolved
 
       @pendings.delete(target_id)

lib/ferrum/errors.rb

@@ -49,7 +49,8 @@ def message
       "Timed out waiting for response. It's possible that this happened " \
         "because something took a very long time (for example a page load " \
         "was slow). If so, setting the :timeout option to a higher value might " \
-        "help."
+        "help. If this happened on an internal protocol call instead, try " \
+        "raising :protocol_timeout."
     end
   end
 

lib/ferrum/page.rb

@@ -426,23 +426,28 @@ def activate
     #   Whether to sleep for `Browser::Options#slowmo` seconds before sending
     #   the command.
     #
+    # @param [Numeric, nil] timeout
+    #   Overrides the timeout this command's response is bound by. Defaults
+    #   to the page's `timeout`. Callers with their own budget (e.g. `#pdf`/
+    #   `#screenshot`) pass it explicitly.
+    #
     # @return [Hash{String => Object}]
     #
     # @example
     #   page.command("Page.navigate", url: "https://github.com/")
     #
-    def command(method, wait: 0, slowmoable: false, **params)
+    def command(method, wait: 0, slowmoable: false, timeout: nil, **params)
       iteration = @event.reset if wait.positive?
       sleep(@options.slowmo) if slowmoable && @options.slowmo.positive?
-      result = client.command(method, **params)
+      result = client.command(method, timeout: timeout || self.timeout, **params)
 
       if wait.positive?
         # Wait a bit after command and check if iteration has
-        # changed which means there was some network event for
+        #  changed, which means there was some network event for
         # the main frame, and it started to load new content.
         @event.wait(wait)
         if iteration != @event.iteration
-          set = @event.wait(timeout)
+          set = @event.wait(self.timeout)
           raise TimeoutError unless set
         end
       end

lib/ferrum/page/screenshot.rb

@@ -14,6 +14,7 @@ module Screenshot
 
       DEFAULT_SCREENSHOT_FORMAT = "png"
       SUPPORTED_SCREENSHOT_FORMAT = %w[png jpeg jpg webp].freeze
+      DEFAULT_RENDER_TIMEOUT = 60
 
       DEFAULT_PDF_OPTIONS = {
         landscape: false,
@@ -69,6 +70,11 @@ module Screenshot
       # @option opts [Ferrum::RGBA] :background_color
       #   Sets the background color.
       #
+      # @param [Numeric] timeout
+      #   How long to wait for the screenshot to be captured. Defaults to
+      #   {DEFAULT_RENDER_TIMEOUT} since a full-page capture is a known slow
+      #   outlier among CDP commands.
+      #
       # @example
       #   page.go_to("https://google.com/")
       #
@@ -87,10 +93,10 @@ module Screenshot
       # @example Save with specific background color:
       #   page.screenshot(background_color: Ferrum::RGBA.new(0, 0, 0, 0.0))
       #
-      def screenshot(**opts)
+      def screenshot(timeout: DEFAULT_RENDER_TIMEOUT, **opts)
         path, encoding = common_options(**opts)
         options = screenshot_options(path, **opts)
-        data = capture_screenshot(options, opts[:full], opts[:background_color])
+        data = capture_screenshot(options, opts[:full], opts[:background_color], timeout)
         return data if encoding == :base64
 
         bin = Base64.decode64(data)
@@ -128,15 +134,20 @@ def screenshot(**opts)
       #   See other [native options](https://chromedevtools.github.io/devtools-protocol/tot/Page#method-printToPDF) you
       #   can pass.
       #
+      # @param [Numeric] timeout
+      #   How long to wait for the PDF to be generated. Defaults to
+      #   {DEFAULT_RENDER_TIMEOUT} since large documents are a known slow
+      #   outlier among CDP commands.
+      #
       # @example
       #   page.go_to("https://google.com/")
       #   # Save to disk as a PDF
       #   page.pdf(path: "google.pdf", paper_width: 1.0, paper_height: 1.0) # => true
       #
-      def pdf(**opts)
+      def pdf(timeout: DEFAULT_RENDER_TIMEOUT, **opts)
         path, encoding = common_options(**opts)
         options = pdf_options(**opts).merge(transferMode: "ReturnAsStream")
-        handle = command("Page.printToPDF", **options).fetch("stream")
+        handle = command("Page.printToPDF", timeout: timeout, **options).fetch("stream")
         stream_to(path: path, encoding: encoding, handle: handle)
       end
 
@@ -313,11 +324,11 @@ def to_camel_case(option)
         option.to_s.gsub(%r{(?:_|(/))([a-z\d]*)}) { "#{Regexp.last_match(1)}#{Regexp.last_match(2).capitalize}" }.to_sym
       end
 
-      def capture_screenshot(options, full, background_color)
+      def capture_screenshot(options, full, background_color, timeout)
         options = options.merge(captureBeyondViewport: true) if full
 
         with_background_color(background_color) do
-          command("Page.captureScreenshot", **options)
+          command("Page.captureScreenshot", timeout: timeout, **options)
         end.fetch("data")
       end
 

sig/ferrum/browser.rbs

@@ -22,6 +22,10 @@ module Ferrum
 
     def timeout=: (::Numeric) -> ::Numeric
 
+    def protocol_timeout: () -> ::Numeric
+
+    def protocol_timeout=: (::Numeric) -> ::Numeric
+
     def default_user_agent: () -> String?
 
     def default_user_agent=: (String) -> String

sig/ferrum/browser/options.rbs

@@ -11,6 +11,8 @@ module Ferrum
 
       DEFAULT_TIMEOUT: ::Integer
 
+      DEFAULT_PROTOCOL_TIMEOUT: ::Integer
+
       PROCESS_TIMEOUT: ::Integer
 
       DEBUG_MODE: bool
@@ -19,6 +21,8 @@ module Ferrum
 
       attr_accessor timeout: ::Numeric
 
+      attr_accessor protocol_timeout: ::Numeric
+
       attr_accessor default_user_agent: String?
 
       attr_reader logger: (IO | StringIO | nil)
@@ -69,6 +73,7 @@ module Ferrum
       @port: String
       @host: String
       @timeout: ::Numeric
+      @protocol_timeout: ::Numeric
       @window_size: ::Array[::Integer]
       @js_errors: bool
       @headless: bool

sig/ferrum/client.rbs

@@ -10,7 +10,7 @@ module Ferrum
 
     def initialize: (Client client, String session_id) -> void
 
-    def command: (String method, ?async: bool, **untyped params) -> (bool | Hash[String, untyped])
+    def command: (String method, ?async: bool, ?timeout: ::Numeric?, **untyped params) -> (bool | Hash[String, untyped])
 
     def on: (String event) { (Hash[String, untyped]) -> void } -> Integer
 
@@ -48,9 +48,9 @@ module Ferrum
 
     def initialize: ((String | ::Addressable::URI) ws_url, Browser::Options options) -> void
 
-    def command: (String method, ?async: bool, **untyped params) -> (bool | Hash[String, untyped])
+    def command: (String method, ?async: bool, ?timeout: ::Numeric?, **untyped params) -> (bool | Hash[String, untyped])
 
-    def send_message: (Hash[Symbol, untyped] message, async: bool) -> (bool | Hash[String, untyped])
+    def send_message: (Hash[Symbol, untyped] message, async: bool, ?timeout: ::Numeric?) -> (bool | Hash[String, untyped])
 
     def on: (String event) { (Hash[String, untyped]) -> void } -> Integer
 
@@ -66,9 +66,9 @@ module Ferrum
 
     def build_message: (String method, Hash[Symbol, untyped] params) -> Hash[Symbol, untyped]
 
-    def timeout: () -> ::Numeric
+    def protocol_timeout: () -> ::Numeric
 
-    def timeout=: (::Numeric value) -> ::Numeric
+    def protocol_timeout=: (::Numeric value) -> ::Numeric
 
     private
 

sig/ferrum/errors.rbs

@@ -31,7 +31,7 @@ module Ferrum
   end
 
   class TimeoutError < Error
-    def message: () -> "Timed out waiting for response. It's possible that this happened because something took a very long time (for example a page load was slow). If so, setting the :timeout option to a higher value might help."
+    def message: () -> "Timed out waiting for response. It's possible that this happened because something took a very long time (for example a page load was slow). If so, setting the :timeout option to a higher value might help. If this happened on an internal protocol call instead, try raising :protocol_timeout."
   end
 
   class ScriptTimeoutError < Error

sig/ferrum/page.rbs

@@ -80,7 +80,7 @@ module Ferrum
 
     def set_window_bounds: (Hash[Symbol, (::Integer | String)] bounds) -> Hash[String, untyped]
 
-    def command: (String method, ?wait: ::Numeric?, ?slowmoable: bool, **untyped) -> Hash[String, untyped]
+    def command: (String method, ?wait: ::Numeric?, ?slowmoable: bool, ?timeout: ::Numeric?, **untyped) -> Hash[String, untyped]
 
     def on: ((Symbol | String) event) ?{ (Hash[String, untyped]) -> void } -> Integer
 

sig/ferrum/page/screenshot.rbs

@@ -1,13 +1,15 @@
 module Ferrum
   class Page
     module Screenshot
+      DEFAULT_RENDER_TIMEOUT: ::Integer
+
       DEFAULT_PDF_OPTIONS: { landscape: false, paper_width: ::Float, paper_height: 11, scale: ::Float }
 
       PAPER_FORMATS: { letter: { width: ::Float, height: ::Float }, legal: { width: ::Float, height: ::Float }, tabloid: { width: ::Float, height: ::Float }, ledger: { width: ::Float, height: ::Float }, :A0 => { width: ::Float, height: ::Float }, :A1 => { width: ::Float, height: ::Float }, :A2 => { width: ::Float, height: ::Float }, :A3 => { width: ::Float, height: ::Float }, :A4 => { width: ::Float, height: ::Float }, :A5 => { width: ::Float, height: ::Float }, :A6 => { width: ::Float, height: ::Float } }
 
-      def screenshot: (**untyped opts) -> untyped
+      def screenshot: (?timeout: ::Numeric, **untyped opts) -> untyped
 
-      def pdf: (**untyped opts) -> untyped
+      def pdf: (?timeout: ::Numeric, **untyped opts) -> untyped
 
       def mhtml: (?path: untyped?) -> untyped
 
@@ -33,7 +35,7 @@ module Ferrum
 
       def to_camel_case: (untyped option) -> (:preferCSSPageSize | untyped)
 
-      def capture_screenshot: (untyped options, untyped full, untyped background_color) -> untyped
+      def capture_screenshot: (untyped options, untyped full, untyped background_color, ::Numeric timeout) -> untyped
 
       def maybe_resize_fullscreen: (untyped full) { () -> untyped } -> untyped