[rb] improve error management

SeleniumHQ · Jan 2, 2024 · 4ce5514 · 4ce5514
1 parent cd556fc
commit 4ce5514
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 49 deletions.
diff --git a/rb/lib/selenium/webdriver/common/driver_finder.rb b/rb/lib/selenium/webdriver/common/driver_finder.rb
@@ -20,52 +20,57 @@
 module Selenium
   module WebDriver
     class DriverFinder
-      def self.results(options, klass)
-        path = klass.driver_path
-        path = path.call if path.is_a?(Proc)
+      class << self
+        def results(options, klass)
+          path = klass.driver_path
+          path = path.call if path.is_a?(Proc)
+          exe = klass::EXECUTABLE
 
-        results = if path
-                    {driver_path: path}
-                  else
-                    begin
-                      SeleniumManager.results(to_args(options))
-                    rescue StandardError => e
-                      raise Error::NoSuchDriverError,
-                            "Unable to obtain #{klass::EXECUTABLE} using Selenium Manager; #{e.message}"
-                    end
-                  end
-
-        begin
-          Platform.assert_executable(results[:driver_path])
-        rescue TypeError
-          raise Error::NoSuchDriverError, "Unable to locate or obtain #{klass::EXECUTABLE}"
-        rescue Error::WebDriverError => e
-          raise Error::NoSuchDriverError, "#{klass::EXECUTABLE} located, but: #{e.message}"
+          if path
+            validate_files(exe, driver_path: path)
+          else
+            begin
+              validate_files(exe, **SeleniumManager.results(to_args(options)))
+            rescue StandardError => e
+              raise Error::NoSuchDriverError("Unable to obtain #{exe} using Selenium Manager"), e.message, e.backtrace
+            end
+          end
         end
 
-        results
-      end
+        def path(options, klass)
+          WebDriver.logger.deprecate('`DriverFinder.path`', '`DriverFinder.results`', id: :driver_finder)
+          results(options, klass)[:driver_path]
+        end
 
-      def self.path(options, klass)
-        WebDriver.logger.deprecate('`DriverFinder.path`', '`DriverFinder.results`', id: :driver_finder)
-        results(options, klass)[:driver_path]
-      end
+        private
 
-      def self.to_args(options)
-        args = ['--browser', options.browser_name]
-        if options.browser_version
-          args << '--browser-version'
-          args << options.browser_version
+        def to_args(options)
+          args = ['--browser', options.browser_name]
+          if options.browser_version
+            args << '--browser-version'
+            args << options.browser_version
+          end
+          if options.respond_to?(:binary) && !options.binary.nil?
+            args << '--browser-path'
+            args << options.binary.gsub('\\', '\\\\\\')
+          end
+          if options.proxy
+            args << '--proxy'
+            args << (options.proxy.ssl || options.proxy.http)
+          end
+          args
         end
-        if options.respond_to?(:binary) && !options.binary.nil?
-          args << '--browser-path'
-          args << options.binary.gsub('\\', '\\\\\\')
-        end
-        if options.proxy
-          args << '--proxy'
-          args << (options.proxy.ssl || options.proxy.http)
+
+        def validate_files(exe, **opts)
+          opts.each_value do |value|
+            raise Error::NoSuchDriverError("Unable to locate or obtain #{exe})") if value.nil?
+
+            Platform.assert_executable(value)
+          rescue Error::WebDriverError => e
+            raise Error::NoSuchDriverError("#{exe} located, but has problems"), e.message, e.backtrace
+          end
+          opts
         end
-        args
       end
     end
   end

diff --git a/rb/lib/selenium/webdriver/common/selenium_manager.rb b/rb/lib/selenium/webdriver/common/selenium_manager.rb
@@ -42,11 +42,8 @@ def results(*arguments)
           arguments << '--debug' if WebDriver.logger.debug?
           output = run(binary, *arguments)
 
-          browser_path = Platform.cygwin? ? Platform.cygwin_path(output['browser_path']) : output['browser_path']
-          driver_path = Platform.cygwin? ? Platform.cygwin_path(output['driver_path']) : output['driver_path']
-          Platform.assert_executable driver_path
-
-          {driver_path: driver_path, browser_path: browser_path}
+          {driver_path: Platform.cygwin? ? Platform.cygwin_path(output['driver_path']) : output['driver_path'],
+           browser_path: Platform.cygwin? ? Platform.cygwin_path(output['browser_path']) : output['browser_path']}
         end
 
         private
@@ -77,14 +74,12 @@ def binary
         end
 
         def validate_location(location)
+          raise Error::WebDriverError('Unable to locate or obtain Selenium Manager binary') if value.nil?
+
           begin
-            Platform.assert_file(location)
             Platform.assert_executable(location)
-          rescue TypeError
-            raise Error::WebDriverError,
-                  "Unable to locate or obtain Selenium Manager binary; #{location} is not a valid file object"
           rescue Error::WebDriverError => e
-            raise Error::WebDriverError, "Selenium Manager binary located, but #{e.message}"
+            raise Error::WebDriverError, "#{exe} located, but has problems: #{e.message}"
           end
 
           WebDriver.logger.debug("Selenium Manager binary found at #{location}", id: :selenium_manager)