metosin / reitit

A fast data-driven routing library for Clojure/Script
https://cljdoc.org/d/metosin/reitit/
Eclipse Public License 1.0
1.43k stars 257 forks source link

Default exception handler fails #343

Open kanwei opened 4 years ago

kanwei commented 4 years ago

My code:

(exception/create-exception-middleware
                                            (merge exception/default-handlers
                                                   {:reitit.coercion/request-coercion
                                                    (fn [_ _]
                                                      {:status  400
                                                       :headers {"Content-Type" "text/plain"}
                                                       :body    "Malformed request - Did not conform to API specifications"})}))

Throwing an error anywhere results in:

HTTP ERROR 500 java.lang.IllegalArgumentException: No implementation of method: :write-body-to-stream of protocol: #'ring.core.protocols/StreamableResponseBody found for class: clojure.lang.PersistentArrayMap
olymk2 commented 4 years ago

I just hit this exact same issue, trying to implement the expound handler as per this link https://github.com/metosin/reitit/blob/master/doc/ring/coercion.md#pretty-printing-spec-errors

using this versions.

  ring/ring-jetty-adapter {:mvn/version "1.8.0"}
  metosin/reitit {:mvn/version "0.4.2"}
  metosin/reitit-dev {:mvn/version "0.4.2"}
  metosin/spec-tools {:mvn/version "0.10.1"}
  metosin/reitit-swagger {:mvn/version "0.4.2"}
  metosin/jsonista {:mvn/version "0.2.5"}
  metosin/muuntaja {:mvn/version "0.6.6"}

The complete traceback, commenting out the custom middleware and everything works.

java.lang.IllegalArgumentException: No implementation of method: :write-body-to-stream of protocol: #'ring.core.protocols/StreamableResponseBody found for class: clojure.lang.PersistentArrayMap
    at clojure.core$_cache_protocol_fn.invokeStatic(core_deftype.clj:583)
    at clojure.core$_cache_protocol_fn.invoke(core_deftype.clj:575)
    at ring.core.protocols$eval444$fn__445$G__435__454.invoke(protocols.clj:8)
    at ring.util.servlet$update_servlet_response.invokeStatic(servlet.clj:106)
    at ring.util.servlet$update_servlet_response.invoke(servlet.clj:91)
    at ring.util.servlet$update_servlet_response.invokeStatic(servlet.clj:95)
    at ring.util.servlet$update_servlet_response.invoke(servlet.clj:91)
    at ring.adapter.jetty$proxy_handler$fn__17787.invoke(jetty.clj:28)
    at ring.adapter.jetty.proxy$org.eclipse.jetty.server.handler.AbstractHandler$ff19274a.handle(Unknown Source)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
    at org.eclipse.jetty.server.Server.handle(Server.java:500)
    at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:386)
    at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:562)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:378)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:270)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
    at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:129)
    at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:388)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
    at java.base/java.lang.Thread.run(Thread.java:834)
TimoKramer commented 4 years ago

me too. how to resolve this error?

ikitommi commented 4 years ago

I think the error is originating from somewhere else as it says the response body is a map, and for coercion errors you are giving a String. I would look fro middleware differ to see what is going on. Could be response coercion error? https://github.com/metosin/reitit/blob/master/examples/ring-spec-swagger/src/example/server.clj#L80

olymk2 commented 4 years ago

Just in relation to the issue I had, I went back to it and tried to make expound work, in the end this is what i used for the exception handler so that api users get a nicer error message.


(defn coercion-error-handler [status]
  (let [printer (expound/custom-printer {:theme :figwheel-theme, :print-specs? false})
        handler (exception/create-coercion-handler status)]
    (fn [exception request]
      {:status 500
       :body (expound/explain-result-str (-> exception ex-data :problems))})))
devurandom commented 3 years ago

The default handler returns a map:

(ns reitit.ring.middleware.exception ,,,)

(defn default-handler
  "Default safe handler for any exception."
  [^Exception e _]
  {:status 500
   :body {:type "exception"
          :class (.getName (.getClass e))}})

(def default-handlers
  {::default default-handler
   ,,,})

If Ring has no middleware to turn this into a string, an exception will be thrown:

Caused by: java.lang.IllegalArgumentException: No implementation of method: :write-body-to-stream of protocol: #'ring.core.protocols/StreamableResponseBody found for class: clojure.lang.PersistentArrayMap
        at clojure.core$_cache_protocol_fn.invokeStatic(core_deftype.clj:583)
        at clojure.core$_cache_protocol_fn.invoke(core_deftype.clj:575)
        at ring.core.protocols$eval10938$fn__10939$G__10929__10948.invoke(protocols.clj:8)
        at ring.util.servlet$update_servlet_response.invokeStatic(servlet.clj:108)
        at ring.util.servlet$update_servlet_response.invoke(servlet.clj:93)
        at ring.util.servlet$update_servlet_response.invokeStatic(servlet.clj:97)
        at ring.util.servlet$update_servlet_response.invoke(servlet.clj:93)
        at ring.adapter.jetty$proxy_handler$fn__11060.invoke(jetty.clj:28)
        at ring.adapter.jetty.proxy$org.eclipse.jetty.server.handler.AbstractHandler$ff19274a.handle(Unknown Source)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
        at org.eclipse.jetty.server.Server.handle(Server.java:516)
        at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388)
        at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:279)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
        at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:882)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1036)
        at java.base/java.lang.Thread.run(Unknown Source)

This can be avoided by wrapping the default exception handler and converting its response body to a JSON string:

