Fix HTTP client response body coercion for error status responses

When using the HTTP client's response body coercion middleware (via the `:as` request parameter) in
combination with `:throw-exceptions? true` (the default), one would run into an error when the
coercion result is not a manifold stream or byte array. Specifically, the commonly used `:as
:string` would result in such an error because the error status handler would assume a manifold
stream as the default case.

This patch fixes this by turning manifold streams into a special case and passing all other response
body types through untouched. A basic dedicated test case for the response body coercion middleware
is included (there was none so far) while the offending interaction with `:throw-exceptions? true`
is covered by `test-client-throw-error-responses-as-exceptions`.

Author: DerGuteMoritz

src/aleph/http/client_middleware.clj

@@ -278,15 +278,15 @@
                   (error-status-deferred
                    (assoc rsp :body (ByteArrayInputStream. body)))))
 
-      (nil? body)
-      (error-status-deferred rsp)
-
-      :else
+      (s/stream? body)
       (d/chain'
        (s/reduce conj [] body)
        (fn [body]
          (error-status-deferred
-          (assoc rsp :body (s/->source body))))))))
+          (assoc rsp :body (s/->source body)))))
+
+      :else
+      (error-status-deferred rsp))))
 
 (defn wrap-method
   "Middleware converting the :method option into the :request-method option"

test/aleph/http/client_middleware_test.clj

@@ -53,6 +53,14 @@
                                          :form-params {:foo :bar}})
         (middleware/coerce-form-params {:form-params {:foo :bar}}))))
 
+(deftest test-coerce-response-body
+  (is (= "foo" (:body (middleware/coerce-response-body
+                       {:as :string}
+                       {:body (s/->source ["f" "o" "o"])}))))
+  (is (= {:foo "bar"} (:body (middleware/coerce-response-body
+                              {:as :clojure}
+                              {:body "{:foo \"bar\"}"})))))
+
 (deftest test-nested-query-params
   (let [req {:request-method :get
              :query-params {:foo {:bar "baz"}}}
@@ -246,6 +254,16 @@
           (is (instance? ExceptionInfo e))
           (is (= 400 (:status (ex-data e))))
           (is (nil? (-> e ex-data :body))))))
+    (testing "response body is string (e.g. via `:as :string`)"
+      (try
+        (let [rsp (handle-error-status {:throw-exceptions? true}
+                                       {:status 400
+                                        :body "hello"})]
+          (is (= ::thrown? rsp) "should have thrown"))
+        (catch Exception e
+          (is (instance? ExceptionInfo e))
+          (is (= 400 (:status (ex-data e))))
+          (is (= "hello" (-> e ex-data :body))))))
     (testing "error status but :throw-exceptions is false"
       (let [rsp (handle-error-status {:throw-exceptions? false}
                                      {:status 400})]

test/aleph/http_test.clj

@@ -1224,12 +1224,13 @@
                        {:status 429
                         :body "nope"}) {}
     (try
-      (-> (http-get "/" {:throw-exceptions? true})
+      (-> (http-get "/" {:throw-exceptions? true :as :string})
           (d/timeout! 1e3)
           deref)
       (is (= false true) "should have thrown an exception")
       (catch Exception e
-        (is (= 429 (:status (ex-data e))))))))
+        (is (= 429 (:status (ex-data e))))
+        (is (= "nope" (:body (ex-data e))))))))
 
 (deftest test-client-errors-handling
   (testing "writing invalid request message"