✨ 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

@@ -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)
+- Add the ability to 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 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
 

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)

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
+(defn- export-binfile
-  [cfg {:keys [file-id include-libraries embed-assets] :as params}]
+  [{:keys [::sto/storage] :as cfg} {:keys [file-id include-libraries embed-assets]}]
-  (rph/stream
+  (let [output  (tmp/tempfile*)]
-   (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))
+          (bf.v3/export-files! output))
-       (catch Throwable cause
-         (l/err :hint "exception on exporting file"
-                :file-id (str file-id)
-                :cause cause))))))
 
-(defn stream-export-v3
+      (let [data   (sto/content output)
-  [cfg {:keys [file-id include-libraries embed-assets] :as params}]
+            object (sto/put-object! storage
-  (rph/stream
+                                    {::sto/content data
-   (fn [_ output-stream]
+                                     ::sto/touched-at (ct/in-future {:minutes 60})
-     (try
+                                     :content-type "application/zip"
-       (-> cfg
+                                     :bucket "tempfile"})]
-           (assoc ::bfc/ids #{file-id})
+
-           (assoc ::bfc/embed-assets embed-assets)
+        (-> (cf/get :public-uri)
-           (assoc ::bfc/include-libraries include-libraries)
+            (u/join "/assets/by-id/")
-           (bf.v3/export-files! output-stream))
+            (u/join (str (:id object)))))
-       (catch Throwable cause
+
-         (l/err :hint "exception on exporting file"
+      (finally
-                :file-id (str file-id)
+        (fs/delete output)))))
-                :cause cause))))))
 
 (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)]
+  (sse/response (partial export-binfile cfg params)))
-    (case (int version)
-      1 (stream-export-v1 cfg params)
-      2 (throw (ex-info "not-implemented" {}))
-      3 (stream-export-v3 cfg params))))
 
 ;; --- Command: import-binfile
 

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))
 

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

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,24 +33,17 @@
 (def check-export-files
   (sm/check-fn schema:export-files))
 
-(defn export-files
+(defn open-export-dialog
-  [files format]
+  [files]
-  (assert (contains? valid-formats format)
-          "expected valid export format")
-
   (let [files (check-export-files files)]
-
     (ptk/reify ::export-files
       ptk/WatchEvent
       (watch [_ state _]
-        (let [team-id  (get state :current-team-id)
+        (let [team-id (get state :current-team-id)]
-              evname   (if (= format :legacy-zip)
-                         "export-standard-files"
-                         "export-binary-files")]
           (rx/merge
-           (rx/of (ptk/event ::ev/event {::ev/name evname
+           (rx/of (ev/event {::ev/name "export-binary-files"
                              ::ev/origin "dashboard"
-                                         :format format
+                             :format "binfile-v3"
                              :num-files (count files)}))
            (->> (rx/from files)
                 (rx/mapcat
@@ -58,11 +52,29 @@
                         (rx/map #(assoc file :has-libraries %)))))
                 (rx/reduce conj [])
                 (rx/map (fn [files]
-                          (modal/show
+                          (modal/show {:type ::export-files
-                           {:type ::export-files
                                        :team-id team-id
-                            :files files
+                                       :files files}))))))))))
-                            :format format}))))))))))
+
+(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

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}
 

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)
+        (fn []
-
+          (st/emit! (-> (fexp/open-export-dialog files)
-        on-export-binary-files-v3
+                        (with-meta {::ev/origin "dashboard"}))))]
-        (partial on-export-files :binfile-v3)
-
-        on-export-standard-files
-        (partial on-export-files :legacy-zip)]
 
     (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})
+              :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})
 
              (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})
+              :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})
 
              (when (and (not is-lib-page?) (not is-search-page?) can-edit)
                {:name   :separator})

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!
+           (->> (fexp/export-files :files files :type type)
-                 {:cmd :export-files
-                  :format format
-                  :team-id team-id
-                  :type selected
-                  :files files})
-                (rx/mapcat #(->> (rx/of %)
-                                 (rx/delay 1000)))
                 (rx/subs!
-                 (fn [msg]
+                 (fn [{:keys [file-id error filename uri] :as result}]
-                   (cond
+                   (if error
-                     (= :error (:type msg))
+                     (swap! state* update :files mark-file-error file-id)
-                     (swap! state* update :files mark-file-error (:file-id msg))
+                     (do
-
+                       (swap! state* update :files mark-file-success file-id)
-                     (= :finish (:type msg))
+                       (dom/trigger-download-uri filename "application/penpot" uri))))))))
-                     (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))))))))
 
         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)

frontend/src/app/main/ui/workspace/main_menu.cljs

@@ -602,12 +602,9 @@
         on-export-file
         (mf/use-fn
          (mf/deps file)
-         (fn [event]
+         (fn [_]
-           (let [target  (dom/get-current-target event)
+           (st/emit! (-> (fexp/open-export-dialog [file])
-                 format  (-> (dom/get-data target "format")
+                         (with-meta {::ev/origin "workspace"})))))
-                             (keyword))]
-             (st/emit! (st/emit! (with-meta (fexp/export-files [file] format)
-                                   {::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")]])
+       (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")]])
 
      (when (seq frames)
        [:> dropdown-menu-item* {:class (stl/css :submenu-item)

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!
+  (assert (check-message message))
-   "expected valid message"
-   (check-message! message))
 
   (letfn [(post [msg]
             (let [msg (-> msg (assoc :reply-to sender-id) (wm/encode))]

frontend/src/app/worker/export.cljs

@@ -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)))))))))