🐛 Add download report to the error toast (#11762)

* 🐛 Add download report to the error toast

* 🐛 Restore old behavior for some cases

frontend/src/app/main/errors.cljs

@@ -19,9 +19,11 @@
    [app.main.router :as rt]
    [app.main.store :as st]
    [app.main.worker]
+   [app.util.dom :as dom]
    [app.util.globals :as g]
    [app.util.i18n :refer [tr]]
    [app.util.timers :as ts]
+   [app.util.webapi :as wapi]
    [beicon.v2.core :as rx]
    [cuerdas.core :as str]
    [potok.v2.core :as ptk]))
@@ -170,6 +172,14 @@
                 :href (rt/get-current-href)
                 :report report}))))
 
+(defn- download-report!
+  [report event]
+  (dom/prevent-default event)
+  (let [blob (wapi/create-blob report "text/plain")
+        uri  (wapi/create-uri blob)]
+    (dom/trigger-download-uri "report" "text/plain" uri)
+    (ts/schedule-on-idle #(wapi/revoke-uri uri))))
+
 (defn flash
   "Show error notification banner and emit error report.
   A nil timeout keeps the notification visible until dismissed or replaced.
@@ -180,23 +190,28 @@
   synchronously from inside an error handler creates a re-entrant
   event-processing cycle that can exhaust the JS call stack
   (RangeError: Maximum call stack size exceeded)."
-  [& {:keys [type hint cause timeout] :or {type :handled timeout 5000}}]
+  [& {:keys [type hint cause timeout report-link?]
-  (when (ex/exception? cause)
+      :or {type :handled timeout 5000}}]
+  (let [report (when (ex/exception? cause) (generate-report cause))]
+    (when report
       (when-let [event-name (case type
                               :handled "handled-exception"
                               :unhandled "unhandled-exception"
                               :silent nil)]
-      (let [report (generate-report cause)]
         (submit-report :event-name event-name
                        :report report
-                       :hint (ex/get-hint cause)))))
+                       :hint (ex/get-hint cause))))
 
     (ts/schedule
      #(st/emit!
-     (ntf/show {:content (or ^boolean hint (tr "errors.generic"))
+       (ntf/show
+        (cond-> {:content (or ^boolean hint (tr "errors.generic"))
                  :type :toast
                  :level :error
-                :timeout timeout}))))
+                 :timeout timeout}
+          (and report-link? report)
+          (assoc :links [{:label (tr "labels.download" "report.txt")
+                          :callback (partial download-report! report)}])))))))
 
 (defmethod ptk/handle-error :network
   [error]
@@ -207,13 +222,31 @@
     (ex/print-throwable cause :prefix "Network Error"))
   (flash :cause (::instance error) :type :handled))
 
+(def ^:private delegated-persistence-types
+  "Save failure causes routed to their own error handler: retaining the
+  changes cannot resolve them."
+  #{:authentication :not-found})
+
+(defn- delegated-persistence-failure?
+  [{:keys [type cause-type code]}]
+  (or (contains? delegated-persistence-types type)
+      (contains? delegated-persistence-types cause-type)
+      ;; The retained changes no longer apply to the restored version.
+      (= :vern-conflict code)))
+
 (defn flash-persistence
   [cause]
-  (let [{:keys [type cause-type]} (ex-data cause)]
+  (let [data (ex-data cause)]
-    ;; Authentication has its own UI. `flash :silent` only skips reporting;
+    (if (delegated-persistence-failure? data)
-    ;; it still shows a toast, so do not call it for these failures.
+      ;; The persistence state wraps the failure and records the original
-    (when-not (or (= :authentication type) (= :authentication cause-type))
+      ;; type under :cause-type; dispatch on it to reach the cause's handler.
-      (flash :cause cause :type :handled :timeout nil :hint (tr "errors.save-failed")))))
+      (on-error (-> (exception->error-data cause)
+                    (assoc :type (or (:cause-type data) (:type data)))))
+      (flash :cause cause
+             :type :handled
+             :timeout nil
+             :report-link? true
+             :hint (tr "errors.save-failed")))))
 
 (defmethod ptk/handle-error :persistence
   [error]

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

@@ -5,6 +5,7 @@
 ;; Copyright (c) KALEIDOS INC Sucursal en España SL
 
 (ns app.main.ui.notifications
+  (:require-macros [app.main.style :as stl])
   (:require
    [app.main.data.notifications :as ntf]
    [app.main.store :as st]
@@ -27,7 +28,20 @@
                          (= :floating (:position notification)))
         toast?       (or (= :toast (:type notification))
                          (some? (:timeout notification)))
-        content     (or (:content notification) "")]
+        content      (or (:content notification) "")
+        toast-content
+        (if-let [links (seq (:links notification))]
+          (mf/html
+           [:div {:class (stl/css :toast-body)}
+            [:div content]
+            [:nav {:class (stl/css :toast-links)}
+             (for [[index {:keys [label callback]}] (map-indexed vector links)]
+               [:a {:key (str "link-" index)
+                    :class (stl/css :toast-link)
+                    :href "#"
+                    :on-click callback}
+                label])]])
+          content)]
 
     (when notification
       (cond
@@ -38,7 +52,7 @@
           :is-html (boolean (:is-html notification))
           :detail (:detail notification)
           :on-close on-close}
-         content]
+         toast-content]
 
         inline?
         [:& inline-notification
@@ -60,4 +74,4 @@
           :type (:type notification)
           :is-html (boolean (:is-html notification))
           :detail (:detail notification)
-          :on-close on-close} content]))))
+          :on-close on-close} toast-content]))))

