fix: user data corruption on unclean Windows shutdown

src/main/lib/mainStore.ts

@@ -1,4 +1,4 @@
-import { Account, LocalStorageData } from '@shared/types';
+import { Account, AuthAppData, LocalStorageData } from '@shared/types';
 import { app, ipcMain } from 'electron';
 import path from 'path';
 import fs from 'fs';
@@ -8,6 +8,105 @@ import { difference } from 'lodash';
 
 const AVAILABLE_USER_DATA_PATH = path.join(app.getPath("userData"), `available_user_data.json`);
 
+// on Windows the rename can transiently fail while an antivirus keeps the target open
+const RENAME_RETRY_DELAYS_MS = [10, 30, 90]
+
+function sleepSync(ms: number) {
+  Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms)
+}
+
+function renameWithRetry(from: string, to: string) {
+  for (let attempt = 0; ; attempt++) {
+    try {
+      fs.renameSync(from, to)
+      return
+    } catch (e: any) {
+      if (attempt >= RENAME_RETRY_DELAYS_MS.length) {
+        throw e
+      }
+      Log.warning('rename of', from, 'failed, retrying:', e?.code || e)
+      sleepSync(RENAME_RETRY_DELAYS_MS[attempt])
+    }
+  }
+}
+
+/**
+ * NTFS journals metadata only: after an unclean shutdown the new file size can be
+ * recovered from the journal while the data pages are still sitting in the page
+ * cache, leaving a file of the expected length filled with 0x00. Writing to a
+ * temporary file, flushing it to the platters and only then renaming it over the
+ * target keeps the previous content intact until the new one is durable on disk.
+ * The fsync is the load bearing part: without it the rename alone still allows a
+ * NUL-filled file to show up under the final name.
+ */
+function writeJsonAtomic(target: string, content: string) {
+  const tmp = `${target}.tmp`
+  const fd = fs.openSync(tmp, 'w')
+  try {
+    fs.writeFileSync(fd, content)
+    fs.fsyncSync(fd)
+  } finally {
+    fs.closeSync(fd)
+  }
+  try {
+    if (fs.existsSync(target)) {
+      fs.copyFileSync(target, `${target}.bak`)
+    }
+  } catch (e) {
+    Log.warning('unable to refresh the backup of', target, e)
+  }
+  renameWithRetry(tmp, target)
+}
+
+function isAllZeroes(buffer: Buffer) {
+  for (let i = 0; i < buffer.length; i++) {
+    if (buffer[i] !== 0) return false
+  }
+  return true
+}
+
+/**
+ * Returns undefined - never throws - when the file is missing, empty, recovered
+ * as NUL bytes after a crash, or simply not valid JSON.
+ */
+function parseJsonFile(filePath: string): any | undefined {
+  try {
+    if (!fs.existsSync(filePath)) return undefined
+    const buffer = fs.readFileSync(filePath)
+    if (buffer.length === 0) {
+      Log.warning('STORE', filePath, 'is empty')
+      return undefined
+    }
+    if (isAllZeroes(buffer)) {
+      Log.warning('STORE', filePath, 'contains only NUL bytes - corrupted by an unclean shutdown')
+      return undefined
+    }
+    return JSON.parse(buffer.toString('utf-8'))
+  } catch (e) {
+    Log.warning('STORE unable to read', filePath, e)
+    return undefined
+  }
+}
+
+/**
+ * The backup is consulted only when the primary file is there but unusable: a file
+ * that is simply missing means a fresh install - or someone removing it on purpose to
+ * force a clean login - and bringing it back from the backup would defeat that.
+ */
+function readJsonSafe<T>(filePath: string, fallback: T): T {
+  const data = parseJsonFile(filePath)
+  if (data !== undefined) return data as T
+  if (fs.existsSync(filePath)) {
+    const backup = parseJsonFile(`${filePath}.bak`)
+    if (backup !== undefined) {
+      Log.warning('STORE restored', filePath, 'from its backup')
+      return backup as T
+    }
+  }
+  Log.warning('STORE no usable data for', filePath, '- falling back to defaults')
+  return fallback
+}
+
 class Store<T> {
   assignedInstanceID;
   USER_DATA_PATH
@@ -21,23 +120,19 @@ class Store<T> {
     }
     Log.info({ assignedInstanceID: this.assignedInstanceID })
     this.USER_DATA_PATH = path.join(app.getPath("userData"), `user_data${this.assignedInstanceID || ''}.json`);
-    if (!fs.existsSync(this.USER_DATA_PATH) || !fs.existsSync(AVAILABLE_USER_DATA_PATH)) {
-      if (!fs.existsSync(AVAILABLE_USER_DATA_PATH)) {
-        if (!fs.existsSync(this.USER_DATA_PATH)) {
-          fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify({}))
-        } else {
-          const data = fs.readFileSync(this.USER_DATA_PATH, 'utf-8');
-          const retrivedStore: LocalStorageData = JSON.parse(data);
-          if (retrivedStore.auth?.availableAccounts) {
-            fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify(retrivedStore.auth!.availableAccounts))
-          } else {
-            fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify({}))
-          }
-        }
+    const hasUserData = fs.existsSync(this.USER_DATA_PATH)
+    const hasAvailableUserData = fs.existsSync(AVAILABLE_USER_DATA_PATH)
+    if (!hasUserData || !hasAvailableUserData) {
+      if (!hasAvailableUserData) {
+        // migration path for the installations where the accounts lived only in user_data.json
+        const userData = readJsonSafe<Partial<LocalStorageData>>(this.USER_DATA_PATH, {})
+        writeJsonAtomic(AVAILABLE_USER_DATA_PATH, JSON.stringify(userData?.auth?.availableAccounts ?? {}))
+      }
+      if (!hasUserData) {
+        writeJsonAtomic(this.USER_DATA_PATH, JSON.stringify({}))
       }
-      !fs.existsSync(this.USER_DATA_PATH) && fs.writeFileSync(this.USER_DATA_PATH, JSON.stringify({}))
     } else {
-      this.store = this.getFromDisk()
+      this.store = this.getFromDisk() ?? ({} as T)
     }
   }
 
