fix(client): keep Ferrum's threads from taking the host process down

CHANGELOG.md

@@ -25,6 +25,9 @@
   the browser; a failed `IO.close` is raised to the caller.
 
 ### Fixed
+- An error in the client or websocket thread was re-raised in the main thread wherever it happened to be, or took
+  the process down with it, since `Utils::Thread.spawn` defaulted to `abort_on_exception: true` and neither thread
+  rescued. Ferrum's threads no longer abort, they report to stderr and mark the connection dead [#470]
 - Commands in flight when the browser died waited out `:protocol_timeout` each before raising, so a dead browser
   turned into a long parade of slow failures. They're released as soon as the socket closes, and a command sent
   after that raises `Ferrum::DeadBrowserError` right away [#470]

lib/ferrum/browser/process.rb

@@ -177,7 +177,7 @@ def async_stop
         user_data_dir = @user_data_dir
         @pid = @user_data_dir = nil
 
-        Utils::Thread.spawn(abort_on_exception: false) do
+        Utils::Thread.spawn do
           Killer.kill(pid) if pid
           Killer.kill(xvfb_pid) if xvfb_pid
           Killer.remove_directory(user_data_dir) if user_data_dir

lib/ferrum/client.rb

@@ -342,7 +342,12 @@ def start
             @pendings[message["id"]]&.set(message)
           end
         end
-
+      rescue StandardError => e
+        # Nothing delivers responses once this thread is gone, so the connection
+        # is done for. Close it rather than leave every command hanging.
+        warn("Ferrum: dispatch stopped, #{e.class}: #{e.message}\n  #{e.backtrace&.first}")
+        @ws.messages.close
+      ensure
         release_pendings
       end
     end

lib/ferrum/client/subscriber.rb

@@ -106,7 +106,7 @@ def clear(session_id:)
       private
 
       def start
-        @regular_thread = Utils::Thread.spawn(abort_on_exception: false) do
+        @regular_thread = Utils::Thread.spawn do
           loop do
             message = @regular.pop
             break unless message
@@ -115,7 +115,7 @@ def start
           end
         end
 
-        @priority_thread = Utils::Thread.spawn(abort_on_exception: false) do
+        @priority_thread = Utils::Thread.spawn do
           loop do
             message = @priority.pop
             break unless message

lib/ferrum/client/web_socket.rb

@@ -152,6 +152,10 @@ def start
             @driver_mutex.synchronize { @driver.parse(data) }
           end
         rescue EOFError, Errno::ECONNRESET, Errno::EPIPE, IOError # rubocop:disable Lint/ShadowedException
+          # The browser went away, nothing to report.
+        rescue StandardError => e
+          warn("Ferrum: websocket reader stopped, #{e.class}: #{e.message}\n  #{e.backtrace&.first}")
+        ensure
           @messages.close
         end
       end

lib/ferrum/utils/thread.rb

@@ -14,10 +14,12 @@ module Thread
       #
       # @param [Boolean] abort_on_exception
       #   Whether the thread aborts the process if it raises an unhandled exception.
+      #   Off by default: an exception in one of Ferrum's own threads reaches the
+      #   main thread wherever it happens to be, or kills the process outright.
       #
       # @return [Thread]
       #
-      def spawn(abort_on_exception: true)
+      def spawn(abort_on_exception: false)
         ::Thread.new(abort_on_exception) do |whether_abort_on_exception|
           ::Thread.current.abort_on_exception = whether_abort_on_exception
           ::Thread.current.report_on_exception = true if ::Thread.current.respond_to?(:report_on_exception=)

spec/client_spec.rb

@@ -37,6 +37,16 @@ def messages
       expect { page.go_to("/") }.to raise_error(Ferrum::DeadBrowserError)
       expect(Ferrum::Utils::ElapsedTime.elapsed_time(start)).to be < 1
     end
+
+    it "reports an unexpected reader error instead of raising in the main thread" do
+      page = remote.create_page
+      driver = remote.client.instance_variable_get(:@ws).instance_variable_get(:@driver)
+      allow(driver).to receive(:parse).and_raise(Errno::ETIMEDOUT)
+
+      expect do
+        expect { page.go_to("/") }.to raise_error(Ferrum::DeadBrowserError)
+      end.to output(/websocket reader stopped, Errno::ETIMEDOUT/).to_stderr
+    end
   end
 
   describe "event callbacks" do