github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-28 21:40:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Keep the workspace open when an export fails

Browse Source 
A failed export raised the exporter error to the global handler, which
replaced the workspace with the error page. Users then reloaded and
lost the changes that were not saved yet.

- A failed single or multiple export now shows an error toast and
  keeps the workspace. Exporter defects still report as unhandled.
- A failed multiple export request ends its progress listeners, so the
  leave-page warning it turned on is released.
- Before exporting, the pending changes of the exported files are sent
  and the export waits up to 10s for them, instead of 400ms. Exports
  are built from the server copy, so if some changes are still unsaved
  a toast warns that the export may not include them.

force-persist-files-and-wait waits only for the given files, so a stuck
save of another file does not delay the export.
This commit is contained in:
alonso.torres 2026-10-16 13:07:36 +02:00 committed by Andrey Antukh
parent 781ac534e0
commit a1e51aacba
6 changed files with 286 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
frontend/src/app/main/data/exports/assets.cljs
View File

@ -13,11 +13,14 @@
[app.main.data.exports.wasm :as wasm.exports]
[app.main.data.helpers :as dsh]
[app.main.data.modal :as modal]
[app.main.data.notifications :as ntf]
[app.main.data.persistence :as dwp]
[app.main.errors :as errors]
[app.main.features :as features]
[app.main.repo :as rp]
[app.main.store :as st]
[app.util.dom :as dom]
[app.util.i18n :refer [tr]]
[app.util.websocket :as ws]
[beicon.v2.core :as rx]
[cuerdas.core :as str]
@ -25,6 +28,37 @@
(def default-timeout 5000)
;; Longest wait for the pending changes of the exported files to be saved.
(def save-wait-timeout-ms 10000)
(defn- export-file-ids
[exports]
(into #{} (keep :file-id) exports))
(defn- warn-unsaved-changes
"Warns that the export may miss the changes of `file-ids` not saved yet."
[file-ids]
(ptk/reify ::warn-unsaved-changes
ptk/WatchEvent
(watch [_ state _]
(when (dwp/unsaved-changes? state file-ids)
(rx/of (ntf/show {:content (tr "workspace.export.unsaved-changes")
:type :toast
:level :warning
:timeout 8000}))))))
(defn- export-failed
"Shows an export failure as a toast, keeping the workspace open."
[cause]
(ptk/reify ::export-failed
ptk/EffectEvent
(effect [_ _ _]
(errors/flash :cause cause
:type (if (#{:server-error :internal} (:type (ex-data cause)))
:unhandled
:handled)
:hint (tr "errors.export-failed")))))
(defn normalize-export
[{:keys [object-id name] :as export}]
(assoc export :name (if (str/blank? name)
@ -226,16 +260,16 @@
:wait true
:is-wasm (wasm-export-enabled? state)})]
(rx/concat
(dwp/force-persist-and-wait 400)
(dwp/force-persist-files-and-wait (export-file-ids [export]) save-wait-timeout-ms)
(rx/of (warn-unsaved-changes (export-file-ids [export])))
(->> (rp/cmd! :export params)
(rx/map (fn [{:keys [filename mtype uri]}]
(dom/trigger-download-uri filename mtype uri)
(clear-export-state uuid/zero)))
(rx/catch (fn [cause]
(rx/concat
(rx/of (clear-export-state uuid/zero))
(rx/throw cause))))))))))))
(rx/of (clear-export-state uuid/zero)
(export-failed cause))))))))))))
(defn request-multiple-export
[{:keys [exports cmd name]
@ -246,6 +280,8 @@
ptk/WatchEvent
(watch [_ state _]
(let [resource-id (volatile! nil)
;; Emits when the export request fails.
failed-s (rx/subject)
profile-id (:profile-id state)
ws-conn (:ws-conn state)
params (cond->
@ -263,46 +299,48 @@
(rx/map :payload)
(rx/filter #(= :export-update (:type %)))
(rx/filter #(= @resource-id (:resource-id %)))
(rx/take-until failed-s)
(rx/share))
stopper
(rx/filter #(or (= "ended" (:status %))
(= "error" (:status %)))
progress-stream)]
progress-stream)
file-ids
(export-file-ids exports)]
(swap! st/ongoing-tasks conj :export)
(rx/merge
;; Force that all data is persisted; best effort.
(rx/of ::dwp/force-persist)
(rx/concat
(dwp/force-persist-files-and-wait file-ids save-wait-timeout-ms)
(rx/of (warn-unsaved-changes file-ids))
(rx/merge
;; Updates the export state with each progress message, until
;; shortly after the export ends or fails.
(->> progress-stream
(rx/map update-export-status)
(rx/take-until (rx/delay 500 stopper))
(rx/finalize (fn []
(swap! st/ongoing-tasks disj :export))))
;; Launch the exportation process and stores the resource id
;; locally.
(->> (rp/cmd! :export params)
(rx/map (fn [{:keys [id] :as resource}]
(vreset! resource-id id)
(initialize-export-status exports cmd resource))))
;; Clears the export state a while after the export ends.
(->> progress-stream
(rx/filter #(= "ended" (:status %)))
(rx/take 1)
(rx/delay default-timeout)
(rx/map #(clear-export-state @resource-id))
(rx/take-until (rx/delay 6000 stopper)))
;; We proceed to update the export state with incoming
;; progress updates. We delay the stopper for give some time
;; to update the status with ended or errored status before
;; close the stream.
(->> progress-stream
(rx/map update-export-status)
(rx/take-until (rx/delay 500 stopper))
(rx/finalize (fn []
(swap! st/ongoing-tasks disj :export))))
;; We hide need to hide the ui elements of the export after
;; some interval. We also delay a little bit more the stopper
;; for ensure that after some security time, the stream is
;; completely closed.
(->> progress-stream
(rx/filter #(= "ended" (:status %)))
(rx/take 1)
(rx/delay default-timeout)
(rx/map #(clear-export-state @resource-id))
(rx/take-until (rx/delay 6000 stopper)))))))))
;; Sends the export request and keeps its resource id. It goes
;; last so a failure reaches the progress listeners above.
(->> (rp/cmd! :export params)
(rx/map (fn [{:keys [id] :as resource}]
(vreset! resource-id id)
(initialize-export-status exports cmd resource)))
(rx/catch (fn [cause]
(rx/push! failed-s cause)
(rx/of (export-failed cause))))))))))))
(defn request-export
[{:keys [exports] :as params}]

24
frontend/src/app/main/data/persistence.cljs
View File

@ -109,6 +109,30 @@
([timeout-ms]
(rx/concat (rx/of ::force-persist) (wait-persisted timeout-ms))))
(defn- files-queued?
"True when the queue in `pstate` still holds changes of any of `file-ids`."
[{:keys [queue index]} file-ids]
(boolean (some #(contains? file-ids (get-in index [% :file-id])) queue)))
(defn unsaved-changes?
"True when any of `file-ids` has changes the server does not have yet."
[state file-ids]
(files-queued? (:persistence state) file-ids))
(defn force-persist-files-and-wait
"Sends the pending changes and waits until no change of `file-ids` is
queued, saving fails, or `timeout-ms` passes. Never raises; check
`unsaved-changes?` afterwards."
[file-ids timeout-ms]
(rx/concat
(rx/of ::force-persist)
(->> (rx/from-atom refs/persistence {:emit-current-value? true})
(rx/filter #(or (= :error (:status %))
(not (files-queued? % file-ids))))
(rx/take 1)
(rx/ignore)
(rx/timeout timeout-ms (rx/empty)))))
(defn- next-status
"Refuses downgrades: a save in progress stays :saving, a failed save
stays :error until persistence is resumed, and a retrying episode stays

108
frontend/test/frontend_tests/data/exports_assets_test.cljs
View File

@ -9,12 +9,15 @@
[app.common.uuid :as uuid]
[app.main.data.exports.assets :as de]
[app.main.data.persistence :as dwp]
[app.main.errors :as errors]
[app.main.repo :as repo]
[app.main.store :as st]
[app.util.dom :as dom]
[app.util.i18n :as i18n]
[app.util.websocket :as ws]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.async :as async]
[frontend-tests.helpers.events :as the]
[frontend-tests.helpers.mock :as mock]
[potok.v2.core :as ptk]))
@ -55,7 +58,7 @@
(rx/of {:filename "export.png"
:mtype "image/png"
:uri "blob:export"})))
dwp/force-persist-and-wait (mock/stub (fn [_] (rx/of ::force-persisted)))
dwp/force-persist-files-and-wait (mock/stub (fn [_ _] (rx/of ::force-persisted)))
dom/trigger-download-uri (mock/stub (fn [& _] nil))}
(fn [done']
(let [completed (fn [_state]
@ -97,3 +100,106 @@
(de/request-multiple-export {:exports exports})
:the/end)))
done))))
(defn- export-store
"A store for the export tests that records store errors in `store-errors`."
[state store-errors]
(ptk/store {:state state :on-error #(swap! store-errors conj %)}))
(defn- unsaved-state
"Test state whose save queue still holds a change of `file-id`."
[file-id]
(let [commit-id (uuid/next)]
(assoc (test-state)
:persistence {:queue (conj #queue [] commit-id)
:index {commit-id {:id commit-id :file-id file-id}}})))
;; Scenario: the exporter fails a single export. Proves: the user gets a
;; toast and stays in the workspace; the failure is not raised to the global
;; error handler, which would replace the workspace with an error page.
(t/deftest ^:async failed-simple-export-shows-a-toast-instead-of-raising
(let [store-errors (atom [])
flashed (atom [])
store (export-store (test-state) store-errors)]
(await
(mock/with-mocks*
{repo/cmd! (mock/stub (fn [_ _]
(rx/throw (ex-info "exporter failed"
{:type :server-error
:code :unhandled}))))
dwp/force-persist-files-and-wait (mock/stub (fn [_ _] (rx/empty)))
errors/flash (fn [& {:keys [hint type]}]
(swap! flashed conj [hint type]))
i18n/tr (mock/stub #(str "translated:" %))}
(ptk/emit! store (de/request-simple-export {:export export}))
(await (async/wait-for #(seq @flashed) "the failure is shown"))
(await (async/settle))
(t/is (= [["translated:errors.export-failed" :unhandled]] @flashed)
"an exporter defect reports as unhandled")
(t/is (empty? @store-errors) "nothing reaches the global error handler")
(t/is (nil? (:export @store)) "the export state is cleared")))))
;; Scenario: the export request of a multiple export fails. Proves: the user
;; gets a toast, and the leave-page warning the export turned on is released
;; even though no progress message will ever arrive.
(t/deftest ^:async failed-multiple-export-shows-a-toast-and-releases-the-leave-warning
(let [store-errors (atom [])
flashed (atom [])
tasks (atom #{})
store (export-store (test-state) store-errors)]
(await
(mock/with-mocks*
{repo/cmd! (mock/stub (fn [_ _]
(rx/throw (ex-info "exporter failed"
{:type :server-error}))))
dwp/force-persist-files-and-wait (mock/stub (fn [_ _] (rx/empty)))
ws/get-rcv-stream (mock/stub (fn [_] (rx/subject)))
st/ongoing-tasks tasks
errors/flash (fn [& {:keys [hint]}] (swap! flashed conj hint))
i18n/tr (mock/stub #(str "translated:" %))}
(ptk/emit! store (de/request-multiple-export {:exports [(assoc export :enabled true)]}))
(await (async/wait-for #(seq @flashed) "the failure is shown"))
(await (async/wait-for #(not (contains? @tasks :export))
"the leave-page warning is released"))
(t/is (= ["translated:errors.export-failed"] @flashed))
(t/is (empty? @store-errors) "nothing reaches the global error handler")))))
;; Scenario: an export starts while its file has a change not saved yet.
;; Proves: the user is warned that the export may miss it, and the export
;; runs anyway.
(t/deftest ^:async export-warns-when-the-file-has-unsaved-changes
(let [file-id (uuid/next)
downloaded (atom false)
store (export-store (unsaved-state file-id) (atom []))]
(await
(mock/with-mocks*
{repo/cmd! (mock/stub (fn [_ _]
(rx/of {:filename "export.png"
:mtype "image/png"
:uri "blob:export"})))
dwp/force-persist-files-and-wait (mock/stub (fn [_ _] (rx/empty)))
dom/trigger-download-uri (mock/stub (fn [& _] (reset! downloaded true)))
i18n/tr (mock/stub #(str "translated:" %))}
(ptk/emit! store (de/request-simple-export {:export (assoc export :file-id file-id)}))
(await (async/wait-for #(true? @downloaded) "the export runs anyway"))
(t/is (= "translated:workspace.export.unsaved-changes"
(get-in @store [:notification :content])))
(t/is (= :warning (get-in @store [:notification :level])))))))
;; Scenario: an export starts with every change of its file saved. Proves:
;; no warning is shown.
(t/deftest ^:async export-shows-no-warning-when-the-file-is-saved
(let [downloaded (atom false)
store (export-store (test-state) (atom []))]
(await
(mock/with-mocks*
{repo/cmd! (mock/stub (fn [_ _]
(rx/of {:filename "export.png"
:mtype "image/png"
:uri "blob:export"})))
dwp/force-persist-files-and-wait (mock/stub (fn [_ _] (rx/empty)))
dom/trigger-download-uri (mock/stub (fn [& _] (reset! downloaded true)))}
(ptk/emit! store (de/request-simple-export {:export (assoc export :file-id (uuid/next))}))
(await (async/wait-for #(true? @downloaded) "the export runs"))
(await (async/settle))
(t/is (nil? (:notification @store)))))))

67
frontend/test/frontend_tests/data/persistence_test.cljs
View File

@ -26,6 +26,7 @@
[app.main.data.render-wasm :as drw]
[app.main.data.workspace :as dw]
[app.main.errors :as errors]
[app.main.refs :as refs]
[app.main.repo :as rp]
[app.main.store :as st]
[app.util.i18n :as i18n]
@ -33,6 +34,7 @@
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.async :as async]
[frontend-tests.helpers.mock :as mock]
[okulary.core :as l]
[potok.v2.core :as ptk]))
(t/deftest reopening-restores-only-the-retained-edits-of-the-loaded-file
@ -1003,6 +1005,71 @@
"the release turns the guard off"))
(t/is (= :saved (get-in @store [:persistence :status])))))))
(defn- wait-files-saved
"Emits into `store` an event that runs `force-persist-files-and-wait` for
`file-ids` and then flips `done`."
[store file-ids timeout-ms done]
(ptk/emit! store
(ptk/reify ::wait-files-saved
ptk/WatchEvent
(watch [_ _ _]
(rx/concat
(dps/force-persist-files-and-wait file-ids timeout-ms)
(rx/of (ptk/reify ::files-saved
ptk/EffectEvent
(effect [_ _ _] (reset! done true)))))))))
;; Scenario: an edit waits in the buffer when an export asks for its file
;; to be saved. Proves: the wait sends the edit and only ends once the save
;; lands.
(t/deftest ^:async waiting-for-a-file-sends-and-waits-for-its-edits
(let [done (atom false)]
(await
(with-persistence
(^:async fn [{:keys [file-id response requests store]}]
(await
(mock/with-mocks*
{refs/persistence (l/derived :persistence store)}
(ptk/emit! store (local-commit file-id))
(wait-files-saved store #{file-id} 60000 done)
(await (async/wait-for #(= 1 (count @requests)) "the edit is sent at once"))
(await (async/settle))
(t/is (false? @done) "the wait holds while the save is in flight")
(rx/push! response {:revn 1})
(await (async/wait-for #(true? @done) "the landed save ends the wait"))
(t/is (not (dps/unsaved-changes? @store #{file-id}))))))))))
;; Scenario: a save of one file hangs while another file is exported.
;; Proves: the wait only covers the exported files.
(t/deftest ^:async waiting-for-a-file-ignores-other-files
(let [done (atom false)]
(await
(with-persistence
(^:async fn [{:keys [file-id requests store]}]
(await
(mock/with-mocks*
{refs/persistence (l/derived :persistence store)}
(ptk/emit! store (local-commit file-id) ::dps/force-persist)
(await (async/wait-for #(= 1 (count @requests)) "the other file's save hangs"))
(wait-files-saved store #{(uuid/next)} 60000 done)
(await (async/wait-for #(true? @done) "the wait ends at once")))))))))
;; Scenario: the file's save hangs past the wait. Proves: the wait gives up
;; without raising, and the file still reports unsaved changes.
(t/deftest ^:async waiting-for-a-file-gives-up-after-the-timeout
(let [done (atom false)]
(await
(with-persistence
(^:async fn [{:keys [file-id requests store]}]
(await
(mock/with-mocks*
{refs/persistence (l/derived :persistence store)}
(ptk/emit! store (local-commit file-id))
(wait-files-saved store #{file-id} 1 done)
(await (async/wait-for #(true? @done) "the wait times out"))
(t/is (= 1 (count @requests)))
(t/is (dps/unsaved-changes? @store #{file-id})))))))))
(defn- save-failed-events
"Starts collecting the `save-failed` audit events that `store` processes."
[store]

8
frontend/translations/en.po
View File

@ -1589,6 +1589,10 @@ msgstr "The email «%s» has many permanent bounce reports."
msgid "errors.email-spam-or-permanent-bounces"
msgstr "The email «%s» has been reported as spam or permanently bounce."
#: src/app/main/data/exports/assets.cljs
msgid "errors.export-failed"
msgstr "The export failed. Try again in a moment."
#: src/app/main/errors.cljs:391
msgid "errors.feature-mismatch"
msgstr ""
@ -6919,6 +6923,10 @@ msgstr "Remove guide"
msgid "workspace.debug.title"
msgstr "Debugging tools"
#: src/app/main/data/exports/assets.cljs
msgid "workspace.export.unsaved-changes"
msgstr "Some of your latest changes are not saved yet, so this export may not include them."
#: src/app/main/ui/workspace/sidebar/layers.cljs:841
msgid "workspace.focus.focus-mode"
msgstr "Focus mode"

8
frontend/translations/es.po
View File

@ -1573,6 +1573,10 @@ msgstr "El correo electrónico «%s» tiene varios reportes de rebote permanente
msgid "errors.email-spam-or-permanent-bounces"
msgstr "El email «%s» tiene reportes de spam o de rebote permanente."
#: src/app/main/data/exports/assets.cljs
msgid "errors.export-failed"
msgstr "La exportación ha fallado. Inténtalo de nuevo en un momento."
#: src/app/main/errors.cljs:391
msgid "errors.feature-mismatch"
msgstr ""
@ -6757,6 +6761,10 @@ msgstr "Duplicar fila"
msgid "workspace.debug.title"
msgstr "Herramientas de depuración"
#: src/app/main/data/exports/assets.cljs
msgid "workspace.export.unsaved-changes"
msgstr "Algunos de tus últimos cambios aún no se han guardado, así que puede que esta exportación no los incluya."
#: src/app/main/ui/workspace/sidebar/layers.cljs:841
msgid "workspace.focus.focus-mode"
msgstr "Modo foco"