Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected handling of empty body response. #691

Open
bombaywalla opened this issue Aug 19, 2024 · 4 comments
Open

Unexpected handling of empty body response. #691

bombaywalla opened this issue Aug 19, 2024 · 4 comments
Labels

Comments

@bombaywalla
Copy link

bombaywalla commented Aug 19, 2024

Hi folks

The following code does not behave as expected.

  (def test-handler
    (ring/ring-handler
     (ring/router [["/openapi.json"
                    {:get {:no-doc true
                           :openapi {:info {:title "testreitit"
                                            :description "Test Open API spec"
                                            :version "0.0.1"}}
                           :handler (openapi/create-openapi-handler)}}]
                   ["/test/{id}"
                    {:get {:description "id < 5 ==> 400 (null body); otherwise 200 and JSON"
                           :handler (fn [{{{:keys [id]} :path} :parameters}]
                                      (if (< id 5)
                                        {:status 400}
                                        {:status 200
                                         :body {:a 1}}))
                           :parameters {:path [:map [:id int?]]}
                           :responses
                           {:default {:description "default is json"
                                      :content {"application/json" {:schema [:map
                                                                             {:closed false}
                                                                             [:a int?]]}}}
                            400 {:description "400 and empty response"
                                 #_#_:content {:default {:schema nil?}}}}}}]]
                  {:validate spec/validate
                   :data {:coercion reitit.coercion.malli/coercion
                          :muuntaja m/instance
                          :middleware [openapi/openapi-feature
                                       swagger/swagger-feature
                                       parameters/parameters-middleware
                                       muuntaja/format-negotiate-middleware
                                       muuntaja/format-response-middleware
                                       exception/exception-middleware
                                       muuntaja/format-request-middleware
                                       coercion/coerce-response-middleware
                                       coercion/coerce-request-middleware
                                       multipart/multipart-middleware]}})
     (ring/routes (swagger-ui/create-swagger-ui-handler
                   {:path "/"
                    :config {:validatorUrl nil
                             :operationsSorter "alpha"
                             :urls [{:name "openapi" :url "/openapi.json"}]
                             :urls.primaryName "openapi"}} )
                  (ring/create-default-handler))))
  (def testx (jetty/run-jetty #'test-handler {:port 7777 :join? false}))
  (.stop ^org.eclipse.jetty.server.Server testx)

When I call the endpoint with a GET of /test/2, I get a 500 response code with the following body

{
  "value": null,
  "type": "reitit.coercion/response-coercion",
  "coercion": "malli",
  "in": [
    "response",
    "body"
  ],
  "humanized": [
    "invalid type"
  ]
}

I was expecting a 400 response code with an empty body.

According to OpenAPI spec for a Response Object and the Swagger docs on an empty response body, to specify an empty response body, you just leave off the content property, which is what I did.

However, if I uncomment the two #_s in the Reitit specification, and use :content {:default {:schema nil?}} then it does work as expected.

What is the recommended way of specifying an empty response body in Reitit?

Thanks.

@opqdonut
Copy link
Member

Either the malli coercion or -get-apidocs-openapi doesn't handle a missing :content properly. This is a bug, the workaround is to use :schema nil?.

@opqdonut opqdonut added the bug label Aug 20, 2024
@opqdonut
Copy link
Member

Based on my testing, it looks like it should work if you use :responses {200 ... 400 ...} instead of using :responses {:default ... 400 ...}. I'm digging into the :default handling, there's probably a bug somewhere there.

@opqdonut
Copy link
Member

The problem seems to be that when there is no :content or :body specified for a response, no coercer gets created in response-coercer:

Then, when it is time to coerce the response, coerce-response falls back to the :default coercer here:

(if-let [coercer (or (coercers (:status response)) (coercers :default))]

The fix would be to create an -identity-coercer in these cases, but that feels like it might break the behaviour of :default for some people. The :default key isn't really documented, just mentioned in examples, as far as I can see.

@bombaywalla
Copy link
Author

Thanks for investigating.
Your analysis of the problem and proposed fix seem reasonable.
Is there such a thing as a "null coercer" (a coercer that coerces anything to nil)?
I'm not sure that would be something useful, just asking whether it exists already.

According to the OpenAPI Spec 3.1.0, the recommended way to specify an empty response body is to leave off the content key.
What is the recommended way to specify an empty response body in Reitit?

In the OpenAPI Spec 3.1.0, the default key is documented to match those response codes that are not explicitly specified. That definition seems to be reasonable, and seems (from the code and examples) to be what Reitit also intended / intends. If so, it should be added to the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

No branches or pull requests

2 participants