Fix electron app (#1003)

Updates to the latest version of electron and moves the backend-frontend
communication from node-ipc to websockets. This resolves the previous
roadblock regarding `nodeIntegration` .

Done

- Remove node-ipc in favour of websockets. 
- Move file copying out of `preload.js` to avoid importing module `fs`
there
- Bump all electron pacakge versions to the latest
- Added new package for finding open ports as node-ipc is gone
- Tweaked webpack config for above changes


Partially fixes #468

Questions/ Pending:
- Literally every single test fails for me, presumably some issue with
my setup/environment.
- The websocket communication is not using TLS. I'm not sure how to
enable this, or if we even need to as its all local.
- Still need to create the CI for building/deploying but I'm not sure
where start in this regard as i have no exp with it. Presumably we will
need to point the electron auto-updater to the github releases url's. If
people are happy with this PR I will look at adding the CI before its
merged.
- In dev mode only, I have disabled TLS security becuase my docker
container's cert is not signed. I _assume_ this will be true for other
people who spin up the server on thier own hardware. Perhaps I just need
to change my cert to one from letsencrypt or something...

Notes.
I have not touched javascript in eons so my apologies if the commit
trail is a bit fragmented. I tried to keep them fairly contained and
then there is a slightly gnarly final commit fixing all the linter
issues... Please let me know if you want me to squash some commits etc.

I initially tried to move this to web workers the same way the web app
does it but this was unsuccessful. I have found no way to spin up a
worker in one place (frontend/backend) and then pass this worker to the
other. The electron ipc channels don't allow you to directly pass
objects such as workers, everything is cloned/serialised. Passing a port
number so the other end can spin up its own socket works fine.

---------

Co-authored-by: Shazib Hussain <contact@shazib.com>
Co-authored-by: Jed Fox <git@jedfox.com>

.gitattributes

@@ -8,6 +8,8 @@
 
 # Declare files that will always have LF line endings on checkout.
 *.js text eol=lf
+*.ts text eol=lf
+*.sh text eol=lf
 yarn.lock text eol=lf
 
 # Denote all files that are truly binary and should not be modified.

.github/workflows/build.yml

@@ -45,20 +45,26 @@ jobs:
           name: actual-web
           path: packages/desktop-client/build
 
-  # TODO: re-enable after solving https://github.com/actualbudget/actual/issues/468
-  # electron:
-  #   # As electron builds take longer, we only run them in master.
-  #   if: github.event_name != 'pull_request'
-  #   strategy:
-  #     matrix:
-  #       os:
-  #         - ubuntu-latest
-  #         - windows-latest
-  #         - macos-latest
-  #   runs-on: ${{ matrix.os }}
-  #   steps:
-  #     - uses: actions/checkout@v3
-  #     - name: Set up environment
-  #       uses: ./.github/actions/setup
-  #     - name: Build Electron
-  #       run: ./bin/package
+  electron:
+    # As electron builds take longer, we only run them in master.
+    strategy:
+      matrix:
+        os:
+          - ubuntu-latest
+          - windows-latest
+          - macos-latest
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up environment
+        uses: ./.github/actions/setup
+      - name: Build Electron
+        run: ./bin/package-electron
+      - name: Upload Build
+        uses: actions/upload-artifact@v3
+        with:
+          name: actual-electron-${{ matrix.os }}
+          path: |
+            packages/desktop-electron/dist/*.dmg
+            packages/desktop-electron/dist/*.exe
+            packages/desktop-electron/dist/*.AppImage

bin/make-release

@@ -42,7 +42,7 @@ git tag -a "$VERSION" -m "$NOTES"
 git push origin "$VERSION"
 
 # Make a macOS version
-./bin/package --release --version "$VERSION"
+./bin/package-electron --release --version "$VERSION"
 
 # TODO: browser version
 

bin/package-electron

@@ -74,19 +74,15 @@ if [ "$OSTYPE" == "msys" ]; then
     fi
 fi
 
-# We only need to run linting once (and this doesn't seem to work on
-# Windows for some reason)
-if [[ $CI != true && "$OSTYPE" == "darwin"* ]]; then
-    yarn lint
-fi
-
 yarn patch-package
 
+yarn rebuild-electron
+
 yarn workspace loot-core build:node
 
 yarn workspace @actual-app/web build
 
-yarn workspace Actual update-client
+yarn workspace desktop-electron update-client
 
 (
     cd packages/desktop-electron;
@@ -103,7 +99,7 @@ yarn workspace Actual update-client
         echo "\nCreated release $VERSION with release notes \"$RELEASE_NOTES\""
     elif [ "$RELEASE" == "beta" ]; then
         yarn build --publish never --arm64 --x64
-        
+
         echo "\nCreated beta release $VERSION"
     else
         SKIP_NOTARIZATION=true yarn build --publish never --x64

package.json

@@ -22,7 +22,7 @@
     "start:desktop": "npm-run-all --parallel 'start:desktop-*'",
     "start:desktop-node": "yarn workspace loot-core watch:node",
     "start:desktop-client": "yarn workspace @actual-app/web watch",
-    "start:desktop-electron": "yarn workspace Actual watch",
+    "start:desktop-electron": "yarn workspace desktop-electron watch",
     "start:browser": "npm-run-all --parallel 'start:browser-*'",
     "start:browser-backend": "yarn workspace loot-core watch:browser",
     "start:browser-frontend": "yarn workspace @actual-app/web start:browser",

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

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

packages/desktop-client/src/components/manager/ConfigServer.js

@@ -5,6 +5,7 @@ import { useHistory } from 'react-router-dom';
 import { createBudget } from 'loot-core/src/client/actions/budgets';
 import { signOut, loggedIn } from 'loot-core/src/client/actions/user';
 import { isNonProductionEnvironment } from 'loot-core/src/shared/environment';
+import { isElectron } from 'loot-core/src/shared/environment';
 
 import { useSetThemeColor } from '../../hooks';
 import { colors } from '../../style';
@@ -165,17 +166,19 @@ export default function ConfigServer() {
           </Button>
         ) : (
           <>
-            <Button
-              bare
-              style={{
-                color: colors.n4,
-                margin: 5,
-                marginRight: 15,
-              }}
-              onClick={onSameDomain}
-            >
-              Use {window.location.origin.replace(/https?:\/\//, '')}
-            </Button>
+            {!isElectron() && (
+              <Button
+                bare
+                style={{
+                  color: colors.n4,
+                  margin: 5,
+                  marginRight: 15,
+                }}
+                onClick={onSameDomain}
+              >
+                Use {window.location.origin.replace(/https?:\/\//, '')}
+              </Button>
+            )}
             <Button
               bare
               style={{ color: colors.n4, margin: 5 }}

packages/desktop-electron/findOpenSocket.js

@@ -1,33 +0,0 @@
-const ipc = require('node-ipc');
-
-ipc.config.silent = true;
-
-function isSocketTaken(name, fn) {
-  return new Promise((resolve, reject) => {
-    ipc.connectTo(name, () => {
-      ipc.of[name].on('error', () => {
-        ipc.disconnect(name);
-        resolve(false);
-      });
-
-      ipc.of[name].on('connect', () => {
-        console.log('connected');
-        ipc.disconnect(name);
-        resolve(true);
-      });
-    });
-  });
-}
-
-async function findOpenSocket() {
-  let currentSocket = 1;
-  while (await isSocketTaken('actual' + currentSocket)) {
-    currentSocket++;
-    console.log('checking', currentSocket);
-  }
-  let socketName = 'actual' + currentSocket;
-  console.log(`Listening... (${socketName})`);
-  return socketName;
-}
-
-module.exports = findOpenSocket;

packages/desktop-electron/index.js

@@ -1,8 +1,15 @@
 /* eslint-disable import/order */
 // (I have no idea why the imports are like this. Not touching them.)
 const isDev = require('electron-is-dev');
+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,
@@ -23,7 +30,7 @@ protocol.registerSchemesAsPrivileged([
 global.fetch = require('node-fetch');
 
 const about = require('./about');
-const findOpenSocket = require('./findOpenSocket');
+const { getRandomPort } = require('get-port-please');
 const getMenu = require('./menu');
 const updater = require('./updater');
 
@@ -92,28 +99,6 @@ function createBackgroundProcess(socketName) {
   });
 }
 
-function createBackgroundWindow(socketName) {
-  const win = new BrowserWindow({
-    show: true,
-    title: 'Actual Server',
-    webPreferences: {
-      nodeIntegration: true,
-      contextIsolation: false,
-    },
-  });
-  win.loadURL(`file://${__dirname}/server.html`);
-
-  win.webContents.on('did-finish-load', () => {
-    win.webContents.send('set-socket', { name: socketName });
-  });
-
-  win.on('closed', () => {
-    serverWin = null;
-  });
-
-  serverWin = win;
-}
-
 async function createWindow() {
   const windowState = await WindowState.get();
 
@@ -127,7 +112,10 @@ async function createWindow() {
     titleBarStyle: 'hiddenInset',
     webPreferences: {
       nodeIntegration: false,
+      nodeIntegrationInWorker: false,
+      nodeIntegrationInSubFrames: false,
       contextIsolation: true,
+      enableRemoteModule: false,
       preload: __dirname + '/preload.js',
     },
   });
@@ -222,7 +210,7 @@ function updateMenu(isBudgetOpen) {
 app.setAppUserModelId('com.shiftreset.actual');
 
 app.on('ready', async () => {
-  serverSocket = await findOpenSocket();
+  serverSocket = await getRandomPort();
 
   // Install an `app://` protocol that always returns the base HTML
   // file no matter what URL it is. This allows us to use react-router
@@ -267,11 +255,7 @@ app.on('ready', async () => {
     console.log('Suspending', new Date());
   });
 
-  if (isDev) {
-    createBackgroundWindow(serverSocket);
-  } else {
-    createBackgroundProcess(serverSocket);
-  }
+  createBackgroundProcess(serverSocket);
 });
 
 app.on('window-all-closed', () => {
@@ -314,9 +298,21 @@ ipcMain.handle('open-file-dialog', (event, { filters, properties }) => {
   });
 });
 
-ipcMain.handle('save-file-dialog', (event, { title, defaultPath }) => {
-  return dialog.showSaveDialogSync({ title, defaultPath });
-});
+ipcMain.handle(
+  'save-file-dialog',
+  (event, { title, defaultPath, fileContents }) => {
+    let fileLocation = dialog.showSaveDialogSync({ title, defaultPath });
+
+    return new Promise((resolve, reject) => {
+      if (fileLocation) {
+        fs.writeFile(fileLocation, fileContents, error => {
+          return reject(error);
+        });
+      }
+      resolve();
+    });
+  },
+);
 
 ipcMain.handle('open-external-url', (event, url) => {
   shell.openExternal(url);

packages/desktop-electron/package.json

@@ -1,9 +1,9 @@
 {
-  "name": "Actual",
+  "name": "desktop-electron",
   "productName": "Actual",
   "author": "Shift Reset LLC",
   "description": "A simple and powerful personal finance system",
-  "version": "22.12.3",
+  "version": "23.5.0",
   "scripts": {
     "clean": "rm -rf dist",
     "update-client": "bin/update-client",
@@ -48,15 +48,16 @@
     "electron-is-dev": "2.0.0",
     "electron-log": "4.3.2",
     "electron-updater": "4.3.8",
+    "get-port-please": "3.0.1",
     "loot-core": "*",
     "node-fetch": "^2.6.9",
-    "node-ipc": "9.1.4",
-    "promise-retry": "^2.0.1"
+    "promise-retry": "^2.0.1",
+    "ws": "8.13.0"
   },
   "devDependencies": {
-    "electron": "12.2.1",
-    "electron-builder": "22.10.5",
-    "electron-notarize": "1.0.0",
-    "electron-rebuild": "2.3.5"
+    "@electron/notarize": "1.2.3",
+    "@electron/rebuild": "3.2.13",
+    "electron": "24.1.3",
+    "electron-builder": "23.6.0"
   }
 }

packages/desktop-electron/preload.js

@@ -1,7 +1,4 @@
-const fs = require('fs');
-
 const { ipcRenderer, contextBridge } = require('electron');
-const ipc = require('node-ipc');
 
 let { version: VERSION, isDev: IS_DEV } =
   ipcRenderer.sendSync('get-bootstrap-data');
@@ -22,22 +19,6 @@ contextBridge.exposeInMainWorld('Actual', {
     require('console').log(...args);
   },
 
-  ipcConnect: (id, func) => {
-    ipc.config.silent = true;
-    ipc.connectTo(id, () => {
-      let client = ipc.of[id];
-
-      func({
-        on(name, handler) {
-          return client.on(name, handler);
-        },
-        emit(name, data) {
-          return client.emit(name, data);
-        },
-      });
-    });
-  },
-
   relaunch: () => {
     ipcRenderer.invoke('relaunch');
   },
@@ -47,21 +28,10 @@ contextBridge.exposeInMainWorld('Actual', {
   },
 
   saveFile: async (contents, filename, dialogTitle) => {
-    const fileLocation = await ipcRenderer.invoke('save-file-dialog', {
+    await ipcRenderer.invoke('save-file-dialog', {
       title: dialogTitle,
       defaultPath: filename,
-    });
-
-    return new Promise((resolve, reject) => {
-      if (fileLocation) {
-        fs.writeFile(fileLocation, contents, error => {
-          if (error) {
-            return reject(error);
-          }
-
-          resolve();
-        });
-      }
+      fileContents: contents,
     });
   },
 

packages/loot-core/package.json

@@ -34,7 +34,8 @@
     "md5": "^2.3.0",
     "mitt": "^3.0.0",
     "node-fetch": "^2.6.9",
-    "node-libofx": "*"
+    "node-libofx": "*",
+    "ws": "8.13.0"
   },
   "devDependencies": {
     "@actual-app/api": "*",
@@ -45,7 +46,6 @@
     "@types/better-sqlite3": "^7.6.4",
     "@types/jest": "^27.5.0",
     "@types/jlongster__sql.js": "npm:@types/sql.js@latest",
-    "@types/node-ipc": "^9.2.0",
     "@types/pegjs": "^0.10.3",
     "@types/webpack": "^5.28.0",
     "adm-zip": "^0.5.9",

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

@@ -8,73 +8,74 @@ let listeners = new Map();
 let messageQueue = [];
 let socketClient = null;
 
-function connectSocket(name, onOpen) {
-  global.Actual.ipcConnect(name, function (client) {
-    client.on('message', data => {
-      const msg = data;
+function connectSocket(port, onOpen) {
+  let client = new WebSocket('ws://localhost:' + port);
 
-      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;
+  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 { id, result, 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) {
         replyHandlers.delete(id);
-      } else if (msg.type === 'reply') {
-        let { id, result, 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);
+        if (!mutated) {
+          undo.gc(undoTag);
         }
 
-        const handler = replyHandlers.get(id);
-        if (handler) {
-          replyHandlers.delete(id);
+        handler.resolve(result);
+      }
+    } else if (msg.type === 'push') {
+      const { name, args } = msg;
 
-          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++) {
-            let stop = listens[i](args);
-            if (stop === true) {
-              break;
-            }
+      const listens = listeners.get(name);
+      if (listens) {
+        for (let i = 0; i < listens.length; i++) {
+          let 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));
+    }
+  };
 
-    client.on('connect', () => {
-      socketClient = client;
+  client.onopen = event => {
+    socketClient = client;
+    // Send any messages that were queued while closed
+    if (messageQueue.length > 0) {
+      messageQueue.forEach(msg => {
+        socketClient.send(msg);
+      });
+      messageQueue = [];
+    }
 
-      // Send any messages that were queued while closed
-      if (messageQueue.length > 0) {
-        messageQueue.forEach(msg => client.emit('message', msg));
-        messageQueue = [];
-      }
+    onOpen();
+  };
 
-      onOpen();
-    });
-
-    client.on('disconnect', () => {
-      socketClient = null;
-    });
-  });
+  client.onclose = event => {
+    socketClient = null;
+  };
 }
 
 export const init: T.Init = async function (socketName) {
@@ -91,21 +92,25 @@ export const send: T.Send = function (
       replyHandlers.set(id, { resolve, reject });
 
       if (socketClient) {
-        socketClient.emit('message', {
-          id,
-          name,
-          args,
-          undoTag: undo.snapshot(),
-          catchErrors: !!catchErrors,
-        });
+        socketClient.send(
+          JSON.stringify({
+            id,
+            name,
+            args,
+            undoTag: undo.snapshot(),
+            catchErrors: !!catchErrors,
+          }),
+        );
       } else {
-        messageQueue.push({
-          id,
-          name,
-          args,
-          undoTag: undo.snapshot(),
-          catchErrors,
-        });
+        messageQueue.push(
+          JSON.stringify({
+            id,
+            name,
+            args,
+            undoTag: undo.snapshot(),
+            catchErrors,
+          }),
+        );
       }
     });
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

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

@@ -1,7 +1,7 @@
 import type { ServerEvents } from '../../../types/server-events';
 
 export function init(
-  channel: Window | string,
+  channel: Window | number, // in electron the port number, in web the worker
   handlers: Record<string, () => void>,
 ): void;
 export type Init = typeof init;

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

@@ -1,10 +1,14 @@
-import ipc from 'node-ipc';
-
 import { runHandler, isMutating } from '../../../server/mutators';
 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;
@@ -14,12 +18,12 @@ function coerceError(error) {
 }
 
 export const init: T.Init = function (socketName, handlers) {
-  ipc.config.id = socketName as string;
-  ipc.config.silent = true;
+  wss = new WebSocketServer({ port: socketName });
 
-  ipc.serve(() => {
-    ipc.server.on('message', (data, socket) => {
-      let msg = data;
+  // websockets doesn't support sending objects so parse/stringify needed
+  wss.on('connection', function connection(ws) {
+    ws.on('message', data => {
+      let msg = JSON.parse(data);
       let { id, name, args, undoTag, catchErrors } = msg;
 
       if (handlers[name]) {
@@ -29,16 +33,18 @@ export const init: T.Init = function (socketName, handlers) {
               result = { data: result, error: null };
             }
 
-            ipc.server.emit(socket, 'message', {
-              type: 'reply',
-              id,
-              result,
-              mutated:
-                isMutating(handlers[name]) &&
-                name !== 'undo' &&
-                name !== 'redo',
-              undoTag,
-            });
+            ws.send(
+              JSON.stringify({
+                type: 'reply',
+                id,
+                result,
+                mutated:
+                  isMutating(handlers[name]) &&
+                  name !== 'undo' &&
+                  name !== 'redo',
+                undoTag,
+              }),
+            );
           },
           nativeError => {
             let error = coerceError(nativeError);
@@ -46,15 +52,17 @@ export const init: T.Init = function (socketName, handlers) {
             if (name.startsWith('api/')) {
               // The API is newer and does automatically forward
               // errors
-              ipc.server.emit(socket, 'message', { type: 'reply', id, error });
+              ws.send(JSON.stringify({ type: 'reply', id, error }));
             } else if (catchErrors) {
-              ipc.server.emit(socket, 'message', {
-                type: 'reply',
-                id,
-                result: { error, data: null },
-              });
+              ws.send(
+                JSON.stringify({
+                  type: 'reply',
+                  id,
+                  result: { error, data: null },
+                }),
+              );
             } else {
-              ipc.server.emit(socket, 'message', { type: 'error', id });
+              ws.send(JSON.stringify({ type: 'error', id }));
             }
 
             if (error.type === 'InternalError' && name !== 'api/load-budget') {
@@ -70,26 +78,31 @@ export const init: T.Init = function (socketName, handlers) {
       } else {
         console.warn('Unknown method: ' + name);
         captureException(new Error('Unknown server method: ' + name));
-        ipc.server.emit(socket, 'message', {
-          type: 'reply',
-          id,
-          result: null,
-          error: { type: 'APIError', message: 'Unknown method: ' + name },
-        });
+        ws.send(
+          JSON.stringify({
+            type: 'reply',
+            id,
+            result: null,
+            error: { type: 'APIError', message: 'Unknown method: ' + name },
+          }),
+        );
       }
     });
   });
-
-  ipc.server.start();
 };
 
 export const getNumClients: T.GetNumClients = function () {
-  // @ts-expect-error server type does not have sockets
-  return ipc.server.sockets.length;
+  if (wss) {
+    return wss.clients.length;
+  }
+
+  return 0;
 };
 
 export const send: T.Send = function (name, args) {
-  if (ipc.server) {
-    ipc.server.broadcast('message', { type: 'push', name, args });
+  if (wss) {
+    wss.clients.forEach(client =>
+      client.send(JSON.stringify({ type: 'push', name, args })),
+    );
   }
 };

packages/loot-core/src/shared/environment.ts

@@ -9,3 +9,10 @@ export function isDevelopmentEnvironment() {
 export function isNonProductionEnvironment() {
   return isPreviewEnvironment() || isDevelopmentEnvironment();
 }
+
+export function isElectron() {
+  if (navigator.userAgent.indexOf('Electron') >= 0) {
+    return true;
+  }
+  return false;
+}

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

@@ -24,14 +24,16 @@ module.exports = {
       '.ts',
       '.tsx',
       '.json',
+      'pegjs',
     ],
   },
   externals: [
     'better-sqlite3',
-    'node-ipc',
     'electron-log',
     'node-fetch',
     'node-libofx',
+    'ws',
+    'fs',
   ],
   plugins: [
     new webpack.IgnorePlugin({

upcoming-release-notes/1003.md

@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [Shazib]
+---
+
+Fixing Electron App