diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index 2ca4855b58..66c19f551d 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -32,6 +32,7 @@ - Tests instant-trigger retries by stubbing `rx/timer` (recording delays to assert the schedule); dynamic bindings do not survive `await` continuations, so no dynamic var for delays. - `save-request-timeout-ms` is 7 minutes and wraps the `update-file` request in `rx/timeout`, failing it as a transient `:network` error so the retry path resends instead of blocking the queue forever. It is deliberately longer than `saving-stall-timeout-ms` (5 min) so the stalled report fires before the retry. - `sync-unload-guard` keeps `:persistence` in `st/ongoing-tasks` while edits are buffered or queued, which is what holds the browser leave-page warning on. It syncs on `::append-commit`, `::discard-commit` and `::release-failed-file` (every queue change). It is the one effect that must stay in `store.cljs`'s `omitset`, and `store.cljs`'s `ongoing-tasks` watcher is guarded by `exists? js/window` because tests run without a window. +- `persistence-failed` emits a `save-failed` audit event (`:cause-type :code :retrying :queued-commits`) whenever the user is notified (`notify-failure?`, same rule as the toast), including delegated auth/not-found failures. - Undo transactions can stay open only temporarily; timed-out pending transactions are force-committed after about 20s. Undo entries are capped at 50. - Undo/redo are ignored while a normal editor/drawing interaction is active, except grid-layout edition handles undo through this path. - After local commits and when render-wasm is active, text shapes get derived `:position-data` recomputed in a separate commit tagged `#{:position-data}`; that tag is excluded from the position-data watcher to avoid loops. diff --git a/frontend/src/app/main/data/persistence.cljs b/frontend/src/app/main/data/persistence.cljs index 0a857da12c..f7f7779478 100644 --- a/frontend/src/app/main/data/persistence.cljs +++ b/frontend/src/app/main/data/persistence.cljs @@ -14,6 +14,7 @@ [app.common.uuid :as uuid] [app.main.data.changes :as dch] [app.main.data.common :as-alias dc] + [app.main.data.event :as ev] [app.main.data.helpers :as dsh] [app.main.data.notifications :as ntf] [app.main.data.workspace :as-alias dw] @@ -313,6 +314,23 @@ (update :queue #(into (empty %) (remove dropped) %)) (update :index #(apply dissoc % dropped))))))) +(defn- notify-failure? + "False for a failed attempt of the slow cycle: it repeats a failure the + user was already told about." + [pstate] + (not (and (:recovering pstate) + (slow-retry-pending? pstate)))) + +(defn- save-failed-event + "Audit event for a save failure the user is told about, so failed saves + can be counted." + [{:keys [error queue] :as pstate}] + (ev/event {::ev/name "save-failed" + :cause-type (:cause-type error) + :code (:code error) + :retrying (slow-retry-pending? pstate) + :queued-commits (count queue)})) + (defn- persistence-failed [commit-id cause] (ptk/reify ::persistence-failed @@ -336,14 +354,16 @@ ptk/WatchEvent (watch [_ state stream] - (rx/merge - ;; The terminal toast supersedes the reconnect notice; hide it - ;; explicitly instead of relying on the single-toast replacement. - (rx/of (ptk/data-event ::error cause) - (ntf/hide :tag reconnecting-tag)) - ;; A transport failure that outlasts the backoff can still pass, so - ;; the queue keeps trying at a slow pace. - (let [pstate (:persistence state)] + (let [pstate (:persistence state)] + (rx/merge + ;; The terminal toast replaces the reconnect notice, hidden by tag. + (rx/of (ptk/data-event ::error cause) + (ntf/hide :tag reconnecting-tag)) + (if (notify-failure? pstate) + (rx/of (save-failed-event pstate)) + (rx/empty)) + ;; A transport failure that outlasts the backoff can still pass, so + ;; the queue keeps trying at a slow pace. (if (and (not (:suspended pstate)) (slow-retry-pending? pstate)) (slow-retry-cycle stream) @@ -351,13 +371,10 @@ ptk/EffectEvent (effect [_ state _] - ;; A failed slow attempt repeats a failure the user was already warned - ;; about. Report without invoking global handlers that may reload the - ;; file or navigate away before the user can recover the retained - ;; changes. + ;; Notifies without the global handlers, which may reload the file or + ;; navigate away and lose the retained changes. (let [pstate (:persistence state)] - (when-not (and (:recovering pstate) - (slow-retry-pending? pstate)) + (when (notify-failure? pstate) (errors/flash-persistence cause :retrying (slow-retry-pending? pstate))))))) (defn- persistence-transient-failure diff --git a/frontend/test/frontend_tests/data/persistence_test.cljs b/frontend/test/frontend_tests/data/persistence_test.cljs index f6555fa165..80ba7608bd 100644 --- a/frontend/test/frontend_tests/data/persistence_test.cljs +++ b/frontend/test/frontend_tests/data/persistence_test.cljs @@ -21,6 +21,7 @@ [app.common.types.shape :as cts] [app.common.uuid :as uuid] [app.main.data.changes :as dch] + [app.main.data.event :as ev] [app.main.data.persistence :as dps] [app.main.data.render-wasm :as drw] [app.main.data.workspace :as dw] @@ -1002,3 +1003,35 @@ "the release turns the guard off")) (t/is (= :saved (get-in @store [:persistence :status]))))))) +(defn- save-failed-events + "Starts collecting the `save-failed` audit events that `store` processes." + [store] + (let [events (atom [])] + (->> (ptk/input-stream store) + (rx/filter #(= ::ev/event (ptk/type %))) + (rx/map deref) + (rx/filter #(= "save-failed" (::ev/name %))) + (rx/subs! #(swap! events conj %))) + events)) + +;; Scenario: the backend rejects a save as invalid data. Proves: the user's +;; failure toast comes with one `save-failed` audit event that records why. +(t/deftest ^:async a-failed-save-emits-one-save-failed-event + (await + (with-persistence + (^:async fn [{:keys [file-id store]}] + (let [events (save-failed-events store)] + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (await (async/wait-for #(= :error (get-in @store [:persistence :status])) + "the save fails")) + (await (async/wait-for #(seq @events) "the event is emitted")) + (await (async/settle)) + (t/is (= 1 (count @events))) + (t/is (= {:cause-type :validation + :code :referential-integrity + :retrying false + :queued-commits 1} + (select-keys (first @events) + [:cause-type :code :retrying :queued-commits]))))) + (fn [_ _] + (rx/throw (ex-info "invalid" {:type :validation :code :referential-integrity}))))))