frontend/src/app/main/ui/notifications.scss

@@ -0,0 +1,24 @@
+// 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 Sucursal en España SL
+
+// Stacks the message and its links as a single item of the notification
+// pill's flex row.
+.toast-body {
+  display: flex;
+  flex-direction: column;
+  gap: var(--sp-s);
+}
+
+.toast-links {
+  display: flex;
+  flex-wrap: wrap;
+  gap: var(--sp-s);
+}
+
+.toast-links .toast-link {
+  color: currentcolor;
+  text-decoration: underline;
+}

frontend/test/frontend_tests/main_errors_test.cljs

@@ -13,16 +13,22 @@
     - on-error re-entrancy guard  – prevents recursive invocations
     - flash schedules async emit  – ntf/show is not emitted synchronously
     - organization SSO recovery   – expired SSO sessions go back to the provider
-    - invalid-sso-config handler  – requires :organization-id to promote to :sso-error"
+    - invalid-sso-config handler  – requires :organization-id to promote to :sso-error
+    - save failure notification   – sticky toast carrying the error report
+    - delegated save failures     – causes handled by the handler for their type"
   (:require
    [app.common.uuid :as uuid]
    [app.main.data.persistence :as dps]
+   [app.main.data.workspace :as-alias dw]
    [app.main.errors :as errors]
    [app.main.refs :as refs]
    [app.main.repo :as rp]
    [app.main.router :as rt]
    [app.main.store :as st]
+   [app.util.dom :as dom]
+   [app.util.i18n :as i18n]
    [app.util.timers :as tm]
+   [app.util.webapi :as wapi]
    [beicon.v2.core :as rx]
    [cljs.test :as t :include-macros true]
    [frontend-tests.helpers.mock :as mock]
@@ -416,6 +422,46 @@
           (t/is (= timeout (get-in state [:notification :timeout])))
           (t/is (= :visible (get-in state [:notification :status]))))))))
 
+(t/deftest persistence-notifications-include-an-error-report-download
+  (let [scheduled      (atom [])
+        idle-callbacks (atom [])
+        events         (atom [])
+        downloads      (atom [])
+        revoked        (atom [])
+        report         "generated error report"
+        cause          (ex-info "Save failed" {:type :validation})]
+    (with-redefs [dom/prevent-default         (fn [_])
+                  dom/trigger-download-uri    (fn [& params]
+                                                (swap! downloads conj params))
+                  errors/generate-report      (fn [_] report)
+                  errors/submit-report        (fn [& _])
+                  ;; `tr` is called with one and with two arguments, and its
+                  ;; two-argument arity is variadic: the stub exposes both
+                  ;; shapes so the compiled static calls resolve.
+                  i18n/tr                     (fn ([key] (str key ":"))
+                                                ([key & args]
+                                                 (str key ":" (first args))))
+                  st/emit!                    (mock/stub (fn [& emitted]
+                                                           (swap! events into emitted)))
+                  tm/schedule                 (mock/stub (fn [callback]
+                                                           (swap! scheduled conj callback)))
+                  tm/schedule-on-idle         (mock/stub (fn [callback]
+                                                           (swap! idle-callbacks conj callback)))
+                  wapi/create-blob            (mock/stub (fn [content media-type]
+                                                           {:content content :media-type media-type}))
+                  wapi/create-uri             (fn [_] "blob:report")
+                  wapi/revoke-uri             (fn [uri]
+                                                (swap! revoked conj uri))]
+      (errors/flash-persistence cause)
+      (doseq [callback @scheduled] (callback))
+      (let [state    (ptk/update (first @events) {})
+            download (get-in state [:notification :links 0])]
+        (t/is (= "labels.download:report.txt" (:label download)))
+        ((:callback download) nil)
+        (t/is (= [["report" "text/plain" "blob:report"]] @downloads))
+        (doseq [callback @idle-callbacks] (callback))
+        (t/is (= ["blob:report"] @revoked))))))
+
 (t/deftest persistence-waiters-do-not-report-an-already-handled-failure
   (let [reports  (atom [])
         rejected (atom [])
@@ -444,3 +490,84 @@
         (t/is (= 3 (count @reports)))
         (finally
           (rx/dispose! store))))))
