rejigger http error response parsing and handling, resolves #218 (#11)

rejigger http error response parsing and handling, resolves #11

src/cognitect/aws/client/impl.clj

@@ -22,11 +22,13 @@
 
 ;; TODO convey throwable back from impl
 (defn ^:private handle-http-response
-  [service op-map http-response]
+  [service op-map {:keys [status] :as http-response}]
   (try
     (if (:cognitect.anomalies/category http-response)
       http-response
-      (aws.protocols/parse-http-response service op-map http-response))
+      (if (< status 400)
+        (aws.protocols/parse-http-response service op-map http-response)
+        (aws.protocols/parse-http-error-response http-response)))
     (catch Throwable t
       {:cognitect.anomalies/category :cognitect.anomalies/fault
        ::throwable t})))

src/cognitect/aws/protocols.clj

@@ -44,15 +44,21 @@
       (contains? #{"query" "ec2"} protocol)
       (assoc "content-type" "application/x-www-form-urlencoded; charset=utf-8"))))
 
-(defn parse-error*
-  [{:keys [status] :as http-response} response-body]
-  (with-meta (assoc response-body :cognitect.anomalies/category (status-code->anomaly status))
-    http-response))
-
-(defn xml-parse-error
-  [{:keys [body] :as http-response}]
-  (parse-error* http-response (some-> body util/bbuf->str util/xml-read util/xml->map)))
+(defn- parse-encoded-string*
+  "Given non-nil String, determine the encoding (currently either XML or JSON). Return a Map
+  representation of the encoded data."
+  [encoded-str]
+  (if (= \< (first encoded-str))
+    (-> encoded-str util/xml-read util/xml->map)
+    (-> encoded-str (json/read-str :key-fn keyword))))
 
-(defn json-parse-error
-  [{:keys [body] :as http-response}]
-  (parse-error* http-response (some-> body util/bbuf->str (json/read-str :key-fn keyword))))
+(defn parse-http-error-response
+  "Given an http error response (any status code 400 or above), return an aws-api-specific response
+  Map."
+  [{:keys [status body] :as http-response}]
+  (with-meta
+    (assoc
+     (some-> body util/bbuf->str parse-encoded-string*)
+     :cognitect.anomalies/category
+     (status-code->anomaly status))
+    http-response))

src/cognitect/aws/protocols/json.clj

@@ -34,14 +34,10 @@
      :body           (serialize input-shape (or request {}))}))
 
 (defmethod aws.protocols/parse-http-response "json"
-  [service {:keys [op]} {:keys [status body] :as http-response}]
-  (if (:cognitect.anomalies/category http-response)
-    http-response
-    (let [operation    (get-in service [:operations op])
-          output-shape (service/shape service (:output operation))
-          body-str     (util/bbuf->str body)]
-      (if (< status 400)
-        (if output-shape
-          (shape/json-parse output-shape body-str)
-          {})
-        (aws.protocols/json-parse-error http-response)))))
+  [service {:keys [op]} {:keys [body]}]
+  (let [operation (get-in service [:operations op])
+        output-shape (service/shape service (:output operation))
+        body-str (util/bbuf->str body)]
+    (if output-shape
+      (shape/json-parse output-shape body-str)
+      {})))

src/cognitect/aws/protocols/query.clj

