fix: build

.github/workflows/tests.yml

@@ -16,6 +16,7 @@ jobs:
     env:
       FERRUM_PROCESS_TIMEOUT: 25
       FERRUM_DEFAULT_TIMEOUT: 15
+      FERRUM_CHROME_DOCKERIZE: true
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -31,11 +32,6 @@ jobs:
         with:
           chrome-version: stable
 
-      - name: Fix GA Chrome Permissions
-        run: |
-          sudo chown root:root /opt/hostedtoolcache/setup-chrome/chromium/stable/x64/chrome-sandbox
-          sudo chmod 4755 /opt/hostedtoolcache/setup-chrome/chromium/stable/x64/chrome-sandbox
-
       - name: Run tests
         run: |
           mkdir -p /tmp/ferrum

.rubocop.yml

@@ -6,6 +6,9 @@ AllCops:
 Layout/FirstArrayElementIndentation:
   EnforcedStyle: consistent
 
+Naming/PredicateMethod:
+  Enabled: false
+
 Naming/MethodParameterName:
   MinNameLength: 2
   AllowedNames:
@@ -56,5 +59,5 @@ Metrics/ModuleLength:
 Metrics/PerceivedComplexity:
   Max: 14
 
-require:
+plugins:
   - rubocop-rake

CHANGELOG.md

@@ -2,10 +2,12 @@
 
 ### Added
 - `Ferrum::Network::Response#body!` returns body or throws error if implicable
+- `Ferrum::Browser#new(dockerize: true)` whether to add CLI flags to run a browser in a container, `false` by default
 
 ### 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
 
 ### Fixed
 

README.md

@@ -134,13 +134,15 @@ browser.quit
 
 ## Docker
 
-In docker as root you must pass the no-sandbox browser option:
+Running in docker as root you should pass:
 
 ```ruby
-Ferrum::Browser.new(browser_options: { "no-sandbox": nil })
+Ferrum::Browser.new(dockerize: true)
 ```
 
-It has also been reported that the Chrome process repeatedly crashes when running inside a Docker container on an M1 Mac preventing Ferrum from working. Ferrum should work as expected when deployed to a Docker container on a non-M1 Mac.
+Essentially it just sets CLI flags for a browser to make it start. On CI, you can just set `FERRUM_CHROME_DOCKERIZE=true` environment variable, and it will be
+passed to all browser instances.
+
 
 ## Customization
 
@@ -153,6 +155,7 @@ Ferrum::Browser.new(options)
 * options `Hash`
   * `:headless` (Boolean) - Set browser as headless or not, `true` by default.
   * `:incognito` (Boolean) - Create an incognito profile for the browser startup window, `true` by default.
+  * `:dockerize` (Boolean) - Provide CLI flags to the browser to run it in a container.
   * `:xvfb` (Boolean) - Run browser in a virtual framebuffer, `false` by default.
   * `:flatten` (Boolean) - Use one websocket connection to the browser and all the pages in flatten mode.
   * `:window_size` (Array) - The dimensions of the browser window in which to
@@ -167,9 +170,8 @@ Ferrum::Browser.new(options)
   * `:timeout` (Numeric) - The number of seconds we'll wait for a response when
       communicating with browser. Default is 5.
   * `:js_errors` (Boolean) - When true, JavaScript errors get re-raised in Ruby.
