mirror of
https://github.com/penpot/penpot.git
synced 2026-10-28 21:40:34 +00:00
✨ Count save failures with a save-failed audit event
We only learn how often users lose work from support tickets. Each time the user is told a save failed, persistence now emits a save-failed audit event with the cause type, the code, whether the queue is still retrying and how many changes are waiting. It follows the same rule as the failure toast: a repeated failure of the slow retry cycle is not counted again. Failures handed to another handler, such as an expired session, are counted too, since they also stop the save.
This commit is contained in:
parent
4b668acf96
commit
781ac534e0
@ -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.
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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}))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user