Allow retry on failed WebDriver process launch (#473)

This allows us to compensate for mysterious, but recoverable,
safaridriver failed launches.

Default for safaridriver is set to 4 retries (total of 5 attempts).

Also host/port socket connectivity check updated to:
- include an explicit timeout of 1 second
- always close socket no matter what happens

Closes #468

doc/01-user-guide.adoc

@@ -2032,6 +2032,12 @@ a| As you would expect, varies by vendor:
 Example: `:args-driver ["-b" "/path/to/firefox/binary"]`
 | <not set>
 
+| `:webdriver-failed-launch-retries`
+
+Introduced to compensate for mysterious but recoverable failed launches of safari driver.
+a| - safari `4` (for a total of 5 tries)
+- all other drivers `0`
+
 | `:path-browser` to *web browser* binary. +
 Typically used if your browser is not on the PATH.
 

src/etaoin/api.clj

@@ -169,7 +169,8 @@
 
 (def ^{:doc "WebDriver global option defaults"} defaults-global
   {:log-level :all
-   :locator default-locator})
+   :locator default-locator
+   :webdriver-failed-launch-retries 0})
 
 (def ^{:doc "WebDriver driver type specific option defaults.
              Note that for locally launched WebDriver processes the default port is a random free port."}
@@ -181,7 +182,8 @@
    :phantom {:port 8910
              :path-driver "phantomjs"}
    :safari  {:port 4445
-             :path-driver "safaridriver"}
+             :path-driver "safaridriver"
+             :webdriver-failed-launch-retries 4}
    :edge    {:port 17556
              :path-driver "msedgedriver"}})
 
@@ -2216,13 +2218,23 @@
 (defn running?
   "Return true if `driver` seems accessable via its `:host` and `:port`.
 
-  Throws if using `:webdriver-url`"
+  Throws
+  - if using `:webdriver-url`
+  - if driver process is found not be running"
   [driver]
   (when (:webdriver-url driver)
     (throw (ex-info "Not supported for driver using :webdriver-url" {})))
+
+  (when-let [process (:process driver)]
+    (when-not (proc/alive? process)
+      (throw (ex-info (format "WebDriver process exited unexpectedly with a value: %d"
+                              (:exit (proc/result process)))
+                      {}))))
   (util/connectable? (:host driver)
                      (:port driver)))
 
+
+
 ;;
 ;; predicates
 ;;
@@ -2599,15 +2611,17 @@
 (defn wait-running
   "Waits until `driver` is reachable via its host and port via [[running?]].
 
-  Throws if using `:webdriver-url`.
+  Throws
+  - if using `:webdriver-url`.
+  - if driver process is found to be no longer running while waiting
 
   - `opts`: see [[wait-predicate]] opts."
   [driver & [opts]]
   (when (:webdriver-url driver)
     (throw (ex-info "Not supported for driver using :webdriver-url" {})))
   (log/debugf "Waiting until %s:%s is running"
               (:host driver) (:port driver))
-  (wait-predicate #(running? driver) opts))
+  (wait-predicate #(running? driver) (merge {:message "Wait until driver is running"} opts)))
 
 ;;
 ;; visible actions
@@ -3403,6 +3417,40 @@
       http (assoc :http http)
       ssl  (assoc :ssl ssl))))
 
