Merge pull request #5019 from penpot/niwinz-static-error-duplication

✨ Add improvements on how UI (React) errors are handled

frontend/deps.edn

@@ -20,8 +20,8 @@
    :git/url "https://github.com/funcool/beicon.git"}
 
   funcool/rumext
-  {:git/tag "v2.13"
+  {:git/tag "v2.14"
-   :git/sha "dc8e1e5"
+   :git/sha "0016623"
    :git/url "https://github.com/funcool/rumext.git"}
 
   instaparse/instaparse {:mvn/version "1.5.0"}

frontend/package.json

@@ -103,6 +103,7 @@
     "randomcolor": "^0.6.2",
     "react": "18.3.1",
     "react-dom": "18.3.1",
+    "react-error-boundary": "^4.0.13",
     "react-virtualized": "^9.22.5",
     "rxjs": "8.0.0-alpha.14",
     "sax": "^1.4.1",

frontend/src/app/main/errors.cljs

@@ -56,6 +56,14 @@
                           (print-explain! cause)
                           (print-trace! cause))))
 
+(defn exception->error-data
+  [cause]
+  (let [data (ex-data cause)]
+    (-> data
+        (assoc :hint (or (:hint data) (ex-message cause)))
+        (assoc ::instance cause)
+        (assoc ::trace (.-stack cause)))))
+
 (defn print-error!
   [cause]
   (cond
@@ -66,22 +74,14 @@
     (print-cause! (ex-message cause) (ex-data cause))
 
     :else
-    (let [trace (.-stack cause)]
+    (print-cause! (ex-message cause) (exception->error-data cause))))
-      (print-cause! (ex-message cause)
-                    {:hint (ex-message cause)
-                     ::trace trace
-                     ::instance cause}))))
 
 (defn on-error
   "A general purpose error handler."
   [error]
   (if (map? error)
     (ptk/handle-error error)
-    (let [data (ex-data error)
+    (let [data (exception->error-data error)]
-          data (-> data
-                   (assoc :hint (or (:hint data) (ex-message error)))
-                   (assoc ::instance error)
-                   (assoc ::trace (.-stack error)))]
       (ptk/handle-error data))))
 
 ;; Set the main potok error handler
@@ -285,6 +285,7 @@
             (let [message (ex-message cause)]
               (or (= message "Possible side-effect in debug-evaluate")
                   (= message "Unexpected end of input")
+                  (str/starts-with? message "invalid props on component")
                   (str/starts-with? message "Unexpected token "))))
 
           (on-unhandled-error [event]

frontend/src/app/main/ui.cljs

@@ -8,9 +8,9 @@
   (:require
    [app.config :as cf]
    [app.main.refs :as refs]
-   [app.main.store :as st]
    [app.main.ui.context :as ctx]
    [app.main.ui.debug.icons-preview :refer [icons-preview]]
+   [app.main.ui.error-boundary :refer [error-boundary*]]
    [app.main.ui.frame-preview :as frame-preview]
    [app.main.ui.icons :as i]
    [app.main.ui.notifications :as notifications]
@@ -21,7 +21,6 @@
    [app.main.ui.static :as static]
    [app.util.dom :as dom]
    [app.util.i18n :refer [tr]]
-   [app.util.router :as rt]
    [rumext.v2 :as mf]))
 
 (def auth-page
@@ -42,15 +41,8 @@
 (def workspace-page
   (mf/lazy-component app.main.ui.workspace/workspace))
 
-(mf/defc on-main-error
-  [{:keys [error] :as props}]
-  (mf/with-effect
-    (st/emit! (rt/assign-exception error)))
-  [:span "Internal application error"])
-
 (mf/defc main-page
-  {::mf/wrap [#(mf/catch % {:fallback on-main-error})]
+  {::mf/props :obj}
-   ::mf/props :obj}
   [{:keys [route profile]}]
   (let [{:keys [data params]} route]
     [:& (mf/provider ctx/current-route) {:value route}
@@ -137,7 +129,7 @@
              {:keys [file-id]} path-params]
          [:? {}
           (if (:token query-params)
-            [:> static/error-container {}
+            [:> static/error-container* {}
              [:div.image i/detach]
              [:div.main-message (tr "viewer.breaking-change.message")]
              [:div.desc-message (tr "viewer.breaking-change.description")]]
@@ -186,8 +178,8 @@
     [:& (mf/provider ctx/current-route) {:value route}
      [:& (mf/provider ctx/current-profile) {:value profile}
       (if edata
-        [:& static/exception-page {:data edata :route route}]
+        [:> static/exception-page* {:data edata :route route}]
-        [:*
+        [:> error-boundary* {:fallback static/internal-error*}
          [:& notifications/current-notification]
          (when route
            [:& main-page {:route route :profile profile}])])]]))

frontend/src/app/main/ui/error_boundary.cljs

@@ -0,0 +1,47 @@
+;; 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) KALEIDOS INC
+
+(ns app.main.ui.error-boundary
+  "React error boundary components"
+  (:require
+   ["react-error-boundary" :as reb]
+   [app.main.errors :as errors]
+   [app.main.refs :as refs]
+   [goog.functions :as gfn]
+   [rumext.v2 :as mf]))
+
+(mf/defc error-boundary*
+  {::mf/props :obj}
+  [{:keys [fallback children]}]
+  (let [fallback-wrapper
+        (mf/with-memo [fallback]
+          (mf/fnc fallback-wrapper*
+            {::mf/props :obj}
+            [{:keys [error reset-error-boundary]}]
+            (let [route (mf/deref refs/route)
+                  data  (errors/exception->error-data error)]
+              [:> fallback {:data data
+                            :route route
+                            :on-reset reset-error-boundary}])))
+
+        on-error
+        (mf/with-memo []
+          ;; NOTE: The debounce is necessary just for simplicity,
+          ;; becuase for some reasons the error is reported twice in a
+          ;; very small amount of time, so we debounce for 100ms for
+          ;; avoid duplicate and redundant reports
+          (gfn/debounce (fn [error info]
+                          (js/console.error
+                           "Component trace: \n"
+                           (unchecked-get info "componentStack")
+                           "\n"
+                           error))
+                        100))]
+
+    [:> reb/ErrorBoundary
+     {:FallbackComponent fallback-wrapper
+      :onError on-error}
+     children]))

frontend/src/app/main/ui/static.cljs

@@ -29,8 +29,8 @@
    [potok.v2.core :as ptk]
    [rumext.v2 :as mf]))
 
-(mf/defc error-container
+(mf/defc error-container*
-  {::mf/wrap-props false}
+  {::mf/props :obj}
   [{:keys [children]}]
   (let [profile-id (:profile-id @st/state)]
     [:section {:class (stl/css :exception-layout)}
@@ -57,12 +57,10 @@
 
 (mf/defc invalid-token
   []
-  [:> error-container {}
+  [:> error-container* {}
    [:div {:class (stl/css :main-message)} (tr "errors.invite-invalid")]
    [:div {:class (stl/css :desc-message)} (tr "errors.invite-invalid.info")]])
 
-
-
 (mf/defc login-dialog
   {::mf/props :obj}
   [{:keys [show-dialog]}]
@@ -247,37 +245,49 @@
          [:& request-dialog {:title (tr "not-found.no-permission.project") :button-text (tr "not-found.no-permission.go-dashboard")}]
 
          (and (some? file-id) (:already-requested requested))
-         [:& request-dialog {:title (tr "not-found.no-permission.already-requested.file") :content [(tr "not-found.no-permission.already-requested.or-others.file")] :button-text (tr "not-found.no-permission.go-dashboard")}]
+         [:& request-dialog {:title (tr "not-found.no-permission.already-requested.file")
+                             :content [(tr "not-found.no-permission.already-requested.or-others.file")]
+                             :button-text (tr "not-found.no-permission.go-dashboard")}]
 
          (:already-requested requested)
-         [:& request-dialog {:title (tr "not-found.no-permission.already-requested.project") :content [(tr "not-found.no-permission.already-requested.or-others.project")] :button-text (tr "not-found.no-permission.go-dashboard")}]
+         [:& request-dialog {:title (tr "not-found.no-permission.already-requested.project")
+                             :content [(tr "not-found.no-permission.already-requested.or-others.project")]
+                             :button-text (tr "not-found.no-permission.go-dashboard")}]
 
          (:sent requested)
-         [:& request-dialog {:title (tr "not-found.no-permission.done.success") :content [(tr "not-found.no-permission.done.remember")] :button-text (tr "not-found.no-permission.go-dashboard")}]
+         [:& request-dialog {:title (tr "not-found.no-permission.done.success")
+                             :content [(tr "not-found.no-permission.done.remember")]
+                             :button-text (tr "not-found.no-permission.go-dashboard")}]
 
          (some? file-id)
-         [:& request-dialog {:title (tr "not-found.no-permission.file") :content [(tr "not-found.no-permission.you-can-ask.file") (tr "not-found.no-permission.if-approves")] :button-text (tr "not-found.no-permission.ask") :on-button-click on-request-access :cancel-text (tr "not-found.no-permission.go-dashboard")}]
+         [:& request-dialog {:title (tr "not-found.no-permission.file")
+                             :content [(tr "not-found.no-permission.you-can-ask.file")
+                                       (tr "not-found.no-permission.if-approves")]
+                             :button-text (tr "not-found.no-permission.ask")
+                             :on-button-click on-request-access
+                             :cancel-text (tr "not-found.no-permission.go-dashboard")}]
 
          (some? team-id)
-         [:& request-dialog {:title (tr "not-found.no-permission.project") :content [(tr "not-found.no-permission.you-can-ask.project") (tr "not-found.no-permission.if-approves")] :button-text (tr "not-found.no-permission.ask") :on-button-click on-request-access :cancel-text (tr "not-found.no-permission.go-dashboard")}]))]))
+         [:& request-dialog {:title (tr "not-found.no-permission.project")
+                             :content [(tr "not-found.no-permission.you-can-ask.project")
+                                       (tr "not-found.no-permission.if-approves")]
+                             :button-text (tr "not-found.no-permission.ask")
+                             :on-button-click on-request-access
+                             :cancel-text (tr "not-found.no-permission.go-dashboard")}]))]))
 