@@ -57,46 +152,51 @@ class Store<T> {
     }
   }
 
-  updateStore(newState: T, from: string) {
+  updateStore(newState: T | null | undefined, from: string) {
     const diff = difference(Object.values(newState as any || {}), Object.values(this.store as any || {}))
     Log.debug('STORE update shared store from', from, Object.keys(newState as any || {}))
     if (diff.length > 0 || this.store === undefined) {
-      this.store = Object.assign({}, newState)
+      this.store = Object.assign({}, newState ?? ({} as T))
     }
   }
 
   saveToDisk(forceSave: boolean = false) {
-    const availableUserData = (this.store as LocalStorageData).auth?.availableAccounts
-    fs.writeFileSync(this.USER_DATA_PATH, JSON.stringify(this.store));
+    const availableUserData = (this.store as LocalStorageData)?.auth?.availableAccounts
+    try {
+      writeJsonAtomic(this.USER_DATA_PATH, JSON.stringify(this.store ?? {}));
+    } catch (e) {
+      Log.error('STORE unable to persist', this.USER_DATA_PATH, e)
+    }
     if (Object.keys(availableUserData || {}).length > 0 || forceSave) {
-      fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify(availableUserData));
+      try {
+        writeJsonAtomic(AVAILABLE_USER_DATA_PATH, JSON.stringify(availableUserData ?? {}));
+      } catch (e) {
+        Log.error('STORE unable to persist', AVAILABLE_USER_DATA_PATH, e)
+      }
     }
   }
 
   getAvailableFromDisk(): { [accountUID: string]: Account; } {
-    if (fs.existsSync(AVAILABLE_USER_DATA_PATH)) {
-      const availableUserData = fs.readFileSync(AVAILABLE_USER_DATA_PATH, 'utf-8');
-      return JSON.parse(availableUserData)
+    const availableUserData = readJsonSafe<{ [accountUID: string]: Account } | null>(AVAILABLE_USER_DATA_PATH, null)
+    if (!availableUserData || typeof availableUserData !== 'object') {
+      return {}
     }
-    return {}
+    return availableUserData
   }
 
-  getFromDisk() {
-    try {
-      const data = fs.readFileSync(this.USER_DATA_PATH, 'utf-8');
-      const availableUserData = fs.readFileSync(AVAILABLE_USER_DATA_PATH, 'utf-8');
-      const retrivedStore = JSON.parse(data);
-      (retrivedStore as LocalStorageData).auth!.availableAccounts = JSON.parse(availableUserData)
-      return retrivedStore
-    } catch (error) {
-      Log.error('retrieving user data', error);
-      // you may want to propagate the error, up to you
-      return null;
+  getFromDisk(): T | null {
+    const data = readJsonSafe<any>(this.USER_DATA_PATH, null)
+    if (!data || typeof data !== 'object') {
+      Log.error('retrieving user data: no usable content in', this.USER_DATA_PATH)
+      return null
     }
+    const retrivedStore = data as LocalStorageData
+    retrivedStore.auth = {
+      ...(retrivedStore.auth ?? {}),
+      availableAccounts: this.getAvailableFromDisk()
+    } as AuthAppData
+    return retrivedStore as T
   }
 }
 
 export const store: Store<LocalStorageData> = new Store<LocalStorageData>()
-
-
-

src/main/main.ts

@@ -6,7 +6,7 @@ import { registerIpcEvents, isCallActive, disableCommandBarShortcuts } from '@/l
 import { AccountController } from './classes/controllers'
 import { PhoneIslandController } from './classes/controllers/PhoneIslandController'
 import { CommandBarController } from './classes/controllers/CommandBarController'
-import { Account, AuthAppData, AvailableThemes } from '@shared/types'
+import { Account, AuthAppData, AvailableThemes, LocalStorageData } from '@shared/types'
 import { TrayController } from './classes/controllers/TrayController'
 import { LoginController } from './classes/controllers/LoginController'
 import { join, resolve } from 'path'
@@ -56,6 +56,7 @@ function startup() {
   app.setAppUserModelId('com.nethesis.nethlink.app') //must match electron-builder appId so Windows notification permissions and taskbar pinning work correctly
   ///LOGGER
   startLogger()
+  attachGlobalErrorHandlers()
 
   //windows
   const gotTheLock = multipleInstances || app.requestSingleInstanceLock()
@@ -214,6 +215,21 @@ function startLogger() {
   deleteLogFile()
 }
 
+/**
+ * Without these the main process swallows any rejection coming from the several fire
+ * and forget async flows, which makes issues that only show up on customer machines
+ * - a crash during startup leaving the app stuck on the splash screen - invisible in
+ * the logs.
+ */
+function attachGlobalErrorHandlers() {
+  process.on('unhandledRejection', (reason) => {
+    Log.error('UNHANDLED REJECTION', reason)
+  })
+  process.on('uncaughtException', (error) => {
+    Log.error('UNCAUGHT EXCEPTION', error)
+  })
+}
+
 function attachOnReadyProcess() {
   new AppController(app)
   new NetworkController()
@@ -242,13 +258,13 @@ function attachOnReadyProcess() {
           // Wait for unlock-screen event before starting
           powerMonitor.once('unlock-screen', () => {
             Log.info('Windows unlocked, starting app now...')
-            setTimeout(startApp, 1000)
+            setTimeout(() => safeStartApp(0), 1000)
           })
           return
         }
       }
       // Normal flow: start after 1 second
-      setTimeout(startApp, 1000)
+      setTimeout(() => safeStartApp(0), 1000)
     })
     await attachProtocolListeners()
 
@@ -300,12 +316,12 @@ function attachOnReadyProcess() {
   let waitingForConnection = false
 
   async function startApp(attempt = 0) {
-    let data = store.store || store.getFromDisk()
+    let data: LocalStorageData | null = store.store || store.getFromDisk()
     if (!checkData(data)) {
       if (attempt === 0) {
         data = store.getFromDisk()
         store.updateStore(data, 'startApp')
-        startApp(++attempt)
+        safeStartApp(++attempt)
         return;
       } else {
         await resetApp()
@@ -337,7 +353,7 @@ function attachOnReadyProcess() {
         }
 
         retryAppStart = setTimeout(() => {
-          startApp(++attempt)
+          safeStartApp(++attempt)
         }, 1000)
       } else {
         if (retryAppStart) {
@@ -382,6 +398,36 @@ function attachOnReadyProcess() {
     }
   }
 
+  /**
+   * startApp is always launched in a fire and forget fashion: without this guard an
+   * unexpected rejection - a corrupted user data file being the known case - leaves
+   * the splash screen up forever, since nothing would ever reach showLogin() nor
+   * SplashScreenController.quit(). Degrading to the login window is always better
+   * than a frozen "starting application" screen.
+   */
+  function safeStartApp(attempt = 0) {
+    startApp(attempt).catch((error) => {
+      Log.error('START - unrecoverable error while starting the app', error)
+      try {
+        showLogin()
+      } catch (e) {
+        Log.error('START - unable to show the login window', e)
+      }
+      try {
+        SplashScreenController.instance.window.quit(true)
+      } catch (e) {
+        Log.error('START - unable to close the splash screen', e)
+      }
+      try {
+        TrayController.instance.updateTray({
+          enableShowButton: true
+        })
+      } catch (e) {
+        Log.error('START - unable to update the tray', e)
+      }
+    })
+  }
+
   // Polling interval for checking connection when waiting
   let connectionCheckInterval: NodeJS.Timeout | null = null
 
@@ -404,7 +450,7 @@ function attachOnReadyProcess() {
         } catch (e) {
           // Splash screen might be closed
         }
-        startApp(0)
+        safeStartApp(0)
       }
     }, 5000) // Check every 5 seconds
   }
@@ -421,7 +467,7 @@ function attachOnReadyProcess() {
     Log.info('START - user requested connection retry')
     waitingForConnection = false
     stopConnectionPolling()
-    startApp(0)
+    safeStartApp(0)
   })
 
   app.on('window-all-closed', () => {