Package: a failed save before packaging writes no package

With Background Save on, Package's OK started the save of a dirty
document on the worker and then wrote the package whether or not that
save succeeded. With both preferences off, a failed save already stopped
the package, so the result depended on the preferences.

The package now starts only after the save is written, with both
preferences in every combination. With Background Export on, it is still
collected from the document as it was at OK and starts once the save's
result arrives; when the save fails, or asks a question first, no package
is written and the save's error stays in the status bar. With Background
Export off, the dialog shows the save's error.

background::after runs a step on the UI thread once a job's result is
applied.

crates/ui-egui/src/background.rs

@@ -190,6 +190,19 @@ pub fn run_with_progress(
     then(app, r)
 }
 
+/// Run `f` on the UI thread once job `id` has finished, right after the job's own result is
+/// applied, with that applied result (an error when the work failed). Nothing runs when no job
+/// `id` is in progress.
+pub fn after(app: &mut VectorcraftApp, id: u64, f: impl FnOnce(&mut VectorcraftApp, &Result<Value, String>) + 'static) {
+    let Some(job) = app.background.jobs.iter_mut().find(|j| j.id == id) else { return };
+    let then = std::mem::replace(&mut job.then, Box::new(|_, r| r));
+    job.then = Box::new(move |app, r| {
+        let r = then(app, r);
+        f(app, &r);
+        r
+    });
+}
+
 /// Run `f` with every save and export it starts written at once, as with the preferences off, so
 /// its result is the written file or the error (a control client's `wait: true`). The jobs already
 /// running finish first, so none of them writes over its files afterwards.

crates/ui-egui/src/dialogs/package.rs

@@ -1,14 +1,14 @@
 //! File → Package…: where the package folder goes, its name, and what it collects (the linked
 //! files, in a folder of their own or not, relinked or not; the fonts; the report). A document
-//! never saved is saved first (Save As); unsaved changes are saved before packaging. The web
-//! downloads the package as a zip.
+//! never saved is saved first (Save As); unsaved changes are saved before packaging, and when that
+//! save fails no package is written. The web downloads the package as a zip.
 //!
 //! Fields: `folder` (the location), `name` (the package folder), `copyLinks`, `linksFolder`,
 //! `relink`, `copyFonts`, `report`. OK runs `file.package` with them, then offers to show the
 //! package folder (`file.showPackage`). On the desktop with Background Export on, the package is
 //! collected and written on the background thread ([`crate::background`]) after the dialog closes.
 
-use std::cell::Cell;
+use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 
 use serde_json::{Value, json};
@@ -108,21 +108,35 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
     } else if d.str("folder").trim().is_empty() {
         return Err("choose a location for the package".into());
     }
-    // The package holds the document as saved.
+    // The package holds the document as saved: when the save fails, or asks a question and writes
+    // nothing yet, no package is written.
+    let mut saving = None;
     if app.session.active().is_some_and(|st| st.is_dirty()) {
         let r = io::save(app, vectorcraft_engine::cmd::fileio::SaveMode::Save, &json!({}), false)?;
         // The save asks first (it would replace a file that loses what opening it left out).
         if r.get("pending").is_some() {
             return Ok(r);
         }
+        // Background Save: the save is written on the worker, and its result decides the package.
+        if r["background"] == true {
+            saving = app.background.jobs.last().map(background::Job::id);
+        }
     }
     // Background Export (Preferences › File Handling): the linked files and fonts are collected
     // and the package is written from the document as it is now, while editing goes on. The jobs
-    // started before it (a save started above, an earlier package to the same folder) are written
-    // first: the worker runs its jobs in order, and a package made at once waits for them.
+    // started before it (an earlier package to the same folder) are written first: the worker
+    // runs its jobs in order, and a package made at once waits for them.
     let enabled = app.session.prefs.background_export;
     if !enabled {
+        let outcome: Rc<RefCell<Option<Result<Value, String>>>> = Rc::default();
+        if let Some(id) = saving {
+            let applied = outcome.clone();
+            background::after(app, id, move |_, r| *applied.borrow_mut() = Some(r.clone()));
+        }
         background::wait_all(app);
+        if let Some(r) = outcome.take().filter(|r| !saved(r)) {
+            return r;
+        }
     }
     let package = app.session.prepare_package(&params).map_err(|e| e.to_string()).inspect_err(|e| app.ui.status = e.clone())?;
     let label = format!("Packaging {}", d.str("name"));
@@ -134,7 +148,23 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
             |path: &str, bytes: &[u8]| write(path, bytes).map_err(|e| EngineError::Other(if e.contains(path) { e } else { format!("{path}: {e}") }));
         package.write(&mut write).map_err(|e| e.to_string())
     };
