save files more safely (temp then move)

package-deps.json

@@ -14,6 +14,7 @@
     "eslint-config-scratch": "^6.0.0",
     "eslint-plugin-import": "^2.20.0",
     "eslint-plugin-react": "^7.20.0",
+    "fs-extra": "^9.0.1",
     "mkdirp": "^1.0.4",
     "nets": "^3.2.0",
     "rimraf": "^3.0.2",

package-lock.json

@@ -7513,9 +7513,9 @@
       }
     },
     "fs-extra": {
-      "version": "9.0.0",
-      "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-9.0.0.tgz",
-      "integrity": "sha512-pmEYSk3vYsG/bF651KPUXZ+hvjpgWYw/Gc7W9NFUe3ZVLczKKWIij3IKpOrQcdw4TILtibFslZ0UmR8Vvzig4g==",
+      "version": "9.0.1",
+      "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-9.0.1.tgz",
+      "integrity": "sha512-h2iAoN838FqAFJY2/qVpzFXy+EBxfVE220PalAqQLDVsFOHLJrZvut5puAbCdNv6WJk+B8ihI+k0c7JK5erwqQ==",
       "dev": true,
       "requires": {
         "at-least-node": "^1.0.0",

package.json

@@ -68,6 +68,7 @@
     "eslint-plugin-jest": "^22.14.1",
     "eslint-plugin-react": "^7.20.0",
     "file-loader": "2.0.0",
+    "fs-extra": "^9.0.1",
     "get-float-time-domain-data": "0.1.0",
     "get-user-media-promise": "1.1.4",
     "gh-pages": "github:rschamp/gh-pages#publish-branch-to-subfolder",

src/main/index.js

@@ -1,5 +1,5 @@
 import {BrowserWindow, Menu, app, dialog, ipcMain, systemPreferences} from 'electron';
-import fs from 'fs';
+import fs from 'fs-extra';
 import path from 'path';
 import {URL} from 'url';
 
@@ -223,9 +223,9 @@ const createMainWindow = () => {
     });
     const webContents = window.webContents;
 
-    webContents.session.on('will-download', (ev, item) => {
-        const isProjectSave = getIsProjectSave(item);
-        const itemPath = item.getFilename();
+    webContents.session.on('will-download', (willDownloadEvent, downloadItem) => {
+        const isProjectSave = getIsProjectSave(downloadItem);
+        const itemPath = downloadItem.getFilename();
         const baseName = path.basename(itemPath);
         const extName = path.extname(baseName);
         const options = {
@@ -236,30 +236,51 @@ const createMainWindow = () => {
             options.filters = [getFilterForExtension(extNameNoDot)];
         }
         const userChosenPath = dialog.showSaveDialogSync(window, options);
+        // this will be falsy if the user canceled the save
         if (userChosenPath) {
-            // If the file exists the browser will first download to a temp file then rename to the userChosenPath.
-            // The MAS sandbox allows accessing userChosenPath but not the temp file, so overwriting fails on MAS.
-            // Deleting the file first could be considered risky but works around the sandbox problem.
-            // Security bookmarks might work to fix the problem but they're only supported by async showSaveDialog.
-            // Since we need to use showSaveDialogSync (see WARNING below) this workaround might be the best option.
-            if (fs.existsSync(userChosenPath)) {
-                fs.unlinkSync(userChosenPath);
-            }
+            const userBaseName = path.basename(userChosenPath);
+            const tempPath = path.join(app.getPath('temp'), userBaseName);
+
             // WARNING: `setSavePath` on this item is only valid during the `will-download` event. Calling the async
             // version of `showSaveDialog` means the event will finish before we get here, so `setSavePath` will be
             // ignored. For that reason we need to call `showSaveDialogSync` above.
-            item.setSavePath(userChosenPath);
+            downloadItem.setSavePath(tempPath);
+
+            downloadItem.on('done', async (doneEvent, doneState) => {
+                try {
+                    if (doneState !== 'completed') {
+                        // The download was canceled or interrupted. Cancel the telemetry event and delete the file.
+                        throw new Error(`save ${doneState}`); // "save cancelled" or "save interrupted"
+                    }
+                    await fs.move(tempPath, userChosenPath, {overwrite: true});
                     if (isProjectSave) {
                         const newProjectTitle = path.basename(userChosenPath, extName);
                         webContents.send('setTitleFromSave', {title: newProjectTitle});
 
-                // "setTitleFromSave" will set the project title but GUI has already reported the telemetry event
-                // using the old title. This call lets the telemetry client know that the save was actually completed
-                // and the event should be committed to the event queue with this new title.
+                        // "setTitleFromSave" will set the project title but GUI has already reported the telemetry
+                        // event using the old title. This call lets the telemetry client know that the save was
+                        // actually completed and the event should be committed to the event queue with this new title.
                         telemetry.projectSaveCompleted(newProjectTitle);
                     }
+                } catch (e) {
+                    if (isProjectSave) {
+                        telemetry.projectSaveCanceled();
+                    }
+                    // don't clean up until after the message box to allow troubleshooting / recovery
+                    await dialog.showMessageBox(window, {
+                        type: 'error',
+                        message: `Save failed:\n${userChosenPath}`,
+                        detail: e.message
+                    });
+                    fs.exists(tempPath).then(exists => {
+                        if (exists) {
+                            fs.unlink(tempPath);
+                        }
+                    });
+                }
+            });
         } else {
-            item.cancel();
+            downloadItem.cancel();
             if (isProjectSave) {
                 telemetry.projectSaveCanceled();
             }