diff --git a/CHANGES.md b/CHANGES.md index 6759e02da5..888a2dc045 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,6 +9,19 @@ ### :heart: Community contributions by (Thank you!) +# 1.10.4-beta + +### :sparkles: Enhacements + +- Allow parametrice file snapshoting interval. + +### :bug: Bugs fixed + +- Fix issue on :mov-object change impl. +- Minor fix on how file changes log is persisted. +- Fix many issues on error reporting. + + # 1.10.3-beta ### :sparkles: Enhacements diff --git a/backend/resources/error-list.tmpl b/backend/resources/error-list.tmpl new file mode 100644 index 0000000000..360734e142 --- /dev/null +++ b/backend/resources/error-list.tmpl @@ -0,0 +1,94 @@ + + + + + + + penpot - error report {{id}} + + + + + + +
+ +
+ + + diff --git a/backend/resources/error-report.tmpl b/backend/resources/error-report.tmpl index 2ff0abb1f2..ad663f604b 100644 --- a/backend/resources/error-report.tmpl +++ b/backend/resources/error-report.tmpl @@ -13,15 +13,40 @@ } pre { margin: 0px; + line-height: 17px; } + + main { + margin: 20px; + } + + nav { + position: fixed; + width: 100vw; + top: 0; + left: 0; + padding: 5px 20px; + display: flex; + background: #e3e3e3; + } + + nav > div { + text-transform: uppercase; + font-weight: bold; + } + + nav > div:not(:last-child) { + margin-right: 10px; + } + * { font-family: "JetBrains Mono", monospace; font-size: 12px; } .table { + margin-top: 25px; display: flex; flex-direction: column; - margin: 10px; } .table-row { @@ -34,6 +59,9 @@ font-weight: 600; width: 60px; padding: 4px; + + padding-top: 40px; + margin-top: -40px; } .table-val { @@ -57,147 +85,70 @@ -
-
-
ERID:
-
{{id}}
-
- {% if profile-id %} -
-
PFID:
-
{{profile-id}}
-
- {% endif %} - {% if user-agent %} -
-
UAGT:
-
{{user-agent}}
-
- {% endif %} - - {% if frontend-version %} -
-
FVER:
-
{{frontend-version}}
-
- {% endif %} - -
-
BVER:
-
{{version}}
-
- - {% if host %} -
-
HOST:
-
{{host}}
-
- {% endif %} - - {% if tenant %} -
-
ENV:
-
{{tenant}}
-
- {% endif %} - - {% if public-uri %} -
-
PURI:
-
{{public-uri}}
-
- {% endif %} - - {% if type %} -
-
TYPE:
-
{{type}}
-
- {% endif %} - - {% if code %} -
-
CODE:
-
{{code}}
-
- {% endif %} - - {% if error %} -
-
CLSS:
-
{{error.class}}
-
- {% endif %} - - {% if hint %} -
-
HINT:
-
{{hint}}
-
- {% endif %} - - {% if method %} -
-
PATH:
-
{{method|upper}} {{path}}
-
- {% endif %} - -
(go to explain)
-
(go to edata)
-
(go to trace)
- - {% if params %} -
-
PARAMS:
-
- 
{{params}}
-
-
+ +
+
+
+
CONTEXT:
+
+ 
{{context}}
+
+
- {% if spec-problems %} -
-
SPEC PROBLEMS:
-
- 
{{spec-problems}}
+ {% if params %} +
+
PARAMS:
+
+ 
{{params}}
+
-
- {% endif %} + {% endif %} - {% if cause %} -
-
TRACE:
-
- 
{{cause}}
+ + {% if data %} +
+
ERROR DATA:
+
+ 
{{data}}
+
-
- {% elif trace %} -
-
TRACE:
-
- 
{{trace}}
+ {% endif %} + + {% if spec-problems %} +
+
SPEC PROBLEMS:
+
+ 
{{spec-problems}}
+
-
- {% elif error %} -
-
TRACE:
-
- 
{{error.trace}}
+ {% endif %} + + {% if trace %} +
+
TRACE:
+
+ 
{{trace}}
+
+ {% endif %}
- {% endif %} -
+
diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 98c003e550..615401b67c 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -51,6 +51,9 @@ :default-blob-version 3 :loggers-zmq-uri "tcp://localhost:45556" + :file-change-snapshot-every 5 + :file-change-snapshot-timeout "3h" + :public-uri "http://localhost:3449" :redis-uri "redis://redis/0" @@ -98,6 +101,10 @@ (s/def ::audit-log-archive-uri ::us/string) (s/def ::audit-log-gc-max-age ::dt/duration) +(s/def ::admins ::us/set-of-str) +(s/def ::file-change-snapshot-every ::us/integer) +(s/def ::file-change-snapshot-timeout ::dt/duration) + (s/def ::secret-key ::us/string) (s/def ::allow-demo-users ::us/boolean) (s/def ::assets-path ::us/string) @@ -185,6 +192,7 @@ (s/def ::config (s/keys :opt-un [::secret-key ::flags + ::admins ::allow-demo-users ::audit-log-archive-uri ::audit-log-gc-max-age @@ -193,6 +201,8 @@ ::database-username ::default-blob-version ::error-report-webhook + ::file-change-snapshot-every + ::file-change-snapshot-timeout ::user-feedback-destination ::github-client-id ::github-client-secret diff --git a/backend/src/app/http.clj b/backend/src/app/http.clj index 3d8185be19..e413207a38 100644 --- a/backend/src/app/http.clj +++ b/backend/src/app/http.clj @@ -12,6 +12,7 @@ [app.common.spec :as us] [app.http.doc :as doc] [app.http.errors :as errors] + [app.http.debug :as debug] [app.http.middleware :as middleware] [app.metrics :as mtx] [clojure.spec.alpha :as s] @@ -104,17 +105,16 @@ (s/def ::storage map?) (s/def ::assets map?) (s/def ::feedback fn?) -(s/def ::error-report-handler fn?) (s/def ::audit-http-handler fn?) +(s/def ::debug map?) (defmethod ig/pre-init-spec ::router [_] (s/keys :req-un [::rpc ::session ::mtx/metrics ::oauth ::storage ::assets ::feedback - ::error-report-handler - ::audit-http-handler])) + ::debug ::audit-http-handler])) (defmethod ig/init-key ::router - [_ {:keys [session rpc oauth metrics assets feedback] :as cfg}] + [_ {:keys [session rpc oauth metrics assets feedback debug] :as cfg}] (rr/router [["/metrics" {:get (:handler metrics)}] ["/assets" {:middleware [[middleware/format-response-body] @@ -125,8 +125,17 @@ ["/by-file-media-id/:id" {:get (:file-objects-handler assets)}] ["/by-file-media-id/:id/thumbnail" {:get (:file-thumbnails-handler assets)}]] - ["/dbg" - ["/error-by-id/:id" {:get (:error-report-handler cfg)}]] + ["/dbg" {:middleware [[middleware/params] + [middleware/keyword-params] + [middleware/format-response-body] + [middleware/errors errors/handle] + [middleware/cookies] + [(:middleware session)]]} + ["/error-by-id/:id" {:get (:retrieve-error debug)}] + ["/error/:id" {:get (:retrieve-error debug)}] + ["/error" {:get (:retrieve-error-list debug)}] + ["/file/data/:id" {:get (:retrieve-file-data debug)}] + ["/file/changes/:id" {:get (:retrieve-file-changes debug)}]] ["/webhooks" ["/sns" {:post (:sns-webhook cfg)}]] diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj new file mode 100644 index 0000000000..1739a476dc --- /dev/null +++ b/backend/src/app/http/debug.clj @@ -0,0 +1,165 @@ +;; 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/. +;; +;; Copyright (c) UXBOX Labs SL + +(ns app.http.debug + (:require + [app.common.data :as d] + [app.common.exceptions :as ex] + [app.common.spec :as us] + [app.common.transit :as t] + [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] + [app.rpc.queries.profile :as profile] + [app.util.blob :as blob] + [app.util.json :as json] + [app.util.template :as tmpl] + [app.util.time :as dt] + [clojure.java.io :as io] + [clojure.pprint :as ppr] + [clojure.spec.alpha :as s] + [cuerdas.core :as str] + [integrant.core :as ig])) + +(def sql:retrieve-range-of-changes + "select revn, changes from file_change where file_id=? and revn >= ? and revn <= ? order by revn") + +(def sql:retrieve-single-change + "select revn, changes, data from file_change where file_id=? and revn = ?") + +(defn authorized? + [pool {:keys [profile-id]}] + (or (= "devenv" (cf/get :host)) + (let [profile (ex/ignoring (profile/retrieve-profile-data pool profile-id)) + admins (or (cf/get :admins) #{})] + (contains? admins (:email profile))))) + +(defn prepare-response + [body] + (when-not body + (ex/raise :type :not-found + :code :enpty-data + :hint "empty response")) + + {:status 200 + :headers {"content-type" "application/transit+json"} + :body body}) + +(defn retrieve-file-data + [{:keys [pool]} request] + (when-not (authorized? pool request) + (ex/raise :type :authentication + :code :only-admins-allowed)) + + (let [id (some-> (get-in request [:path-params :id]) uuid/uuid) + revn (some-> (get-in request [:params :revn]) d/parse-integer)] + (when-not id + (ex/raise :type :validation + :code :missing-arguments)) + + (if (integer? revn) + (let [fchange (db/exec-one! pool [sql:retrieve-single-change id revn])] + (prepare-response (some-> fchange :data blob/decode))) + + (let [file (db/get-by-id pool :file id)] + (prepare-response (some-> file :data blob/decode)))))) + +(defn retrieve-file-changes + [{:keys [pool]} {:keys [params path-params profile-id] :as request}] + (when-not (authorized? pool request) + (ex/raise :type :authentication + :code :only-admins-allowed)) + + (let [id (some-> (get-in request [:path-params :id]) uuid/uuid) + revn (get-in request [:params :revn] "latest")] + + (when (or (not id) (not revn)) + (ex/raise :type :validation + :code :invalid-arguments + :hint "missing arguments")) + + (cond + (d/num-string? revn) + (let [item (db/exec-one! pool [sql:retrieve-single-change id (d/parse-integer revn)])] + (prepare-response (some-> item :changes blob/decode vec))) + + (str/includes? revn ":") + (let [[start end] (->> (str/split revn #":") + (map str/trim) + (map d/parse-integer)) + items (db/exec! pool [sql:retrieve-range-of-changes id start end])] + (prepare-response (some->> items + (map :changes) + (map blob/decode) + (mapcat identity) + (vec)))) + + :else + (ex/raise :type :validation :code :invalid-arguments)))) + + +(defn retrieve-error + [{:keys [pool]} request] + (letfn [(parse-id [request] + (let [id (get-in request [:path-params :id]) + id (us/uuid-conformer id)] + (when (uuid? id) + id))) + + (retrieve-report [id] + (ex/ignoring + (some-> (db/get-by-id pool :server-error-report id) :content db/decode-transit-pgobject))) + + (render-template [report] + (binding [ppr/*print-right-margin* 300] + (let [context (dissoc report :trace :cause :params :data :spec-prob :spec-problems) + params {:context (with-out-str (ppr/pprint context)) + :data (:data report) + :trace (or (:cause report) + (:trace report) + (some-> report :error :trace)) + :params (:params report)}] + (-> (io/resource "error-report.tmpl") + (tmpl/render params))))) + ] + + (when-not (authorized? pool request) + (ex/raise :type :authentication + :code :only-admins-allowed)) + + (let [result (some-> (parse-id request) + (retrieve-report) + (render-template))] + (if result + {:status 200 + :headers {"content-type" "text/html; charset=utf-8" + "x-robots-tag" "noindex"} + :body result} + {:status 404 + :body "not found"})))) + +(def sql:error-reports + "select id, created_at from server_error_report order by created_at desc limit 100") + +(defn retrieve-error-list + [{:keys [pool]} request] + (when-not (authorized? pool request) + (ex/raise :type :authentication + :code :only-admins-allowed)) + (let [items (db/exec! pool [sql:error-reports]) + items (map #(update % :created-at dt/format-instant :rfc1123) items)] + {:status 200 + :headers {"content-type" "text/html; charset=utf-8" + "x-robots-tag" "noindex"} + :body (-> (io/resource "error-list.tmpl") + (tmpl/render {:items items}))})) + +(defmethod ig/init-key ::handlers + [_ {:keys [pool] :as cfg}] + {:retrieve-file-data (partial retrieve-file-data cfg) + :retrieve-file-changes (partial retrieve-file-changes cfg) + :retrieve-error (partial retrieve-error cfg) + :retrieve-error-list (partial retrieve-error-list cfg)}) diff --git a/backend/src/app/http/errors.clj b/backend/src/app/http/errors.clj index fac7094471..99992a9677 100644 --- a/backend/src/app/http/errors.clj +++ b/backend/src/app/http/errors.clj @@ -30,14 +30,13 @@ :hint (or (:hint data) (ex-message error)) :params (l/stringify-data (:params request)) :spec-problems (some-> data ::s/problems) + :data (some-> data (dissoc ::s/problems)) :ip-addr (parse-client-ip request) :profile-id (:profile-id request)} (let [headers (:headers request)] {:user-agent (get headers "user-agent") - :frontend-version (get headers "x-frontend-version" "unknown")}) - - (dissoc data ::s/problems)))) + :frontend-version (get headers "x-frontend-version" "unknown")})))) (defmulti handle-exception (fn [err & _rest] @@ -54,18 +53,9 @@ {:status 400 :body (ex-data err)}) (defmethod handle-exception :validation - [err req] - (let [header (get-in req [:headers "accept"]) - edata (ex-data err)] - (if (and (= :spec-validation (:code edata)) - (str/starts-with? header "text/html")) - {:status 400 - :headers {"content-type" "text/html"} - :body (str "
"
-                  (:explain edata)
-                  "
\n")} - {:status 400 - :body (dissoc edata ::s/problems)}))) + [err _] + (let [edata (ex-data err)] + {:status 400 :body (dissoc edata ::s/problems)})) (defmethod handle-exception :assertion [error request] diff --git a/backend/src/app/http/session.clj b/backend/src/app/http/session.clj index 2888961402..f341f91da2 100644 --- a/backend/src/app/http/session.clj +++ b/backend/src/app/http/session.clj @@ -73,6 +73,7 @@ (if-let [{:keys [id profile-id] :as session} (retrieve-from-request cfg request)] (do (a/>!! (::events-ch cfg) id) + (l/set-context! {:profile-id profile-id}) (handler (assoc request :profile-id profile-id))) (handler request)))) diff --git a/backend/src/app/loggers/database.clj b/backend/src/app/loggers/database.clj index ca0fb5d6eb..3094405688 100644 --- a/backend/src/app/loggers/database.clj +++ b/backend/src/app/loggers/database.clj @@ -14,10 +14,8 @@ [app.config :as cf] [app.db :as db] [app.util.async :as aa] - [app.util.template :as tmpl] [app.worker :as wrk] [clojure.core.async :as a] - [clojure.java.io :as io] [clojure.spec.alpha :as s] [cuerdas.core :as str] [integrant.core :as ig])) @@ -60,8 +58,10 @@ [{:keys [executor] :as cfg} event] (aa/with-thread executor (try - (let [event (parse-event event)] - (l/debug :hint "registering error on database" :id (:id event)) + (let [event (parse-event event) + uri (cf/get :public-uri)] + (l/debug :hint "registering error on database" :id (:id event) + :uri (str uri "/dbg/error/" (:id event))) (persist-on-database! cfg event)) (catch Exception e (l/warn :hint "unexpected exception on database error logger" @@ -89,39 +89,3 @@ (defmethod ig/halt-key! ::reporter [_ output] (a/close! output)) - -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; Http Handler -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(defmethod ig/pre-init-spec ::handler [_] - (s/keys :req-un [::db/pool])) - -(defmethod ig/init-key ::handler - [_ {:keys [pool] :as cfg}] - (letfn [(parse-id [request] - (let [id (get-in request [:path-params :id]) - id (us/uuid-conformer id)] - (when (uuid? id) - id))) - (retrieve-report [id] - (ex/ignoring - (when-let [{:keys [content] :as row} (db/get-by-id pool :server-error-report id)] - (assoc row :content (db/decode-transit-pgobject content))))) - - (render-template [{:keys [content] :as report}] - (some-> (io/resource "error-report.tmpl") - (tmpl/render content)))] - - - (fn [request] - (let [result (some-> (parse-id request) - (retrieve-report) - (render-template))] - (if result - {:status 200 - :headers {"content-type" "text/html; charset=utf-8" - "x-robots-tag" "noindex"} - :body result} - {:status 404 - :body "not found"}))))) diff --git a/backend/src/app/loggers/mattermost.clj b/backend/src/app/loggers/mattermost.clj index 07da27aadc..367fe6603e 100644 --- a/backend/src/app/loggers/mattermost.clj +++ b/backend/src/app/loggers/mattermost.clj @@ -25,7 +25,7 @@ [cfg {:keys [host id public-uri] :as event}] (try (let [uri (:uri cfg) - text (str "Exception on (host: " host ", url: " public-uri "/dbg/error-by-id/" id ")\n" + text (str "Exception on (host: " host ", url: " public-uri "/dbg/error/" id ")\n" (when-let [pid (:profile-id event)] (str "- profile-id: #uuid-" pid "\n"))) rsp (http/send! {:uri uri diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 0681363363..d90acb5f07 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -106,8 +106,11 @@ :storage (ig/ref :app.storage/storage) :sns-webhook (ig/ref :app.http.awsns/handler) :feedback (ig/ref :app.http.feedback/handler) - :audit-http-handler (ig/ref :app.loggers.audit/http-handler) - :error-report-handler (ig/ref :app.loggers.database/handler)} + :debug (ig/ref :app.http.debug/handlers) + :audit-http-handler (ig/ref :app.loggers.audit/http-handler)} + + :app.http.debug/handlers + {:pool (ig/ref :app.db/pool)} :app.http.assets/handlers {:metrics (ig/ref :app.metrics/metrics) @@ -306,9 +309,6 @@ :pool (ig/ref :app.db/pool) :executor (ig/ref :app.worker/executor)} - :app.loggers.database/handler - {:pool (ig/ref :app.db/pool)} - :app.loggers.sentry/reporter {:dsn (cf/get :sentry-dsn) :trace-sample-rate (cf/get :sentry-trace-sample-rate 1.0) diff --git a/backend/src/app/rpc/mutations/files.clj b/backend/src/app/rpc/mutations/files.clj index d84911ef84..7f089eff8f 100644 --- a/backend/src/app/rpc/mutations/files.clj +++ b/backend/src/app/rpc/mutations/files.clj @@ -11,6 +11,7 @@ [app.common.pages.migrations :as pmg] [app.common.spec :as us] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.metrics :as mtx] [app.rpc.permissions :as perms] @@ -280,11 +281,13 @@ (defn- take-snapshot? "Defines the rule when file `data` snapshot should be saved." [{:keys [revn modified-at] :as file}] - ;; The snapshot will be saved every 20 changes or if the last - ;; modification is older than 3 hour. - (or (zero? (mod revn 20)) - (> (inst-ms (dt/diff modified-at (dt/now))) - (inst-ms (dt/duration {:hours 3}))))) + (let [freq (or (cf/get :file-change-snapshot-every) 20) + timeout (or (cf/get :file-change-snapshot-timeout) + (dt/duration {:hours 1}))] + (or (= 1 freq) + (zero? (mod revn freq)) + (> (inst-ms (dt/diff modified-at (dt/now))) + (inst-ms timeout))))) (defn- delete-from-storage [{:keys [storage] :as cfg} file] @@ -309,6 +312,8 @@ (mapcat :changes changes-with-metadata) changes) + changes (vec changes) + ;; Trace the number of changes processed _ ((::mtx/fn mtx1) {:by (count changes)}) diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index d43d9eb304..a23dfe101d 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -252,6 +252,11 @@ #?(:clj (Object.) :cljs (js/Object.))) +(defn getf + "Returns a function to access a map" + [coll] + (partial get coll)) + (defn update-in-when [m key-seq f & args] (let [found (get-in m key-seq sentinel)] diff --git a/common/src/app/common/logging.cljc b/common/src/app/common/logging.cljc index f9356e5880..b9940f1ea2 100644 --- a/common/src/app/common/logging.cljc +++ b/common/src/app/common/logging.cljc @@ -65,7 +65,7 @@ val (coll? val) - (binding [clojure.pprint/*print-right-margin* 120] + (binding [clojure.pprint/*print-right-margin* 200] (-> (with-out-str (pprint val)) (simple-prune (* 1024 1024 3)))) @@ -83,6 +83,12 @@ (stringify-data val)]))) data))) +#?(:clj + (defn set-context! + [data] + (ThreadContext/putAll (data->context-map data)) + nil)) + #?(:clj (defmacro with-context [data & body] diff --git a/common/src/app/common/pages/changes.cljc b/common/src/app/common/pages/changes.cljc index dd469f2542..295489f284 100644 --- a/common/src/app/common/pages/changes.cljc +++ b/common/src/app/common/pages/changes.cljc @@ -195,7 +195,8 @@ [data {:keys [parent-id shapes index page-id component-id ignore-touched]}] (letfn [(is-valid-move? [objects shape-id] (let [invalid-targets (cph/calculate-invalid-targets shape-id objects)] - (and (not (invalid-targets parent-id)) + (and (contains? objects shape-id) + (not (invalid-targets parent-id)) (cph/valid-frame-target shape-id parent-id objects)))) (insert-items [prev-shapes index shapes] diff --git a/common/src/app/common/spec.cljc b/common/src/app/common/spec.cljc index 5a2f37065d..e674a32b5a 100644 --- a/common/src/app/common/spec.cljc +++ b/common/src/app/common/spec.cljc @@ -256,7 +256,7 @@ (let [data (s/explain-data spec data)] (throw (ex/error :type :validation :code :spec-validation - :data data)))) + ::s/problems (::s/problems data))))) result)) (defmacro instrument! diff --git a/frontend/src/app/main/data/workspace/changes.cljs b/frontend/src/app/main/data/workspace/changes.cljs index 309d47c523..241229fe88 100644 --- a/frontend/src/app/main/data/workspace/changes.cljs +++ b/frontend/src/app/main/data/workspace/changes.cljs @@ -120,8 +120,7 @@ (log/debug :msg "commit-changes" :js/redo-changes redo-changes :js/undo-changes undo-changes) - - (let [error (volatile! nil)] + (let [error (volatile! nil)] (ptk/reify ::commit-changes cljs.core/IDeref (-deref [_]