From 6a73fcfb0aeca0436f2b0807cc9827bdcd4c924d Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin <d.vorotilin@gmail.com> Date: Fri, 5 Jan 2024 17:13:34 +0300 Subject: [PATCH] ref: rename Client --- CHANGELOG.md | 3 +- lib/ferrum/browser.rb | 2 +- lib/ferrum/browser/client.rb | 115 ------------------- lib/ferrum/client.rb | 113 ++++++++++++++++++ lib/ferrum/{browser => client}/subscriber.rb | 2 +- lib/ferrum/{browser => client}/web_socket.rb | 2 +- lib/ferrum/target.rb | 2 +- spec/browser_spec.rb | 2 +- spec/unit/process_spec.rb | 4 +- 9 files changed, 122 insertions(+), 123 deletions(-) delete mode 100644 lib/ferrum/browser/client.rb create mode 100644 lib/ferrum/client.rb rename lib/ferrum/{browser => client}/subscriber.rb (98%) rename lib/ferrum/{browser => client}/web_socket.rb (99%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76f5e565..bc210a71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,8 @@ instead of passing browser and making cyclic dependency on the browser instance, - Got rid of `Concurrent::Async` in `Ferrum::Browser::Subscriber` [#432] - `Ferrum::Page#set_window_bounds` is renamed to `Ferrum::Page#window_bounds=` - `Ferrum::Page` get right client from the Target and passes it down everywhere [#433] -- `Ferrum::Network::InterceptedRequest` accepts `Browser::Client` instead of `Ferrum::Page` [#433] +- `Ferrum::Network::InterceptedRequest` accepts `Ferrum::Browser::Client` instead of `Ferrum::Page` [#433] +- `Ferrum::Browser::Client` -> `Ferrum::Client` [#433] ### Fixed diff --git a/lib/ferrum/browser.rb b/lib/ferrum/browser.rb index 15faf4a7..e1186472 100644 --- a/lib/ferrum/browser.rb +++ b/lib/ferrum/browser.rb @@ -4,11 +4,11 @@ require "forwardable" require "ferrum/page" require "ferrum/proxy" +require "ferrum/client" require "ferrum/contexts" require "ferrum/browser/xvfb" require "ferrum/browser/options" require "ferrum/browser/process" -require "ferrum/browser/client" require "ferrum/browser/binary" require "ferrum/browser/version_info" diff --git a/lib/ferrum/browser/client.rb b/lib/ferrum/browser/client.rb deleted file mode 100644 index f8d678b3..00000000 --- a/lib/ferrum/browser/client.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -require "forwardable" -require "ferrum/browser/subscriber" -require "ferrum/browser/web_socket" - -module Ferrum - class Browser - class Client - extend Forwardable - delegate %i[timeout timeout=] => :options - - attr_reader :options - - def initialize(ws_url, options) - @command_id = 0 - @options = options - @pendings = Concurrent::Hash.new - @ws = WebSocket.new(ws_url, options.ws_max_receive_size, options.logger) - @subscriber = Subscriber.new - - start - end - - def command(method, async: false, **params) - message = build_message(method, params) - - if async - @ws.send_message(message) - true - else - pending = Concurrent::IVar.new - @pendings[message[:id]] = pending - @ws.send_message(message) - data = pending.value!(timeout) - @pendings.delete(message[:id]) - - raise DeadBrowserError if data.nil? && @ws.messages.closed? - raise TimeoutError unless data - - error, response = data.values_at("error", "result") - raise_browser_error(error) if error - response - end - end - - def on(event, &block) - @subscriber.on(event, &block) - end - - def subscribed?(event) - @subscriber.subscribed?(event) - end - - def close - @ws.close - # Give a thread some time to handle a tail of messages - @pendings.clear - @thread.kill unless @thread.join(1) - @subscriber.close - end - - def inspect - "#<#{self.class} " \ - "@command_id=#{@command_id.inspect} " \ - "@pendings=#{@pendings.inspect} " \ - "@ws=#{@ws.inspect}>" - end - - private - - def start - @thread = Utils::Thread.spawn do - loop do - message = @ws.messages.pop - break unless message - - if message.key?("method") - @subscriber << message - else - @pendings[message["id"]]&.set(message) - end - end - end - end - - def build_message(method, params) - { method: method, params: params }.merge(id: next_command_id) - end - - def next_command_id - @command_id += 1 - end - - def raise_browser_error(error) - case error["message"] - # Node has disappeared while we were trying to get it - when "No node with given id found", - "Could not find node with given id", - "Inspected target navigated or closed" - raise NodeNotFoundError, error - # Context is lost, page is reloading - when "Cannot find context with specified id" - raise NoExecutionContextError, error - when "No target with given id found" - raise NoSuchPageError - when /Could not compute content quads/ - raise CoordinatesNotFoundError - else - raise BrowserError, error - end - end - end - end -end diff --git a/lib/ferrum/client.rb b/lib/ferrum/client.rb new file mode 100644 index 00000000..37afa112 --- /dev/null +++ b/lib/ferrum/client.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require "forwardable" +require "ferrum/client/subscriber" +require "ferrum/client/web_socket" + +module Ferrum + class Client + extend Forwardable + delegate %i[timeout timeout=] => :options + + attr_reader :options + + def initialize(ws_url, options) + @command_id = 0 + @options = options + @pendings = Concurrent::Hash.new + @ws = WebSocket.new(ws_url, options.ws_max_receive_size, options.logger) + @subscriber = Subscriber.new + + start + end + + def command(method, async: false, **params) + message = build_message(method, params) + + if async + @ws.send_message(message) + true + else + pending = Concurrent::IVar.new + @pendings[message[:id]] = pending + @ws.send_message(message) + data = pending.value!(timeout) + @pendings.delete(message[:id]) + + raise DeadBrowserError if data.nil? && @ws.messages.closed? + raise TimeoutError unless data + + error, response = data.values_at("error", "result") + raise_browser_error(error) if error + response + end + end + + def on(event, &block) + @subscriber.on(event, &block) + end + + def subscribed?(event) + @subscriber.subscribed?(event) + end + + def close + @ws.close + # Give a thread some time to handle a tail of messages + @pendings.clear + @thread.kill unless @thread.join(1) + @subscriber.close + end + + def inspect + "#<#{self.class} " \ + "@command_id=#{@command_id.inspect} " \ + "@pendings=#{@pendings.inspect} " \ + "@ws=#{@ws.inspect}>" + end + + private + + def start + @thread = Utils::Thread.spawn do + loop do + message = @ws.messages.pop + break unless message + + if message.key?("method") + @subscriber << message + else + @pendings[message["id"]]&.set(message) + end + end + end + end + + def build_message(method, params) + { method: method, params: params }.merge(id: next_command_id) + end + + def next_command_id + @command_id += 1 + end + + def raise_browser_error(error) + case error["message"] + # Node has disappeared while we were trying to get it + when "No node with given id found", + "Could not find node with given id", + "Inspected target navigated or closed" + raise NodeNotFoundError, error + # Context is lost, page is reloading + when "Cannot find context with specified id" + raise NoExecutionContextError, error + when "No target with given id found" + raise NoSuchPageError + when /Could not compute content quads/ + raise CoordinatesNotFoundError + else + raise BrowserError, error + end + end + end +end diff --git a/lib/ferrum/browser/subscriber.rb b/lib/ferrum/client/subscriber.rb similarity index 98% rename from lib/ferrum/browser/subscriber.rb rename to lib/ferrum/client/subscriber.rb index 1a9fe7ee..7fc52a64 100644 --- a/lib/ferrum/browser/subscriber.rb +++ b/lib/ferrum/client/subscriber.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Ferrum - class Browser + class Client class Subscriber INTERRUPTIONS = %w[Fetch.requestPaused Fetch.authRequired].freeze diff --git a/lib/ferrum/browser/web_socket.rb b/lib/ferrum/client/web_socket.rb similarity index 99% rename from lib/ferrum/browser/web_socket.rb rename to lib/ferrum/client/web_socket.rb index 02204298..2fcb3a06 100644 --- a/lib/ferrum/browser/web_socket.rb +++ b/lib/ferrum/client/web_socket.rb @@ -5,7 +5,7 @@ require "websocket/driver" module Ferrum - class Browser + class Client class WebSocket WEBSOCKET_BUG_SLEEP = 0.05 SKIP_LOGGING_SCREENSHOTS = !ENV["FERRUM_LOGGING_SCREENSHOTS"] diff --git a/lib/ferrum/target.rb b/lib/ferrum/target.rb index 63bb36ec..8dfc8af2 100644 --- a/lib/ferrum/target.rb +++ b/lib/ferrum/target.rb @@ -69,7 +69,7 @@ def maybe_sleep_if_new_window def build_client options = @client.options ws_url = options.ws_url.merge(path: "/devtools/page/#{id}").to_s - Browser::Client.new(ws_url, options) + Client.new(ws_url, options) end end end diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index 854b46c2..508a6f5e 100644 --- a/spec/browser_spec.rb +++ b/spec/browser_spec.rb @@ -280,7 +280,7 @@ allow(Ferrum::Browser::Process).to receive(:new).and_return(process) error = StandardError.new - allow(Ferrum::Browser::Client).to receive(:new).and_raise(error) + allow(Ferrum::Client).to receive(:new).and_raise(error) expect { Ferrum::Browser.new }.to raise_error(error) expect(process.pid).to be(nil) diff --git a/spec/unit/process_spec.rb b/spec/unit/process_spec.rb index 8fde4395..519a2ee7 100644 --- a/spec/unit/process_spec.rb +++ b/spec/unit/process_spec.rb @@ -7,7 +7,7 @@ it "forcibly kills the child if it does not respond to SIGTERM" do allow(Process).to receive(:spawn).and_return(5678) allow(Process).to receive(:wait).and_return(nil) - allow(Ferrum::Browser::Client).to receive(:new).and_return(double.as_null_object) + allow(Ferrum::Client).to receive(:new).and_return(double.as_null_object) allow_any_instance_of(Ferrum::Browser::Process).to receive(:parse_ws_url) @@ -25,7 +25,7 @@ it "passes through env" do allow(Process).to receive(:wait).and_return(nil) - allow(Ferrum::Browser::Client).to receive(:new).and_return(double.as_null_object) + allow(Ferrum::Client).to receive(:new).and_return(double.as_null_object) allow(Process).to receive(:spawn).with({ "LD_PRELOAD" => "some.so" }, any_args).and_return(123_456_789)