mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 22:10:34 +00:00
🐛 Reduce thumbnail re-renders and report throttled update loops (#12610)
* 🐛 Stop thumbnail subscribers re-rendering on every store change A thumbnail loaded from the server keeps a media id as :uri, and workspace-thumbnail-by-id resolves it into a new map on each read. The ref compared values with identical?, so every root frame, every component in the assets panel and the interactions layer re-rendered on every store change, mouse moves included. Compare by value instead. This does not loop by itself, but it adds work to every commit and brings other update loops closer to React's limit. Relates to #12580 AI-assisted-by: claude-opus-5-5 * ✨ Report update loops throttled by the update scheduler When the scheduler throttles an update loop, it only logs a console warning, so a loop that no longer crashes leaves no trace. Send an error report too, with the events in flight and the last events, at most once per report window. Relates to #12580 AI-assisted-by: claude-opus-5-5 * 🐛 Report throttled update loops as unhandled exceptions The throttled update loop report used the handled-exception event, which only lands in the audit log: error reports and alerts read unhandled-exception and exception-page. A throttled loop is an application defect (its cause has :type :internal), and the point of the report is to find those loops, so it is now an unhandled exception. AI-assisted-by: claude-opus-5-5
This commit is contained in:
parent
a3fe43c1ec
commit
1962860f04
@ -440,16 +440,31 @@
|
||||
|
||||
(defn on-update-loop
|
||||
"Handler for the rumext update scheduler trips: logs the throttled
|
||||
update loop with the events being processed, at most once per
|
||||
`report-window-ms`. Runs inside store watchers and effect bodies, so it
|
||||
never throws."
|
||||
[_]
|
||||
update loop and reports it as an unhandled exception with the events
|
||||
being processed, at most once per `report-window-ms`. Runs inside store
|
||||
watchers and effect bodies, so it never throws."
|
||||
[{:keys [rounds]}]
|
||||
(try
|
||||
(let [now (inst-ms (ct/now))]
|
||||
(when (>= (- now @update-loop-warned-at) report-window-ms)
|
||||
(vreset! update-loop-warned-at now)
|
||||
(.warn js/console "[update-scheduler] update loop throttled; in-flight events:"
|
||||
(into-array (map ptk/repr-event (ptk/in-flight-events st/state))))))
|
||||
(let [events (ptk/in-flight-events st/state)
|
||||
cause (ex-info "update loop throttled"
|
||||
{:type :internal
|
||||
:code :update-loop-throttled
|
||||
:rounds rounds})]
|
||||
(.warn js/console "[update-scheduler] update loop throttled; in-flight events:"
|
||||
(into-array (map ptk/repr-event events)))
|
||||
(when (seq events)
|
||||
(.set in-flight-by-cause cause (in-flight-lines events)))
|
||||
;; Report outside the loop that tripped the scheduler, as an
|
||||
;; unhandled exception: a throttled loop is an application defect,
|
||||
;; so it must reach the error reports and alerts, not only audit.
|
||||
(ts/schedule
|
||||
#(submit-report :event-name "unhandled-exception"
|
||||
:hint (ex-message cause)
|
||||
:report (fn [] (generate-report cause))
|
||||
:cause cause)))))
|
||||
(catch :default _
|
||||
nil)))
|
||||
|
||||
|
||||
@ -609,6 +609,8 @@
|
||||
|
||||
(defn workspace-thumbnail-by-id
|
||||
[object-id]
|
||||
;; Resolving a media id builds a new map on each read; compare by
|
||||
;; value so subscribers do not re-render on every store change.
|
||||
(l/derived
|
||||
(fn [state]
|
||||
(when-let [entry (dm/get-in state [:thumbnails object-id])]
|
||||
@ -616,7 +618,8 @@
|
||||
(and (:uri entry)
|
||||
(not (resolved-uri? (:uri entry))))
|
||||
(update :uri cf/resolve-media))))
|
||||
st/state))
|
||||
st/state
|
||||
=))
|
||||
|
||||
(def workspace-text-modifier
|
||||
(l/derived :workspace-text-modifier st/state))
|
||||
|
||||
@ -8,6 +8,7 @@
|
||||
(:require
|
||||
[app.common.uuid :as uuid]
|
||||
[app.main.refs :as refs]
|
||||
[app.main.store :as st]
|
||||
[cljs.test :as t :include-macros true]))
|
||||
|
||||
(def ^:private resolved-uri? @#'refs/resolved-uri?)
|
||||
@ -28,3 +29,23 @@
|
||||
(t/is (false? (resolved-uri? (uuid/next))))
|
||||
(t/is (false? (resolved-uri? nil)))
|
||||
(t/is (false? (resolved-uri? 42))))
|
||||
|
||||
(t/deftest thumbnail-ref-ignores-unrelated-state-changes
|
||||
;; A thumbnail loaded from the server keeps a media id as :uri, and
|
||||
;; the ref resolves it to a new map on each read. Subscribers (root
|
||||
;; frames, assets) must not re-render on every unrelated store change.
|
||||
(let [object-id (str (uuid/next))
|
||||
ref (refs/workspace-thumbnail-by-id object-id)
|
||||
calls (atom 0)]
|
||||
(swap! st/state assoc-in [:thumbnails object-id] {:uri (uuid/next) :rendered-at nil})
|
||||
(add-watch ref ::probe (fn [_ _ _ _] (swap! calls inc)))
|
||||
(try
|
||||
(swap! st/state assoc ::probe (uuid/next))
|
||||
(t/is (zero? @calls) "unrelated change does not notify")
|
||||
(swap! st/state assoc-in [:thumbnails object-id :uri] (uuid/next))
|
||||
(t/is (= 1 @calls) "a new thumbnail uri notifies")
|
||||
(finally
|
||||
(remove-watch ref ::probe)
|
||||
(swap! st/state #(-> %
|
||||
(dissoc ::probe)
|
||||
(update :thumbnails dissoc object-id)))))))
|
||||
|
||||
@ -14,7 +14,8 @@
|
||||
- events in flight – the full report lists the events the
|
||||
store was processing when it caught the exception; a failing stream
|
||||
pipeline lists the emitters of the value, not its own event
|
||||
- on-update-loop – never throws into the scheduler's caller
|
||||
- on-update-loop – never throws into the scheduler's caller;
|
||||
reports the throttled loop as unhandled, with the events in flight
|
||||
- flash schedules async report and toast – neither the report nor
|
||||
ntf/show is emitted synchronously
|
||||
- organization SSO recovery – expired SSO sessions go back to the provider
|
||||
@ -554,6 +555,30 @@
|
||||
{ptk/in-flight-events (fn [_] (throw (js/Error. "in-flight read failure")))}
|
||||
(t/is (nil? (errors/on-update-loop {:rounds 26}))))))
|
||||
|
||||
(t/deftest ^:async update-loop-is-reported-with-the-events-in-flight
|
||||
(errors/reset-report-governor!)
|
||||
(vreset! @#'errors/update-loop-warned-at 0)
|
||||
(let [events (atom [])
|
||||
loop-ev (ptk/reify ::test-update-loop-source)]
|
||||
(await
|
||||
(mock/with-mocks*
|
||||
{st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))
|
||||
ptk/in-flight-events (fn [_] [loop-ev])
|
||||
st/format-last-events (mock/stub (fn [& _] "(stub last events)"))
|
||||
rt/get-current-href (constantly "https://penpot.example.com/#/workspace")
|
||||
tm/schedule (mock/stub (fn [f] (f)))}
|
||||
(errors/on-update-loop {:rounds 26})
|
||||
;; A second trip within the window is not reported again
|
||||
(errors/on-update-loop {:rounds 26})
|
||||
(t/is (= 1 (count @events)))
|
||||
(let [props (deref (first @events))]
|
||||
;; An unhandled report: it reaches the error reports and alerts,
|
||||
;; not only the audit log.
|
||||
(t/is (= "unhandled-exception" (::ev/name props)))
|
||||
(t/is (= "update loop throttled" (:hint props)))
|
||||
(t/is (str/includes? (in-flight-section (:report props))
|
||||
"test-update-loop-source")))))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; Expired organization SSO session
|
||||
;;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user