From 4d7a34a998a9fca8b9e45d2fdad8369854b16d05 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 4 Dec 2020 20:38:38 +0100 Subject: [PATCH] :tada: Add better error reporting. --- backend/deps.edn | 1 + backend/resources/log4j2.xml | 10 +++- backend/src/app/config.clj | 7 +++ backend/src/app/error_reporter.clj | 83 +++++++++++++++++++++++++++++ backend/src/app/http.clj | 2 +- backend/src/app/http/errors.clj | 56 ++++++++++++++----- backend/src/app/http/handlers.clj | 48 ++++++++++------- backend/src/app/services.clj | 43 +++++++++++++++ backend/src/app/services/init.clj | 31 +---------- backend/src/app/worker.clj | 2 +- backend/tests/app/tests/helpers.clj | 6 +-- common/app/common/data.cljc | 13 ++++- common/app/common/exceptions.cljc | 4 ++ common/app/common/spec.cljc | 4 +- 14 files changed, 238 insertions(+), 72 deletions(-) create mode 100644 backend/src/app/error_reporter.clj create mode 100644 backend/src/app/services.clj diff --git a/backend/deps.edn b/backend/deps.edn index 095b0e4fb..782997986 100644 --- a/backend/deps.edn +++ b/backend/deps.edn @@ -34,6 +34,7 @@ org.postgresql/postgresql {:mvn/version "42.2.16"} com.zaxxer/HikariCP {:mvn/version "3.4.5"} + funcool/log4j2-clojure {:mvn/version "2020.11.23-1"} funcool/datoteka {:mvn/version "1.2.0"} funcool/promesa {:mvn/version "5.1.0"} funcool/cuerdas {:mvn/version "2020.03.26-3"} diff --git a/backend/resources/log4j2.xml b/backend/resources/log4j2.xml index df7bc897e..26a71e7bc 100644 --- a/backend/resources/log4j2.xml +++ b/backend/resources/log4j2.xml @@ -12,6 +12,10 @@ + + + + @@ -23,13 +27,17 @@ + + + + + - diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 41d1da236..9a6f3a3c6 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -40,6 +40,8 @@ :smtp-default-reply-to "no-reply@example.com" :smtp-default-from "no-reply@example.com" + :host "devenv" + :allow-demo-users true :registration-enabled true :registration-domain-whitelist "" @@ -78,6 +80,9 @@ (s/def ::media-uri ::us/string) (s/def ::media-directory ::us/string) (s/def ::secret-key ::us/string) + +(s/def ::host ::us/string) +(s/def ::error-report-webhook ::us/string) (s/def ::smtp-enabled ::us/boolean) (s/def ::smtp-default-reply-to ::us/email) (s/def ::smtp-default-from ::us/email) @@ -135,6 +140,7 @@ ::assets-uri ::media-directory ::media-uri + ::error-report-webhook ::secret-key ::smtp-default-from ::smtp-default-reply-to @@ -145,6 +151,7 @@ ::smtp-password ::smtp-tls ::smtp-ssl + ::host ::file-trimming-threshold ::debug-humanize-transit ::allow-demo-users diff --git a/backend/src/app/error_reporter.clj b/backend/src/app/error_reporter.clj new file mode 100644 index 000000000..a567c17ab --- /dev/null +++ b/backend/src/app/error_reporter.clj @@ -0,0 +1,83 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; This Source Code Form is "Incompatible With Secondary Licenses", as +;; defined by the Mozilla Public License, v. 2.0. +;; +;; Copyright (c) 2020 Andrey Antukh + +(ns app.error-reporter + "A mattermost integration for error reporting." + (:require + [app.common.exceptions :as ex] + [app.common.spec :as us] + [app.config :as cfg] + [app.db :as db] + [app.tasks :as tasks] + [app.util.async :as aa] + [app.worker :as wrk] + [app.util.http :as http] + [clojure.core.async :as a] + [clojure.data.json :as json] + [clojure.spec.alpha :as s] + [clojure.tools.logging :as log] + [cuerdas.core :as str] + [mount.core :as mount :refer [defstate]] + [promesa.exec :as px])) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Public API +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defonce enqueue identity) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Implementation +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn- send-to-mattermost! + [log-event] + (try + (let [text (str/fmt "Unhandled exception: `host='%s'`, `version=%s`.\n@channel ⇊\n```%s\n```" + (:host cfg/config) + (:full @cfg/version) + (str log-event)) + rsp (http/send! {:uri (:error-reporter-webhook cfg/config) + :method :post + :headers {"content-type" "application/json"} + :body (json/write-str {:text text})})] + (when (not= (:status rsp) 200) + (log/warnf "Error reporting webhook replying with unexpected status: %s\n%s" + (:status rsp) + (pr-str rsp)))) + (catch Exception e + (log/warnf e "Unexpected exception on error reporter.")))) + +(defn- send! + [val] + (aa/thread-call wrk/executor (partial send-to-mattermost! val))) + +(defn- start + [] + (let [qch (a/chan (a/sliding-buffer 128))] + (log/info "Starting error reporter loop.") + + ;; Only enable when a valid URL is provided. + (when (:error-reporter-webhook cfg/config) + (alter-var-root #'enqueue (constantly #(a/>!! qch %))) + (a/go-loop [] + (let [val (a/" (:explain response) "\n")} - + :body (str "
"
+                  (with-out-str
+                    (:data edata))
+                  "
\n")} :else {:status 400 - :body response}))) + :body edata}))) (defmethod handle-exception :ratelimit [_ _] @@ -60,11 +65,38 @@ :body {:type :parse :message (ex-message err)}}) +(defn get-context-string + [err request] + (str + "=| uri: " (pr-str (:uri request)) "\n" + "=| method: " (pr-str (:request-method request)) "\n" + "=| path-params: " (pr-str (:path-params request)) "\n" + "=| query-params: " (pr-str (:query-params request)) "\n" + + (when-let [bparams (:body-params request)] + (str "=| body-params: " (pr-str bparams) "\n")) + + (when (ex/ex-info? err) + (str "=| ex-data: " (pr-str (ex-data err)) "\n")) + + "\n")) + + +(defmethod handle-exception :assertion + [err request] + (let [{:keys [data] :as edata} (ex-data err)] + (log/errorf err + (str "Assertion error\n" + (get-context-string err request) + (with-out-str (expound/printer data)))) + {:status 500 + :body {:type :internal-error + :message "Assertion error" + :data (ex-data err)}})) + (defmethod handle-exception :default - [err req] - (log/error "Unhandled exception on request:" (:path req) "\n" - (with-out-str - (.printStackTrace ^Throwable err (java.io.PrintWriter. *out*)))) + [err request] + (log/errorf err (str "Internal Error\n" (get-context-string err request))) {:status 500 :body {:type :internal-error :message (ex-message err) diff --git a/backend/src/app/http/handlers.clj b/backend/src/app/http/handlers.clj index 3d3855175..1265a97e1 100644 --- a/backend/src/app/http/handlers.clj +++ b/backend/src/app/http/handlers.clj @@ -9,6 +9,10 @@ (ns app.http.handlers (:require + [app.common.data :as d] + [app.common.exceptions :as ex] + [app.emails :as emails] + [app.http.session :as session] [app.services.init] [app.services.mutations :as sm] [app.services.queries :as sq])) @@ -25,36 +29,40 @@ :login}) (defn query-handler - [req] - (let [type (keyword (get-in req [:path-params :type])) - data (merge (:params req) - {::sq/type type}) - data (cond-> data - (:profile-id req) (assoc :profile-id (:profile-id req)))] - (if (or (:profile-id req) (contains? unauthorized-services type)) + [{:keys [profile-id] :as request}] + (let [type (keyword (get-in request [:path-params :type])) + data (assoc (:params request) ::sq/type type) + data (if profile-id + (assoc data :profile-id profile-id) + (dissoc data :profile-id))] + + (if (or (uuid? profile-id) + (contains? unauthorized-services type)) {:status 200 - :body (sq/handle (with-meta data {:req req}))} + :body (sq/handle (with-meta data {:req request}))} {:status 403 :body {:type :authentication :code :unauthorized}}))) (defn mutation-handler - [req] - (let [type (keyword (get-in req [:path-params :type])) - data (merge (:params req) - (:body-params req) - (:uploads req) - {::sm/type type}) - data (cond-> data - (:profile-id req) (assoc :profile-id (:profile-id req)))] - (if (or (:profile-id req) (contains? unauthorized-services type)) - (let [result (sm/handle (with-meta data {:req req})) + [{:keys [profile-id] :as request}] + (let [type (keyword (get-in request [:path-params :type])) + data (d/merge (:params request) + (:body-params request) + (:uploads request) + {::sm/type type}) + data (if profile-id + (assoc data :profile-id profile-id) + (dissoc data :profile-id))] + + (if (or (uuid? profile-id) + (contains? unauthorized-services type)) + (let [result (sm/handle (with-meta data {:req request})) mdata (meta result) resp {:status (if (nil? (seq result)) 204 200) :body result}] (cond->> resp - (:transform-response mdata) ((:transform-response mdata) req))) - + (:transform-response mdata) ((:transform-response mdata) request))) {:status 403 :body {:type :authentication :code :unauthorized}}))) diff --git a/backend/src/app/services.clj b/backend/src/app/services.clj new file mode 100644 index 000000000..2268ed791 --- /dev/null +++ b/backend/src/app/services.clj @@ -0,0 +1,43 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; This Source Code Form is "Incompatible With Secondary Licenses", as +;; defined by the Mozilla Public License, v. 2.0. +;; +;; Copyright (c) 2020 UXBOX Labs SL + +(ns app.services + "A initialization of services." + (:require + [app.services.middleware :as middleware] + [app.util.dispatcher :as uds] + [mount.core :as mount :refer [defstate]])) + +;; --- Initialization + +(defn- load-query-services + [] + (require 'app.services.queries.projects) + (require 'app.services.queries.files) + (require 'app.services.queries.comments) + (require 'app.services.queries.profile) + (require 'app.services.queries.recent-files) + (require 'app.services.queries.viewer)) + +(defn- load-mutation-services + [] + (require 'app.services.mutations.demo) + (require 'app.services.mutations.media) + (require 'app.services.mutations.projects) + (require 'app.services.mutations.files) + (require 'app.services.mutations.comments) + (require 'app.services.mutations.profile) + (require 'app.services.mutations.viewer) + (require 'app.services.mutations.verify-token)) + +(defstate query-services + :start (load-query-services)) + +(defstate mutation-services + :start (load-mutation-services)) diff --git a/backend/src/app/services/init.clj b/backend/src/app/services/init.clj index d0b69fa73..6223b121b 100644 --- a/backend/src/app/services/init.clj +++ b/backend/src/app/services/init.clj @@ -7,33 +7,4 @@ ;; ;; Copyright (c) 2020 UXBOX Labs SL -(ns app.services.init - "A initialization of services." - (:require - [mount.core :as mount :refer [defstate]])) - -(defn- load-query-services - [] - (require 'app.services.queries.projects) - (require 'app.services.queries.files) - (require 'app.services.queries.comments) - (require 'app.services.queries.profile) - (require 'app.services.queries.recent-files) - (require 'app.services.queries.viewer)) - -(defn- load-mutation-services - [] - (require 'app.services.mutations.demo) - (require 'app.services.mutations.media) - (require 'app.services.mutations.projects) - (require 'app.services.mutations.files) - (require 'app.services.mutations.comments) - (require 'app.services.mutations.profile) - (require 'app.services.mutations.viewer) - (require 'app.services.mutations.verify-token)) - -(defstate query-services - :start (load-query-services)) - -(defstate mutation-services - :start (load-mutation-services)) +(ns app.services.init) diff --git a/backend/src/app/worker.clj b/backend/src/app/worker.clj index 2b71beacd..f7de49abf 100644 --- a/backend/src/app/worker.clj +++ b/backend/src/app/worker.clj @@ -380,7 +380,7 @@ (defn thread-pool ([] (thread-pool {})) ([{:keys [min-threads max-threads name] - :or {min-threads 0 max-threads 128}}] + :or {min-threads 0 max-threads 256}}] (let [executor (QueuedThreadPool. max-threads min-threads)] (.setName executor (or name "default-tp")) (.start executor) diff --git a/backend/tests/app/tests/helpers.clj b/backend/tests/app/tests/helpers.clj index 2ae597a02..4ff4232df 100644 --- a/backend/tests/app/tests/helpers.clj +++ b/backend/tests/app/tests/helpers.clj @@ -17,7 +17,7 @@ [mount.core :as mount] [environ.core :refer [env]] [app.common.pages :as cp] - [app.services.init] + [app.services] [app.services.mutations.profile :as profile] [app.services.mutations.projects :as projects] [app.services.mutations.teams :as teams] @@ -50,8 +50,8 @@ #'app.redis/client #'app.redis/conn #'app.media/semaphore - #'app.services.init/query-services - #'app.services.init/mutation-services + #'app.services/query-services + #'app.services/mutation-services #'app.migrations/migrations #'app.media-storage/assets-storage #'app.media-storage/media-storage}) diff --git a/common/app/common/data.cljc b/common/app/common/data.cljc index 5cde2320a..a52991a68 100644 --- a/common/app/common/data.cljc +++ b/common/app/common/data.cljc @@ -6,7 +6,7 @@ (ns app.common.data "Data manipulation and query helper functions." - (:refer-clojure :exclude [concat read-string hash-map]) + (:refer-clojure :exclude [concat read-string hash-map merge]) #?(:cljs (:require-macros [app.common.data])) (:require @@ -210,6 +210,17 @@ (assoc m key v) m))) +(defn merge + "A faster merge." + [& maps] + (loop [res (transient (first maps)) + maps (next maps)] + (if (nil? maps) + (persistent! res) + (recur (reduce-kv assoc! res (first maps)) + (next maps))))) + + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Data Parsing / Conversion ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/common/app/common/exceptions.cljc b/common/app/common/exceptions.cljc index 69885361b..389178255 100644 --- a/common/app/common/exceptions.cljc +++ b/common/app/common/exceptions.cljc @@ -48,3 +48,7 @@ (defmacro try [& exprs] `(try* (^:once fn* [] ~@exprs) identity)) + +(defn ex-info? + [v] + (instance? #?(:clj clojure.lang.ExceptionInfo :cljs cljs.core.ExceptionInfo) v)) diff --git a/common/app/common/spec.cljc b/common/app/common/spec.cljc index 1292e1bf9..929396e91 100644 --- a/common/app/common/spec.cljc +++ b/common/app/common/spec.cljc @@ -189,9 +189,7 @@ (let [edata (s/explain-data spec data)] (throw (ex/error :type :validation :code :spec-validation - :explain (with-out-str - (expound/printer edata)) - :data (::s/problems edata))))) + :data data)))) result)) (defmacro instrument!