Merge pull request #2373 from element-hq/dbkr/safestorage_fixloop

Fix restart loop in safeStorage

package.json

@@ -58,7 +58,6 @@
     },
     "dependencies": {
         "@sentry/electron": "^6.0.0",
-        "@standardnotes/electron-clear-data": "^1.0.5",
         "auto-launch": "^5.0.5",
         "counterpart": "^0.18.6",
         "electron-store": "^10.0.0",

src/electron-main.ts

@@ -453,14 +453,6 @@ app.on("ready", async () => {
         store,
     });
 
-    try {
-        console.debug("Ensuring storage is ready");
-        await store.safeStorageReady();
-    } catch (e) {
-        console.error(e);
-        app.exit(1);
-    }
-
     // Load the previous window state with fallback to defaults
     const mainWindowState = windowStateKeeper({
         defaultWidth: 1024,
@@ -492,6 +484,15 @@ app.on("ready", async () => {
             webgl: true,
         },
     });
+
+    try {
+        console.debug("Ensuring storage is ready");
+        if (!(await store.prepareSafeStorage(global.mainWindow.webContents.session))) return;
+    } catch (e) {
+        console.error(e);
+        app.exit(1);
+    }
+
     void global.mainWindow.loadURL("vector://vector/webapp/");
 
     if (process.platform === "darwin") {

src/ipc.ts

@@ -174,7 +174,7 @@ ipcMain.on("ipcCall", async function (_ev: IpcMainEvent, payload) {
             break;
 
         case "clearStorage":
-            await clearDataAndRelaunch();
+            await clearDataAndRelaunch(global.mainWindow.webContents.session);
             return; // the app is about to stop, we don't need to reply to the IPC
 
         case "breadcrumbs": {

src/store.ts

@@ -16,8 +16,7 @@ limitations under the License.
 
 import ElectronStore from "electron-store";
 import keytar from "keytar-forked";
-import { app, safeStorage, dialog, type SafeStorage } from "electron";
+import { app, safeStorage, dialog, type SafeStorage, type Session } from "electron";
-import { clearAllUserData, relaunchApp } from "@standardnotes/electron-clear-data";
 
 import { _t } from "./language-helper.js";
 
@@ -61,12 +60,19 @@ const safeStorageBackendMap: Omit<
     kwallet5: "kwallet5",
 };
 
+function relaunchApp(): void {
+    console.info("Relaunching app...");
+    app.relaunch();
+    app.exit();
+}
+
 /**
  * Clear all data and relaunch the app.
  */
-export async function clearDataAndRelaunch(): Promise<void> {
+export async function clearDataAndRelaunch(electronSession: Session): Promise<void> {
     Store.instance?.clear();
-    clearAllUserData();
+    electronSession.flushStorageData();
+    await electronSession.clearStorageData();
     relaunchApp();
 }
 
@@ -227,10 +233,10 @@ class Store extends ElectronStore<StoreData> {
         });
     }
 
-    private safeStorageReadyPromise?: Promise<unknown>;
+    private safeStorageReadyPromise?: Promise<boolean>;
     public async safeStorageReady(): Promise<void> {
         if (!this.safeStorageReadyPromise) {
-            this.safeStorageReadyPromise = this.prepareSafeStorage();
+            throw new Error("prepareSafeStorage must be called before using storage methods");
         }
         await this.safeStorageReadyPromise;
     }
@@ -270,8 +276,18 @@ class Store extends ElectronStore<StoreData> {
      * Prepare the safeStorage backend for use.
      * We don't eagerly import from keytar as that would bring in data for all Element profiles and not just the current one,
      * so we import lazily in getSecret.
+     *
+     * This will relaunch the app in some cases, in which case it will return false and the caller should abort startup.
+     *
+     * @param electronSession - The Electron session to use for storage (will be used to clear storage if necessary).
+     * @returns true if safeStorage was initialised successfully or false if the app will be relaunched
      */
-    private async prepareSafeStorage(): Promise<void> {
+    public async prepareSafeStorage(electronSession: Session): Promise<boolean> {
+        this.safeStorageReadyPromise = this.reallyPrepareSafeStorage(electronSession);
+        return this.safeStorageReadyPromise;
+    }
+
+    private async reallyPrepareSafeStorage(electronSession: Session): Promise<boolean> {
         await app.whenReady();
 
         // The backend the existing data is written with if any
@@ -282,11 +298,13 @@ class Store extends ElectronStore<StoreData> {
         // Handle migrations
         if (existingSafeStorageBackend) {
             if (existingSafeStorageBackend === "basic_text" && backend !== "plaintext" && backend !== "basic_text") {
-                return this.prepareMigrateBasicTextToPlaintext();
+                this.prepareMigrateBasicTextToPlaintext();
+                return false;
             }
 
             if (this.get("safeStorageBackendMigrate") && backend === "basic_text") {
-                return this.migrateBasicTextToPlaintext();
+                this.migrateBasicTextToPlaintext();
+                return false;
             }
 
             if (existingSafeStorageBackend === "plaintext" && backend !== "plaintext") {
@@ -305,13 +323,21 @@ class Store extends ElectronStore<StoreData> {
             // Store the backend used for the safeStorage data so we can detect if it changes, and we know how the data is encoded
             this.recordSafeStorageBackend(backend);
         } else if (existingSafeStorageBackend !== backend) {
+            // We already appear to have started using a backend other than the one that we picked, so
+            // set the override flag and relaunch with the backend we were previously using, unless we
+            // already have the override flag, in which case we must assume the previous backend is no
+            // longer usable, in which case we should fall into the next block and warn the user we can't
+            // migrate.
             console.warn(`safeStorage backend changed from ${existingSafeStorageBackend} to ${backend}`);
 
-            if (existingSafeStorageBackend in safeStorageBackendMap) {
+            if (existingSafeStorageBackend in safeStorageBackendMap && !this.get("safeStorageBackendOverride")) {
                 this.set("safeStorageBackendOverride", true);
-                return relaunchApp();
+                relaunchApp();
+                return false;
             } else {
-                await this.consultUserBackendChangedUnableToMigrate();
+                // This will either relaunch the app or throw an execption
+                await this.consultUserBackendChangedUnableToMigrate(electronSession);
+                return false;
             }
         }
 
@@ -321,9 +347,11 @@ class Store extends ElectronStore<StoreData> {
         } else {
             this.secrets = new StorageWriter(this);
         }
+
+        return true;
     }
 
-    private async consultUserBackendChangedUnableToMigrate(): Promise<void> {
+    private async consultUserBackendChangedUnableToMigrate(electronSession: Session): Promise<void> {
         const { response } = await dialog.showMessageBox({
             title: _t("store|error|backend_changed_title"),
             message: _t("store|error|backend_changed"),
@@ -336,7 +364,7 @@ class Store extends ElectronStore<StoreData> {
         if (response === 0) {
             throw new Error("safeStorage backend changed and cannot migrate");
         }
-        return clearDataAndRelaunch();
+        return clearDataAndRelaunch(electronSession);
     }
 
     private async consultUserConsentDegradedMode(backend: "plaintext" | "basic_text"): Promise<void> {

yarn.lock

@@ -1048,7 +1048,6 @@
 
 "@electron/node-gyp@https://github.com/electron/node-gyp#06b29aafb7708acef8b3669835c8a7857ebc92d2":
   version "10.2.0-electron.1"
-  uid "06b29aafb7708acef8b3669835c8a7857ebc92d2"
   resolved "https://github.com/electron/node-gyp#06b29aafb7708acef8b3669835c8a7857ebc92d2"
   dependencies:
     env-paths "^2.2.0"
@@ -1334,7 +1333,6 @@
 
 "@fastify/otel@github:getsentry/fastify-otel#otel-v1":
   version "0.8.0"
-  uid ae3088d65e286bdc94ac5d722573537d6a6671bb
   resolved "https://codeload.github.com/getsentry/fastify-otel/tar.gz/ae3088d65e286bdc94ac5d722573537d6a6671bb"
   dependencies:
     "@opentelemetry/core" "^1.30.1"
@@ -2034,7 +2032,7 @@
   resolved "https://registry.yarnpkg.com/@sentry/node/-/node-9.18.0.tgz#682ad11030548eb0e0674ff091afc65319da6d8d"
   integrity sha512-n0H13YVfynZJnKQLHoTlyBK2P960X8+B08za9VaRnJ4zikDx23Xk2Owtj006ZUItUKtKLFi70NyQGGDp7gVAyw==
   dependencies:
-    "@fastify/otel" "github:getsentry/fastify-otel#otel-v1"
+    "@fastify/otel" getsentry/fastify-otel#otel-v1
     "@opentelemetry/api" "^1.9.0"
     "@opentelemetry/context-async-hooks" "^1.30.1"
     "@opentelemetry/core" "^1.30.1"
@@ -2127,11 +2125,6 @@
   resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-4.6.0.tgz#3c7c9c46e678feefe7a2e5bb609d3dbd665ffb3f"
   integrity sha512-t09vSN3MdfsyCHoFcTRCH/iUtG7OJ0CsjzB8cjAmKc/va/kIgeDI/TxsigdncE/4be734m0cvIYwNaV4i2XqAw==
 
-"@standardnotes/electron-clear-data@^1.0.5":
-  version "1.1.1"
-  resolved "https://registry.yarnpkg.com/@standardnotes/electron-clear-data/-/electron-clear-data-1.1.1.tgz#45eab118ed5d1ee9369b540d7e62a9ca96778335"
-  integrity sha512-R0YivtSwSQpNt5nPOi7YRTGlk6kpcz6/2/sAQZf6ZCU8vIGm1cBMo++6kkGQcDEumkwbmagxmLWinL9d1W5g3Q==
-
 "@stylistic/eslint-plugin@^4.0.0":
   version "4.4.0"
   resolved "https://registry.yarnpkg.com/@stylistic/eslint-plugin/-/eslint-plugin-4.4.0.tgz#e1a3c9fd7109411d32dc0bcb575d2b4066fbbc63"