From ab4e195ccabafad5c5af67ac0a57103686acc81f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 17 Mar 2026 15:04:06 +0100 Subject: [PATCH] :sparkles: Add protection for stale cache of js assets loading issues (#8638) * :sparkles: Use update-when for update dashboard state This make updates more consistent and reduces possible eventual consistency issues in out of order events execution. * :bug: Detect stale JS modules at boot and force reload When the browser serves cached JS files from a previous deployment alongside a fresh index.html, code-split modules reference keyword constants that do not exist in the stale shared.js, causing TypeError crashes. This adds a compile-time version tag (via goog-define / closure-defines) that is baked into the JS bundle. At boot, it is compared against the runtime version tag from index.html (which is always fresh due to no-cache headers). If they differ, the app forces a hard page reload before initializing, ensuring all JS modules come from the same build. * :paperclip: Ensure consistent version across builds on github e2e test workflow --------- Signed-off-by: Andrey Antukh --- .github/workflows/tests.yml | 2 +- frontend/shadow-cljs.edn | 3 +- frontend/src/app/config.cljs | 55 ++++++++++++++++++- frontend/src/app/main.cljs | 24 ++++++--- frontend/src/app/main/data/dashboard.cljs | 8 +-- frontend/src/app/main/errors.cljs | 57 ++++++++++++++------ frontend/src/app/main/ui/error_boundary.cljs | 21 +++++--- frontend/src/app/util/storage.cljs | 23 +++++--- 8 files changed, 149 insertions(+), 44 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index fa21646cac..0c444f2bc2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -272,7 +272,7 @@ jobs: - name: Build Bundle working-directory: ./frontend run: | - ./scripts/build 0.0.0 + ./scripts/build - name: Store Bundle Cache uses: actions/cache@v4 diff --git a/frontend/shadow-cljs.edn b/frontend/shadow-cljs.edn index bd09d46e6d..fadcb4131f 100644 --- a/frontend/shadow-cljs.edn +++ b/frontend/shadow-cljs.edn @@ -70,7 +70,8 @@ :release {:closure-defines {goog.DEBUG false - goog.debug.LOGGING_ENABLED true} + goog.debug.LOGGING_ENABLED true + app.config/compiled-version-tag #shadow/env ["VERSION_TAG" :default "develop"]} :compiler-options {:fn-invoke-direct true :optimizations #shadow/env ["PENPOT_BUILD_OPTIMIZATIONS" :as :keyword :default :advanced] diff --git a/frontend/src/app/config.cljs b/frontend/src/app/config.cljs index 6ea679328c..0cf0e6fc9e 100644 --- a/frontend/src/app/config.cljs +++ b/frontend/src/app/config.cljs @@ -6,8 +6,11 @@ (ns app.config (:require + [app.common.data :as d] [app.common.data.macros :as dm] [app.common.flags :as flags] + [app.common.logging :as log] + [app.common.time :as ct] [app.common.uri :as u] [app.common.version :as v] [app.util.avatars :as avatars] @@ -15,6 +18,8 @@ [app.util.globals :refer [global location]] [app.util.navigator :as nav] [app.util.object :as obj] + [app.util.storage :as sto] + [app.util.timers :as ts] [cuerdas.core :as str])) (set! *assert* js/goog.DEBUG) @@ -81,6 +86,16 @@ "unknown" date))) +;; --- Compile-time version tag +;; +;; This value is baked into the compiled JS at build time via closure-defines, +;; so it travels with the JS bundle. In contrast, `version-tag` (below) is read +;; at runtime from globalThis.penpotVersionTag which is set by the always-fresh +;; index.html. Comparing the two lets us detect when the browser has loaded +;; stale cached JS files. + +(goog-define compiled-version-tag "develop") + ;; --- Global Config Vars (def default-theme "default") @@ -90,12 +105,50 @@ (def build-date (parse-build-date global)) (def flags (parse-flags global)) -(def version (parse-version global)) (def target (parse-target global)) (def browser (parse-browser)) (def platform (parse-platform)) +(def version (parse-version global)) (def version-tag (obj/get global "penpotVersionTag")) + +(defn stale-build? + "Returns true when the compiled JS was built with a different version + tag than the one present in the current index.html. This indicates + the browser has cached JS from a previous deployment." + ^boolean + [] + (not= compiled-version-tag version-tag)) + +;; --- Throttled reload +;; +;; A generic reload mechanism with loop protection via sessionStorage. +;; Used by both the boot-time stale-build check and the runtime +;; stale-asset error handler. + +(def ^:private reload-storage-key "penpot-last-reload-timestamp") +(def ^:private reload-cooldown-ms 30000) + +(defn throttled-reload + "Force a hard page reload unless one was already triggered within the + last 30 seconds (tracked in sessionStorage). Returns true when a + reload is initiated, false when suppressed." + [& {:keys [reason]}] + (let [now (ct/now) + prev-ts (-> (sto/get-item sto/session-storage reload-storage-key) + (d/parse-integer))] + (if (and (some? prev-ts) + (< (- now prev-ts) reload-cooldown-ms)) + (do + (log/wrn :hint "reload suppressed (cooldown active)" + :reason reason) + false) + (do + (log/wrn :hint "forcing page reload" :reason reason) + (sto/set-item sto/session-storage reload-storage-key (str now)) + (ts/asap #(.reload ^js location true)) + true)))) + (def terms-of-service-uri (obj/get global "penpotTermsOfServiceURI")) (def privacy-policy-uri (obj/get global "penpotPrivacyPolicyURI")) (def flex-help-uri (obj/get global "penpotGridHelpURI" "https://help.penpot.app/user-guide/flexible-layouts/")) diff --git a/frontend/src/app/main.cljs b/frontend/src/app/main.cljs index 9994856f60..870f8a82bb 100644 --- a/frontend/src/app/main.cljs +++ b/frontend/src/app/main.cljs @@ -100,16 +100,24 @@ (defn ^:export init [options] - (some-> (unchecked-get options "defaultTranslations") - (i18n/set-default-translations)) + ;; Before initializing anything, check if the browser has loaded + ;; stale JS from a previous deployment. If so, do a hard reload so + ;; the browser fetches fresh assets matching the current index.html. + (if (cf/stale-build?) + (cf/throttled-reload + :reason (dm/str "stale JS: compiled=" cf/compiled-version-tag + " expected=" cf/version-tag)) + (do + (some-> (unchecked-get options "defaultTranslations") + (i18n/set-default-translations)) - (mw/init!) - (i18n/init) - (cur/init-styles) + (mw/init!) + (i18n/init) + (cur/init-styles) - (init-ui) - (st/emit! (plugins/initialize) - (initialize))) + (init-ui) + (st/emit! (plugins/initialize) + (initialize))))) (defn ^:export reinit ([] diff --git a/frontend/src/app/main/data/dashboard.cljs b/frontend/src/app/main/data/dashboard.cljs index f4f6ead4be..a5ce2cd2c3 100644 --- a/frontend/src/app/main/data/dashboard.cljs +++ b/frontend/src/app/main/data/dashboard.cljs @@ -362,7 +362,7 @@ (ptk/reify ::toggle-project-pin ptk/UpdateEvent (update [_ state] - (assoc-in state [:projects id :is-pinned] (not is-pinned))) + (d/update-in-when state [:projects id] assoc :is-pinned (not is-pinned))) ptk/WatchEvent (watch [_ state _] @@ -379,7 +379,7 @@ ptk/UpdateEvent (update [_ state] (-> state - (update-in [:projects id :name] (constantly name)) + (d/update-in-when [:projects id] assoc :name name) (update :dashboard-local dissoc :project-for-edit))) ptk/WatchEvent @@ -409,7 +409,7 @@ (ptk/reify ::file-deleted ptk/UpdateEvent (update [_ state] - (update-in state [:projects project-id :count] dec)))) + (d/update-in-when state [:projects project-id :count] dec)))) (defn delete-file [{:keys [id project-id] :as params}] @@ -514,7 +514,7 @@ (-> state (assoc-in [:files id] file) (assoc-in [:recent-files id] file) - (update-in [:projects project-id :count] inc)))))) + (d/update-in-when [:projects project-id :count] inc)))))) (defn create-file [{:keys [project-id name] :as params}] diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 973163b3fc..f09c323bee 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -33,6 +33,27 @@ ;; Will contain last uncaught exception (def last-exception nil) +;; --- Stale-asset error detection and auto-reload +;; +;; When the browser loads JS modules from different builds (e.g. shared.js from +;; build A and main-dashboard.js from build B because you loaded it in the +;; middle of a deploy per example), keyword constants referenced across modules +;; will be undefined. This manifests as TypeError messages containing +;; "$cljs$cst$" and "is undefined" or "is null". + +(defn stale-asset-error? + "Returns true if the error matches the signature of a cross-build + module mismatch: accessing a ClojureScript keyword constant that + doesn't exist on the shared $APP object." + [cause] + (when (some? cause) + (let [message (ex-message cause)] + (and (string? message) + (str/includes? message "$cljs$cst$") + (or (str/includes? message "is undefined") + (str/includes? message "is null") + (str/includes? message "is not a function")))))) + (defn exception->error-data [cause] (let [data (ex-data cause)] @@ -364,27 +385,31 @@ (.preventDefault ^js event) (when-let [cause (unchecked-get event "error")] (when-not (is-ignorable-exception? cause) - (let [data (ex-data cause) - type (get data :type)] - (set! last-exception cause) - (if (= :wasm-error type) - (on-error cause) - (do - (ex/print-throwable cause :prefix "Uncaught Exception") - (ts/asap #(flash :cause cause :type :unhandled)))))))) + (if (stale-asset-error? cause) + (cf/throttled-reload :reason (ex-message cause)) + (let [data (ex-data cause) + type (get data :type)] + (set! last-exception cause) + (if (= :wasm-error type) + (on-error cause) + (do + (ex/print-throwable cause :prefix "Uncaught Exception") + (ts/asap #(flash :cause cause :type :unhandled))))))))) (on-unhandled-rejection [event] (.preventDefault ^js event) (when-let [cause (unchecked-get event "reason")] (when-not (is-ignorable-exception? cause) - (let [data (ex-data cause) - type (get data :type)] - (set! last-exception cause) - (if (= :wasm-error type) - (on-error cause) - (do - (ex/print-throwable cause :prefix "Uncaught Rejection") - (ts/asap #(flash :cause cause :type :unhandled))))))))] + (if (stale-asset-error? cause) + (cf/throttled-reload :reason (ex-message cause)) + (let [data (ex-data cause) + type (get data :type)] + (set! last-exception cause) + (if (= :wasm-error type) + (on-error cause) + (do + (ex/print-throwable cause :prefix "Uncaught Rejection") + (ts/asap #(flash :cause cause :type :unhandled)))))))))] (.addEventListener g/window "error" on-unhandled-error) (.addEventListener g/window "unhandledrejection" on-unhandled-rejection) diff --git a/frontend/src/app/main/ui/error_boundary.cljs b/frontend/src/app/main/ui/error_boundary.cljs index 91e09920cf..7c68c84144 100644 --- a/frontend/src/app/main/ui/error_boundary.cljs +++ b/frontend/src/app/main/ui/error_boundary.cljs @@ -9,6 +9,7 @@ (:require ["react-error-boundary" :as reb] [app.common.exceptions :as ex] + [app.config :as cf] [app.main.errors :as errors] [app.main.refs :as refs] [goog.functions :as gfn] @@ -33,13 +34,19 @@ ;; very small amount of time, so we debounce for 100ms for ;; avoid duplicate and redundant reports (gfn/debounce (fn [error info] - (set! errors/last-exception error) - (ex/print-throwable error) - (js/console.error - "Component trace: \n" - (unchecked-get info "componentStack") - "\n" - error)) + ;; If the error is a stale-asset error (cross-build + ;; module mismatch), force a hard page reload instead + ;; of showing the error page to the user. + (if (errors/stale-asset-error? error) + (cf/throttled-reload :reason (ex-message error)) + (do + (set! errors/last-exception error) + (ex/print-throwable error) + (js/console.error + "Component trace: \n" + (unchecked-get info "componentStack") + "\n" + error)))) 100))] [:> reb/ErrorBoundary diff --git a/frontend/src/app/util/storage.cljs b/frontend/src/app/util/storage.cljs index b67c805d25..dd97d7ea35 100644 --- a/frontend/src/app/util/storage.cljs +++ b/frontend/src/app/util/storage.cljs @@ -17,10 +17,10 @@ ;; Using ex/ignoring because can receive a DOMException like this when ;; importing the code as a library: Failed to read the 'localStorage' ;; property from 'Window': Storage is disabled inside 'data:' URLs. -(defonce ^:private local-storage-backend +(defonce local-storage (ex/ignoring (unchecked-get g/global "localStorage"))) -(defonce ^:private session-storage-backend +(defonce session-storage (ex/ignoring (unchecked-get g/global "sessionStorage"))) (def ^:dynamic *sync* @@ -69,6 +69,17 @@ (persistent! result)))) {})) +(defn set-item + [storage key val] + (when (and (some? storage) + (string? key)) + (.setItem ^js storage key val))) + +(defn get-item + [storage key] + (when (some? storage) + (.getItem storage key))) + (defn create-storage [backend prefix] (let [initial (load-data backend prefix) @@ -154,10 +165,10 @@ (-remove-watch [_ key] (.delete watches key))))) -(defonce global (create-storage local-storage-backend "penpot-global")) -(defonce user (create-storage local-storage-backend "penpot-user")) -(defonce storage (create-storage local-storage-backend "penpot")) -(defonce session (create-storage session-storage-backend "penpot")) +(defonce global (create-storage local-storage "penpot-global")) +(defonce user (create-storage local-storage "penpot-user")) +(defonce storage (create-storage local-storage "penpot")) +(defonce session (create-storage session-storage "penpot")) (defonce before-unload (letfn [(on-before-unload [_]