ref(page): deprecate :pending_connection_errors in favor of go_to's on_timeout:

CHANGELOG.md

@@ -1,6 +1,8 @@
 ## [Unreleased](https://github.com/rubycdp/ferrum/compare/v0.20.1...main) ##
 
 ### Added
+- `go_to(url, on_timeout: :raise)` raises `NavigationTimeoutError`, a `TimeoutError` naming the requests still
+  pending, when the page doesn't load in time
 
 ### Changed
 - Every script runs the new way: keyword arguments are named parameters, promises and thenables are awaited, all
@@ -11,6 +13,8 @@
 - `current_url` and `current_title` on pages and frames, will be removed in 0.22. Use `url` and `title`, or
   `frame.page.url` for the top window from a frame, `frame.url` is the frame's own
 - `:js_errors`, will be removed in 0.22. Subscribe with `page.on("Runtime.exceptionThrown")` instead
+- `:pending_connection_errors` and `PendingConnectionsError`, will be removed in 0.22. Pass `on_timeout: :raise` to
+  `go_to` instead
 
 ### Fixed
 

docs/2-customization.md

@@ -31,8 +31,8 @@ Ferrum::Browser.new(options)
       communicating with browser. Default is 5.
     * `:js_errors` (Boolean) - Deprecated, will be removed in 0.22. When true, JavaScript errors get re-raised in
       Ruby. Subscribe with `page.on("Runtime.exceptionThrown") { |params| ... }` instead.
-    * `:pending_connection_errors` (Boolean) - Raise `PendingConnectionsError` when main frame is still waiting
-      for slow responses and timeout is reached. Default is false.
+    * `:pending_connection_errors` (Boolean) - Deprecated, will be removed in 0.22. Pass `on_timeout: :raise` to `go_to`
+      instead: `page.go_to(url, on_timeout: :raise)`.
     * `:browser_name` (Symbol) - `:chrome` by default, `:opera` and `:edge` are
       also supported.
     * `:browser_path` (String) - Path to Chrome binary, you can also set ENV

docs/3-navigation.md

@@ -4,15 +4,19 @@ sidebar_position: 4
 
 # Navigation
 
-#### go_to(url) : `String`
+#### go_to(url, \*\*options) : `String`
 
 Navigate page to.
 
 * url `String` The url should include scheme unless you set `base_url` when
   configuring driver.
+* options `Hash`
+  * :on_timeout `Symbol` What to do when the timeout is reached: `:ignore` (default) returns `nil`,
+    `:raise` raises `Ferrum::NavigationTimeoutError` naming the requests still pending.
 
 ```ruby
 page.go_to("https://github.com/")
+page.go_to("https://github.com/", on_timeout: :raise)
 ```
 
 #### back

lib/ferrum/browser.rb

@@ -101,11 +101,9 @@ class Browser
     #   errors get re-raised in Ruby. Subscribe with
     #   `page.on("Runtime.exceptionThrown")` instead.
     #
-    # @option options [Boolean] :pending_connection_errors (true)
-    #   When main frame is still waiting for slow responses while timeout is
-    #   reached {PendingConnectionsError} is raised. It's better to figure out
-    #   why you have slow responses and fix or block them rather than turn this
-    #   setting off.
+    # @option options [Boolean] :pending_connection_errors (false)
+    #   Deprecated, will be removed in Ferrum 0.22. Default for `go_to`'s
+    #   `on_timeout:`, pass `on_timeout: :raise` to `go_to` instead.
     #
     # @option options [:chrome, :opera, :edge] :browser_name (:chrome)
     #   Sets the browser's name.

lib/ferrum/browser/options.rb

@@ -36,12 +36,17 @@ def initialize(options = nil)
         @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 = parse_js_errors
+        @js_errors = fetch_deprecated(
+          :js_errors,
+          "Subscribe with page.on(\"Runtime.exceptionThrown\") { |params| ... } to handle JavaScript errors."
+        )
         @headless = @options.fetch(:headless, true)
         @incognito = @options.fetch(:incognito, true)
         @dockerize = @options.fetch(:dockerize, false)
         @flatten = @options.fetch(:flatten, true)
-        @pending_connection_errors = @options.fetch(:pending_connection_errors, false)
+        @pending_connection_errors = fetch_deprecated(
+          :pending_connection_errors, "Pass on_timeout: :raise to go_to instead: page.go_to(url, on_timeout: :raise)."
+        )
         @process_timeout = @options.fetch(:process_timeout, PROCESS_TIMEOUT)
         @slowmo = @options[:slowmo].to_f
 
@@ -132,15 +137,11 @@ def to_h
 
       private
 
-      def parse_js_errors
-        return false unless @options.key?(:js_errors)
+      def fetch_deprecated(name, instead)
+        return false unless @options.key?(name)
 
