Improve error messages when op conversion fails

- Wrap errors during operation processing in an
  ex-info. The message contains the operation name
  and the ex-data contains the source operation.
- Throw ex-infos with a "missing schema" message
  instead of NPEs when a necessary schema is null.

This turns errors that previously looked like:
`java.lang.NullPointerException: Cannot invoke "io.swagger.v3.oas.models.media.Schema.getTypes()" because "schema" is null`
into this:
`clojure.lang.ExceptionInfo: Exception processing operation "TestOp": Missing schema`

Author: john-shaffer

src/navi/core.clj

@@ -41,17 +41,19 @@
                m)))
 
 (defn schema->spec [^Schema schema]
-  (let [types (.getTypes schema)]
-    (if (= 1 (count types))
-      (spec schema)
-      (try
-        (->> (map (fn [type]
-                    (.setTypes schema #{type})
-                    (spec schema))
-                  types)
-             (into [:or]))
-        (finally
-          (.setTypes schema types))))))
+  (if schema
+    (let [types (.getTypes schema)]
+      (if (= 1 (count types))
+        (spec schema)
+        (try
+          (->> (map (fn [type]
+                      (.setTypes schema #{type})
+                      (spec schema))
+                    types)
+               (into [:or]))
+          (finally
+            (.setTypes schema types)))))
+    (throw (ex-info "Missing schema" {}))))
 
 ;; TODO: Better
 (defn ->prop-schema
@@ -192,7 +194,9 @@
 (defn media-type->data
   "Convert a Java Schema's MediaType to a spec that Reitit will accept."
   [^MediaType mt]
-  {:schema (spec (.getSchema mt))})
+  (if-let [schema (some-> mt .getSchema spec)]
+    {:schema schema}
+    (throw (ex-info "MediaType has no schema" {:media-type mt}))))
 
 (defn handle-media-type-key
   "If the media type is \"default\", then return it as a keyword, otherwise pass through."
@@ -220,22 +224,29 @@
   "Converts a Java Operation to a map of parameters, responses, schemas and handler
   that conforms to reitit."
   [^Operation op handlers]
-  (let [params (into [] (.getParameters op))
-        request-body (.getRequestBody op)
-        params (if (nil? request-body)
-                 params
-                 (conj params request-body))
-        schemas (->> params
-                     (map param->data)
-                     (apply merge-with into)
-                     (wrap-map :path)
-                     (wrap-map :query)
-                     (wrap-map :header))
-        responses (-> (.getResponses op)
-                      (update-kvs handle-response-key response->data))]
-    (cond-> {:handler (get handlers (.getOperationId op))}
-      (seq schemas) (assoc :parameters schemas)
-      (seq responses) (assoc :responses responses))))
+  (try
+    (let [params (into [] (.getParameters op))
+          request-body (.getRequestBody op)
+          params (if (nil? request-body)
+                   params
+                   (conj params request-body))
+          schemas (->> params
+                       (map param->data)
+                       (apply merge-with into)
+                       (wrap-map :path)
+                       (wrap-map :query)
+                       (wrap-map :header))
+          responses (-> (.getResponses op)
+                        (update-kvs handle-response-key response->data))]
+      (cond-> {:handler (get handlers (.getOperationId op))}
+        (seq schemas) (assoc :parameters schemas)
+        (seq responses) (assoc :responses responses)))
+    (catch Exception e
+      (throw (ex-info (str "Exception processing operation "
+                           (pr-str (.getOperationId op))
+                           ": "(ex-message e))
+                      {:operation op}
+                      e)))))
 
 (defn path-item->data
   "Converts a path to its corresponding vector of method and the operation map"

test/navi/core_test.clj

@@ -7,7 +7,8 @@
 (ns navi.core-test
   (:require [clojure.test :refer [deftest testing is]]
             [navi.core :as core])
-  (:import [io.swagger.v3.oas.models Operation PathItem]
+  (:import [clojure.lang ExceptionInfo]
+           [io.swagger.v3.oas.models Operation PathItem]
            [io.swagger.v3.oas.models.media Content StringSchema IntegerSchema JsonSchema
             NumberSchema ObjectSchema ArraySchema MediaType UUIDSchema Schema]
            [io.swagger.v3.oas.models.parameters Parameter PathParameter HeaderParameter QueryParameter RequestBody]
@@ -228,6 +229,30 @@
               :responses {200 {:content {"application/json" {:schema [:map {:closed false}]}}}}}
              (core/operation->data operation handlers))))))
 
+(deftest openapi-operation-to-malli-spec-missing-schema
+  (testing "Missing response schema results in an informative error"
+    (let [param (doto (PathParameter.)
+                  (.setName "x")
+                  (.setSchema (IntegerSchema.)))
+          hparam (doto (HeaderParameter.)
+                   (.setName "y")
+                   (.setSchema (StringSchema.)))
+          response (doto (ApiResponse.)
+                     (.setContent (doto (Content.)
+                                    (.put "application/json" (MediaType.)))))
+          responses (doto (ApiResponses.)
+                      (.put "200" response))
+          operation (doto (Operation.)
+                      (.setParameters [param hparam])
+                      (.setResponses responses)
+                      (.setOperationId "TestOp"))
+          handlers {"TestOp" "a handler"}]
+      (is (thrown-with-msg?
+           ExceptionInfo
+           #".*TestOp.*schema"
+           (core/operation->data operation handlers))
+          "Error message contains operation name and mentions the missing schema"))))
+
 (deftest openapi-path-to-malli-spec
   (testing "OpenAPI path to reitit route"
     (let [param (doto (PathParameter.)
@@ -242,3 +267,19 @@
       (is (= {:get {:handler "a handler"
                     :parameters {:path [:map [:x int?]]}}}
              (core/path-item->data path-item handlers))))))
+
+(deftest openapi-path-to-malli-spec-missing-schema
+  (testing "Missing path parameter schema results in an informative error"
+    (let [param (doto (PathParameter.)
+                  (.setName "x"))
+          operation (doto (Operation.)
+                      (.setParameters [param])
+                      (.setOperationId "TestOp"))
+          handlers {"TestOp" "a handler"}
+          path-item (doto (PathItem.)
+                      (.setGet operation))]
+      (is (thrown-with-msg?
+           ExceptionInfo
+           #".*TestOp.*schema"
+           (core/path-item->data path-item handlers))
+          "Error message contains operation name and mentions the missing schema"))))