Improve error handling for :paths-fn (nextjournal#302)

Catch exceptions thrown by requiring-resolve add more tests Co-authored-by: Martin Kavalar <martin@nextjournal.com>
witek · Nov 30, 2022 · 6d7e3a4 · 6d7e3a4
1 parent bdf7908
commit 6d7e3a4
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 16 deletions.
diff --git a/resources/viewer-js-hash b/resources/viewer-js-hash
@@ -1 +1 @@
-2wraHWJPXmjSxFQAWjXt8jr69Tqr
+tjFXyBi69ZzjvJw2a6D3L3G8zyW
diff --git a/src/nextjournal/clerk/builder.clj b/src/nextjournal/clerk/builder.clj
@@ -132,16 +132,21 @@
   (->> (cond paths (if (sequential? paths)
                      paths
                      (throw (ex-info "`:paths` must be sequential" {:paths paths})))
-             paths-fn (if (qualified-symbol? paths-fn)
-                        (try
-                          (if-let [resolved-var  (requiring-resolve paths-fn)]
-                            (let [resolved-paths (cond-> @resolved-var
-                                                   (fn? @resolved-var) (apply []))]
-                              (when-not (sequential? resolved-paths)
-                                (throw (ex-info (str "#'" paths-fn " must be sequential.") {:paths-fn paths-fn :resolved-paths resolved-paths})))
-                              resolved-paths)
-                            (throw (ex-info (str "#'" paths-fn " cannot be resolved.") {:paths-fn paths-fn}))))
-                        (throw (ex-info "`:path-fn` must be a qualified symbol pointing at an existing var." {:paths-fn paths-fn}))))
+             paths-fn (let [ex-msg "`:path-fn` must be a qualified symbol pointing at an existing var."]
+                        (when-not (qualified-symbol? paths-fn)
+                          (throw (ex-info ex-msg {:paths-fn paths-fn})))
+                        (if-let [resolved-var  (try (requiring-resolve paths-fn)
+                                                    (catch Exception e
+                                                      (throw (ex-info ex-msg {:paths-fn paths-fn}))))]
+                          (let [resolved-paths (try (cond-> @resolved-var
+                                                      (fn? @resolved-var) (apply []))
+                                                    (catch Exception e
+                                                      (throw (ex-info (str "An error occured invoking `" (pr-str resolved-var) "`: " (ex-message e))
+                                                                      {:paths-fn paths-fn} e))))]
+                            (when-not (sequential? resolved-paths)
+                              (throw (ex-info (str "`:paths-fn` must compute sequential value.") {:paths-fn paths-fn :resolved-paths resolved-paths})))
+                            resolved-paths)
+                          (throw (ex-info ex-msg {:paths-fn paths-fn})))))
        (maybe-add-index build-opts)
        (mapcat (partial fs/glob "."))
        (filter (complement fs/directory?))

diff --git a/test/nextjournal/clerk/builder_test.clj b/test/nextjournal/clerk/builder_test.clj
@@ -4,7 +4,8 @@
             [clojure.string :as str]
             [clojure.test :refer :all]
             [nextjournal.clerk.builder :as builder])
-  (:import (java.io File)))
+  (:import (clojure.lang ExceptionInfo)
+           (java.io File)))
 
 (deftest url-canonicalize
   (testing "canonicalization of file components into url components"
@@ -37,10 +38,24 @@
     (is (= ["book.clj"] (builder/expand-paths {:paths ["book.clj"]}))))
 
   (testing "invalid args"
-    (is (thrown? Exception (builder/expand-paths {})))
-    (is (thrown? Exception (builder/expand-paths {:paths-fn 'foo})))
-    (is (thrown? Exception (builder/expand-paths {:paths-fn "hi"})))
-    (is (thrown? Exception (builder/expand-paths {:index ["book.clj"]})))))
+    (let [msg #"must be a qualified symbol pointing at an existing var"]
+      (is (thrown-with-msg? ExceptionInfo #"must set either"
+                            (builder/expand-paths {})))
+      (is (thrown-with-msg? ExceptionInfo #"must be a qualified symbol pointing at an existing var"
+                            (builder/expand-paths {:paths-fn :foo})))
+      (is (thrown-with-msg? ExceptionInfo #"must be a qualified symbol pointing at an existing var"
+                            (builder/expand-paths {:paths-fn 'foo})))
+      (is (thrown-with-msg? ExceptionInfo #"must be a qualified symbol pointing at an existing var"
+                            (builder/expand-paths {:paths-fn 'foo/bar})))
+      (is (thrown-with-msg? ExceptionInfo #"must be a qualified symbol pointing at an existing var"
+                            (builder/expand-paths {:paths-fn 'clojure.core/non-existant-var})))
+      (is (thrown-with-msg? ExceptionInfo #"must be a qualified symbol pointing at an existing var"
+                            (builder/expand-paths {:paths-fn "hi"})))
+      (is (thrown-with-msg? ExceptionInfo #"An error occured invoking"
+                            (builder/expand-paths {:paths-fn 'clojure.core/inc})))
+      (is (thrown-with-msg? ExceptionInfo #"`:paths-fn` must compute sequential value"
+                            (builder/expand-paths {:paths-fn 'clojure.core/+})))
+      (is (thrown? ExceptionInfo (builder/expand-paths {:index ["book.clj"]}))))))
 
 (deftest process-build-opts
   (testing "assigns index when only one path is given"