♻️ refactor(electron): moving back from websockets to IPC (#2190)

.gitattributes

@@ -16,4 +16,4 @@ yarn.lock text eol=lf
 
 # Denote all files that are truly binary and should not be modified.
 *.png binary
-*.jpg binary
+*.jpg binary

packages/desktop-client/src/browser-preload.browser.js

@@ -125,6 +125,7 @@ global.Actual = {
   applyAppUpdate: () => {},
   updateAppMenu: isBudgetOpen => {},
 
+  ipcConnect: () => {},
   getServerSocket: async () => {
     return worker;
   },

packages/desktop-electron/index.js

@@ -5,11 +5,6 @@ const fs = require('fs');
 
 require('module').globalPaths.push(__dirname + '/..');
 
-// Allow unsecure in dev
-if (isDev) {
-  process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0';
-}
-
 const {
   app,
   ipcMain,
@@ -18,6 +13,7 @@ const {
   dialog,
   shell,
   protocol,
+  utilityProcess,
 } = require('electron');
 const promiseRetry = require('promise-retry');
 
@@ -30,15 +26,12 @@ protocol.registerSchemesAsPrivileged([
 global.fetch = require('node-fetch');
 
 const about = require('./about');
-const { getRandomPort } = require('get-port-please');
 const getMenu = require('./menu');
 const updater = require('./updater');
 
 require('./security');
 
-const { fork } = require('child_process');
 const path = require('path');
-const http = require('http');
 
 require('./setRequireHook');
 
@@ -57,7 +50,6 @@ const WindowState = require('./window-state.js');
 // be closed automatically when the JavaScript object is garbage collected.
 let clientWin;
 let serverProcess;
-let serverSocket;
 
 updater.onEvent((type, data) => {
   // Notify both the app and the about window
@@ -74,10 +66,10 @@ if (isDev) {
   process.traceProcessWarnings = true;
 }
 
-function createBackgroundProcess(socketName) {
-  serverProcess = fork(
+function createBackgroundProcess() {
+  serverProcess = utilityProcess.fork(
     __dirname + '/server.js',
-    ['--subprocess', app.getVersion(), socketName],
+    ['--subprocess', app.getVersion()],
     isDev ? { execArgv: ['--inspect'] } : undefined,
   );
 
@@ -93,52 +85,20 @@ function createBackgroundProcess(socketName) {
           updater.stop();
         }
         break;
+      case 'reply':
+      case 'error':
+      case 'push':
+        if (clientWin) {
+          clientWin.webContents.send('message', msg);
+        }
+        break;
       default:
         console.log('Unknown server message: ' + msg.type);
     }
   });
-
-  return serverProcess;
-}
-
-const isPortFree = port =>
-  new Promise(resolve => {
-    const server = http
-      .createServer()
-      .listen(port, () => {
-        server.close();
-        resolve(true);
-      })
-      .on('error', () => {
-        resolve(false);
-      });
-  });
-
-async function createSocketConnection() {
-  if (!serverSocket) serverSocket = await getRandomPort();
-
-  // Spawn the child process if it is not already running
-  // (sometimes long child processes die, so we need to set them
-  // up again)
-  const isFree = await isPortFree(serverSocket);
-  if (isFree) {
-    await createBackgroundProcess(serverSocket);
-  }
-
-  if (!clientWin) {
-    return;
-  }
-
-  // Send a heartbeat to the client whenever we attempt to create a new
-  // sockets connection
-  clientWin.webContents.executeJavaScript(
-    `window.__actionsForMenu && window.__actionsForMenu.reconnect(${serverSocket})`,
-  );
 }
 
 async function createWindow() {
-  await createSocketConnection();
-
   const windowState = await WindowState.get();
 
   // Create the browser window.
@@ -194,10 +154,6 @@ async function createWindow() {
     }
   });
 
-  win.webContents.on('did-finish-load', () => {
-    win.webContents.send('set-socket', { name: serverSocket });
-  });
-
   // hit when middle-clicking buttons or <a href/> with a target set to _blank
   // always deny, optionally redirect to browser
   win.webContents.setWindowOpenHandler(({ url }) => {
@@ -304,6 +260,8 @@ app.on('ready', async () => {
   require('electron').powerMonitor.on('suspend', () => {
     console.log('Suspending', new Date());
   });
+
+  createBackgroundProcess();
 });
 
 app.on('window-all-closed', () => {
@@ -326,14 +284,6 @@ app.on('activate', () => {
   }
 });
 
-app.on('did-become-active', () => {
-  // Reconnect whenever the window becomes active;
-  // We don't know what might have happened in-between, so it's better
-  // to be safe than sorry; the client can then decide if it wants to
-  // reconnect or not.
-  createSocketConnection();
-});
-
 ipcMain.on('get-bootstrap-data', event => {
   event.returnValue = {
     version: app.getVersion(),
@@ -377,6 +327,14 @@ ipcMain.on('show-about', () => {
   about.openAboutWindow();
 });
 
+ipcMain.on('message', (_event, msg) => {
+  if (!serverProcess) {
+    return;
+  }
+
+  serverProcess.postMessage(msg.args);
+});
+
 ipcMain.on('screenshot', () => {
   if (isDev) {
     const width = 1100;

packages/desktop-electron/package.json

@@ -46,11 +46,9 @@
     "electron-is-dev": "2.0.0",
     "electron-log": "4.4.8",
     "electron-updater": "6.1.4",
-    "get-port-please": "3.0.1",
     "loot-core": "*",
     "node-fetch": "^2.6.9",
-    "promise-retry": "^2.0.1",
-    "ws": "8.13.0"
+    "promise-retry": "^2.0.1"
   },
   "devDependencies": {
     "@electron/notarize": "2.1.0",

packages/desktop-electron/preload.js

@@ -3,15 +3,6 @@ const { ipcRenderer, contextBridge } = require('electron');
 const { version: VERSION, isDev: IS_DEV } =
   ipcRenderer.sendSync('get-bootstrap-data');
 
-let resolveSocketPromise;
-const socketPromise = new Promise(resolve => {
-  resolveSocketPromise = resolve;
-});
-
-ipcRenderer.on('set-socket', (event, { name }) => {
-  resolveSocketPromise(name);
-});
-
 contextBridge.exposeInMainWorld('Actual', {
   IS_DEV,
   ACTUAL_VERSION: VERSION,
@@ -19,6 +10,17 @@ contextBridge.exposeInMainWorld('Actual', {
     require('console').log(...args);
   },
 
+  ipcConnect: func => {
+    func({
+      on(name, handler) {
+        return ipcRenderer.on(name, (_event, value) => handler(value));
+      },
+      emit(name, data) {
+        return ipcRenderer.send('message', { name, args: data });
+      },
+    });
+  },
+
   relaunch: () => {
     ipcRenderer.invoke('relaunch');
   },
@@ -52,7 +54,7 @@ contextBridge.exposeInMainWorld('Actual', {
   },
 
   getServerSocket: () => {
-    return socketPromise;
+    return null;
   },
 
   setTheme: theme => {

packages/desktop-electron/server.js

@@ -9,22 +9,7 @@ function getBackend() {
   return require('loot-core/lib-dist/bundle.desktop.js');
 }
 
-if (process.argv[2] === '--subprocess') {
-  const isDev = false;
-  // let version = process.argv[3];
-  const socketName = process.argv[4];
+const isDev = false;
 
-  // Start the app
-  getBackend().initApp(isDev, socketName);
-} else if (process.argv[2] === '--standalone') {
-  require('source-map-support').install();
-  getBackend().initApp(true, 'actual-standalone');
-} else {
-  const { ipcRenderer } = require('electron');
-  const isDev = true;
-
-  ipcRenderer.on('set-socket', (event, { name }) => {
-    // Start the app
-    getBackend().initApp(isDev, name);
-  });
-}
+// Start the app
+getBackend().initApp(isDev);

packages/loot-core/package.json

@@ -37,8 +37,7 @@
     "path-browserify": "^1.0.1",
     "process": "^0.11.10",
     "reselect": "^4.1.8",
-    "stream-browserify": "^3.0.0",
-    "ws": "8.13.0"
+    "stream-browserify": "^3.0.0"
   },
   "devDependencies": {
     "@actual-app/api": "*",
@@ -80,7 +79,6 @@
     "webpack": "^5.88.2",
     "webpack-bundle-analyzer": "^4.9.1",
     "webpack-cli": "^5.1.4",
-    "ws": "^4.1.0",
     "yargs": "^9.0.1"
   }
 }

packages/loot-core/src/client/actions/app.ts

@@ -1,4 +1,4 @@
-import { init as initConnection, send } from '../../platform/client/fetch';
+import { send } from '../../platform/client/fetch';
 import * as constants from '../constants';
 import type {
   AppState,
@@ -15,12 +15,6 @@ export function setAppState(state: Partial<AppState>): SetAppStateAction {
   };
 }
 
-export function reconnect(connectionName: string) {
-  return () => {
-    initConnection(connectionName);
-  };
-}
-
 export function updateApp() {
   return async (dispatch: Dispatch) => {
     global.Actual.applyAppUpdate();

packages/loot-core/src/platform/client/fetch/index.web.ts

@@ -8,89 +8,76 @@ const replyHandlers = new Map();
 const listeners = new Map();
 let messageQueue = [];
 let socketClient = null;
-let activePort = null;
 
-function connectSocket(port, onOpen) {
-  // Do nothing if connection to this port is already active
-  if (socketClient && port === activePort) {
-    return;
-  }
+function connectSocket(onOpen) {
+  global.Actual.ipcConnect(function (client) {
+    client.on('message', data => {
+      const msg = data;
 
-  const client = new WebSocket('ws://localhost:' + port);
-  socketClient = client;
-  activePort = port;
-
-  client.onmessage = event => {
-    const msg = JSON.parse(event.data);
-
-    if (msg.type === 'error') {
-      // An error happened while handling a message so cleanup the
-      // current reply handler. We don't care about the actual error -
-      // generic backend errors are handled separately and if you want
-      // more specific handling you should manually forward the error
-      // through a normal reply.
-      const { id } = msg;
-      replyHandlers.delete(id);
-    } else if (msg.type === 'reply') {
-      let { result } = msg;
-      const { id, mutated, undoTag } = msg;
-
-      // Check if the result is a serialized buffer, and if so
-      // convert it to a Uint8Array. This is only needed when working
-      // with node; the web version connection layer automatically
-      // supports buffers
-      if (result && result.type === 'Buffer' && Array.isArray(result.data)) {
-        result = new Uint8Array(result.data);
-      }
-
-      const handler = replyHandlers.get(id);
-      if (handler) {
+      if (msg.type === 'error') {
+        // An error happened while handling a message so cleanup the
+        // current reply handler. We don't care about the actual error -
+        // generic backend errors are handled separately and if you want
+        // more specific handling you should manually forward the error
+        // through a normal reply.
+        const { id } = msg;
         replyHandlers.delete(id);
+      } else if (msg.type === 'reply') {
+        let { result } = msg;
+        const { id, mutated, undoTag } = msg;
 
-        if (!mutated) {
-          undo.gc(undoTag);
+        // Check if the result is a serialized buffer, and if so
+        // convert it to a Uint8Array. This is only needed when working
+        // with node; the web version connection layer automatically
+        // supports buffers
+        if (result && result.type === 'Buffer' && Array.isArray(result.data)) {
+          result = new Uint8Array(result.data);
         }
 
-        handler.resolve(result);
-      }
-    } else if (msg.type === 'push') {
-      const { name, args } = msg;
+        const handler = replyHandlers.get(id);
+        if (handler) {
+          replyHandlers.delete(id);
 
-      const listens = listeners.get(name);
-      if (listens) {
-        for (let i = 0; i < listens.length; i++) {
-          const stop = listens[i](args);
-          if (stop === true) {
-            break;
+          if (!mutated) {
+            undo.gc(undoTag);
+          }
+
+          handler.resolve(result);
+        }
+      } else if (msg.type === 'push') {
+        const { name, args } = msg;
+
+        const listens = listeners.get(name);
+        if (listens) {
+          for (let i = 0; i < listens.length; i++) {
+            const stop = listens[i](args);
+            if (stop === true) {
+              break;
+            }
           }
         }
+      } else {
+        throw new Error('Unknown message type: ' + JSON.stringify(msg));
       }
-    } else {
-      throw new Error('Unknown message type: ' + JSON.stringify(msg));
-    }
-  };
+    });
+
+    socketClient = client;
 
-  client.onopen = event => {
     // Send any messages that were queued while closed
     if (messageQueue.length > 0) {
-      messageQueue.forEach(msg => {
-        socketClient.send(msg);
-      });
+      messageQueue.forEach(msg => client.emit('message', msg));
       messageQueue = [];
     }
 
     onOpen();
-  };
-
-  client.onclose = () => {
-    socketClient = null;
-  };
+  });
 }
 
-export const init: T.Init = async function (socketName) {
-  return new Promise(resolve => connectSocket(socketName, resolve));
+export const init: T.Init = async function () {
+  return new Promise(connectSocket);
 };
 
+// @ts-expect-error Figure out why typechecker is suddenly breaking here
 export const send: T.Send = function (
   name,
   args,
@@ -101,28 +88,23 @@ export const send: T.Send = function (
     replyHandlers.set(id, { resolve, reject });
 
     if (socketClient) {
-      socketClient.send(
-        JSON.stringify({
-          id,
-          name,
-          args,
-          undoTag: undo.snapshot(),
-          catchErrors: !!catchErrors,
-        }),
-      );
+      socketClient.emit('message', {
+        id,
+        name,
+        args,
+        undoTag: undo.snapshot(),
+        catchErrors: !!catchErrors,
+      });
     } else {
-      messageQueue.push(
-        JSON.stringify({
-          id,
-          name,
-          args,
-          undoTag: undo.snapshot(),
-          catchErrors,
-        }),
-      );
+      messageQueue.push({
+        id,
+        name,
+        args,
+        undoTag: undo.snapshot(),
+        catchErrors,
+      });
     }
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  }) as any;
+  });
 };
 
 export const sendCatch: T.SendCatch = function (name, args) {

packages/loot-core/src/platform/server/connection/index.electron.ts

@@ -3,12 +3,6 @@ import { captureException } from '../../exceptions';
 
 import type * as T from '.';
 
-// for some reason import doesn't work
-const WebSocketServer = require('ws').Server;
-
-// the websocket server
-let wss = null;
-
 function coerceError(error) {
   if (error.type && error.type === 'APIError') {
     return error;
@@ -17,105 +11,74 @@ function coerceError(error) {
   return { type: 'InternalError', message: error.message };
 }
 
-export const init: T.Init = function (socketName, handlers) {
-  wss = new WebSocketServer({ port: socketName });
+export const init: T.Init = function (_socketName, handlers) {
+  process.parentPort.on('message', ({ data }) => {
+    const { id, name, args, undoTag, catchErrors } = data;
 
-  // websockets doesn't support sending objects so parse/stringify needed
-  wss.on('connection', function connection(ws) {
-    ws.on('error', console.error);
+    if (handlers[name]) {
+      runHandler(handlers[name], args, { undoTag, name }).then(
+        result => {
+          if (catchErrors) {
+            result = { data: result, error: null };
+          }
 
-    ws.on('message', data => {
-      const msg = JSON.parse(data);
-
-      if (ws.readyState !== 1) {
-        return;
-      }
-
-      const { id, name, args, undoTag, catchErrors } = msg;
-
-      if (handlers[name]) {
-        runHandler(handlers[name], args, { undoTag, name }).then(
-          result => {
-            if (ws.readyState !== 1) {
-              return;
-            }
-            if (catchErrors) {
-              result = { data: result, error: null };
-            }
-
-            ws.send(
-              JSON.stringify({
-                type: 'reply',
-                id,
-                result,
-                mutated:
-                  isMutating(handlers[name]) &&
-                  name !== 'undo' &&
-                  name !== 'redo',
-                undoTag,
-              }),
-            );
-          },
-          nativeError => {
-            if (ws.readyState !== 1) {
-              return;
-            }
-            const error = coerceError(nativeError);
-
-            if (name.startsWith('api/')) {
-              // The API is newer and does automatically forward
-              // errors
-              ws.send(JSON.stringify({ type: 'reply', id, error }));
-            } else if (catchErrors) {
-              ws.send(
-                JSON.stringify({
-                  type: 'reply',
-                  id,
-                  result: { error, data: null },
-                }),
-              );
-            } else {
-              ws.send(JSON.stringify({ type: 'error', id }));
-            }
-
-            if (error.type === 'InternalError' && name !== 'api/load-budget') {
-              captureException(nativeError);
-            }
-
-            if (!catchErrors) {
-              // Notify the frontend that something bad happend
-              send('server-error');
-            }
-          },
-        );
-      } else {
-        console.warn('Unknown method: ' + name);
-        captureException(new Error('Unknown server method: ' + name));
-        ws.send(
-          JSON.stringify({
+          process.parentPort.postMessage({
             type: 'reply',
             id,
-            result: null,
-            error: { type: 'APIError', message: 'Unknown method: ' + name },
-          }),
-        );
-      }
-    });
+            result,
+            mutated:
+              isMutating(handlers[name]) && name !== 'undo' && name !== 'redo',
+            undoTag,
+          });
+        },
+        nativeError => {
+          const error = coerceError(nativeError);
+
+          if (name.startsWith('api/')) {
+            // The API is newer and does automatically forward
+            // errors
+            process.parentPort.postMessage({
+              type: 'reply',
+              id,
+              error,
+            });
+          } else if (catchErrors) {
+            process.parentPort.postMessage({
+              type: 'reply',
+              id,
+              result: { error, data: null },
+            });
+          } else {
+            process.parentPort.postMessage({ type: 'error', id });
+          }
+
+          if (error.type === 'InternalError' && name !== 'api/load-budget') {
+            captureException(nativeError);
+          }
+
+          if (!catchErrors) {
+            // Notify the frontend that something bad happend
+            send('server-error');
+          }
+        },
+      );
+    } else {
+      console.warn('Unknown method: ' + name);
+      captureException(new Error('Unknown server method: ' + name));
+      process.parentPort.postMessage({
+        type: 'reply',
+        id,
+        result: null,
+        error: { type: 'APIError', message: 'Unknown method: ' + name },
+      });
+    }
   });
 };
 
 export const getNumClients: T.GetNumClients = function () {
-  if (wss) {
-    return wss.clients.length;
-  }
-
   return 0;
 };
 
 export const send: T.Send = function (name, args) {
-  if (wss) {
-    wss.clients.forEach(client =>
-      client.send(JSON.stringify({ type: 'push', name, args })),
-    );
-  }
+  process.parentPort.postMessage({ type: 'push', name, args });
 };

packages/loot-core/src/server/main.ts

@@ -1348,7 +1348,10 @@ handlers['save-global-prefs'] = async function (prefs) {
   }
   if ('autoUpdate' in prefs) {
     await asyncStorage.setItem('auto-update', '' + prefs.autoUpdate);
-    process.send({ type: 'shouldAutoUpdate', flag: prefs.autoUpdate });
+    process.parentPort.postMessage({
+      type: 'shouldAutoUpdate',
+      flag: prefs.autoUpdate,
+    });
   }
   if ('documentDir' in prefs) {
     if (await fs.exists(prefs.documentDir)) {
@@ -2235,7 +2238,7 @@ export async function initApp(isDev, socketName) {
 
   if (!isDev && !Platform.isMobile && !Platform.isWeb) {
     const autoUpdate = await asyncStorage.getItem('auto-update');
-    process.send({
+    process.parentPort.postMessage({
       type: 'shouldAutoUpdate',
       flag: autoUpdate == null || autoUpdate === 'true',
     });

packages/loot-core/webpack/webpack.desktop.config.js

@@ -28,14 +28,7 @@ module.exports = {
       'pegjs',
     ],
   },
-  externals: [
-    'better-sqlite3',
-    'electron-log',
-    'node-fetch',
-    'node-libofx',
-    'ws',
-    'fs',
-  ],
+  externals: ['better-sqlite3', 'electron-log', 'node-fetch', 'node-libofx'],
   plugins: [
     new webpack.IgnorePlugin({
       resourceRegExp: /original-fs/,

upcoming-release-notes/2190.md

@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [MatissJanis]
+---
+
+electron: move back from WebSockets to IPC for internal communications. This should improve the stability of the desktop app.

yarn.lock

@@ -5913,13 +5913,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"async-limiter@npm:~1.0.0":
-  version: 1.0.1
-  resolution: "async-limiter@npm:1.0.1"
-  checksum: 2b849695b465d93ad44c116220dee29a5aeb63adac16c1088983c339b0de57d76e82533e8e364a93a9f997f28bbfc6a92948cefc120652bd07f3b59f8d75cf2b
-  languageName: node
-  linkType: hard
-
 "async@npm:^3.2.3":
   version: 3.2.4
   resolution: "async@npm:3.2.4"
@@ -8269,11 +8262,9 @@ __metadata:
     electron-is-dev: "npm:2.0.0"
     electron-log: "npm:4.4.8"
     electron-updater: "npm:6.1.4"
-    get-port-please: "npm:3.0.1"
     loot-core: "npm:*"
     node-fetch: "npm:^2.6.9"
     promise-retry: "npm:^2.0.1"
-    ws: "npm:8.13.0"
   languageName: unknown
   linkType: soft
 
@@ -10399,13 +10390,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"get-port-please@npm:3.0.1":
-  version: 3.0.1
-  resolution: "get-port-please@npm:3.0.1"
-  checksum: a5de771314986e45872354a72e24f27c13884c14be9e00b7332bc53de971c50787d2ae61dd81ef6d1eb2df5c35b1e3528906de29ac4a5127769f7ebee6e8f100
-  languageName: node
-  linkType: hard
-
 "get-proxy@npm:^2.0.0":
   version: 2.1.0
   resolution: "get-proxy@npm:2.1.0"
@@ -13406,7 +13390,6 @@ __metadata:
     webpack: "npm:^5.88.2"
     webpack-bundle-analyzer: "npm:^4.9.1"
     webpack-cli: "npm:^5.1.4"
-    ws: "npm:^4.1.0"
     yargs: "npm:^9.0.1"
   languageName: unknown
   linkType: soft
@@ -21541,31 +21524,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"ws@npm:8.13.0, ws@npm:^8.13.0":
-  version: 8.13.0
-  resolution: "ws@npm:8.13.0"
-  peerDependencies:
-    bufferutil: ^4.0.1
-    utf-8-validate: ">=5.0.2"
-  peerDependenciesMeta:
-    bufferutil:
-      optional: true
-    utf-8-validate:
-      optional: true
-  checksum: 1769532b6fdab9ff659f0b17810e7501831d34ecca23fd179ee64091dd93a51f42c59f6c7bb4c7a384b6c229aca8076fb312aa35626257c18081511ef62a161d
-  languageName: node
-  linkType: hard
-
-"ws@npm:^4.1.0":
-  version: 4.1.0
-  resolution: "ws@npm:4.1.0"
-  dependencies:
-    async-limiter: "npm:~1.0.0"
-    safe-buffer: "npm:~5.1.0"
-  checksum: afdb3c55defe9ff39474fd769f7a7c718e3fa868b8bc945546bdcb1277ee0bc086c19ff688e551fdcd2a2359218724f9088acfad4bb05a6c7afc2068ea9ec144
-  languageName: node
-  linkType: hard
-
 "ws@npm:^7.3.1, ws@npm:^7.4.6":
   version: 7.5.9
   resolution: "ws@npm:7.5.9"
@@ -21581,6 +21539,21 @@ __metadata:
   languageName: node
   linkType: hard
 
+"ws@npm:^8.13.0":
+  version: 8.13.0
+  resolution: "ws@npm:8.13.0"
+  peerDependencies:
+    bufferutil: ^4.0.1
+    utf-8-validate: ">=5.0.2"
+  peerDependenciesMeta:
+    bufferutil:
+      optional: true
+    utf-8-validate:
+      optional: true
+  checksum: 1769532b6fdab9ff659f0b17810e7501831d34ecca23fd179ee64091dd93a51f42c59f6c7bb4c7a384b6c229aca8076fb312aa35626257c18081511ef62a161d
+  languageName: node
+  linkType: hard
+
 "xml-name-validator@npm:^3.0.0":
   version: 3.0.0
   resolution: "xml-name-validator@npm:3.0.0"