Skip to content

Commit

Permalink
Include driver process liveness in some exceptions
Browse files Browse the repository at this point in the history 
When an http request, for example, times out one might wonder if the
WebDriver process is still running.

We now use slingshot throw+ as we do for http status errors. This throw+
still includes the :driver :process but checks if it is alive and
realizes its exit value in the thrown map if dead.

Also: if an exception occurs when booting a driver, any launched WebDriver
process is now cleaned up.

Also: reworked some proc tests to always attempt cleanup even when
exceptions occur.

Closes clj-commons#469
  • Loading branch information
@lread
lread committed Jul 4, 2022
1 parent b6c3afd commit 5b63645
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 76 deletions.
41 changes: 24 additions & 17 deletions src/etaoin/api.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3621,12 +3621,23 @@
(proc/kill (:process driver))
(dissoc driver :process :args :env :capabilities))

(defn quit
"Have `driver` close the current session, then, if Etaoin launched it, kill the WebDriver process."
[driver]
(let [process (:process driver)]
(try
(disconnect-driver driver)
(finally
(when process
(stop-driver driver))))))

(defn boot-driver
"Launch and return a driver of `type` (e.g. `:chrome`, `:firefox` `:safari` `:edge` `:phantom`)
with `opts` options.
- creates a driver
- launches a WebDriver process (or connects to an existing running process if `:host` is specified)
- launches a WebDriver process (or connects to an existing running process if `:host`
or `:webdriver-url` is specified)
- creates a session for driver
Defaults taken from [[defaults-global]] then [[defaults]] for `type`:
Expand All @@ -3641,22 +3652,18 @@
(dissoc (type defaults) :capabilities))
;; if host, we are launching webdriver, default port is random
(not host) (assoc :port (util/get-free-port)))
opts (merge default-opts opts)]
(cond-> type
:always (-create-driver opts)
(and (not host)
(not webdriver-url)) (-run-driver opts)
:always (-connect-driver opts)))))

(defn quit
"Have `driver` close the current session, then, if Etaoin launched it, kill the WebDriver process."
[driver]
(let [process (:process driver)]
(try
(disconnect-driver driver)
(finally
(when process
(stop-driver driver))))))
opts (merge default-opts opts)
driver (cond-> (-create-driver type opts)
(and (not host) (not webdriver-url)) (-run-driver opts))]
(try
(-connect-driver driver opts)
(catch Throwable ex
(when (:process driver)
(try
(quit driver)
;; silently ignore failure to quit driver on cleanup
(catch Throwable _ex)))
(throw ex))))))

(def ^{:arglists '([] [opts])} firefox
"Launch and return a Firefox driver.
Expand Down
58 changes: 41 additions & 17 deletions src/etaoin/impl/client.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
[cheshire.core :as json]
[clojure.string :as str]
[clojure.tools.logging :as log]
[etaoin.impl.proc :as proc]
[etaoin.impl.util :as util]
#?(:bb [clj-http.lite.client :as client]
:clj [clj-http.client :as client])
Expand All @@ -13,9 +14,9 @@
;;

(def default-timeout
"HTTP timeout in seconds. The current value may seems to high,
"HTTP timeout in seconds. The current value may seem high,
but according to my experience with SPA application full of React
modules even 20 seconds could not be enough for a driver to process
modules even 20 seconds can insufficient time for a driver to process
your request."
60)

Expand Down Expand Up @@ -70,6 +71,23 @@
(parse-json body)
body))

(defn- realized-driver
"Realize process liveness (or actually deadness, if dead)"
[{:keys [process] :as driver}]
(try
(if (and process (not (proc/alive? process)))
(assoc driver :process (proc/result process))
driver)
(catch Throwable ex
;; if, by chance, something goes wrong while trying to realize process liveness
(assoc driver :process-liveness-ex ex))))


(defn http-request
"an isolated http-request to support mocking"
[params]
(client/request params))

