From e9d177eae3cc1b524212f11bd83204d65accc30f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 13 Nov 2025 15:10:34 +0100 Subject: [PATCH] :sparkles: Make the binfile export process more resilent to errors The current binfile export process uses a streaming technique. The major problem with the streaming approach is the case when an error happens on the middle of generation, because we have no way to notify the user about the error (because the response is already is sent and contents are streaming directly to the user client/browser). This commit replaces the streaming with temporal files and SSE encoded response for emit the export progress events; once the exportation is finished, a temporal uri to the exported artifact is emited to the user via "end" event and the frontend code will automatically trigger the download. Using the SSE approach removes possible transport timeouts on export large files by sending progress data over the open connection. This commit also removes obsolete code related to old binfile formats. --- CHANGES.md | 6 +- backend/src/app/binfile/v3.clj | 4 ++ backend/src/app/rpc/commands/binfile.clj | 68 ++++++++----------- backend/src/app/util/services.clj | 2 +- common/src/app/common/flags.cljc | 1 - frontend/src/app/main/data/exports/files.cljs | 50 ++++++++------ frontend/src/app/main/repo.cljs | 3 + .../src/app/main/ui/dashboard/file_menu.cljs | 50 +++----------- frontend/src/app/main/ui/exports/files.cljs | 41 +++-------- .../src/app/main/ui/workspace/main_menu.cljs | 42 +++--------- frontend/src/app/worker.cljs | 7 +- frontend/src/app/worker/export.cljs | 44 ------------ 12 files changed, 103 insertions(+), 215 deletions(-) delete mode 100644 frontend/src/app/worker/export.cljs diff --git a/CHANGES.md b/CHANGES.md index 6bdb25a3b6..6f7596a8c1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -60,8 +60,10 @@ penpot on-premise you will need to apply the same changes on your own ### :sparkles: New features & Enhancements -- Select boards to export as PDF [Taiga #12320](https://tree.taiga.io/project/penpot/issue/12320) -- Toggle for switching boolean property values [Taiga #12341](https://tree.taiga.io/project/penpot/us/12341) +- Add the ability to select boards to export as PDF [Taiga #12320](https://tree.taiga.io/project/penpot/issue/12320) +- Add toggle for switching boolean property values [Taiga #12341](https://tree.taiga.io/project/penpot/us/12341) +- Make the file export process more reliable [Taiga #12555](https://tree.taiga.io/project/penpot/us/12555) +- Add auth flow changes [Taiga #12333](https://tree.taiga.io/project/penpot/us/12333) ### :bug: Bugs fixed diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index cf88f71551..e044ef2e8b 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -255,6 +255,8 @@ (write-entry! output path params) + (events/tap :progress {:section :storage-object :id id}) + (with-open [input (sto/get-object-data storage sobject)] (.putNextEntry ^ZipOutputStream output (ZipEntry. (str "objects/" id ext))) (io/copy input output :size (:size sobject)) @@ -279,6 +281,8 @@ thumbnails (bfc/get-file-object-thumbnails cfg file-id)] + (events/tap :progress {:section :file :id file-id}) + (vswap! bfc/*state* update :files assoc file-id {:id file-id :name (:name file) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 98b2c04193..743303c6a2 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -11,9 +11,9 @@ [app.binfile.v1 :as bf.v1] [app.binfile.v3 :as bf.v3] [app.common.features :as cfeat] - [app.common.logging :as l] [app.common.schema :as sm] [app.common.time :as ct] + [app.common.uri :as u] [app.config :as cf] [app.db :as db] [app.http.sse :as sse] @@ -25,10 +25,12 @@ [app.rpc.commands.projects :as projects] [app.rpc.commands.teams :as teams] [app.rpc.doc :as-alias doc] - [app.rpc.helpers :as rph] + [app.storage :as sto] + [app.storage.tmp :as tmp] [app.tasks.file-gc] [app.util.services :as sv] - [app.worker :as-alias wrk])) + [app.worker :as-alias wrk] + [datoteka.fs :as fs])) (set! *warn-on-reflection* true) @@ -38,52 +40,42 @@ schema:export-binfile [:map {:title "export-binfile"} [:file-id ::sm/uuid] - [:version {:optional true} ::sm/int] [:include-libraries ::sm/boolean] [:embed-assets ::sm/boolean]]) -(defn stream-export-v1 - [cfg {:keys [file-id include-libraries embed-assets] :as params}] - (rph/stream - (fn [_ output-stream] - (try - (-> cfg - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/embed-assets embed-assets) - (assoc ::bfc/include-libraries include-libraries) - (bf.v1/export-files! output-stream)) - (catch Throwable cause - (l/err :hint "exception on exporting file" - :file-id (str file-id) - :cause cause)))))) +(defn- export-binfile + [{:keys [::sto/storage] :as cfg} {:keys [file-id include-libraries embed-assets]}] + (let [output (tmp/tempfile*)] + (try + (-> cfg + (assoc ::bfc/ids #{file-id}) + (assoc ::bfc/embed-assets embed-assets) + (assoc ::bfc/include-libraries include-libraries) + (bf.v3/export-files! output)) -(defn stream-export-v3 - [cfg {:keys [file-id include-libraries embed-assets] :as params}] - (rph/stream - (fn [_ output-stream] - (try - (-> cfg - (assoc ::bfc/ids #{file-id}) - (assoc ::bfc/embed-assets embed-assets) - (assoc ::bfc/include-libraries include-libraries) - (bf.v3/export-files! output-stream)) - (catch Throwable cause - (l/err :hint "exception on exporting file" - :file-id (str file-id) - :cause cause)))))) + (let [data (sto/content output) + object (sto/put-object! storage + {::sto/content data + ::sto/touched-at (ct/in-future {:minutes 60}) + :content-type "application/zip" + :bucket "tempfile"})] + + (-> (cf/get :public-uri) + (u/join "/assets/by-id/") + (u/join (str (:id object))))) + + (finally + (fs/delete output))))) (sv/defmethod ::export-binfile "Export a penpot file in a binary format." {::doc/added "1.15" + ::doc/changes [["2.12" "Remove version parameter, only one version is supported"]] ::webhooks/event? true ::sm/params schema:export-binfile} - [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id version file-id] :as params}] + [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id file-id] :as params}] (files/check-read-permissions! pool profile-id file-id) - (let [version (or version 1)] - (case (int version) - 1 (stream-export-v1 cfg params) - 2 (throw (ex-info "not-implemented" {})) - 3 (stream-export-v3 cfg params)))) + (sse/response (partial export-binfile cfg params))) ;; --- Command: import-binfile diff --git a/backend/src/app/util/services.clj b/backend/src/app/util/services.clj index b7d2159ea1..ac814b4d17 100644 --- a/backend/src/app/util/services.clj +++ b/backend/src/app/util/services.clj @@ -27,7 +27,7 @@ (throw (IllegalArgumentException. "Missing arguments on `defmethod` macro."))) (let [mdata (assoc mdata - ::docstring (some-> docs str/<<-) + ::docstring (some-> docs str/unindent) ::spec sname ::name (name sname)) diff --git a/common/src/app/common/flags.cljc b/common/src/app/common/flags.cljc index 4177de8855..3c5311f7af 100644 --- a/common/src/app/common/flags.cljc +++ b/common/src/app/common/flags.cljc @@ -132,7 +132,6 @@ :v2-migration :webhooks ;; TODO: deprecate this flag and consolidate the code - :export-file-v3 :render-wasm-dpr :hide-release-modal :subscriptions diff --git a/frontend/src/app/main/data/exports/files.cljs b/frontend/src/app/main/data/exports/files.cljs index 74001f1f9f..b292f2fecb 100644 --- a/frontend/src/app/main/data/exports/files.cljs +++ b/frontend/src/app/main/data/exports/files.cljs @@ -12,6 +12,7 @@ [app.main.data.event :as ev] [app.main.data.modal :as modal] [app.main.repo :as rp] + [app.util.sse :as sse] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -32,25 +33,18 @@ (def check-export-files (sm/check-fn schema:export-files)) -(defn export-files - [files format] - (assert (contains? valid-formats format) - "expected valid export format") - +(defn open-export-dialog + [files] (let [files (check-export-files files)] - (ptk/reify ::export-files ptk/WatchEvent (watch [_ state _] - (let [team-id (get state :current-team-id) - evname (if (= format :legacy-zip) - "export-standard-files" - "export-binary-files")] + (let [team-id (get state :current-team-id)] (rx/merge - (rx/of (ptk/event ::ev/event {::ev/name evname - ::ev/origin "dashboard" - :format format - :num-files (count files)})) + (rx/of (ev/event {::ev/name "export-binary-files" + ::ev/origin "dashboard" + :format "binfile-v3" + :num-files (count files)})) (->> (rx/from files) (rx/mapcat (fn [file] @@ -58,11 +52,29 @@ (rx/map #(assoc file :has-libraries %))))) (rx/reduce conj []) (rx/map (fn [files] - (modal/show - {:type ::export-files - :team-id team-id - :files files - :format format})))))))))) + (modal/show {:type ::export-files + :team-id team-id + :files files})))))))))) + +(defn export-files + [& {:keys [type files]}] + (->> (rx/from files) + (rx/mapcat + (fn [file] + (->> (rp/cmd! ::sse/export-binfile {:file-id (:id file) + :version 3 + :include-libraries (= type :all) + :embed-assets (= type :merge)}) + (rx/filter sse/end-of-stream?) + (rx/map sse/get-payload) + (rx/map (fn [uri] + {:file-id (:id file) + :uri uri + :filename (:name file)})) + (rx/catch (fn [cause] + (let [error (ex-data cause)] + (rx/of {:file-id (:id file) + :error error}))))))))) ;;;;;;;;;;;;;;;;;;;;;; ;; Team Request diff --git a/frontend/src/app/main/repo.cljs b/frontend/src/app/main/repo.cljs index adb1d5deea..fe24df216d 100644 --- a/frontend/src/app/main/repo.cljs +++ b/frontend/src/app/main/repo.cljs @@ -77,6 +77,9 @@ {:query-params [:file-id :revn] :form-data? true} + ::sse/export-binfile + {:stream? true} + ::sse/clone-template {:stream? true} diff --git a/frontend/src/app/main/ui/dashboard/file_menu.cljs b/frontend/src/app/main/ui/dashboard/file_menu.cljs index 2d8a0c0271..4006caa690 100644 --- a/frontend/src/app/main/ui/dashboard/file_menu.cljs +++ b/frontend/src/app/main/ui/dashboard/file_menu.cljs @@ -6,7 +6,6 @@ (ns app.main.ui.dashboard.file-menu (:require - [app.config :as cf] [app.main.data.common :as dcm] [app.main.data.dashboard :as dd] [app.main.data.event :as-alias ev] @@ -185,19 +184,10 @@ :on-accept del-shared :count-libraries file-count}))) - on-export-files - (fn [format] - (st/emit! (with-meta (fexp/export-files files format) - {::ev/origin "dashboard"}))) - on-export-binary-files - (partial on-export-files :binfile-v1) - - on-export-binary-files-v3 - (partial on-export-files :binfile-v3) - - on-export-standard-files - (partial on-export-files :legacy-zip)] + (fn [] + (st/emit! (-> (fexp/open-export-dialog files) + (with-meta {::ev/origin "dashboard"}))))] (mf/with-effect [] (->> (rp/cmd! :get-all-projects) @@ -248,20 +238,9 @@ :id "file-move-multi" :options sub-options}) - (when-not (contains? cf/flags :export-file-v3) - {:name (tr "dashboard.export-binary-multi" file-count) - :id "file-binary-export-multi" - :handler on-export-binary-files}) - - (when (contains? cf/flags :export-file-v3) - {:name (tr "dashboard.export-binary-multi" file-count) - :id "file-binary-export-multi" - :handler on-export-binary-files-v3}) - - (when-not (contains? cf/flags :export-file-v3) - {:name (tr "dashboard.export-standard-multi" file-count) - :id "file-standard-export-multi" - :handler on-export-standard-files}) + {:name (tr "dashboard.export-binary-multi" file-count) + :id "file-binary-export-multi" + :handler on-export-binary-files} (when (and (:is-shared file) can-edit) {:name (tr "labels.unpublish-multi-files" file-count) @@ -307,20 +286,9 @@ {:name :separator} - (when-not (contains? cf/flags :export-file-v3) - {:name (tr "dashboard.download-binary-file") - :id "download-binary-file" - :handler on-export-binary-files}) - - (when (contains? cf/flags :export-file-v3) - {:name (tr "dashboard.download-binary-file") - :id "download-binary-file" - :handler on-export-binary-files-v3}) - - (when-not (contains? cf/flags :export-file-v3) - {:name (tr "dashboard.download-standard-file") - :id "download-standard-file" - :handler on-export-standard-files}) + {:name (tr "dashboard.download-binary-file") + :id "download-binary-file" + :handler on-export-binary-files} (when (and (not is-lib-page?) (not is-search-page?) can-edit) {:name :separator}) diff --git a/frontend/src/app/main/ui/exports/files.cljs b/frontend/src/app/main/ui/exports/files.cljs index 0dd31cb174..93ab836597 100644 --- a/frontend/src/app/main/ui/exports/files.cljs +++ b/frontend/src/app/main/ui/exports/files.cljs @@ -10,13 +10,11 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] - [app.config :as cf] [app.main.data.exports.files :as fexp] [app.main.data.modal :as modal] [app.main.store :as st] [app.main.ui.ds.product.loader :refer [loader*]] [app.main.ui.icons :as deprecated-icon] - [app.main.worker :as mw] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] [beicon.v2.core :as rx] @@ -71,7 +69,7 @@ {::mf/register modal/components ::mf/register-as ::fexp/export-files ::mf/props :obj} - [{:keys [team-id files format]}] + [{:keys [team-id files]}] (let [state* (mf/use-state (partial initialize-state files)) has-libs? (some :has-libraries files) @@ -79,40 +77,19 @@ selected (:selected state) status (:status state) - binary? (not= format :legacy-zip) - - ;; We've deprecated the merge option on non-binary files - ;; because it wasn't working and we're planning to remove this - ;; export in future releases. - export-types (if binary? fexp/valid-types [:all :detach]) - start-export (mf/use-fn (mf/deps team-id selected files) (fn [] (swap! state* assoc :status :exporting) - (->> (mw/ask-many! - {:cmd :export-files - :format format - :team-id team-id - :type selected - :files files}) - (rx/mapcat #(->> (rx/of %) - (rx/delay 1000))) + (->> (fexp/export-files :files files :type type) (rx/subs! - (fn [msg] - (cond - (= :error (:type msg)) - (swap! state* update :files mark-file-error (:file-id msg)) - - (= :finish (:type msg)) - (let [mtype (if (contains? cf/flags :export-file-v3) - "application/penpot" - (:mtype msg)) - fname (:filename msg) - uri (:uri msg)] - (swap! state* update :files mark-file-success (:file-id msg)) - (dom/trigger-download-uri fname mtype uri)))))))) + (fn [{:keys [file-id error filename uri] :as result}] + (if error + (swap! state* update :files mark-file-error file-id) + (do + (swap! state* update :files mark-file-success file-id) + (dom/trigger-download-uri filename "application/penpot" uri)))))))) on-cancel (mf/use-fn @@ -155,7 +132,7 @@ [:p {:class (stl/css :modal-msg)} (tr "dashboard.export.explain")] [:p {:class (stl/css :modal-scd-msg)} (tr "dashboard.export.detail")] - (for [type export-types] + (for [type fexp/valid-types] [:div {:class (stl/css :export-option true) :key (name type)} [:label {:for (str "export-" type) diff --git a/frontend/src/app/main/ui/workspace/main_menu.cljs b/frontend/src/app/main/ui/workspace/main_menu.cljs index 494341cb5a..a964d27475 100644 --- a/frontend/src/app/main/ui/workspace/main_menu.cljs +++ b/frontend/src/app/main/ui/workspace/main_menu.cljs @@ -602,12 +602,9 @@ on-export-file (mf/use-fn (mf/deps file) - (fn [event] - (let [target (dom/get-current-target event) - format (-> (dom/get-data target "format") - (keyword))] - (st/emit! (st/emit! (with-meta (fexp/export-files [file] format) - {::ev/origin "workspace"})))))) + (fn [_] + (st/emit! (-> (fexp/open-export-dialog [file]) + (with-meta {::ev/origin "workspace"}))))) on-export-file-key-down (mf/use-fn @@ -684,32 +681,13 @@ (for [sc (scd/split-sc (sc/get-tooltip :export-shapes))] [:span {:class (stl/css :shortcut-key) :key sc} sc])]] - (when-not (contains? cf/flags :export-file-v3) - [:> dropdown-menu-item* {:class (stl/css :submenu-item) - :on-click on-export-file - :on-key-down on-export-file-key-down - :data-format "binfile-v1" - :id "file-menu-binary-file"} - [:span {:class (stl/css :item-name)} - (tr "dashboard.download-binary-file")]]) - - (when (contains? cf/flags :export-file-v3) - [:> dropdown-menu-item* {:class (stl/css :submenu-item) - :on-click on-export-file - :on-key-down on-export-file-key-down - :data-format "binfile-v3" - :id "file-menu-binary-file"} - [:span {:class (stl/css :item-name)} - (tr "dashboard.download-binary-file")]]) - - (when-not (contains? cf/flags :export-file-v3) - [:> dropdown-menu-item* {:class (stl/css :submenu-item) - :on-click on-export-file - :on-key-down on-export-file-key-down - :data-format "legacy-zip" - :id "file-menu-standard-file"} - [:span {:class (stl/css :item-name)} - (tr "dashboard.download-standard-file")]]) + [:> dropdown-menu-item* {:class (stl/css :submenu-item) + :on-click on-export-file + :on-key-down on-export-file-key-down + :data-format "binfile-v3" + :id "file-menu-binary-file"} + [:span {:class (stl/css :item-name)} + (tr "dashboard.download-binary-file")]] (when (seq frames) [:> dropdown-menu-item* {:class (stl/css :submenu-item) diff --git a/frontend/src/app/worker.cljs b/frontend/src/app/worker.cljs index 89476d0408..e6ca3fb1d2 100644 --- a/frontend/src/app/worker.cljs +++ b/frontend/src/app/worker.cljs @@ -11,7 +11,6 @@ [app.common.schema :as sm] [app.common.types.objects-map] [app.util.object :as obj] - [app.worker.export] [app.worker.impl :as impl] [app.worker.import] [app.worker.index] @@ -32,7 +31,7 @@ [:cmd :keyword]]] [:buffer? {:optional true} :boolean]]) -(def ^:private check-message! +(def ^:private check-message (sm/check-fn schema:message)) (def buffer (rx/subject)) @@ -41,9 +40,7 @@ "Process the message and returns to the client" [{:keys [sender-id payload transfer] :as message}] - (dm/assert! - "expected valid message" - (check-message! message)) + (assert (check-message message)) (letfn [(post [msg] (let [msg (-> msg (assoc :reply-to sender-id) (wm/encode))] diff --git a/frontend/src/app/worker/export.cljs b/frontend/src/app/worker/export.cljs deleted file mode 100644 index f120ea90fc..0000000000 --- a/frontend/src/app/worker/export.cljs +++ /dev/null @@ -1,44 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS INC - -(ns app.worker.export - (:require - [app.common.exceptions :as ex] - [app.main.repo :as rp] - [app.util.webapi :as wapi] - [app.worker.impl :as impl] - [beicon.v2.core :as rx])) - -(defmethod impl/handler :export-files - [{:keys [files type format] :as message}] - (assert (or (= format :binfile-v1) - (= format :binfile-v3)) - "expected valid format") - - (->> (rx/from files) - (rx/mapcat - (fn [file] - (->> (rp/cmd! :export-binfile {:file-id (:id file) - :version (if (= format :binfile-v3) 3 1) - :include-libraries (= type :all) - :embed-assets (= type :merge)}) - (rx/map wapi/create-blob) - (rx/map wapi/create-uri) - (rx/map (fn [uri] - {:type :finish - :file-id (:id file) - :filename (:name file) - :mtype (if (= format :binfile-v3) - "application/zip" - "application/penpot") - :uri uri})) - (rx/catch - (fn [cause] - (rx/of (ex/raise :type :internal - :code :export-error - :hint "unexpected error on exporting file" - :file-id (:id file) - :cause cause)))))))))