From ea5501ffd38803df04ffc27f4294a8bd9a160389 Mon Sep 17 00:00:00 2001 From: "Howard M. Lewis Ship" Date: Tue, 11 Jun 2024 14:17:03 -0300 Subject: [PATCH] Non-success status codes should not always result in a thrown exception (#30) * Add a throw? flag, defaults to true, so that non-success status do not automatically throw an exception --- project.clj | 24 ++++++++--------- src/clj_github/httpkit_client.clj | 34 ++++++++++++++++++++----- test/clj_github/httpkit_client_test.clj | 13 +++++++--- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/project.clj b/project.clj index 43bc918..b409a26 100644 --- a/project.clj +++ b/project.clj @@ -9,29 +9,29 @@ :password :env/clojars_passwd :sign-releases false}]] - :plugins [[lein-cljfmt "0.6.4" :exclusions [org.clojure/clojure]] + :plugins [[lein-cljfmt "0.9.2" :exclusions [org.clojure/clojure]] [lein-nsorg "0.3.0" :exclusions [org.clojure/clojure]] - [lein-ancient "0.6.14" :exclusions [commons-logging com.fasterxml.jackson.core/jackson-databind com.fasterxml.jackson.core/jackson-core]]] + [lein-ancient "0.7.0" :exclusions [commons-logging com.fasterxml.jackson.core/jackson-databind com.fasterxml.jackson.core/jackson-core]]] - :dependencies [[org.clojure/clojure "1.10.3"] - [cheshire "5.10.1"] - [clj-commons/clj-yaml "0.7.107"] - [http-kit "2.5.3"] + :dependencies [[org.clojure/clojure "1.11.3"] + [cheshire "5.13.0"] + [clj-commons/clj-yaml "1.0.27"] + [http-kit "2.8.0"] [nubank/clj-github-app "0.2.1"] - [nubank/state-flow "5.14.0"] - [clj-commons/fs "1.6.310"] + [nubank/state-flow "5.17.0"] + [clj-commons/fs "1.6.311"] [ring/ring-codec "1.2.0"]] :cljfmt {:indents {flow [[:block 1]] assoc-some [[:block 0]]}} :profiles {:dev {:plugins [[lein-project-version "0.1.0"]] - :dependencies [[ch.qos.logback/logback-classic "1.3.0-alpha4" :exclusions [com.sun.mail/javax.mail]] + :dependencies [[ch.qos.logback/logback-classic "1.3.0" :exclusions [com.sun.mail/javax.mail]] [org.clojure/test.check "1.1.1"] - [nubank/matcher-combinators "3.3.1" :exclusions [mvxcvi/puget commons-codec]] - [tortue/spy "2.9.0"] + [nubank/matcher-combinators "3.9.1" :exclusions [mvxcvi/puget commons-codec]] + [tortue/spy "2.14.0"] [http-kit.fake "0.2.2"] - [metosin/reitit-core "0.5.15"] + [metosin/reitit-core "0.7.0"] [dev.nubank/clj-github-mock "0.2.0"]]}} :aliases {"coverage" ["cloverage" "-s" "coverage"] diff --git a/src/clj_github/httpkit_client.clj b/src/clj_github/httpkit_client.clj index 64de116..1d35367 100644 --- a/src/clj_github/httpkit_client.clj +++ b/src/clj_github/httpkit_client.clj @@ -29,13 +29,35 @@ (or (get-in response [:headers :content-type]) (get-in response [:headers "Content-Type"]))) -(defn request [client req-map] - (let [response @(httpkit/request (prepare client req-map))] - (if (success-codes (:status response)) - (update response :body (partial parse-body (content-type response))) +(defn request + "Sends a synchronous request to GitHub, largely as a wrapper around HTTP Kit. + + In addition to normal HTTP Kit keys in the request map, two additional keys are added. + + :path - used to create the :url key for HTTP kit; this is the path relative to https://api.github.com. + + :throw? - if true (the default), then non-success status codes (codes outside the range of 200 to 204) + result in a thrown exception. If set to false, then the response is returned, regardless and the + caller can decide what to do with failure statuses." + [client req-map] + (let [{:keys [throw?] + :or {throw? true}} req-map + {:keys [error status body opts] :as response} @(httpkit/request (prepare client req-map))] + (cond + error + (throw (ex-info "Failure sending request to GitHub" + {:opts opts} + error)) + + (and throw? + (not (success-codes status))) (throw (ex-info "Request to GitHub failed" - {:response (select-keys response [:status :body])} - (:error response)))))) + {:response (select-keys response [:status :body]) + :opts opts} + error)) + + :else + (assoc response :body (parse-body (content-type response) body))))) (defn new-client [{:keys [app-id private-key token org] :as opts}] (cond diff --git a/test/clj_github/httpkit_client_test.clj b/test/clj_github/httpkit_client_test.clj index 143b805..b998e60 100644 --- a/test/clj_github/httpkit_client_test.clj +++ b/test/clj_github/httpkit_client_test.clj @@ -31,11 +31,18 @@ "Content-Type" "application/json"}} {:status 200}] (is (match? {:status 200} (sut/request client {}))))) - (testing "it throws an exception when status code is not succesful" + (testing "it throws an exception when status code is not successful" (with-fake-http [{} {:status 400}] (is (thrown-with-msg? clojure.lang.ExceptionInfo #"(?i)Request to GitHub failed" (sut/request client {}))))) - ;; in case of lower level errors, e.g. DNS lookup failue there will not be a status code + (testing "it does not throw with a failure status if so enabled" + (with-fake-http [{:headers {"Authorization" "Bearer token" + "Content-Type" "application/json"}} + {:status 404}] + (is (match? + {:status 404} + (sut/request client {:throw? false}))))) + ;; in case of lower level errors, e.g. DNS lookup failure there will not be a status code (testing "it throws an exception when there is no status code" (with-fake-http [{} {:status nil}] (is (thrown-with-msg? clojure.lang.ExceptionInfo #"(?i)Request to GitHub failed" @@ -44,7 +51,7 @@ (let [cause (Exception. "Exception for testing purpose")] (with-fake-http [{} {:error cause :status nil}] (let [e (try (sut/request client {}) (catch Exception e e))] - (is (re-matches #"(?i)Request to GitHub failed" (.getMessage e))) + (is (re-matches #"(?i)Failure sending request to GitHub" (.getMessage e))) (is (= cause (.getCause e))))))) (testing "url path contains special character `|`" (with-fake-http [{:url "https://api.github.com/test%7Ctest"}