chore: revert default for :pending_connection_errors to true and update related docs and specs (#572)

* chore: revert default for :pending_connection_errors to true and update related docs and specs

* fix: add back downloads specs

That was a sinatra disposition header issue, not chrome that was opening pdf extension. The header was not set because public folder was handled by rack static and file was returned to chrome without disposition header making chrome extension to intercept pdf request.

Author: route

CHANGELOG.md

@@ -8,7 +8,6 @@
 ### Changed
 - `Ferrum::Network::Response#body` returns body or nil in case of errors
 - Disable Chrome code sign clones [#555]
-- `Ferrum::Browser` option `:pending_connection_errors` is set to false by default
 - Ruby version required is >= 3.1 [#565]
 
 ### Fixed

docs/2-customization.md

@@ -29,7 +29,7 @@ Ferrum::Browser.new(options)
       communicating with browser. Default is 5.
     * `:js_errors` (Boolean) - When true, JavaScript errors get re-raised in Ruby.
     * `:pending_connection_errors` (Boolean) - Raise `PendingConnectionsError` when main frame is still waiting
-      for slow responses and timeout is reached. Default is false.
+      for slow responses and timeout is reached. Default is true.
     * `:browser_name` (Symbol) - `:chrome` by default, only experimental support
       for `:firefox` for now.
     * `:browser_path` (String) - Path to Chrome binary, you can also set ENV

lib/ferrum/browser/options.rb

@@ -30,7 +30,7 @@ def initialize(options = nil)
         @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 = @options.fetch(:pending_connection_errors, true)
         @process_timeout = @options.fetch(:process_timeout, PROCESS_TIMEOUT)
         @slowmo = @options[:slowmo].to_f
 

lib/ferrum/contexts.rb

@@ -72,7 +72,7 @@ def size
 
     private
 
-    def subscribe # rubocop:disable Metrics/PerceivedComplexity
+    def subscribe # rubocop:disable Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity
       @client.on("Target.attachedToTarget") do |params|
         info, session_id = params.values_at("targetInfo", "sessionId")
         next unless ALLOWED_TARGET_TYPES.include?(info["type"])

lib/ferrum/cookies/cookie.rb

@@ -161,7 +161,7 @@ def ==(other)
       #   The raw cookie string.
       #
       def to_s
-        string = String.new("#{@attributes['name']}=#{@attributes['value']}")
+        string = "#{@attributes['name']}=#{@attributes['value']}"
 
         @attributes.each do |key, value|
           case key

spec/browser_spec.rb

@@ -236,9 +236,9 @@
     end
 
     it "supports :pending_connection_errors argument" do
-      browser = Ferrum::Browser.new(base_url: base_url, pending_connection_errors: true, timeout: 0.5)
+      browser = Ferrum::Browser.new(base_url: base_url, pending_connection_errors: false, timeout: 0.5)
 
-      expect { browser.go_to("/really_slow") }.to raise_error(Ferrum::PendingConnectionsError)
+      expect { browser.go_to("/really_slow") }.not_to raise_error
     ensure
       browser&.quit
     end

spec/cookies_spec.rb

@@ -23,7 +23,6 @@
                                         "secure" => false,
                                         "session" => true,
                                         "priority" => "Medium",
-                                        "sameParty" => false,
                                         "sourceScheme" => "NonSecure",
                                         "sourcePort" => server.port)
           ]
@@ -47,7 +46,6 @@
                                         "secure" => false,
                                         "session" => true,
                                         "priority" => "Medium",
-                                        "sameParty" => false,
                                         "sourceScheme" => "NonSecure",
                                         "sourcePort" => server.port)
           ]
@@ -72,7 +70,6 @@
                                                                        "secure" => false,
                                                                        "session" => true,
                                                                        "priority" => "Medium",
-                                                                       "sameParty" => false,
                                                                        "sourceScheme" => "NonSecure",
                                                                        "sourcePort" => server.port) })
     end

spec/downloads_spec.rb

@@ -4,15 +4,8 @@
   let(:filename) { "attachment.pdf" }
   let(:save_path) { "/tmp/ferrum" }
 
-  def skip_browser_bug
-    # Also https://github.com/puppeteer/puppeteer/issues/10161
-    skip "https://bugs.chromium.org/p/chromium/issues/detail?id=1444729"
-  end
-
   describe "#files" do
     it "saves an attachment" do
-      skip_browser_bug
-
       page.downloads.set_behavior(save_path: save_path)
       page.go_to("/#{filename}")
       page.downloads.wait
@@ -50,8 +43,6 @@ def skip_browser_bug
   describe "#set_behavior" do
     context "with absolute path" do
       it "saves an attachment" do
-        skip_browser_bug
-
         page.downloads.set_behavior(save_path: save_path)
         page.go_to("/#{filename}")
         page.downloads.wait
@@ -62,17 +53,13 @@ def skip_browser_bug
       end
 
       it "saves no attachment when behavior is deny" do
-        skip_browser_bug
-
         page.downloads.set_behavior(save_path: save_path, behavior: :deny)
         page.downloads.wait { page.go_to("/#{filename}") }
 
         expect(File.exist?("#{save_path}/#{filename}")).to be false
       end
 
       it "saves an attachment on click" do
-        skip_browser_bug
-
         page.downloads.set_behavior(save_path: save_path)
         page.go_to("/attachment")
         page.downloads.wait { page.at_css("#download").click }

spec/network/exchange_spec.rb

@@ -197,7 +197,12 @@
     it "determines if exchange is not fully loaded" do
       allow(page).to receive(:timeout) { 2 }
 
-      page.go_to("/visit_timeout")
+      expect do
+        page.go_to("/visit_timeout")
+      end.to raise_error(
+        Ferrum::PendingConnectionsError,
+        %r{Request to http://.*/visit_timeout reached server, but there are still pending connections: http://.*/really_slow}
+      )
 
       expect(last_exchange.pending?).to be true
     end

spec/page_spec.rb

@@ -25,7 +25,6 @@
 
       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}
@@ -35,7 +34,6 @@
 
       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}
@@ -166,7 +164,6 @@
 
   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