+(defn- -run-driver*
+  [driver & [{:keys [dev
+                     env
+                     log-level
+                     log-stdout
+                     log-stderr
+                     args-driver
+                     path-driver
+                     download-dir
+                     path-browser
+                     driver-log-level]}]]
+  (let [{:keys [port host]} driver
+
+        _ (when (util/connectable? host port)
+            (throw (ex-info
+                     (format "Port %d already in use" port)
+                     {:port port})))
+
+        driver    (cond-> driver
+                    true             (drv/set-browser-log-level log-level)
+                    true             (drv/set-path path-driver)
+                    true             (drv/set-port port)
+                    dev              (drv/set-perf-logging (:perf dev))
+                    driver-log-level (drv/set-driver-log-level driver-log-level)
+                    args-driver      (drv/set-args args-driver)
+                    path-browser     (drv/set-binary path-browser)
+                    download-dir     (drv/set-download-dir download-dir))
+        proc-args (drv/get-args driver)
+        _         (log/debugf "Starting process: %s" (str/join \space proc-args))
+        process   (proc/run proc-args {:log-stdout log-stdout
+                                       :log-stderr log-stderr
+                                       :env        env})]
+    (util/assoc-some driver :env env :process process)))
+
 (defn- -run-driver
   "Runs a driver process locally.
 
@@ -3421,6 +3469,8 @@
   -- `:path-browser` is a string path to the browser's binary
   file. When not passed, the driver discovers it by its own.
 
+  -- `:webdriver-failed-launch-retries` number of times to retry launching webdriver process.
+
   -- `:log-level` a keyword to set browser's log level. Used when fetching
   browser's logs. Possible values are: `:off`, `:debug`, `:warn`, `:info`,
   `:error`, `:all`.
@@ -3440,39 +3490,26 @@
 
   -- `:env` is a map with system ENV variables. Keys are turned into
   upper-case strings."
-  [driver & [{:keys [dev
-                     env
-                     log-level
-                     log-stdout
-                     log-stderr
-                     args-driver
-                     path-driver
-                     download-dir
-                     path-browser
-                     driver-log-level]}]]
-
-  (let [{:keys [port host]} driver
-
-        _ (when (util/connectable? host port)
-            (throw (ex-info
-                     (format "Port %d already in use" port)
-                     {:port port})))
+  [driver {:keys [webdriver-failed-launch-retries] :as opts}]
+  (let [max-tries (inc webdriver-failed-launch-retries)]
+    (loop [try-num 1
+           ex nil]
+      (if (> try-num max-tries)
+        (throw (ex-info (format "gave up trying to launch %s after %d tries" (:type driver) max-tries) {} ex))
+        (do
+          (when ex
+            (log/warnf ex "unexpected exception occurred launching %s, try %d (of a max of %d)"
+                       (:type driver) (dec try-num) max-tries)
+            (Thread/sleep 100))
+          (let [driver (-run-driver* driver opts)
+                res (try
+                      (wait-running driver)
+                      (catch Exception e
+                        {:exception e}))]
+            (if (not (:exception res))
+              driver
+              (recur (inc try-num) (:exception res)))))))))
 
-        driver    (cond-> driver
-                    true             (drv/set-browser-log-level log-level)
-                    true             (drv/set-path path-driver)
-                    true             (drv/set-port port)
-                    dev              (drv/set-perf-logging (:perf dev))
-                    driver-log-level (drv/set-driver-log-level driver-log-level)
-                    args-driver      (drv/set-args args-driver)
-                    path-browser     (drv/set-binary path-browser)
-                    download-dir     (drv/set-download-dir download-dir))
-        proc-args (drv/get-args driver)
-        _         (log/debugf "Starting process: %s" (str/join \space proc-args))
-        process   (proc/run proc-args {:log-stdout log-stdout
-                                       :log-stderr log-stderr
-                                       :env        env})]
-    (util/assoc-some driver :env env :process process)))
 
 (defn- -connect-driver
   "Connects to a running Webdriver server.

src/etaoin/impl/util.clj

@@ -1,5 +1,9 @@
 (ns ^:no-doc etaoin.impl.util
-  (:import java.io.File))
+  (:import
+   [java.io File IOException]
+   [java.net InetSocketAddress ServerSocket Socket]))
+
+(set! *warn-on-reflection* true)
 
 (defn map-or-nil?
   [x]
@@ -37,20 +41,19 @@
    (error (apply format tpl args))))
 
 (defn get-free-port []
-  (let [socket (java.net.ServerSocket. 0)]
+  (let [socket (ServerSocket. 0)]
     (.close socket)
     (.getLocalPort socket)))
 
 (defn connectable?
-  "Checks whether it's possible to connect a given host/port pair."
+  "Returns `true` when it is possible to connect a given `host` over `port`."
   [host port]
-  (when-let [^java.net.Socket socket
-             (try
-               (java.net.Socket. ^String host ^int port)
-               (catch java.io.IOException _))]
-    (when (.isConnected socket)
-      (.close socket)
-      true)))
+  (with-open [socket (Socket.)]
+    (try
+      (.connect socket (InetSocketAddress. ^String host ^int port) 1000)
+      true
+      (catch IOException _e
+        false))))
 
 (defn exit
   [code template & args]

test/etaoin/unit/unit_test.clj

@@ -8,7 +8,8 @@
    [etaoin.ide.impl.spec :as spec]
    [etaoin.impl.proc :as proc]
    [etaoin.impl.util :as util]
-   [etaoin.test-report]))
+   [etaoin.test-report])
+  (:import [clojure.lang ExceptionInfo]))
 
 (deftest test-driver-options
   (with-redefs
@@ -101,10 +102,52 @@
 
 (deftest test-fail-run-driver
   (is (thrown-with-msg?
-        clojure.lang.ExceptionInfo
+        ExceptionInfo
         #"wrong-driver-path"
         (e/chrome {:path-driver "wrong-driver-path"}))))
 
+(deftest test-retry-launch
+  (testing "give up after max tries"
+    (let [run-calls (atom 0)]
+      (with-redefs
+        [etaoin.impl.proc/run  (fn [_ _]
+                                 (swap! run-calls inc)
+                                 {:some :process})
+         e/running? (fn [_] (throw (ex-info "firefox badness" {})))]
+        (is (thrown-with-msg?
+              ExceptionInfo
+              #"gave up trying to launch :firefox after 8 tries"
+              (e/with-firefox {:webdriver-failed-launch-retries 7} driver
+                driver)))
+        (is (= 8 @run-calls)))))
+  (testing "succeed before max tries"
+    (let [run-calls (atom 0)
+          succeed-when-calls 3]
+      (with-redefs
+        [etaoin.impl.proc/run  (fn [_ _]
+                                 (swap! run-calls inc)
+                                 {:some :process})
+         e/create-session   (fn [_ _] "session-key")
+         proc/kill identity
+         e/delete-session   identity
+         e/running? (fn [_]
+                      (if (< @run-calls succeed-when-calls)
+                        (throw (ex-info "safari badness" {}))
+                        true))
+         util/get-free-port (constantly 12345)]
+        ;; safari driver has a default of 4 retries
+        (e/with-safari driver
+          (is (= {:args ["safaridriver" "--port" 12345]
+                  :capabilities {:loggingPrefs {:browser "ALL"}}
+                  :host "127.0.0.1"
+                  :locator "xpath"
+                  :port 12345
+                  :process {:some :process}
+                  :session "session-key"
+                  :type :safari,
+                  :url "http://127.0.0.1:12345"} driver)))
+        (is (= succeed-when-calls @run-calls))))))
+
 (deftest test-actions
   (let [keyboard        (-> (e/make-key-input)
                             (e/with-key-down "H")