-
+(mf/defc not-found*
-
-(mf/defc not-found
   []
-  [:> error-container {}
+  [:> error-container* {}
    [:div {:class (stl/css :main-message)} (tr "labels.not-found.main-message")]
    [:div {:class (stl/css :desc-message)} (tr "not-found.desc-message.error")]
    [:div {:class (stl/css :desc-message)} (tr "not-found.desc-message.doesnt-exist")]])
 
-
+(mf/defc bad-gateway*
-
-(mf/defc bad-gateway
   []
   (let [handle-retry
         (mf/use-fn
          (fn [] (st/emit! (rt/assign-exception nil))))]
-    [:> error-container {}
+    [:> error-container* {}
      [:div {:class (stl/css :main-message)} (tr "labels.bad-gateway.main-message")]
      [:div {:class (stl/css :desc-message)} (tr "labels.bad-gateway.desc-message")]
      [:div {:class (stl/css :sign-info)}
@@ -286,13 +296,12 @@
 (mf/defc service-unavailable
   []
   (let [on-click (mf/use-fn #(st/emit! (rt/assign-exception nil)))]
-    [:> error-container {}
+    [:> error-container* {}
      [:div {:class (stl/css :main-message)} (tr "labels.service-unavailable.main-message")]
      [:div {:class (stl/css :desc-message)} (tr "labels.service-unavailable.desc-message")]
      [:div {:class (stl/css :sign-info)}
       [:button {:on-click on-click} (tr "labels.retry")]]]))
 
-
 (defn generate-report
   [data]
   (try
@@ -336,17 +345,16 @@
 
                        (println))]
       (wapi/create-blob content "text/plain"))
-    (catch :default err
+    (catch :default cause
-      (.error js/console err)
+      (.error js/console "error on generating report.txt" cause)
       nil)))
 
-
+(mf/defc internal-error*
-(mf/defc internal-error
   {::mf/props :obj}
-  [{:keys [data]}]
+  [{:keys [data on-reset] :as props}]
-  (let [on-click   (mf/use-fn #(st/emit! (rt/assign-exception nil)))
+  (let [report-uri (mf/use-ref nil)
-        report-uri (mf/use-ref nil)
         report     (mf/use-memo (mf/deps data) #(generate-report data))
+        on-reset   (or on-reset #(st/emit! (rt/assign-exception nil)))
 
         on-download
         (mf/use-fn
@@ -357,22 +365,24 @@
 
     (mf/with-effect [report]
       (when (some? report)
+
         (let [uri    (wapi/create-uri report)]
           (mf/set-ref-val! report-uri uri)
           (fn []
             (wapi/revoke-uri uri)))))
 
-    [:> error-container {}
+    [:> error-container* {}
      [:div {:class (stl/css :main-message)} (tr "labels.internal-error.main-message")]
      [:div {:class (stl/css :desc-message)} (tr "labels.internal-error.desc-message")]
      (when (some? report)
        [:a {:on-click on-download} "Download report.txt"])
      [:div {:class (stl/css :sign-info)}
-      [:button {:on-click on-click} (tr "labels.retry")]]]))
+      [:button {:on-click on-reset} (tr "labels.retry")]]]))
 
-(mf/defc exception-page
+(mf/defc exception-page*
   {::mf/props :obj}
   [{:keys [data route] :as props}]
+
   (let [file-info    (mf/use-state {:pending true})
         team-info    (mf/use-state {:pending true})
         type         (:type data)
@@ -409,18 +419,24 @@
     (case (:type data)
       :not-found
       (if request-access?
-        [:& request-access {:file-id (:file-id @file-info) :team-id  (:team-id @team-info) :is-default (:is-default @team-info) :workspace? workspace?}]
+        [:& request-access {:file-id (:file-id @file-info)
-        [:& not-found])
+                            :team-id  (:team-id @team-info)
+                            :is-default (:is-default @team-info)
+                            :workspace? workspace?}]
+        [:> not-found* {}])
 
       :authentication
       (if request-access?
-        [:& request-access {:file-id (:file-id @file-info) :team-id  (:team-id @team-info) :is-default (:is-default @team-info) :workspace? workspace?}]
+        [:& request-access {:file-id (:file-id @file-info)
-        [:& not-found])
+                            :team-id  (:team-id @team-info)
+                            :is-default (:is-default @team-info)
+                            :workspace? workspace?}]
+        [:> not-found* {}])
 
       :bad-gateway
-      [:& bad-gateway]
+      [:> bad-gateway* props]
 
       :service-unavailable
       [:& service-unavailable]
 
-      [:> internal-error props])))
+      [:> internal-error* props])))

frontend/src/app/main/ui/workspace/sidebar.cljs

@@ -73,47 +73,40 @@
         on-tab-change
         (mf/use-fn #(st/emit! (dw/go-to-layout (keyword %))))
 
-        tabs (if ^boolean mode-inspect?
+        layers-tab
-               #js [#js {:label (tr "workspace.sidebar.layers")
+        (mf/html
-                         :id "layers"
+         [:article {:class (stl/css :layers-tab)
-                         :content (mf/html [:article {:class (stl/css :layers-tab)
+                    :style #js {"--height" (str size-pages "px")}}
-                                                      :style #js {"--height" (str size-pages "px")}}
 
-                                            [:& sitemap {:layout layout
+          [:& sitemap {:layout layout
-                                                         :toggle-pages toggle-pages
+                       :toggle-pages toggle-pages
-                                                         :show-pages? @show-pages?
+                       :show-pages? @show-pages?
-                                                         :size size-pages}]
+                       :size size-pages}]
 
-                                            (when @show-pages?
+          (when @show-pages?
-                                              [:div {:class (stl/css :resize-area-horiz)
+            [:div {:class (stl/css :resize-area-horiz)
-                                                     :on-pointer-down on-pointer-down-pages
+                   :on-pointer-down on-pointer-down-pages
-                                                     :on-lost-pointer-capture on-lost-pointer-capture-pages
+                   :on-lost-pointer-capture on-lost-pointer-capture-pages
-                                                     :on-pointer-move on-pointer-move-pages}])
+                   :on-pointer-move on-pointer-move-pages}])
 
-                                            [:& layers-toolbox {:size-parent size
+          [:& layers-toolbox {:size-parent size
-                                                                :size size-pages}]])}]
+                              :size size-pages}]])
-               #js [#js {:label (tr "workspace.sidebar.layers")
-                         :id "layers"
-                         :content (mf/html [:article {:class (stl/css :layers-tab)
-                                                      :style #js {"--height" (str size-pages "px")}}
 
-                                            [:& sitemap {:layout layout
-                                                         :toggle-pages toggle-pages
-                                                         :show-pages? @show-pages?
-                                                         :size size-pages}]
 
-                                            (when @show-pages?
+        assets-tab
-                                              [:div {:class (stl/css :resize-area-horiz)
+        (mf/html [:& assets-toolbox {:size (- size 58)}])
-                                                     :on-pointer-down on-pointer-down-pages
-                                                     :on-lost-pointer-capture on-lost-pointer-capture-pages
-                                                     :on-pointer-move on-pointer-move-pages}])
-
-                                            [:& layers-toolbox {:size-parent size
-                                                                :size size-pages}]])}
-                    #js {:label (tr "workspace.toolbar.assets")
-                         :id "assets"
-                         :content (mf/html [:& assets-toolbox {:size (- size 58)}])}])]
 
+        tabs
+        (if ^boolean mode-inspect?
+          #js [#js {:label (tr "workspace.sidebar.layers")
+                    :id "layers"
+                    :content layers-tab}]
+          #js [#js {:label (tr "workspace.sidebar.layers")
+                    :id "layers"
+                    :content layers-tab}
+               #js {:label (tr "workspace.toolbar.assets")
+                    :id "assets"
+                    :content assets-tab}])]
 
     [:& (mf/provider muc/sidebar) {:value :left}
      [:aside {:ref parent-ref

frontend/src/app/main/ui/workspace/sidebar/options.cljs

@@ -146,25 +146,30 @@
               (st/emit! :interrupt (udw/set-workspace-read-only true))
               (st/emit! :interrupt (udw/set-workspace-read-only false)))))
 
-        design-content (mf/html [:& design-menu {:selected selected
+        design-content
-                                                 :objects objects
+        (mf/html [:& design-menu {:selected selected
-                                                 :page-id page-id
+                                  :objects objects
-                                                 :file-id file-id
+                                  :page-id page-id
-                                                 :selected-shapes selected-shapes
+                                  :file-id file-id
-                                                 :shapes-with-children shapes-with-children}])
+                                  :selected-shapes selected-shapes
+                                  :shapes-with-children shapes-with-children}])
+
+        inspect-content
+        (mf/html [:div {:class (stl/css :element-options :inspect-options)}
+                  [:& hrs/right-sidebar {:page-id           page-id
+                                         :objects           objects
+                                         :file-id           file-id
+                                         :frame             shape-parent-frame
+                                         :shapes            selected-shapes
+                                         :on-change-section on-change-section
+                                         :on-expand         on-expand
+                                         :from              :workspace}]])
+
+        interactions-content
+        (mf/html [:div {:class (stl/css :element-options :interaction-options)}
+                  [:& interactions-menu {:shape (first shapes)}]])
 
-        inspect-content (mf/html [:div {:class (stl/css :element-options :inspect-options)}
-                                  [:& hrs/right-sidebar {:page-id           page-id
-                                                         :objects           objects
-                                                         :file-id           file-id
-                                                         :frame             shape-parent-frame
-                                                         :shapes            selected-shapes
-                                                         :on-change-section on-change-section
-                                                         :on-expand         on-expand
-                                                         :from              :workspace}]])
 
-        interactions-content (mf/html [:div {:class (stl/css :element-options :interaction-options)}
-                                       [:& interactions-menu {:shape (first shapes)}]])
         tabs
         #js [#js {:label (tr "workspace.options.design")
                   :id "design"

frontend/yarn.lock

@@ -1738,6 +1738,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@babel/runtime@npm:^7.12.5":
+  version: 7.25.0
+  resolution: "@babel/runtime@npm:7.25.0"
+  dependencies:
+    regenerator-runtime: "npm:^0.14.0"
+  checksum: 10c0/bd3faf246170826cef2071a94d7b47b49d532351360ecd17722d03f6713fd93a3eb3dbd9518faa778d5e8ccad7392a7a604e56bd37aaad3f3aa68d619ccd983d
+  languageName: node
+  linkType: hard
+
 "@babel/runtime@npm:^7.17.8, @babel/runtime@npm:^7.21.0, @babel/runtime@npm:^7.7.2, @babel/runtime@npm:^7.8.4, @babel/runtime@npm:^7.8.7":
   version: 7.24.5
   resolution: "@babel/runtime@npm:7.24.5"
@@ -6657,6 +6666,7 @@ __metadata:
     randomcolor: "npm:^0.6.2"
     react: "npm:18.3.1"
     react-dom: "npm:18.3.1"
+    react-error-boundary: "npm:^4.0.13"
     react-virtualized: "npm:^9.22.5"
     rimraf: "npm:^5.0.7"
     rxjs: "npm:8.0.0-alpha.14"
@@ -10690,6 +10700,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"react-error-boundary@npm:^4.0.13":
+  version: 4.0.13
+  resolution: "react-error-boundary@npm:4.0.13"
+  dependencies:
+    "@babel/runtime": "npm:^7.12.5"
+  peerDependencies:
+    react: ">=16.13.1"
+  checksum: 10c0/6f3e0e4d7669f680ccf49c08c9571519c6e31f04dcfc30a765a7136c7e6fbbbe93423dd5a9fce12107f8166e54133e9dd5c2079a00c7a38201ac811f7a28b8e7
+  languageName: node
+  linkType: hard
+
 "react-is@npm:18.1.0":
   version: 18.1.0
   resolution: "react-is@npm:18.1.0"