-    let r = background::run(app, enabled, label, None, work, move |app, r| packaged(app, r, web, in_background.get()))?;
+    let start =
+        move |app: &mut VectorcraftApp| background::run(app, enabled, label, None, work, move |app, r| packaged(app, r, web, in_background.get()));
+    // Both in the background: the package, collected from the document as it is now, starts once
+    // the save is written.
+    if let Some(id) = saving.filter(|_| enabled) {
+        later.set(true);
+        app.ui.dialog = None;
+        background::after(app, id, move |app, r| {
+            if saved(r)
+                && let Err(e) = start(app)
+            {
+                app.status(e);
+            }
+        });
+        return Ok(json!({ "background": true }));
+    }
+    let r = start(app)?;
     if r["background"] == true {
         later.set(true);
         app.ui.dialog = None;
@@ -142,6 +172,12 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
     Ok(r)
 }
 
+/// Did a save whose applied result is `r` write the document (no error, and no question asked
+/// before writing)?
+fn saved(r: &Result<Value, String>) -> bool {
+    r.as_ref().is_ok_and(|r| r.get("pending").is_none())
+}
+
 /// Report the package `r` (`file.package`'s reply): the status bar, the web's download, and the
 /// offer to show the package folder. `later`: it was written in the background, after the dialog
 /// closed, and a dialog open now is another one, which stays (the status bar names the folder).

crates/ui-egui/src/dialogs/tests_package.rs

@@ -181,7 +181,7 @@ fn a_dirty_documents_save_is_written_before_its_package() {
     let (mut app, files) = saved_with_worker(&dir, &gate);
     let (doc, out) = (dir.path("card.vectorcraft"), dir.path("out"));
     // With Background Export on, the save and the package both run in the background, and the
-    // worker writes the save first.
+    // package starts once the save is written.
     app.run("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap();
     package_dialog(&mut app, &out);
     // A thread opens the gate after 10 s at the latest, so a confirm that waits for the save ends.
@@ -191,7 +191,7 @@ fn a_dirty_documents_save_is_written_before_its_package() {
         opener.open();
     });
     assert_eq!(confirm(&mut app).unwrap()["background"], true);
-    assert_eq!(app.background.jobs.len(), 2, "the save and the package");
+    assert_eq!(app.background.jobs.len(), 1, "the save; the package waits for it");
     gate.open();
     background::wait_all(&mut app);
     let paths = written(&files);
@@ -216,6 +216,38 @@ fn a_dirty_documents_save_is_written_before_its_package() {
     assert!(app.ui.status.starts_with("Packaged 3 files in "), "the package's status came last: {}", app.ui.status);
 }
 
+/// A save before packaging that fails writes no package, with Background Export on or off.
+#[test]
+fn a_failed_save_before_packaging_writes_no_package() {
+    let dir = Folder::new("failed-save");
+    let (doc, out) = (dir.path("card.vectorcraft"), dir.path("out"));
+    let root = std::path::Path::new(&out).join("card Folder");
+    for background_export in [true, false] {
+        let mut app = saved(&dir);
+        let (files, refused) = (Files::default(), doc.clone());
+        let recorded = files.clone();
+        app.services.write_shared = Some(Arc::new(move |p: &str, b: &[u8]| {
+            if p == refused {
+                return Err("Access is denied. (os error 5)".into());
+            }
+            recorded.lock().unwrap().push((p.to_string(), b.to_vec()));
+            Ok(())
+        }));
+        app.session.execute("prefs.set", &json!({"values": {"backgroundExport": background_export}})).unwrap();
+        app.run("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap();
+        package_dialog(&mut app, &out);
+        let r = confirm(&mut app);
+        background::wait_all(&mut app);
+        assert!(written(&files).is_empty(), "Background Export {background_export}: {:?}", written(&files));
+        assert!(!root.exists(), "Background Export {background_export}: no package folder");
+        assert!(app.session.active().unwrap().is_dirty(), "the save failed");
+        match r {
+            Ok(r) => assert!(background_export && r["background"] == true, "{r}"),
+            Err(e) => assert!(!background_export && e.contains("Access is denied"), "{e}"),
+        }
+    }
+}
+
 #[test]
 fn a_package_made_at_once_waits_for_one_in_the_background() {
     let dir = Folder::new("twice");

docs/control-protocol.md

@@ -762,8 +762,8 @@ the `PATH` elsewhere), and any other file, or a raster image without PhotoCraft
 
 Package: File → Package… (`ui.packageDialog`) opens the `package` dialog for a saved document (a document never saved
 gets a `confirm` dialog whose OK runs `file.saveAs`). Fields: `folder` (the location), `name` (the package folder),
-`copyLinks`, `linksFolder`, `relink`, `copyFonts`, `report`. `ui.dialog.confirm` saves unsaved changes, runs
-`file.package` and then opens a `confirm` dialog whose OK runs `file.showPackage {folder}`; the web downloads the zip
+`copyLinks`, `linksFolder`, `relink`, `copyFonts`, `report`. `ui.dialog.confirm` saves unsaved changes (when that save
+fails, no package is written, also with Background Save and Background Export on), runs `file.package` and then opens a `confirm` dialog whose OK runs `file.showPackage {folder}`; the web downloads the zip
 instead. Document Info's flyout picks a category and Save… runs `docInfo.save {path?, selectionOnly?, category?}` with the
 category the panel shows (none for All Categories, which writes every category).