From 1ea781e03fe4dc95a153dee68dfb9f97d4407596 Mon Sep 17 00:00:00 2001
From: Dave Roberts <dave@droberts.com>
Date: Mon, 16 Sep 2024 08:53:39 -0500
Subject: [PATCH] Replace etaoin.impl.util/error with Slingshot throw+ (#667)

---
 CHANGELOG.adoc           |  1 +
 src/etaoin/api.clj       | 16 +++++++++++-----
 src/etaoin/impl/util.clj |  6 ------
 src/etaoin/query.clj     |  8 +++++---
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 391ffb2..2b3f0b4 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -32,6 +32,7 @@ A release with an intentional breaking changes is marked with:
 ** {issue}657[#657]: Make `set-<xyz>-timeout` functions resilient to reasonable non-integer timeouts (e.g., rationals, doubles, etc.). ({person}dgr[@dgr])
 ** {issue}661[#661]: Fix `:fn/enabled`. ({person}dgr[@dgr])
 ** {issue}663[#663]: `query` throws a more accurate exception with a more accurate error message when provided with an empty query vector. ({person}dgr[@dgr])
+** {issue}666[#666]: Previously, in some error conditions, Etaoin would throw a very generic `clojure.lang.Exception` object. Some of those cases have been replaced by throwing a map with Slingshot, providing more information about the problem. ({person}dgr[@dgr])
 * Docs
 ** {issue}656[#656]: Correctly describe behavior when query's parameter is a string. The User Guide and `query` doc strings say that a string passed to `query` is interpreted as an XPath expression. In fact, `query` interprets this as either XPath or CSS depending on the setting of the driver's `:locator` parameter, which can be changed. ({person}dgr[@dgr])
 * Quality
diff --git a/src/etaoin/api.clj b/src/etaoin/api.clj
index 16af594..8daa0bc 100644
--- a/src/etaoin/api.clj
+++ b/src/etaoin/api.clj
@@ -3059,7 +3059,8 @@
                          :path   [:session (:session driver) :screenshot]})
         b64str (-> resp :value not-empty)]
     (when (not b64str)
-      (util/error "Empty screenshot"))
+      (throw+ {:type :etaoin/failed
+               :message "Empty screenshot"}))
     (create-dirs-for-file file)
     (b64-to-file b64str file)))
 
@@ -3078,7 +3079,9 @@
                          :path   [:session (:session driver) :element el :screenshot]})
         b64str (-> resp :value not-empty)]
     (when (not b64str)
-      (util/error "Empty screenshot, query: %s" q))
+      (throw+ {:type :etaoin/failed
+               :message "Empty screenshot"
+               :q q}))
     (create-dirs-for-file file)
     (b64-to-file b64str file)))
 
@@ -3129,8 +3132,10 @@
 
 (defmethod print-page
   :default ;; last checked safari 2024-08-08
-  [_driver _file & [_opts]]
-  (util/error "This driver doesn't support printing pages to PDF."))
+  [driver _file & [_opts]]
+  (throw+ {:type :etaoin/unsupported
+           :message "This driver doesn't support printing pages to PDF."
+           :driver driver}))
 
 (defmethods print-page
   [:chrome :edge :firefox]
@@ -3141,7 +3146,8 @@
                          :data opts})
         b64str (-> resp :value not-empty)]
     (when (not b64str)
-      (util/error "Empty page"))
+      (throw+ {:type :etaoin/failed
+               :message "Empty page"}))
     (create-dirs-for-file file)
     (b64-to-file b64str file)))
 
diff --git a/src/etaoin/impl/util.clj b/src/etaoin/impl/util.clj
index 6033825..b88b3c3 100644
--- a/src/etaoin/impl/util.clj
+++ b/src/etaoin/impl/util.clj
@@ -35,12 +35,6 @@
   [& args]
   (mapv class args))
 
-(defn error
-  ([msg]
-   (throw (Exception. ^String msg)))
-  ([tpl & args]
-   (error (apply format tpl args))))
-
 (defn get-free-port []
   (let [socket (ServerSocket. 0)]
     (.close socket)
diff --git a/src/etaoin/query.clj b/src/etaoin/query.clj
index 58a0b52..f4a4502 100644
--- a/src/etaoin/query.clj
+++ b/src/etaoin/query.clj
@@ -5,8 +5,8 @@
   Why do folks need to use this directly?
   Maybe they are extending defmulti with more conversions?"
   (:require
-   [etaoin.impl.util :as util]
-   [etaoin.impl.xpath :as xpath]))
+   [etaoin.impl.xpath :as xpath]
+   [slingshot.slingshot :refer [throw+]]))
 
 (set! *warn-on-reflection* true)
 
@@ -51,7 +51,9 @@
 
 (defmethod to-query :default
   [_driver q]
-  (util/error "Wrong query: %s" q))
+  (throw+ {:type :etaoin/argument
+           :message "Unsupported query argument type"
+           :q q}))
 
 (defn expand
   "Return expanded query `q` for `driver`."