-        Utils::Deprecate.warn(
-          ":js_errors will be removed in Ferrum 0.22. Subscribe with " \
-          "page.on(\"Runtime.exceptionThrown\") { |params| ... } to handle JavaScript errors.",
-          user_code: true
-        )
-        @options[:js_errors]
+        Utils::Deprecate.warn(":#{name} will be removed in Ferrum 0.22. #{instead}", user_code: true)
+        @options[name]
       end
 
       def read_extension(extension)

lib/ferrum/errors.rb

@@ -26,6 +26,9 @@ def initialize(url, message = nil)
 
   # Raised when the request reached the server, but the page still has
   # pending network connections that never settled.
+  #
+  # @deprecated Will be removed in Ferrum 0.22 with the `:pending_connection_errors`
+  #   option. `go_to(url, on_timeout: :raise)` raises {NavigationTimeoutError}.
   class PendingConnectionsError < StatusError
     attr_reader :pendings
 
@@ -98,6 +101,49 @@ def message
     end
   end
 
+  # Raised by `go_to(url, on_timeout: :raise)` when the page doesn't load in
+  # time.
+  class NavigationTimeoutError < TimeoutError
+    # The URL navigated to.
+    #
+    # @return [String]
+    attr_reader :url
+
+    # URLs of the requests still pending when the timeout was reached.
+    #
+    # @return [Array<String>]
+    attr_reader :pendings
+
+    #
+    # @param [String] url
+    #   The URL navigated to.
+    #
+    # @param [Array<String>] pendings
+    #   URLs of the requests still pending.
+    #
+    def initialize(url, pendings = [])
+      @url = url
+      @pendings = pendings
+      super()
+    end
+
+    #
+    # Explains that the page didn't load in time, naming the requests that
+    # were still pending.
+    #
+    # @return [String]
+    #
+    def message
+      if @pendings.empty?
+        "Timed out waiting for #{@url} to load. The page took too long, setting the :timeout option to a higher " \
+          "value might help."
+      else
+        "Timed out waiting for #{@url} to load, still pending: #{@pendings.join(', ')}. If they're expected to be " \
+          "slow, setting the :timeout option to a higher value might help, otherwise block them with network.blacklist."
+      end
+    end
+  end
+
   # Raised when the browser process doesn't produce a websocket URL within
   # the configured `:process_timeout`.
   class ProcessTimeoutError < Error

lib/ferrum/page.rb

@@ -29,6 +29,7 @@ module Ferrum
   #
   class Page
     GOTO_WAIT = ENV.fetch("FERRUM_GOTO_WAIT", 0.1).to_f
+    ON_TIMEOUT = %i[ignore raise].freeze
 
     extend Forwardable
 
@@ -132,10 +133,32 @@ def initialize(client, context_id:, target_id:, proxy: nil)
     #   The URL to navigate to. The url should include scheme unless you set
     #   `{Browser#base_url = url}` when configuring.
     #
+    # @param [:ignore, :raise, nil] on_timeout
+    #   What to do when the page doesn't load in time: `:ignore` returns `nil`,
+    #   `:raise` raises {NavigationTimeoutError}. `nil` follows the deprecated
+    #   `:pending_connection_errors` option, which raises {PendingConnectionsError}.
+    #
+    # @return [String, nil]
+    #   The navigated frame's id, or `nil` when the timeout is reached.
+    #
+    # @raise [StatusError]
+    #   If the request fails.
+    #
+    # @raise [NavigationTimeoutError]
+    #   If `on_timeout` is `:raise` and the timeout is reached.
+    #
+    # @raise [ArgumentError]
+    #   If `on_timeout` is neither `:ignore`, `:raise` nor `nil`.
+    #
     # @example
     #   page.go_to("https://github.com/")
+    #   page.go_to("/slow", on_timeout: :raise)
     #
-    def go_to(url = nil)
+    def go_to(url = nil, on_timeout: nil)
+      unless on_timeout.nil? || ON_TIMEOUT.include?(on_timeout)
+        raise ArgumentError, "on_timeout should be one of #{ON_TIMEOUT.inspect}, got #{on_timeout.inspect}"
+      end
+
       options = { url: combine_url!(url) }
       options.merge!(referrer: referrer) if referrer
       response = command("Page.navigate", wait: GOTO_WAIT, **options)
@@ -146,10 +169,13 @@ def go_to(url = nil)
 
       response["frameId"]
     rescue TimeoutError
-      if @options.pending_connection_errors
-        pendings = network.traffic.select(&:pending?).map(&:url).compact
-        raise PendingConnectionsError.new(options[:url], Array(pendings))
-      end
+      on_timeout ||= @options.pending_connection_errors ? :pending_connections : :ignore
+      return if on_timeout == :ignore
+
+      pendings = network.traffic.select(&:pending?).map(&:url).compact
+      raise NavigationTimeoutError.new(options[:url], pendings) if on_timeout == :raise
+
+      raise PendingConnectionsError.new(options[:url], pendings)
     end
     alias goto go_to
     alias go go_to

sig/ferrum/browser/options.rbs