;;
;; client
;;
Expand Down Expand Up @@ -98,25 +116,31 @@
(-> method name str/upper-case)
path
(-> payload (or "")))
resp (client/request params)
body #?(:bb (-> resp :body parse-json)
:clj (:body resp))
error (delay {:type :etaoin/http-error
:status (:status resp)
:driver driver
:response (error-response body)
error (delay {:type :etaoin/http-ex
:driver (realized-driver driver)
:webdriver-url webdriver-url
:host host
:port port
:method method
:path path
:payload payload})]
(cond
(-> resp :status (not= 200))
(throw+ @error)
:payload payload})
resp (try (http-request params)
(catch Throwable ex
{:exception ex}))]
(if (:exception resp)
(throw+ @error (:exception resp))
(let [body #?(:bb (some-> resp :body parse-json)
:clj (:body resp))
error (delay (assoc @error
:type :etaoin/http-error
:status (:status resp)
:response (error-response body)))]
(cond
(-> resp :status (not= 200))
(throw+ @error)

(-> body :status (or 0) (> 0))
(throw+ @error)
(-> body :status (or 0) (> 0))
(throw+ @error)

:else
body)))
:else
body)))))
188 changes: 146 additions & 42 deletions test/etaoin/unit/proc_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,73 +5,177 @@
[clojure.string :as str]
[clojure.test :refer [deftest is]]
[etaoin.api :as e]
[etaoin.impl.client :as client]
[etaoin.impl.proc :as proc]
[etaoin.test-report]))

(defn get-count-chromedriver-instances
[]
(defn get-count-driver-instances
[drivername]
(if proc/windows?
(let [instance-report (-> (shell/sh "powershell" "-command" "(Get-Process chromedriver -ErrorAction SilentlyContinue).Path")
(let [instance-report (-> (shell/sh "powershell" "-command" (format "(Get-Process %s -ErrorAction SilentlyContinue).Path" drivername))
:out
str/split-lines)]
;; more flakiness diagnosis
(println "windows chromedriver instance report:" instance-report)
(println "windows" drivername "instance report:" instance-report)
(println "windows full list of running processes:")
;; use Get-CimInstance, because Get-Process, does not have commandline available
(pprint/pprint (-> (shell/sh "powershell" "-command" "Get-CimInstance Win32_Process | select name, commandline")
:out
str/split-lines))
(->> instance-report
(remove #(str/includes? % "\\scoop\\shims\\")) ;; for the scoop users, exclude the shim process
(filter #(str/includes? % "chromedriver"))
(filter #(str/includes? % drivername))
count))
(->> (shell/sh "sh" "-c" "ps aux")
:out
str/split-lines
(filter #(str/includes? % "chromedriver"))
(filter #(str/includes? % drivername))
count)))

(deftest test-process-forking-port-specified
(defn get-count-chromedriver-instances []
(get-count-driver-instances "chromedriver"))

(defn get-count-firefoxdriver-instances []
(get-count-driver-instances "geckodriver"))

(deftest test-process-forking-port-specified-is-in-use
(let [port 9997
process (proc/run ["chromedriver" (format "--port=%d" port)])
_ (e/wait-running {:port port :host "localhost"})]
(is (= 1 (get-count-chromedriver-instances)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"already in use"
(e/chrome {:port port})))
(proc/kill process)))
process (proc/run ["chromedriver" (format "--port=%d" port)])]
(try
(e/wait-running {:port port :host "localhost"})
(is (= 1 (get-count-chromedriver-instances)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"already in use"
(e/chrome {:port port})))
(finally
(proc/kill process)))))

(deftest test-process-forking-port-random
(deftest test-process-forking-port-not-specified-so-random-port-is-picked
(let [port 9998
process (proc/run ["chromedriver" (format "--port=%d" port)])
_ (e/wait-running {:port port :host "localhost"})]
(e/with-chrome {:args ["--no-sandbox"]} driver
;; added to diagnose flakyness on windows on CI
(println "automatically chosen port->" (:port driver))
;; added to diagnose flakyness on windows on CI
(e/wait-running driver)
(is (= 2 (get-count-chromedriver-instances))))
(proc/kill process)))
process (proc/run ["chromedriver" (format "--port=%d" port)])]
(try
(e/wait-running {:port port :host "localhost"})
(e/with-chrome {:args ["--no-sandbox"]} driver
;; added to diagnose flakyness on windows on CI
(println "automatically chosen port->" (:port driver))
;; added to diagnose flakyness on windows on CI
(e/wait-running driver)
(is (= 2 (get-count-chromedriver-instances))))
(finally
(proc/kill process)))))

(deftest test-process-forking-connect-existing-host
(deftest test-process-forking-connect-existing-webdriver-host
(let [port 9999
process (proc/run ["chromedriver" (format "--port=%d" port)])
_ (e/wait-running {:port port :host "localhost"})
;; should connect, not launch
driver (e/chrome {:host "localhost" :port port :args ["--no-sandbox"]})]
(is (= 1 (get-count-chromedriver-instances)))
(e/quit driver)
(proc/kill process)))
process (proc/run ["chromedriver" (format "--port=%d" port)])]
(try
(e/wait-running {:port port :host "localhost"})
;; should connect, not launch
(let [driver (e/chrome {:host "localhost" :port port :args ["--no-sandbox"]})]
(is (= 1 (get-count-chromedriver-instances)))
(e/quit driver))
(finally
(proc/kill process)))))

(deftest test-process-forking-connect-existing-webdriver-url
(let [port 9999
process (proc/run ["chromedriver" (format "--port=%d" port)])
;; normally would not call wait-running for a remote service, we are simulating here and want
;; to make sure the process we launched is up and running
_ (e/wait-running {:port port :host "localhost"})
;; should connect, not launch
driver (e/chrome {:webdriver-url (format "http://localhost:%d" port) :args ["--no-sandbox"]})]
(is (= 1 (get-count-chromedriver-instances)))
(e/quit driver)
(proc/kill process)))
process (proc/run ["chromedriver" (format "--port=%d" port)])]
(try
(e/wait-running {:port port :host "localhost"})
(let [;; should connect, not launch
driver (e/chrome {:webdriver-url (format "http://localhost:%d" port) :args ["--no-sandbox"]})]


(is (= 1 (get-count-chromedriver-instances)))
(e/quit driver))
(finally
(proc/kill process)))))

