Merge pull request #1799 from teamcapybara/selenium_ignore_quit_errors

ignore Selenium UnknownError during quit

History.md

@@ -12,6 +12,7 @@ Release date: unreleased
 * Selenium driver with Chrome should support multiple file upload [Thomas Walpole]
 * Fix visible: :hidden with :text option behavior [Thomas Walpole]
 * App exceptions with multiple parameter initializers now re-raised correctly - Issue #1785 [Michael Lutsiuk]
+* Warn on selenium unknown errors rather than raising when quitting driver [Adam Pohorecki, Thomas Walpole]
 
 #2.10.1
 Release date: 2016-10-08

lib/capybara/selenium/driver.rb

@@ -258,8 +258,10 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
   rescue Errno::ECONNREFUSED
     # Browser must have already gone
   rescue Selenium::WebDriver::Error::UnknownError => e
-    raise unless e.message =~ /Error communicating with the remote browser/
-    # probably already gone
+    unless silenced_unknown_error_message?(e.message) # Most likely already gone
+      # probably already gone but not sure - so warn
+      warn "Ignoring Selenium UnknownError during driver quit: #{e.message}"
+    end
   ensure
     @browser = nil
   end
@@ -311,4 +313,11 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     end
   end
 
+  def silenced_unknown_error_message?(msg)
+    silenced_unknown_error_messages.any? { |r| msg =~ r }
+  end
+
+  def silenced_unknown_error_messages
+    [ /Error communicating with the remote browser/ ]
+  end
 end

spec/selenium_spec_marionette.rb

@@ -52,6 +52,39 @@ RSpec.describe Capybara::Selenium::Driver do
       #access instance variable directly so we don't create a new browser instance
       expect(@driver.instance_variable_get(:@browser)).to be_nil
     end
+
+    context "with errors" do
+      before do
+        @original_browser = @driver.browser
+      end
+      after do
+        # Ensure browser is actually quit so we don't leave hanging processe
+        RSpec::Mocks.space.proxy_for(@original_browser).reset
+        @original_browser.quit
+      end
+
+      it "warns UnknownError returned during quit because the browser is probably already gone" do
+        expect_any_instance_of(Capybara::Selenium::Driver).to receive(:warn).with(/random message/)
+        allow(@driver.browser).to(
+          receive(:quit)
+          .and_raise(Selenium::WebDriver::Error::UnknownError, "random message")
+        )
+
+        expect { @driver.quit }.not_to raise_error
+        expect(@driver.instance_variable_get(:@browser)).to be_nil
+      end
+
+      it "ignores silenced UnknownError returned during quit because the browser is almost definitely already gone" do
+        expect_any_instance_of(Capybara::Selenium::Driver).not_to receive(:warn)
+        allow(@driver.browser).to(
+          receive(:quit)
+          .and_raise(Selenium::WebDriver::Error::UnknownError, "Error communicating with the remote browser")
+        )
+
+        expect { @driver.quit }.not_to raise_error
+        expect(@driver.instance_variable_get(:@browser)).to be_nil
+      end
+    end
   end
 
   context "storage" do