@@ -34,7 +34,7 @@ module Ferrum
 
       attr_reader slowmo: ::Float
 
-      attr_reader pending_connection_errors: bool
+      %a{deprecated} attr_reader pending_connection_errors: bool
 
       attr_reader url: String?
 
@@ -112,7 +112,7 @@ module Ferrum
 
       private
 
-      def parse_js_errors: () -> bool
+      def fetch_deprecated: (Symbol name, String instead) -> bool
 
       def read_extension: (String | Hash[Symbol, String] extension) -> String
 

sig/ferrum/errors.rbs

@@ -24,12 +24,24 @@ module Ferrum
   class ServerError < Error
   end
 
-  class PendingConnectionsError < StatusError
+  %a{deprecated} class PendingConnectionsError < StatusError
     attr_reader pendings: ::Array[String]
 
     def initialize: (String url, ?::Array[String] pendings) -> void
   end
 
+  class NavigationTimeoutError < TimeoutError
+    attr_reader url: String
+    attr_reader pendings: ::Array[String]
+
+    @url: String
+    @pendings: ::Array[String]
+
+    def initialize: (String url, ?::Array[String] pendings) -> void
+
+    def message: () -> String
+  end
+
   class TimeoutError < Error
     attr_reader command: String?
     attr_reader session_id: String?

sig/ferrum/page.rbs

@@ -49,7 +49,7 @@ module Ferrum
 
     def context: () -> Context?
 
-    def go_to: (?String? url) -> String
+    def go_to: (?String? url, ?on_timeout: (:ignore | :raise)?) -> String?
     alias goto go_to
     alias go go_to
 

spec/browser_spec.rb

@@ -672,12 +672,16 @@ def proxy_tunnel(socket, request, browser_url)
       end
     end
 
-    it "supports :pending_connection_errors argument", :launch do
-      browser = Ferrum::Browser.new(base_url: base_url, pending_connection_errors: true, timeout: 0.5)
+    it "supports deprecated :pending_connection_errors argument", :launch do
+      Ferrum::Utils::Deprecate.reset!
+      browser = nil
 
+      expect { browser = Ferrum::Browser.new(base_url: base_url, pending_connection_errors: true, timeout: 0.5) }
+        .to output(/:pending_connection_errors will be removed in Ferrum 0\.22.*called from #{__FILE__}/m).to_stderr
       expect { browser.go_to("/really_slow") }.to raise_error(Ferrum::PendingConnectionsError)
     ensure
       browser&.quit
+      Ferrum::Utils::Deprecate.reset!
     end
 
     context ":save_path argument" do

spec/page_spec.rb

@@ -25,24 +25,27 @@
 
       it "reports pending connection for image" do
         with_timeout(2) do
-          allow(browser.options).to receive(:pending_connection_errors).and_return(true)
-          expect { browser.go_to("/visit_timeout") }.to raise_error(
-            Ferrum::PendingConnectionsError,
-            %r{Request to http://.*/visit_timeout reached server, but there are still pending connections: http://.*/really_slow}
+          expect { browser.go_to("/visit_timeout", on_timeout: :raise) }.to raise_error(
+            Ferrum::NavigationTimeoutError,
+            %r{Timed out waiting for http://.*/visit_timeout to load, still pending: http://.*/really_slow\.}
           )
         end
       end
 
       it "reports pending connection for main frame" do
         with_timeout(0.5) do
-          allow(browser.options).to receive(:pending_connection_errors).and_return(true)
-          expect { browser.go_to("/really_slow") }.to raise_error(
-            Ferrum::PendingConnectionsError,
-            %r{Request to http://.*/really_slow reached server, but there are still pending connections: http://.*/really_slow}
+          expect { browser.go_to("/really_slow", on_timeout: :raise) }.to raise_error(
+            Ferrum::NavigationTimeoutError,
+            %r{Timed out waiting for http://.*/really_slow to load, still pending: http://.*/really_slow\.}
           )
         end
       end
 
+      it "rejects an unknown on_timeout" do
+        expect { browser.go_to("/", on_timeout: :wait) }
+          .to raise_error(ArgumentError, "on_timeout should be one of [:ignore, :raise], got :wait")
+      end
+
       it "handles server error" do
         expect { page.go_to("/server_error") }.to raise_error(
           Ferrum::StatusError,
@@ -247,12 +250,11 @@
 
   describe "#timeout=" do
     it "supports to change timeout dynamically" do
-      allow(browser.options).to receive(:pending_connection_errors).and_return(true)
       page.timeout = 4
-      expect { page.go_to("/really_slow") }.not_to raise_error
+      expect { page.go_to("/really_slow", on_timeout: :raise) }.not_to raise_error
 
       page.timeout = 2
-      expect { page.go_to("/really_slow") }.to raise_error(Ferrum::PendingConnectionsError)
+      expect { page.go_to("/really_slow", on_timeout: :raise) }.to raise_error(Ferrum::NavigationTimeoutError)
     end
   end