-  * `:pending_connection_errors` (Boolean) - 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. Default is true.
+  * `:pending_connection_errors` (Boolean) - When main frame is still waiting for slow responses and timeout is
+      reached, we raise `PendingConnectionsError`. Default is false.
   * `: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.rb

@@ -15,6 +15,7 @@
 module Ferrum
   class Browser
     extend Forwardable
+
     delegate %i[default_context] => :contexts
     delegate %i[targets create_target page pages windows] => :default_context
     delegate %i[go_to goto go back forward refresh reload stop wait_for_reload
@@ -48,6 +49,9 @@ class Browser
     # @option options [Boolean] :incognito (true)
     #   Create an incognito profile for the browser startup window.
     #
+    # @option options [Boolean] :dockerize (false)
+    #   Add CLI flags to a browser to run in a container.
+    #
     # @option options [Boolean] :xvfb (false)
     #   Run browser in a virtual framebuffer.
     #
@@ -77,7 +81,7 @@ class Browser
     # @option options [Boolean] :js_errors
     #   When true, JavaScript errors get re-raised in Ruby.
     #
-    # @option options [Boolean] :pending_connection_errors (true)
+    # @option options [Boolean] :pending_connection_errors (false)
     #   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

lib/ferrum/browser/options.rb

@@ -14,7 +14,7 @@ class Options
       attr_reader :window_size, :logger, :ws_max_receive_size,
                   :js_errors, :base_url, :slowmo, :pending_connection_errors,
                   :url, :ws_url, :env, :process_timeout, :browser_name, :browser_path,
-                  :save_path, :proxy, :port, :host, :headless, :incognito, :browser_options,
+                  :save_path, :proxy, :port, :host, :headless, :incognito, :dockerize, :browser_options,
                   :ignore_default_browser_options, :xvfb, :flatten
       attr_accessor :timeout, :default_user_agent
 
@@ -28,8 +28,9 @@ def initialize(options = nil)
         @js_errors = @options.fetch(:js_errors, false)
         @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, true)
+        @pending_connection_errors = @options.fetch(:pending_connection_errors, false)
         @process_timeout = @options.fetch(:process_timeout, PROCESS_TIMEOUT)
         @slowmo = @options[:slowmo].to_f
 

lib/ferrum/browser/options/chrome.rb

@@ -40,9 +40,6 @@ class Chrome < Base
           "no-startup-window" => nil,
           "remote-allow-origins" => "*",
           "disable-blink-features" => "AutomationControlled"
-          # NOTE: --no-sandbox is not needed if you properly set up a user in the container.
-          # https://github.com/ebidel/lighthouse-ci/blob/master/builder/Dockerfile#L35-L40
-          # "no-sandbox" => nil,
         }.freeze
 
         MAC_BIN_PATH = [
@@ -76,15 +73,23 @@ def merge_required(flags, options, user_data_dir)
         end
 
         def merge_default(flags, options)
-          defaults = except("headless", "disable-gpu") if options.headless == false
-          defaults ||= DEFAULT_OPTIONS
+          defaults = options.headless == false ? except("headless", "disable-gpu") : DEFAULT_OPTIONS
           defaults.delete("no-startup-window") if options.incognito == false
+
+          if options.dockerize || ENV["FERRUM_CHROME_DOCKERIZE"] == "true"
+            # NOTE: --no-sandbox is not needed if you properly set up a user in the container.
+            # https://github.com/ebidel/lighthouse-ci/blob/master/builder/Dockerfile#L35-L40
+            defaults = defaults.merge("no-sandbox" => nil, "disable-setuid-sandbox" => nil)
+          end
+
           # On Windows, the --disable-gpu flag is a temporary workaround for a few bugs.
           # See https://bugs.chromium.org/p/chromium/issues/detail?id=737678 for more information.
           defaults = defaults.merge("disable-gpu" => nil) if Utils::Platform.windows?
+
           # Use Metal on Apple Silicon
           # https://github.com/google/angle#platform-support-via-backing-renderers
           defaults = defaults.merge("use-angle" => "metal") if Utils::Platform.mac_arm?
+
           defaults.merge(flags)
         end
       end

lib/ferrum/browser/process.rb

@@ -18,11 +18,8 @@ class Process
       KILL_TIMEOUT = 2
       WAIT_KILLED = 0.05
 
-      attr_reader :host, :port, :ws_url, :pid, :command,
-                  :default_user_agent, :browser_version, :protocol_version,
-                  :v8_version, :webkit_version, :xvfb
-
       extend Forwardable
+
       delegate path: :command
 
       def self.start(*args)
@@ -61,6 +58,10 @@ def self.directory_remover(path)
         }
       end
 
+      attr_reader :host, :port, :ws_url, :pid, :command,
+                  :default_user_agent, :browser_version, :protocol_version,
+                  :v8_version, :webkit_version, :xvfb
+
       def initialize(options)
         @pid = @xvfb = @user_data_dir = nil
 

lib/ferrum/client.rb

@@ -61,6 +61,7 @@ def event_name(event)
 
   class Client
     extend Forwardable
+
     delegate %i[timeout timeout=] => :options
 
     attr_reader :ws_url, :options, :subscriber

lib/ferrum/errors.rb

@@ -113,9 +113,10 @@ def initialize(response = nil)
   class JavaScriptError < BrowserError
     attr_reader :class_name, :message, :stack_trace
 
-    def initialize(response, stack_trace = nil)
-      @class_name, @message = response.values_at("className", "description")
-      @stack_trace = stack_trace
+    def initialize(response)
+      @class_name, @message = response["exception"]&.values_at("className", "description")
+      @message ||= response["text"]
+      @stack_trace = response["stackTrace"]
       super(response.merge("message" => @message))
     end
   end

lib/ferrum/frame/runtime.rb

@@ -135,7 +135,7 @@ def handle_error(response)
         when /\AError: timed out promise/
           raise ScriptTimeoutError
         else
-          raise JavaScriptError.new(result, response.dig("exceptionDetails", "stackTrace"))
+          raise JavaScriptError, response["exceptionDetails"]
         end
       end
 

lib/ferrum/page.rb

@@ -21,6 +21,7 @@ class Page
     GOTO_WAIT = ENV.fetch("FERRUM_GOTO_WAIT", 0.1).to_f
 
     extend Forwardable
+
     delegate %i[at_css at_xpath css xpath
                 current_url current_title url title body doctype content=
                 execution_id execution_id! evaluate evaluate_on evaluate_async execute evaluate_func
@@ -442,10 +443,7 @@ def subscribe
       if @options.js_errors
         on("Runtime.exceptionThrown") do |params|
           # FIXME: https://jvns.ca/blog/2015/11/27/why-rubys-timeout-is-dangerous-and-thread-dot-raise-is-terrifying/
-          Thread.main.raise JavaScriptError.new(
-            params.dig("exceptionDetails", "exception"),
-            params.dig("exceptionDetails", "stackTrace")
-          )
+          Thread.main.raise JavaScriptError, params["exceptionDetails"]
         end
       end
 

spec/browser/options/chrome_spec.rb

@@ -28,7 +28,7 @@
 
   describe ".version" do
     it "returns an executable version" do
-      expect(described_class.version).to match(/(Chromium|Chrome) \d/)
+      expect(described_class.version).to match(/(Chromium|Chrome)(?: for Testing)? \d/)
     end
   end
 end

spec/browser_spec.rb

@@ -51,7 +51,8 @@
 
     it "supports :ignore_default_browser_options argument" do
       defaults = Ferrum::Browser::Options::Chrome.options.except("disable-web-security")
-      browser = Ferrum::Browser.new(ignore_default_browser_options: true, browser_options: defaults)
+      browser = Ferrum::Browser.new(ignore_default_browser_options: true,
+                                    browser_options: defaults.merge("no-sandbox" => nil))
       browser.go_to(base_url("/console_log"))
     ensure
       browser&.quit
@@ -235,9 +236,9 @@
     end
 
     it "supports :pending_connection_errors argument" do
-      browser = Ferrum::Browser.new(base_url: base_url, pending_connection_errors: false, timeout: 0.5)
+      browser = Ferrum::Browser.new(base_url: base_url, pending_connection_errors: true, timeout: 0.5)
 
-      expect { browser.go_to("/really_slow") }.not_to raise_error
+      expect { browser.go_to("/really_slow") }.to raise_error(Ferrum::PendingConnectionsError)
     ensure
       browser&.quit
     end

spec/network/exchange_spec.rb

@@ -44,6 +44,17 @@
       page.go_to
 
       expect(page.body).to include("Another World")
+
+      # Debug: Check all exchanges
+      puts "Total exchanges: #{network.traffic.size}"
+      network.traffic.each_with_index do |ex, i|
+        puts "Exchange #{i}: url=#{ex.url}, intercepted=#{ex.intercepted?}, #{ex.intercepted_request.inspect}"
+      end
+
+      puts "===="
+      puts last_exchange.inspect
+      puts "----"
+
       expect(last_exchange.intercepted_request).to be
       expect(last_exchange.intercepted_request).to be_a(Ferrum::Network::InterceptedRequest)
     end
@@ -177,12 +188,8 @@
     it "determines if exchange is not fully loaded" do
       allow(page).to receive(:timeout) { 2 }
 
-      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}
-      )
+      page.go_to("/visit_timeout")
+
       expect(last_exchange.pending?).to be true
     end
 

spec/page/tracing_spec.rb

@@ -5,7 +5,6 @@
   let(:file_path2) { "#{PROJECT_ROOT}/spec/tmp/trace2.json" }
   let(:file_path3) { "#{PROJECT_ROOT}/spec/tmp/trace3.json" }
   let(:content) { JSON.parse(File.read(file_path)) }
-  let(:trace_config) { JSON.parse(content["metadata"]["trace-config"]) }
 
   after do
     FileUtils.rm_f(file_path)
@@ -30,21 +29,14 @@
       ) { page.go_to }
 
       expect(File.exist?(file_path)).to be(true)
-      expect(trace_config["excluded_categories"]).to eq(["*"])
-      expect(trace_config["included_categories"]).to eq(["disabled-by-default-devtools.timeline"])
       expect(content["traceEvents"].any? { |o| o["cat"] == "toplevel" }).to eq(false)
+      expect(content["traceEvents"]).to include(hash_including("cat" => "disabled-by-default-devtools.timeline"))
     end
 
     it "runs with default categories" do
       page.tracing.record(path: file_path) { page.go_to }
 
       expect(File.exist?(file_path)).to be(true)
-      expect(trace_config["excluded_categories"]).to eq(["*"])
-      expect(trace_config["included_categories"])
-        .to match_array(%w[devtools.timeline v8.execute disabled-by-default-devtools.timeline
-                           disabled-by-default-devtools.timeline.frame toplevel blink.console
-                           blink.user_timing latencyInfo disabled-by-default-devtools.timeline.stack
-                           disabled-by-default-v8.cpu_profiler disabled-by-default-v8.cpu_profiler.hires])
       expect(content["traceEvents"].any? { |o| o["cat"] == "toplevel" }).to eq(true)
     end
 
@@ -101,20 +93,23 @@
 
     context "with screenshots enabled" do
       it "fills file with screenshot data" do
-        page.tracing.record(path: file_path, screenshots: true) { page.go_to("/grid") }
+        page.tracing.record(path: file_path, screenshots: true) do
+          page.go_to("/grid")
+          sleep 0.1
+        end
 
         expect(File.exist?(file_path)).to be(true)
-        expect(trace_config["included_categories"]).to include("disabled-by-default-devtools.screenshot")
         expect(content["traceEvents"].any? { |o| o["name"] == "Screenshot" }).to eq(true)
       end
 
       it "returns a buffer with screenshot data" do
-        trace = page.tracing.record(screenshots: true) { page.go_to("/grid") }
+        trace = page.tracing.record(screenshots: true) do
+          page.go_to("/grid")
+          sleep 0.1
+        end
 
         expect(File.exist?(file_path)).to be(false)
         content = JSON.parse(trace)
-        trace_config = JSON.parse(content["metadata"]["trace-config"])
-        expect(trace_config["included_categories"]).to include("disabled-by-default-devtools.screenshot")
         expect(content["traceEvents"].any? { |o| o["name"] == "Screenshot" }).to eq(true)
       end
     end