diff --git a/modules/reitit-middleware/src/reitit/ring/middleware/exception.clj b/modules/reitit-middleware/src/reitit/ring/middleware/exception.clj index ed81a41cf..f198c2132 100644 --- a/modules/reitit-middleware/src/reitit/ring/middleware/exception.clj +++ b/modules/reitit-middleware/src/reitit/ring/middleware/exception.clj @@ -69,6 +69,7 @@ "Default safe handler for any exception." [^Exception e _] {:status 500 + :headers {} :body {:type "exception" :class (.getName (.getClass e))}}) @@ -77,6 +78,7 @@ [status] (fn [e _] {:status status + :headers {} :body (coercion/encode-error (ex-data e))})) (defn http-response-handler diff --git a/modules/reitit-ring/src/reitit/ring/coercion.cljc b/modules/reitit-ring/src/reitit/ring/coercion.cljc index 8d7cbe0fc..c870c180f 100644 --- a/modules/reitit-ring/src/reitit/ring/coercion.cljc +++ b/modules/reitit-ring/src/reitit/ring/coercion.cljc @@ -11,6 +11,7 @@ nil)] (respond {:status status + :headers {} :body (coercion/encode-error data)}) (raise e)))) diff --git a/test/clj/reitit/ring/middleware/exception_test.clj b/test/clj/reitit/ring/middleware/exception_test.clj index f905d9947..71b11b58e 100644 --- a/test/clj/reitit/ring/middleware/exception_test.clj +++ b/test/clj/reitit/ring/middleware/exception_test.clj @@ -34,8 +34,8 @@ {:data {:middleware [(exception/create-exception-middleware (merge exception/default-handlers - {::kikka (constantly {:status 400, :body "kikka"}) - SQLException (constantly {:status 400, :body "sql"}) + {::kikka (constantly (http-response/bad-request "kikka")) + SQLException (constantly (http-response/bad-request "sql")) ::exception/wrap wrap}))]}}))))] (testing "normal calls work ok" @@ -44,19 +44,21 @@ (is (= response (app {:request-method :get, :uri "/defaults"}))))) (testing "unknown exception" - (let [app (create (fn [_] (throw (NullPointerException.))))] - (is (= {:status 500 - :body {:type "exception" - :class "java.lang.NullPointerException"}} - (app {:request-method :get, :uri "/defaults"})))) - (let [app (create (fn [_] (throw (ex-info "fail" {:type ::invalid}))))] - (is (= {:status 500 - :body {:type "exception" - :class "clojure.lang.ExceptionInfo"}} - (app {:request-method :get, :uri "/defaults"}))))) + (let [app (create (fn [_] (throw (NullPointerException.)))) + {:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 500)) + (is (= body {:type "exception" + :class "java.lang.NullPointerException"}))) + (let [app (create (fn [_] (throw (ex-info "fail" {:type ::invalid})))) + {:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 500)) + (is (= body {:type "exception" + :class "clojure.lang.ExceptionInfo"})))) (testing "::ring/response" - (let [response {:status 200, :body "ok"} + (let [response (http-response/ok "ok") app (create (fn [_] (throw (ex-info "fail" {:type ::ring/response, :response response}))))] (is (= response (app {:request-method :get, :uri "/defaults"}))))) @@ -75,45 +77,56 @@ (testing "::coercion/request-coercion" (let [app (create (fn [{{{:keys [x y]} :query} :parameters}] - {:status 200, :body {:total (+ x y)}}))] + (http-response/ok {:total (+ x y)})))] - (let [{:keys [status body]} (app {:request-method :get - :uri "/coercion" - :query-params {"x" "1", "y" "2"}})] + (let [{:keys [status body] :as resp} (app {:request-method :get + :uri "/coercion" + :query-params {"x" "1", "y" "2"}})] + (is (http-response/response? resp)) (is (= 200 status)) (is (= {:total 3} body))) - (let [{:keys [status body]} (app {:request-method :get - :uri "/coercion" - :query-params {"x" "abba", "y" "2"}})] + (let [{:keys [status body] :as resp} (app {:request-method :get + :uri "/coercion" + :query-params {"x" "abba", "y" "2"}})] + (is (http-response/response? resp)) (is (= 400 status)) (is (= :reitit.coercion/request-coercion (:type body)))) - (let [{:keys [status body]} (app {:request-method :get - :uri "/coercion" - :query-params {"x" "-10", "y" "2"}})] + (let [{:keys [status body] :as resp} (app {:request-method :get + :uri "/coercion" + :query-params {"x" "-10", "y" "2"}})] + (is (http-response/response? resp)) (is (= 500 status)) (is (= :reitit.coercion/response-coercion (:type body))))))) (testing "exact :type" - (let [app (create (fn [_] (throw (ex-info "fail" {:type ::kikka}))))] - (is (= {:status 400, :body "kikka"} - (app {:request-method :get, :uri "/defaults"}))))) + (let [app (create (fn [_] (throw (ex-info "fail" {:type ::kikka})))) + {:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 400)) + (is (= body "kikka")))) (testing "parent :type" - (let [app (create (fn [_] (throw (ex-info "fail" {:type ::kukka}))))] - (is (= {:status 400, :body "kikka"} - (app {:request-method :get, :uri "/defaults"}))))) + (let [app (create (fn [_] (throw (ex-info "fail" {:type ::kukka})))) + {:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 400)) + (is (= body "kikka")))) (testing "exact Exception" - (let [app (create (fn [_] (throw (SQLException.))))] - (is (= {:status 400, :body "sql"} - (app {:request-method :get, :uri "/defaults"}))))) + (let [app (create (fn [_] (throw (SQLException.)))) + {:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 400)) + (is (= body "sql")))) (testing "Exception SuperClass" - (let [app (create (fn [_] (throw (SQLWarning.))))] - (is (= {:status 400, :body "sql"} - (app {:request-method :get, :uri "/defaults"}))))) + (let [app (create (fn [_] (throw (SQLWarning.)))) + {:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 400)) + (is (= body "sql")))) (testing "::exception/wrap" (let [calls (atom 0) @@ -121,11 +134,17 @@ (fn [handler exception request] (if (< (swap! calls inc) 2) (handler exception request) - {:status 500, :body "too many tries"})))] - (is (= {:status 400, :body "sql"} - (app {:request-method :get, :uri "/defaults"}))) - (is (= {:status 500, :body "too many tries"} - (app {:request-method :get, :uri "/defaults"}))))))) + {:status 500 + :headers {} + :body "too many tries"})))] + (let [{:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 400)) + (is (= body "sql"))) + (let [{:keys [status body] :as resp} (app {:request-method :get, :uri "/defaults"})] + (is (http-response/response? resp)) + (is (= status 500)) + (is (= body "too many tries"))))))) (deftest spec-coercion-exception-test (let [app (ring/ring-handler @@ -135,22 +154,26 @@ {:parameters {:query {:x int?, :y int?}} :responses {200 {:body {:total pos-int?}}} :handler (fn [{{{:keys [x y]} :query} :parameters}] - {:status 200, :body {:total (+ x y)}})}}] + (http-response/ok {:total (+ x y)}))}}] {:data {:coercion reitit.coercion.spec/coercion :middleware [(exception/create-exception-middleware (merge exception/default-handlers - {::coercion/request-coercion (fn [e _] {:status 400, :body (ex-data e)}) - ::coercion/response-coercion (fn [e _] {:status 500, :body (ex-data e)})})) + {::coercion/request-coercion (fn [e _] (http-response/bad-request (ex-data e)) ) + ::coercion/response-coercion (fn [e _] {:status 500 + :headers {} + :body (ex-data e)})})) reitit.ring.coercion/coerce-request-middleware reitit.ring.coercion/coerce-response-middleware]}}))] (testing "success" - (let [{:keys [status body]} (app {:uri "/plus", :request-method :get, :query-params {"x" "1", "y" "2"}})] + (let [{:keys [status body] :as resp} (app {:uri "/plus", :request-method :get, :query-params {"x" "1", "y" "2"}})] + (is (http-response/response? resp)) (is (= 200 status)) (is (= body {:total 3})))) (testing "request error" - (let [{:keys [status body]} (app {:uri "/plus", :request-method :get, :query-params {"x" "1", "y" "fail"}})] + (let [{:keys [status body] :as resp} (app {:uri "/plus", :request-method :get, :query-params {"x" "1", "y" "fail"}})] + (is (http-response/response? resp)) (is (= 400 status)) (testing "spec error is exposed as is" (let [problems (:problems body)] @@ -159,7 +182,8 @@ (is (contains? problems ::s/problems)))))) (testing "response error" - (let [{:keys [status body]} (app {:uri "/plus", :request-method :get, :query-params {"x" "1", "y" "-2"}})] + (let [{:keys [status body] :as resp} (app {:uri "/plus", :request-method :get, :query-params {"x" "1", "y" "-2"}})] + (is (http-response/response? resp)) (is (= 500 status)) (testing "spec error is exposed as is" (let [problems (:problems body)] diff --git a/test/cljc/reitit/ring_coercion_test.cljc b/test/cljc/reitit/ring_coercion_test.cljc index 0a1c6503c..c2e9d1692 100644 --- a/test/cljc/reitit/ring_coercion_test.cljc +++ b/test/cljc/reitit/ring_coercion_test.cljc @@ -478,7 +478,7 @@ (testing "encoding errors" (let [app (->app {:encode-error (fn [error] {:errors (:humanized error)})})] - (is (= {:status 400, :body {:errors {:x ["missing required key"]}}} + (is (= {:status 400, :headers {}, :body {:errors {:x ["missing required key"]}}} (app (assoc (->request "closed") :body-params {})))))) (testing "when schemas are not closed and extra keys are not stripped"