From 0c68545b846cac392ec1bead6a42cd9ffabd343c Mon Sep 17 00:00:00 2001 From: Matt Spencer Date: Wed, 5 Feb 2020 17:59:41 -0800 Subject: [PATCH 1/5] add trimmed? to wrap-stacktrace extend color? to wrap-stacktrace-web The new parameter will display the key :trimmed-elems returned from the processes error instead of :trace-elems which whill show a shorter trace. This is benificial in 1. not flooding the terminal and 2. making it easier to find the trace directly related to the application. Extending color? to wrap-stacktrace-web will allow the same color patterns available in wrap-stacktrace-log. --- ring-devel/src/ring/middleware/stacktrace.clj | 122 +++++++++++------- 1 file changed, 78 insertions(+), 44 deletions(-) diff --git a/ring-devel/src/ring/middleware/stacktrace.clj b/ring-devel/src/ring/middleware/stacktrace.clj index 63a8f535..9c801300 100644 --- a/ring-devel/src/ring/middleware/stacktrace.clj +++ b/ring-devel/src/ring/middleware/stacktrace.clj @@ -5,117 +5,151 @@ This middleware is for debugging purposes, and should be limited to development environments." (:require [clojure.java.io :as io] + [clojure.string :as str] [hiccup.core :refer [html h]] [hiccup.page :refer [html5]] - [clj-stacktrace.core :refer :all] - [clj-stacktrace.repl :refer :all] + [clj-stacktrace.core :refer [parse-exception]] + [clj-stacktrace.repl :refer [clojure-method-str + elem-color + java-method-str + pst + pst-on + source-str]] [ring.util.response :refer [content-type response status]])) +(defn swap-trace-elems + "Recursively replace :trace-elems with :trimmed-elems" + [exception] + (let [trimmed (or (:trimmed-elems exception) '()) + cause (:cause exception)] + (if cause + (assoc exception + :cause (swap-trace-elems cause) + :trace-elems trimmed) + (assoc exception :trace-elems trimmed)))) + +(defn trim-error-elems [ex trimmed?] + (if trimmed? + (swap-trace-elems (parse-exception ex)) + (parse-exception ex))) + (defn wrap-stacktrace-log "Wrap a handler such that exceptions are logged to *err* and then rethrown. Accepts the following options: - - :color? - if true, apply ANSI colors to stacktrace (default false)" + :color? - if true, apply ANSI colors to stacktrace (default false) + :trimmed? - if true, use the trimmed-elems instead of trace-elems (default false)" ([handler] (wrap-stacktrace-log handler {})) ([handler options] - (let [color? (:color? options)] + (let [{:keys [color? trimmed?]} options] (fn ([request] (try (handler request) (catch Throwable ex - (pst-on *err* color? ex) + (pst-on *err* color? (trim-error-elems ex trimmed?)) (throw ex)))) ([request respond raise] (try - (handler request respond (fn [ex] (pst-on *err* color? ex) (raise ex))) + (handler request respond (fn [ex] (pst-on *err* color? (trim-error-elems ex trimmed?)) (raise ex))) (catch Throwable ex - (pst-on *err* color? ex) + (pst-on *err* color? (trim-error-elems ex trimmed?)) (throw ex)))))))) (defn- style-resource [path] (html [:style {:type "text/css"} (slurp (io/resource path))])) -(defn- elem-partial [elem] +(defn color-style + "Returns a style tag with the color appropriate for the given trace elem. + Cyan is replaced with black for readability on the light background." + [elem] + {:style + {:color (str/replace (name (elem-color elem)) "cyan" "black")}}) + +(defn- elem-partial [elem color?] (if (:clojure elem) - [:tr.clojure + [:tr.clojure (when color? (color-style elem)) [:td.source (h (source-str elem))] [:td.method (h (clojure-method-str elem))]] - [:tr.java + [:tr.java (when color? (color-style elem)) [:td.source (h (source-str elem))] [:td.method (h (java-method-str elem))]])) -(defn- html-exception [ex] - (let [[ex & causes] (iterate :cause (parse-exception ex))] +(defn- html-exception [ex color? trimmed?] + (let [[ex & causes] (iterate :cause (trim-error-elems ex trimmed?))] (html5 [:head [:title "Ring: Stacktrace"] (style-resource "ring/css/stacktrace.css")] [:body [:div#exception - [:h1 (h (.getName ^Class (:class ex)))] - [:div.message (h (:message ex))] + [:h1 (h (.getName ^Class (:class ex)))] + [:div.message (h (:message ex))] + (when (pos? (count (:trace-elems ex))) [:div.trace - [:table - [:tbody (map elem-partial (:trace-elems ex))]]] + [:table + [:tbody (map #(elem-partial % color?) (:trace-elems ex))]]]) (for [cause causes :while cause] [:div#causes - [:h2 "Caused by " [:span.class (h (.getName ^Class (:class cause)))]] - [:div.message (h (:message cause))] - [:div.trace - [:table - [:tbody (map elem-partial (:trace-elems cause))]]]])]]))) + [:h2 "Caused by " [:span.class (h (.getName ^Class (:class cause)))]] + [:div.message (h (:message cause))] + [:div.trace + [:table + [:tbody (map #(elem-partial % color?) (:trace-elems cause))]]]])]]))) (defn- text-ex-response [e] (-> (response (with-out-str (pst e))) (status 500) (content-type "text/plain"))) -(defn- html-ex-response [ex] - (-> (response (html-exception ex)) +(defn- html-ex-response [ex color? trimmed?] + (-> (response (html-exception ex color? trimmed?)) (status 500) (content-type "text/html"))) (defn- ex-response "Returns a response showing debugging information about the exception. - Renders HTML if that's in the accept header (indicating that the URL was opened in a browser), but defaults to plain text." - [req ex] + [req ex color? trimmed?] (let [accept (get-in req [:headers "accept"])] (if (and accept (re-find #"^text/html" accept)) - (html-ex-response ex) + (html-ex-response ex color? trimmed?) (text-ex-response ex)))) (defn wrap-stacktrace-web "Wrap a handler such that exceptions are caught and a response containing - a HTML representation of the exception and stacktrace is returned." - [handler] - (fn - ([request] - (try - (handler request) - (catch Throwable ex - (ex-response request ex)))) - ([request respond raise] - (try - (handler request respond (fn [ex] (respond (ex-response request ex)))) - (catch Throwable ex - (respond (ex-response request ex))))))) + a HTML representation of the exception and stacktrace is returned. + Accepts the following option: + :color? - if true, apply ANSI colors to terminal stacktrace (default false) + :trimmed? - if true, use the trimmed-elems instead of trace-elems (default false)" + ([handler] + (wrap-stacktrace-web handler {})) + ([handler options] + (let [{:keys [color? trimmed?]} options] + (fn + ([request] + (try + (handler request) + (catch Throwable ex + (ex-response request ex color? trimmed?)))) + ([request respond raise] + (try + (handler request respond (fn [ex] (respond (ex-response request ex color? trimmed?)))) + (catch Throwable ex + (respond (ex-response request ex color? trimmed?))))))))) (defn wrap-stacktrace "Wrap a handler such that exceptions are caught, a corresponding stacktrace is logged to *err*, and a HTML representation of the stacktrace is returned as a response. - Accepts the following option: - - :color? - if true, apply ANSI colors to terminal stacktrace (default false)" + :color? - if true, apply ANSI colors to terminal stacktrace (default false) + :trimmed? - if true, use the trimmed-elems instead of trace-elems (default false)" {:arglists '([handler] [handler options])} ([handler] (wrap-stacktrace handler {})) ([handler options] (-> handler (wrap-stacktrace-log options) - (wrap-stacktrace-web)))) + (wrap-stacktrace-web options)))) From 0e0b194a63a11e193681177ecfd7e1265fbfe644 Mon Sep 17 00:00:00 2001 From: Matt Spencer Date: Wed, 5 Feb 2020 18:01:14 -0800 Subject: [PATCH 2/5] update wrap-stacktrace test for new parameters --- .../test/ring/middleware/test/stacktrace.clj | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/ring-devel/test/ring/middleware/test/stacktrace.clj b/ring-devel/test/ring/middleware/test/stacktrace.clj index 5dc21dea..e7e63de4 100644 --- a/ring-devel/test/ring/middleware/test/stacktrace.clj +++ b/ring-devel/test/ring/middleware/test/stacktrace.clj @@ -6,8 +6,8 @@ (def assert-app (wrap-stacktrace (fn [_] (assert (= 1 2))))) -(def html-req {:headers {"accept" "text/html"}}) -(def js-req {:headers {"accept" "application/javascript"}}) +(def html-req {:headers {"accept" "text/html"}}) +(def js-req {:headers {"accept" "application/javascript"}}) (def plain-req {}) (deftest wrap-stacktrace-smoke @@ -25,35 +25,39 @@ (is (or (.startsWith body "java.lang.Exception") (.startsWith body "java.lang.AssertionError"))))) (testing "requests without Accept header" - (let [{:keys [status headers body]} (app js-req)] + (let [{:keys [status headers body]} (app plain-req)] (is (= 500 status)) (is (= {"Content-Type" "text/plain"} headers)) (is (or (.startsWith body "java.lang.Exception") (.startsWith body "java.lang.AssertionError")))))))) +(def default-params {}) +(def non-default-params {:color? true :trimmed? true}) + (deftest wrap-stacktrace-cps-test - (testing "no exception" - (let [handler (wrap-stacktrace (fn [_ respond _] (respond :ok))) - response (promise) - exception (promise)] - (handler {} response exception) - (is (= :ok @response)) - (is (not (realized? exception))))) - - (testing "thrown exception" - (let [handler (wrap-stacktrace (fn [_ _ _] (throw (Exception. "fail")))) - response (promise) - exception (promise)] - (binding [*err* (java.io.StringWriter.)] - (handler {} response exception)) - (is (= 500 (:status @response))) - (is (not (realized? exception))))) - - (testing "raised exception" - (let [handler (wrap-stacktrace (fn [_ _ raise] (raise (Exception. "fail")))) - response (promise) - exception (promise)] - (binding [*err* (java.io.StringWriter.)] - (handler {} response exception)) - (is (= 500 (:status @response))) - (is (not (realized? exception)))))) + (doseq [params [default-params non-default-params]] + (testing "no exception" + (let [handler (wrap-stacktrace (fn [_ respond _] (respond :ok)) params) + response (promise) + exception (promise)] + (handler {} response exception) + (is (= :ok @response)) + (is (not (realized? exception))))) + + (testing "thrown exception" + (let [handler (wrap-stacktrace (fn [_ _ _] (throw (Exception. "fail"))) params) + response (promise) + exception (promise)] + (binding [*err* (java.io.StringWriter.)] + (handler {} response exception)) + (is (= 500 (:status @response))) + (is (not (realized? exception))))) + + (testing "raised exception" + (let [handler (wrap-stacktrace (fn [_ _ raise] (raise (Exception. "fail"))) params) + response (promise) + exception (promise)] + (binding [*err* (java.io.StringWriter.)] + (handler {} response exception)) + (is (= 500 (:status @response))) + (is (not (realized? exception))))))) From eccc38b49a4f02f39c914a461036adf1a31ee17d Mon Sep 17 00:00:00 2001 From: Matt Spencer Date: Mon, 10 Feb 2020 10:41:05 -0800 Subject: [PATCH 3/5] Changes made from web review by weavejester --- ring-devel/src/ring/middleware/stacktrace.clj | 55 +++++++++--------- .../test/ring/middleware/test/stacktrace.clj | 56 ++++++++++--------- 2 files changed, 61 insertions(+), 50 deletions(-) diff --git a/ring-devel/src/ring/middleware/stacktrace.clj b/ring-devel/src/ring/middleware/stacktrace.clj index 9c801300..a898a2c4 100644 --- a/ring-devel/src/ring/middleware/stacktrace.clj +++ b/ring-devel/src/ring/middleware/stacktrace.clj @@ -8,16 +8,11 @@ [clojure.string :as str] [hiccup.core :refer [html h]] [hiccup.page :refer [html5]] - [clj-stacktrace.core :refer [parse-exception]] - [clj-stacktrace.repl :refer [clojure-method-str - elem-color - java-method-str - pst - pst-on - source-str]] + [clj-stacktrace.core :refer :all] + [clj-stacktrace.repl :refer :all] [ring.util.response :refer [content-type response status]])) -(defn swap-trace-elems +(defn- swap-trace-elems "Recursively replace :trace-elems with :trimmed-elems" [exception] (let [trimmed (or (:trimmed-elems exception) '()) @@ -28,7 +23,7 @@ :trace-elems trimmed) (assoc exception :trace-elems trimmed)))) -(defn trim-error-elems [ex trimmed?] +(defn- trim-error-elems [ex trimmed?] (if trimmed? (swap-trace-elems (parse-exception ex)) (parse-exception ex))) @@ -37,7 +32,7 @@ "Wrap a handler such that exceptions are logged to *err* and then rethrown. Accepts the following options: :color? - if true, apply ANSI colors to stacktrace (default false) - :trimmed? - if true, use the trimmed-elems instead of trace-elems (default false)" + :trimmed? - if true, use trimmed-elems (default false)" ([handler] (wrap-stacktrace-log handler {})) ([handler options] @@ -51,7 +46,13 @@ (throw ex)))) ([request respond raise] (try - (handler request respond (fn [ex] (pst-on *err* color? (trim-error-elems ex trimmed?)) (raise ex))) + (handler request + respond + (fn [ex] + (pst-on *err* + color? + (trim-error-elems ex trimmed?)) + (raise ex))) (catch Throwable ex (pst-on *err* color? (trim-error-elems ex trimmed?)) (throw ex)))))))) @@ -59,7 +60,7 @@ (defn- style-resource [path] (html [:style {:type "text/css"} (slurp (io/resource path))])) -(defn color-style +(defn- color-style "Returns a style tag with the color appropriate for the given trace elem. Cyan is replaced with black for readability on the light background." [elem] @@ -83,19 +84,20 @@ (style-resource "ring/css/stacktrace.css")] [:body [:div#exception - [:h1 (h (.getName ^Class (:class ex)))] - [:div.message (h (:message ex))] + [:h1 (h (.getName ^Class (:class ex)))] + [:div.message (h (:message ex))] (when (pos? (count (:trace-elems ex))) [:div.trace - [:table - [:tbody (map #(elem-partial % color?) (:trace-elems ex))]]]) + [:table + [:tbody (map #(elem-partial % color?) (:trace-elems ex))]]]) (for [cause causes :while cause] [:div#causes - [:h2 "Caused by " [:span.class (h (.getName ^Class (:class cause)))]] - [:div.message (h (:message cause))] - [:div.trace - [:table - [:tbody (map #(elem-partial % color?) (:trace-elems cause))]]]])]]))) + [:h2 "Caused by " [:span.class (h (.getName ^Class (:class cause)))]] + [:div.message (h (:message cause))] + [:div.trace + [:table + [:tbody + (map #(elem-partial % color?) (:trace-elems cause))]]]])]]))) (defn- text-ex-response [e] (-> (response (with-out-str (pst e))) @@ -122,7 +124,7 @@ a HTML representation of the exception and stacktrace is returned. Accepts the following option: :color? - if true, apply ANSI colors to terminal stacktrace (default false) - :trimmed? - if true, use the trimmed-elems instead of trace-elems (default false)" + :trimmed? - if true, use the trimmed-elems (default false)" ([handler] (wrap-stacktrace-web handler {})) ([handler options] @@ -135,7 +137,10 @@ (ex-response request ex color? trimmed?)))) ([request respond raise] (try - (handler request respond (fn [ex] (respond (ex-response request ex color? trimmed?)))) + (handler request + respond + (fn [ex] + (respond (ex-response request ex color? trimmed?)))) (catch Throwable ex (respond (ex-response request ex color? trimmed?))))))))) @@ -144,8 +149,8 @@ logged to *err*, and a HTML representation of the stacktrace is returned as a response. Accepts the following option: - :color? - if true, apply ANSI colors to terminal stacktrace (default false) - :trimmed? - if true, use the trimmed-elems instead of trace-elems (default false)" + :color? - if true, apply ANSI colors to stacktrace (default false) + :trimmed? - if true, use the trimmed-elems (default false)" {:arglists '([handler] [handler options])} ([handler] (wrap-stacktrace handler {})) diff --git a/ring-devel/test/ring/middleware/test/stacktrace.clj b/ring-devel/test/ring/middleware/test/stacktrace.clj index e7e63de4..b81482c5 100644 --- a/ring-devel/test/ring/middleware/test/stacktrace.clj +++ b/ring-devel/test/ring/middleware/test/stacktrace.clj @@ -36,28 +36,34 @@ (deftest wrap-stacktrace-cps-test (doseq [params [default-params non-default-params]] - (testing "no exception" - (let [handler (wrap-stacktrace (fn [_ respond _] (respond :ok)) params) - response (promise) - exception (promise)] - (handler {} response exception) - (is (= :ok @response)) - (is (not (realized? exception))))) - - (testing "thrown exception" - (let [handler (wrap-stacktrace (fn [_ _ _] (throw (Exception. "fail"))) params) - response (promise) - exception (promise)] - (binding [*err* (java.io.StringWriter.)] - (handler {} response exception)) - (is (= 500 (:status @response))) - (is (not (realized? exception))))) - - (testing "raised exception" - (let [handler (wrap-stacktrace (fn [_ _ raise] (raise (Exception. "fail"))) params) - response (promise) - exception (promise)] - (binding [*err* (java.io.StringWriter.)] - (handler {} response exception)) - (is (= 500 (:status @response))) - (is (not (realized? exception))))))) + (testing "no exception" + (let [handler (wrap-stacktrace (fn [_ respond _] + (respond :ok)) + params) + response (promise) + exception (promise)] + (handler {} response exception) + (is (= :ok @response)) + (is (not (realized? exception))))) + + (testing "thrown exception" + (let [handler (wrap-stacktrace (fn [_ _ _] + (throw (Exception. "fail"))) + params) + response (promise) + exception (promise)] + (binding [*err* (java.io.StringWriter.)] + (handler {} response exception)) + (is (= 500 (:status @response))) + (is (not (realized? exception))))) + + (testing "raised exception" + (let [handler (wrap-stacktrace (fn [_ _ raise] + (raise (Exception. "fail"))) + params) + response (promise) + exception (promise)] + (binding [*err* (java.io.StringWriter.)] + (handler {} response exception)) + (is (= 500 (:status @response))) + (is (not (realized? exception))))))) From 3da13e6803d978efbfaac51f236ea65eb85a908b Mon Sep 17 00:00:00 2001 From: Matt Spencer Date: Mon, 10 Feb 2020 11:08:11 -0800 Subject: [PATCH 4/5] Remove more formatting changes unrelated to this change --- ring-devel/src/ring/middleware/stacktrace.clj | 5 ++++- ring-devel/test/ring/middleware/test/stacktrace.clj | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ring-devel/src/ring/middleware/stacktrace.clj b/ring-devel/src/ring/middleware/stacktrace.clj index a898a2c4..d127ba33 100644 --- a/ring-devel/src/ring/middleware/stacktrace.clj +++ b/ring-devel/src/ring/middleware/stacktrace.clj @@ -111,6 +111,7 @@ (defn- ex-response "Returns a response showing debugging information about the exception. + Renders HTML if that's in the accept header (indicating that the URL was opened in a browser), but defaults to plain text." [req ex color? trimmed?] @@ -122,8 +123,9 @@ (defn wrap-stacktrace-web "Wrap a handler such that exceptions are caught and a response containing a HTML representation of the exception and stacktrace is returned. + Accepts the following option: - :color? - if true, apply ANSI colors to terminal stacktrace (default false) + :color? - if true, apply ANSI colors to HTML stacktrace (default false) :trimmed? - if true, use the trimmed-elems (default false)" ([handler] (wrap-stacktrace-web handler {})) @@ -148,6 +150,7 @@ "Wrap a handler such that exceptions are caught, a corresponding stacktrace is logged to *err*, and a HTML representation of the stacktrace is returned as a response. + Accepts the following option: :color? - if true, apply ANSI colors to stacktrace (default false) :trimmed? - if true, use the trimmed-elems (default false)" diff --git a/ring-devel/test/ring/middleware/test/stacktrace.clj b/ring-devel/test/ring/middleware/test/stacktrace.clj index b81482c5..c8e9698e 100644 --- a/ring-devel/test/ring/middleware/test/stacktrace.clj +++ b/ring-devel/test/ring/middleware/test/stacktrace.clj @@ -6,8 +6,8 @@ (def assert-app (wrap-stacktrace (fn [_] (assert (= 1 2))))) -(def html-req {:headers {"accept" "text/html"}}) -(def js-req {:headers {"accept" "application/javascript"}}) +(def html-req {:headers {"accept" "text/html"}}) +(def js-req {:headers {"accept" "application/javascript"}}) (def plain-req {}) (deftest wrap-stacktrace-smoke @@ -25,7 +25,7 @@ (is (or (.startsWith body "java.lang.Exception") (.startsWith body "java.lang.AssertionError"))))) (testing "requests without Accept header" - (let [{:keys [status headers body]} (app plain-req)] + (let [{:keys [status headers body]} (app js-req)] (is (= 500 status)) (is (= {"Content-Type" "text/plain"} headers)) (is (or (.startsWith body "java.lang.Exception") From 9b01d447e607df757a493748f86b7f8ea90f0bf3 Mon Sep 17 00:00:00 2001 From: Matt Spencer Date: Mon, 10 Feb 2020 11:14:23 -0800 Subject: [PATCH 5/5] Add back blank line for comment --- ring-devel/src/ring/middleware/stacktrace.clj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ring-devel/src/ring/middleware/stacktrace.clj b/ring-devel/src/ring/middleware/stacktrace.clj index d127ba33..b9a3bb22 100644 --- a/ring-devel/src/ring/middleware/stacktrace.clj +++ b/ring-devel/src/ring/middleware/stacktrace.clj @@ -31,6 +31,7 @@ (defn wrap-stacktrace-log "Wrap a handler such that exceptions are logged to *err* and then rethrown. Accepts the following options: + :color? - if true, apply ANSI colors to stacktrace (default false) :trimmed? - if true, use trimmed-elems (default false)" ([handler] @@ -125,6 +126,7 @@ a HTML representation of the exception and stacktrace is returned. Accepts the following option: + :color? - if true, apply ANSI colors to HTML stacktrace (default false) :trimmed? - if true, use the trimmed-elems (default false)" ([handler] @@ -152,6 +154,7 @@ response. Accepts the following option: + :color? - if true, apply ANSI colors to stacktrace (default false) :trimmed? - if true, use the trimmed-elems (default false)" {:arglists '([handler] [handler options])}