Merge pull request #103 from zombocom/schneems/retry-visiting-url

Retry on ReadTimeout from website.visit

Author: schneems

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## HEAD
 
+- Fix: Net::ReadTimeout errors on `website.visit` are now retried by default (https://github.com/zombocom/rundoc/pull/103)
 - Added: When a background process (`background.start`) does not exit cleanly, print its logs for help with debugging. (https://github.com/zombocom/rundoc/pull/104)
 
 ## 4.1.3

lib/rundoc/code_command/website/driver.rb

@@ -6,7 +6,8 @@ class Rundoc::CodeCommand::Website
   class Driver
     attr_reader :session
 
-    def initialize(name:, url:, width: 1024, height: 720, visible: false)
+    def initialize(name:, url:, width: 1024, height: 720, visible: false, io: $stdout, read_timeout: 60)
+      @io = io
       browser_options = ::Selenium::WebDriver::Chrome::Options.new
       browser_options.args << "--headless" unless visible
       browser_options.args << "--disable-gpu" if Gem.win_platform?
@@ -15,7 +16,10 @@ def initialize(name:, url:, width: 1024, height: 720, visible: false)
       @width = width
       @height = height
 
-      @driver = Capybara::Selenium::Driver.new(nil, browser: :chrome, options: browser_options)
+      client = Selenium::WebDriver::Remote::Http::Default.new
+      client.read_timeout = read_timeout
+
+      @driver = Capybara::Selenium::Driver.new(nil, browser: :chrome, options: browser_options, http_client: client)
       driver_name = :"rundoc_driver_#{name}"
       Capybara.register_driver(driver_name) do |app|
         @driver
@@ -24,8 +28,20 @@ def initialize(name:, url:, width: 1024, height: 720, visible: false)
       @session = Capybara::Session.new(driver_name)
     end
 
-    def visit(url)
-      @session.visit(url)
+    def visit(url, max_attempts: 3, delay: 1)
+      attempts = 0
+      begin
+        @session.visit(url)
+      rescue ::Net::ReadTimeout => e
+        attempts += 1
+        if attempts > max_attempts
+          raise e
+        else
+          @io.puts "Error visiting url (#{attempts}/#{max_attempts}) `#{url}`:\n#{e}"
+          sleep delay
+          retry
+        end
+      end
     end
 
     def timestamp
@@ -54,7 +70,7 @@ def safe_eval(code, env = {})
       msg = +""
       msg << "Error running code #{code.inspect} at #{current_url.inspect}\n"
       msg << "saving a screenshot to: `tmp/error.png`"
-      puts msg
+      @io.puts msg
       error_path = env[:context].screenshots_dir.join("error.png")
       session.save_screenshot(error_path)
       raise e
@@ -66,13 +82,13 @@ def screenshot(screenshots_dir:, upload: false)
       file_name = self.class.next_screenshot_name
       file_path = screenshots_dir.join(file_name)
       session.save_screenshot(file_path)
-      puts "Screenshot saved to #{file_path}"
+      @io.puts "Screenshot saved to #{file_path}"
 
       return file_path unless upload
 
       case upload
       when "s3", "aws"
-        puts "Uploading screenshot to S3"
+        @io.puts "Uploading screenshot to S3"
         require "aws-sdk-s3"
         ENV.fetch("AWS_ACCESS_KEY_ID")
         s3 = Aws::S3::Resource.new(region: ENV.fetch("AWS_REGION"))

lib/rundoc/code_command/website/visit.rb

@@ -2,13 +2,14 @@
 
 class Rundoc::CodeCommand::Website
   class Visit < Rundoc::CodeCommand
-    def initialize(name:, url: nil, scroll: nil, height: 720, width: 1024, visible: false)
+    def initialize(name:, url: nil, scroll: nil, height: 720, width: 1024, visible: false, max_attempts: 3)
       @name = name
       @url = url
       @scroll = scroll
       @height = height
       @width = width
       @visible = visible
+      @max_attempts = max_attempts
     end
 
     def driver
@@ -33,7 +34,7 @@ def call(env = {})
 
       puts message
 
-      driver.visit(@url) if @url
+      driver.visit(@url, max_attempts: @max_attempts) if @url
       driver.scroll(@scroll) if @scroll
 
       return "" if contents.nil? || contents.empty?

test/integration/website_test.rb

@@ -34,4 +34,23 @@ def test_screenshot_command
       end
     end
   end
+
+  def test_retry_client_closed_early
+    tcp_unexpected_exit do |port|
+      io = StringIO.new
+      driver = Rundoc::CodeCommand::Website::Driver.new(name: SecureRandom.hex, url: nil, read_timeout: 0.1, io: io)
+      assert_raises(Net::ReadTimeout) do
+        driver.visit("http://localhost:#{port}", max_attempts: 3, delay: 0)
+      end
+
+      logs = io.string
+      assert_logs_include(logs: logs, include_str: "Error visiting url (1/3)")
+      assert_logs_include(logs: logs, include_str: "Error visiting url (2/3)")
+      assert_logs_include(logs: logs, include_str: "Error visiting url (3/3)")
+    end
+  end
+
+  def assert_logs_include(logs:, include_str:)
+    assert logs.include?(include_str), "Expected logs to include #{include_str} but they didnt. Logs:\n#{logs}"
+  end
 end

test/test_helper.rb

@@ -9,6 +9,8 @@
 require "minitest/autorun"
 require "mocha/minitest"
 require "tmpdir"
+require "socket"
+require "timeout"
 
 class Minitest::Test
   SUCCESS_DIRNAME = Rundoc::CLI::DEFAULTS::ON_SUCCESS_DIR
@@ -81,4 +83,30 @@ def called?
 
     attr_reader :value
   end
+
+  # Yields port, exits unexpectedly
+  def tcp_unexpected_exit(timeout: 30)
+    Timeout.timeout(timeout) do
+      threads = []
+      server = TCPServer.new("127.0.0.1", 0)
+      port = server.addr[1]
+      threads << Thread.new do
+        # Accept one connection, then raise an error to simulate unexpected close
+        server.accept
+        raise "Unexpected server error!"
+      rescue
+        # Simulate crash, but let ensure run
+      end.tap { |t| t.abort_on_exception = false }
+
+      threads << Thread.new do
+        yield port
+      end
+
+      while threads.all?(&:alive?)
+        sleep 0.1
+      end
+    ensure
+      server.close if server && !server.closed?
+    end
+  end
 end