+
+;; ---------------------------------------------------------------------------
+;; Save failures that belong to their own handler
+;;
+;; Some causes are not resolved by retaining the changes: the session has to
+;; be renewed, the file is gone, or a different version was restored.  Each
+;; one is handled by the error handler for its own type.
+;; ---------------------------------------------------------------------------
+
+(t/deftest expired-session-during-save-goes-to-the-authentication-handler
+  (t/testing "a lost session is handled as an authentication error, not notified"
+    (let [assigned  (atom nil)
+          scheduled (atom [])]
+      (with-redefs [rt/get-current-href (constantly workspace-href)
+                    rt/assign-exception (fn [error] error)
+                    st/async-emit!      (fn [& events] (reset! assigned (first events)))
+                    tm/schedule         (mock/stub (fn [callback]
+                                                     (swap! scheduled conj callback)))]
+        (errors/flash-persistence (ex-info "Session expired" {:type :authentication}))
+        (t/is (= :authentication (:type @assigned)))
+        (t/is (empty? @scheduled) "An expired session shows no save notification")))))
+
+(t/deftest expired-organization-sso-during-save-renews-the-session
+  (t/async done
+    (t/testing "an SSO-guarded save failure goes back through the identity provider"
+      (let [events (atom [])]
+        (mock/with-mocks
+          {rp/cmd!             (mock/stub
+                                (fn [_command _params]
+                                  (rx/of {:authorized false
+                                          :redirect-uri "https://idp.example.com/authorize"})))
+           rt/get-current-href (constantly workspace-href)
+           st/emit!            (mock/stub (fn [& emitted] (swap! events into emitted)))}
+          (fn [done']
+            (errors/flash-persistence (ex-info "SSO required" (sso-required-error)))
+            (t/is (= [::rt/nav-raw] (mapv ptk/type @events)))
+            (done'))
+          done)))))
+
+(t/deftest a-deleted-file-during-save-shows-the-exception-page
+  (t/testing "a file that no longer exists shows its page, not the save notification"
+    (let [events    (atom [])
+          scheduled (atom [])]
+      (with-redefs [rt/assign-exception (fn [error] error)
+                    st/emit!            (mock/stub (fn [& emitted]
+                                                     (swap! events into emitted)))
+                    tm/schedule         (mock/stub (fn [callback]
+                                                     (swap! scheduled conj callback)))]
+        (errors/flash-persistence (ex-info "File not found" {:type :not-found}))
+        (doseq [callback @scheduled] (callback))
+        (t/is (= [:not-found] (mapv :type @events)))))))
+
+(t/deftest a-restored-version-during-save-reloads-the-file
+  (t/testing "a version restored elsewhere reloads the file instead of notifying"
+    (let [events    (atom [])
+          scheduled (atom [])]
+      (with-redefs [st/emit!    (mock/stub (fn [& emitted] (swap! events into emitted)))
+                    tm/schedule (mock/stub (fn [callback]
+                                             (swap! scheduled conj callback)))]
+        (errors/flash-persistence (ex-info "A different version has been restored"
+                                           {:type :validation :code :vern-conflict}))
+        (t/is (= [::dw/reload-current-file] (mapv ptk/type @events)))
+        (t/is (empty? @scheduled) "A restored version shows no save notification")))))
+
+(t/deftest other-validation-failures-during-save-keep-the-notification
+  (t/testing "a validation failure without a recovery of its own is still notified"
+    (let [events    (atom [])
+          scheduled (atom [])]
+      (with-redefs [errors/generate-report (fn [_] "generated error report")
+                    errors/submit-report   (fn [& _])
+                    st/emit!               (mock/stub (fn [& emitted]
+                                                        (swap! events into emitted)))
+                    tm/schedule            (mock/stub (fn [callback]
+                                                        (swap! scheduled conj callback)))]
+        (errors/flash-persistence (ex-info "Invalid data" {:type :validation
+                                                           :code :invalid-data}))
+        (doseq [callback @scheduled] (callback))
+        (t/is (= 1 (count @events)))
+        (let [state (ptk/update (first @events) {})]
+          (t/is (nil? (get-in state [:notification :timeout])))
+          (t/is (some? (get-in state [:notification :links 0]))))))))

frontend/translations/en.po

@@ -1799,9 +1799,7 @@ msgstr "The registration is currently disabled."
 #: src/app/main/errors.cljs:216
 msgid "errors.save-failed"
 msgstr ""
-"Your latest changes are not confirmed as saved. Saving may not resume "
+"Autosave is not working due to an error. Contact support to report the error and reload to continue from your last saved version."
-"automatically. Do not close or reload this tab: you may lose changes. "
-"Contact support for help."
 
 #: src/app/main/errors.cljs:337
 msgid "errors.svg-parser.invalid-svg"

frontend/translations/es.po

@@ -1763,6 +1763,11 @@ msgstr ""
 msgid "errors.registration-disabled"
 msgstr "El registro está actualmente desactivado."
 
+#: src/app/main/errors.cljs:216
+msgid "errors.save-failed"
+msgstr ""
+"El autoguardado se ha detenido debido a un error. Contacta soporte para informar del error y recarga para continuar desde los últimos cambios guardados."
+
 #: src/app/main/errors.cljs:337
 msgid "errors.svg-parser.invalid-svg"
 msgstr "El SVG no es válido o está mal formado"