@@ -102,15 +102,11 @@
   (build-query-http-request serialize service req-map))
 
 (defn build-query-http-response
-  [service {:keys [op]} {:keys [status body] :as http-response}]
+  [service {:keys [op]} {:keys [body]}]
   (let [operation (get-in service [:operations op])]
-    (if (:cognitect.anomalies/category http-response)
-      http-response
-      (if (< status 400)
-        (if-let [output-shape (service/shape service (:output operation))]
-          (shape/xml-parse output-shape (util/bbuf->str body))
-          (util/xml->map (util/xml-read (util/bbuf->str body))))
-        (aws.protocols/xml-parse-error http-response)))))
+    (if-let [output-shape (service/shape service (:output operation))]
+      (shape/xml-parse output-shape (util/bbuf->str body))
+      (util/xml->map (util/xml-read (util/bbuf->str body))))))
 
 (defmethod aws.protocols/parse-http-response "query"
   [service op-map http-response]

src/cognitect/aws/protocols/rest.clj

@@ -248,15 +248,10 @@
         (parse-fn output-shape body-str)))))
 
 (defn parse-http-response
-  [service {:keys [op]} {:keys [status body] :as http-response}
-   parse-body-str
-   parse-error]
-  (if (:cognitect.anomalies/category http-response)
-    http-response
-    (let [operation    (get-in service [:operations op])
-          output-shape (service/shape service (:output operation))]
-      (if (< status 400)
-        (merge (parse-non-payload-attrs output-shape http-response)
-               (when output-shape
-                 (parse-body output-shape body parse-body-str)))
-        (parse-error http-response)))))
+  [service {:keys [op]} {:keys [body] :as http-response}
+   parse-body-str]
+  (let [operation (get-in service [:operations op])
+        output-shape (service/shape service (:output operation))]
+    (merge (parse-non-payload-attrs output-shape http-response)
+           (when output-shape
+             (parse-body output-shape body parse-body-str)))))

src/cognitect/aws/protocols/rest_json.clj

@@ -41,5 +41,4 @@
   (rest/parse-http-response service
                             op-map
                             http-response
-                            shape/json-parse
-                            aws.protocols/json-parse-error))
+                            shape/json-parse))

src/cognitect/aws/protocols/rest_xml.clj

@@ -26,5 +26,4 @@
   (rest/parse-http-response service
                             op-map
                             http-response
-                            shape/xml-parse
-                            aws.protocols/xml-parse-error))
+                            shape/xml-parse))

test/src/cognitect/aws/protocols_test.clj

@@ -17,7 +17,8 @@
             [cognitect.aws.protocols.rest-json]
             [cognitect.aws.protocols.rest-xml]
             [cognitect.aws.util :as util])
-  (:import (java.util Date)))
+  (:import (java.nio ByteBuffer)
+           (java.util Date)))
 
 (s/fdef cognitect.aws.util/query-string
   :args (s/cat :params (s/or :seq (s/coll-of (s/cat :key (s/or :kw simple-keyword? :str string?)
@@ -434,5 +435,33 @@
                       "rest-json"]]
       (test-protocol protocol))))
 
+(deftest test-parse-http-error-response
+  (testing "parse JSON-encoded error response body"
+    (let [response {:status 401
+                    :body   (ByteBuffer/wrap (.getBytes "{\"FOO\": \"abc\"}" "UTF-8"))}
+          parsed-response (aws.protocols/parse-http-error-response response)]
+      (is (= {:FOO "abc" :cognitect.anomalies/category :cognitect.anomalies/incorrect}
+             parsed-response))
+      (testing "http response is included as metadata on returned parsed error response"
+        (is (= response (meta parsed-response))))))
+  (testing "parse XML-encoded response body - issue 218: AWS returns XML-encoded 404 response when JSON-encoding was expected"
+    (let [response {:status 404
+                    :body   (ByteBuffer/wrap (.getBytes "<UnknownOperationException/>" "UTF-8"))}
+          parsed-response (aws.protocols/parse-http-error-response response)]
+      (is (= {:UnknownOperationException nil
+              :UnknownOperationExceptionAttrs {}
+              :cognitect.anomalies/category :cognitect.anomalies/not-found}
+             parsed-response))
+      (testing "http response is included as metadata on returned parsed error response"
+        (is (= response (meta parsed-response))))))
+  (testing "parse response with empty body"
+    (let [response {:status 404
+                    :body   nil}
+          parsed-response (aws.protocols/parse-http-error-response response)]
+      (is (= {:cognitect.anomalies/category :cognitect.anomalies/not-found}
+             parsed-response))
+      (testing "http response is included as metadata on returned parsed error response"
+        (is (= response (meta parsed-response)))))))
+
 (comment
   (t/run-tests))