(ns user
  (:require
    [clojure.data.json :as json]
    [reitit.ring.middleware.exception :as exception]
    ,,,))

(defn wrap-exception [handler exception request]
  (update (handler exception request) :body json/write-str))

(def exception-middleware
  (exception/create-exception-middleware
    (merge
      exception/default-handlers
      {::exception/wrap wrap-exception})))

(defn run-app ^Server []
  (let [middleware [#(wrap-defaults % api-defaults)
                    exception-middleware]
        handler (reitit-ring/ring-handler
                  (reitit-ring/router
                    [[,,,]])
                  (reitit-ring/routes
                    (reitit-ring/create-default-handler))
                  {:middleware middleware})]
    (run-jetty handler {:port  3000})))

Though this will not have the intended response if the exception handler is ever replaced with something that does not respond with a map in :body. A better solution might be to return something simpler from default-handler, e.g. a plain string.

A more robust workaround that does not involve the default-handler would be to replace it with a more ugly but less fragile string operation until this issue is fixed:

(defn exception-handler [exception request]
  {:status 500
   :body (str "{\"type\": \"exception\", \"class\": \"" (.getName (.getClass exception)) "\"}"))

(def exception-middleware
  (exception/create-exception-middleware
    (merge
      exception/default-handlers
      {::exception/default exception-handler})))
alissocool commented 4 months ago

I encountered a similar error while trying to handle coercion failures, but it depends on the order of the middlewares-- If I place coerce-exceptions-middleware before coerce-request-middleware:

(def handler
  (ring/ring-handler 
    (ring/router 
      [
       ["/api"
         ["/snakes/:id" {:get {:coercion rcs/coercion
                               :parameters {:path {:id s/Int}}
                               :handler get-snake-by-id}}]]]
      {:data {:middleware [rrc/coerce-exceptions-middleware
                           rrc/coerce-request-middleware
                           rrc/coerce-response-middleware
                          ]}})))

I encounter the IllegalArgumentException. But if I place coerce-request-middleware before coerce-exceptions-middleware, I get a "HTTP ERROR 500 clojure.lang.ExceptionInfo: Request coercion failed" error instead:

clojure.lang.ExceptionInfo: Request coercion failed {:schema {:id Int, Keyword Any}, :errors {:id (not (integer? "g"))}, :type :reitit.coercion/request-coercion, :coercion #Coercion{:name :schema}, :value {:id "g"}, :in [:request :path-params], :request {:ring.request/headers {"priority" "u=0, i", "sec-fetch-site" "cross-site", "host" "localhost:3000", "user-agent" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:128.0) Gecko/20100101 Firefox/128.0", "connection" "keep-alive", "upgrade-insecure-requests" "1", "pragma" "no-cache", "accept" "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/png,image/svg+xml,*/*;q=0.8", "accept-language" "en-US,en;q=0.5", "sec-fetch-dest" "document", "accept-encoding" "gzip, deflate, br, zstd", "sec-fetch-mode" "navigate", "dnt" "1", "cache-control" "no-cache"}, :reitit.core/match #reitit.core.Match{:template "/api/snakes/:id", :data {:middleware [{:name :reitit.ring.coercion/coerce-request, :spec :reitit.spec/parameters, :compile #object[reitit.ring.coercion$fn__5281 0x29a5d128 "reitit.ring.coercion$fn__5281@29a5d128"]} {:name :reitit.ring.coercion/coerce-response, :spec :reitit.spec/responses, :compile #object[reitit.ring.coercion$fn__5292 0x2ce90a6c "reitit.ring.coercion$fn__5292@2ce90a6c"]} {:name :reitit.ring.coercion/coerce-exceptions, :compile #object[reitit.ring.coercion$fn__5305 0x66d6d51e "reitit.ring.coercion$fn__5305@66d6d51e"]}], :get {:coercion #Coercion{:name :schema}, :parameters {:path [{:id Int}]}, :handler #object[snakes.core$get_snake_by_id 0x7db790fb "snakes.core$get_snake_by_id@7db790fb"]}}, :result #reitit.ring.Methods{:get #reitit.ring.Endpoint{:data {:middleware [{:name :reitit.ring.coercion/coerce-request, :spec :reitit.spec/parameters, :compile #object[reitit.ring.coercion$fn__5281 0x29a5d128 "reitit.ring.coercion$fn__5281@29a5d128"]} {:name :reitit.ring.coercion/coerce-response, :spec :reitit.spec/responses, :compile #object[reitit.ring.coercion$fn__5292 0x2ce90a6c "reitit.ring.coercion$fn__5292@2ce90a6c"]} {:name :reitit.ring.coercion/coerce-exceptions, :compile #object[reitit.ring.coercion$fn__5305 0x66d6d51e "reitit.ring.coercion$fn__5305@66d6d51e"]}], :coercion #Coercion{:name :schema}, :parameters {:path {:id Int}}, :handler #object[snakes.core$get_snake_by_id 0x7db790fb "snakes.core$get_snake_by_id@7db790fb"]}, :handler #object[reitit.ring.coercion$fn__5281$fn__5283$fn__5284 0x7a9c22f4 "reitit.ring.coercion$fn__5281$fn__5283$fn__5284@7a9c22f4"], :path "/api/snakes/:id", :method :get, :middleware [#reitit.middleware.Middleware{:name :reitit.ring.coercion/coerce-request, :wrap #object[reitit.ring.coercion$fn__5281$fn__5283 0x1ec2fda1 

This didn't align with the "Invalid request" behavior shown in the Ring Coercion docs.