From a956a2ba60a5f8b5e6bf2b8937ba5d2f8db67d8b Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 30 Oct 2026 15:03:10 +0300 Subject: [PATCH] ref(page): deprecate :pending_connection_errors in favor of go_to's on_timeout: Whether a navigation timeout raises is decided per call, so go_to takes on_timeout: :ignore or :raise. A timeout isn't a status error, so :raise raises NavigationTimeoutError, a TimeoutError naming the requests still pending. The browser option and PendingConnectionsError, still raised through it, will be removed in 0.22. --- CHANGELOG.md | 4 +++ docs/2-customization.md | 4 +-- docs/3-navigation.md | 6 ++++- lib/ferrum/browser.rb | 8 +++--- lib/ferrum/browser/options.rb | 21 ++++++++-------- lib/ferrum/errors.rb | 46 ++++++++++++++++++++++++++++++++++ lib/ferrum/page.rb | 36 ++++++++++++++++++++++---- sig/ferrum/browser/options.rbs | 4 +-- sig/ferrum/errors.rbs | 14 ++++++++++- sig/ferrum/page.rbs | 2 +- spec/browser_spec.rb | 8 ++++-- spec/page_spec.rb | 24 ++++++++++-------- 12 files changed, 137 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 942d0430..d607fd22 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/2-customization.md b/docs/2-customization.md index d65a2a94..6c451da1 100644 --- a/docs/2-customization.md +++ b/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 diff --git a/docs/3-navigation.md b/docs/3-navigation.md index 90f88293..98a444b3 100644 --- a/docs/3-navigation.md +++ b/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 diff --git a/lib/ferrum/browser.rb b/lib/ferrum/browser.rb index 05470c58..52d213fd 100644 --- a/lib/ferrum/browser.rb +++ b/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. diff --git a/lib/ferrum/browser/options.rb b/lib/ferrum/browser/options.rb index 7468d0bd..9b58510c 100644 --- a/lib/ferrum/browser/options.rb +++ b/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) diff --git a/lib/ferrum/errors.rb b/lib/ferrum/errors.rb index 57b95cec..7681b5b0 100644 --- a/lib/ferrum/errors.rb +++ b/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] + attr_reader :pendings + + # + # @param [String] url + # The URL navigated to. + # + # @param [Array] 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 diff --git a/lib/ferrum/page.rb b/lib/ferrum/page.rb index 8ca5f87e..c695256a 100644 --- a/lib/ferrum/page.rb +++ b/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 diff --git a/sig/ferrum/browser/options.rbs b/sig/ferrum/browser/options.rbs index 24eb243c..338581a8 100644 --- a/sig/ferrum/browser/options.rbs +++ b/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 diff --git a/sig/ferrum/errors.rbs b/sig/ferrum/errors.rbs index 37821353..eb6723b6 100644 --- a/sig/ferrum/errors.rbs +++ b/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? diff --git a/sig/ferrum/page.rbs b/sig/ferrum/page.rbs index b73620fe..f5de7d27 100644 --- a/sig/ferrum/page.rbs +++ b/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 diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index fe0ed703..5a278dba 100644 --- a/spec/browser_spec.rb +++ b/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 diff --git a/spec/page_spec.rb b/spec/page_spec.rb index 0dbf15fd..a14cdbd3 100644 --- a/spec/page_spec.rb +++ b/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