Skip to content

Commit

Permalink
Improve error messages when op conversion fails
Browse files Browse the repository at this point in the history
- 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`
  • Loading branch information
john-shaffer authored and lispyclouds committed Sep 1, 2024
1 parent f81a15f commit 3fabb61
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 29 deletions.
67 changes: 39 additions & 28 deletions src/navi/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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"
Expand Down
43 changes: 42 additions & 1 deletion test/navi/core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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.)
Expand All @@ -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"))))

0 comments on commit 3fabb61

Please sign in to comment.