diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 5125730363..666a4d846b 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -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))) diff --git a/frontend/src/app/main/refs.cljs b/frontend/src/app/main/refs.cljs index 84f6bd72f2..bb7fc0392a 100644 --- a/frontend/src/app/main/refs.cljs +++ b/frontend/src/app/main/refs.cljs @@ -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)) diff --git a/frontend/test/frontend_tests/main/refs_test.cljs b/frontend/test/frontend_tests/main/refs_test.cljs index 99c9f57657..581ff949d2 100644 --- a/frontend/test/frontend_tests/main/refs_test.cljs +++ b/frontend/test/frontend_tests/main/refs_test.cljs @@ -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))))))) diff --git a/frontend/test/frontend_tests/main_errors_test.cljs b/frontend/test/frontend_tests/main_errors_test.cljs index 5954eab457..32898be282 100644 --- a/frontend/test/frontend_tests/main_errors_test.cljs +++ b/frontend/test/frontend_tests/main_errors_test.cljs @@ -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 ;;