(deftest http-exception-on-create-proc-alive
;; when etaoin tries to create a session we simulate a read timeout
(with-redefs [client/http-request (fn [_] (throw (ex-info "read timeout" {})))]
(let [ex (try
(e/with-firefox _driver
(is false "should not reach here"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-ex (:type exd)))
(is (= nil (-> exd :driver :process :exit)))
(is (= 0 (get-count-firefoxdriver-instances))))))

(deftest http-error-on-create-proc-alive
;; when etaoin tries to create a session return an http error
(with-redefs [client/http-request (fn [_] {:status 400})]
(let [ex (try
(e/with-firefox _driver
(is false "should not reach here"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-error (:type exd)))
(is (= 400 (:status exd)))
(is (= nil (-> exd :driver :process :exit)))
(is (= 0 (get-count-firefoxdriver-instances))))))

(deftest http-exception-after-create-proc-now-dead
(let [orig-http-request client/http-request]
(with-redefs [client/http-request (fn [{:keys [method url] :as params}]
;; allow create session through, fail on everything else
(if (and (= :post method) (str/ends-with? url "/session"))
(orig-http-request params)
(throw (ex-info "read timeout" {}))))]
(let [ex (try
;; go headless to avoid that dangling open web browser
(e/with-firefox-headless driver
(is (= 1 (get-count-firefoxdriver-instances)))
(proc/kill (:process driver))
;; we'll now fail on this call
(e/go driver "https://clojure.org"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-ex (:type exd)))
(is (= 143 (-> exd :driver :process :exit)))
(is (= 0 (get-count-firefoxdriver-instances)))))))

(deftest http-error-after-create-proc-now-dead
;; unlikely, we know we just talked to the driver because it returned an http error, but for completeness
(let [orig-http-request client/http-request]
(with-redefs [client/http-request (fn [{:keys [method url] :as params}]
;; allow create session through, fail on everything else
(if (and (= :post method) (str/ends-with? url "/session"))
(orig-http-request params)
{:status 418}))]
(let [ex (try
;; go headless to avoid that dangling open web browser
(e/with-firefox-headless driver
(is (= 1 (get-count-firefoxdriver-instances)))
(proc/kill (:process driver))
;; we'll now fail on this call
(e/go driver "https://clojure.org"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-error (:type exd)))
(is (= 418 (:status exd)))
(is (= 143 (-> exd :driver :process :exit)))
(is (= 0 (get-count-firefoxdriver-instances)))))))

(deftest test-cleanup-connect-existing-on-create-error
(let [port 9999
process (proc/run ["chromedriver" (format "--port=%d" port)])]
(try
(e/wait-running {:port port :host "localhost"})
(with-redefs [client/http-request (fn [_] (throw (ex-info "read timeout" {})))]
;; attempt connect, not launch
(let [ex (try
(e/with-chrome {:webdriver-url (format "http://localhost:%d" port) :args ["--no-sandbox"]} driver
(is false "should not get here"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-ex (:type exd)))
(is (= 1 (get-count-chromedriver-instances)))))
(finally
(proc/kill process)))))

0 comments on commit 5b63645

Please sign in to comment.