From c8551d517b7d1182e3864934c8a6456b0b762ebb Mon Sep 17 00:00:00 2001 From: Matiss Janis Aboltins Date: Sun, 7 Jan 2024 19:00:25 +0000 Subject: [PATCH] :recycle: refactor(electron): moving back from websockets to IPC --- .gitattributes | 2 +- .../src/browser-preload.browser.js | 1 + packages/desktop-electron/index.js | 85 +++------- packages/desktop-electron/package.json | 4 +- packages/desktop-electron/preload.js | 22 +-- packages/desktop-electron/server.js | 4 +- packages/loot-core/package.json | 4 +- .../src/platform/client/fetch/index.web.ts | 150 ++++++++--------- .../server/connection/index.electron.ts | 153 +++++++----------- packages/loot-core/src/server/main.ts | 7 +- .../webpack/webpack.desktop.config.js | 9 +- yarn.lock | 47 ++---- 12 files changed, 178 insertions(+), 310 deletions(-) diff --git a/.gitattributes b/.gitattributes index 21f3d101a30..a592bc40b1e 100644 --- a/.gitattributes +++ b/.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 \ No newline at end of file +*.jpg binary diff --git a/packages/desktop-client/src/browser-preload.browser.js b/packages/desktop-client/src/browser-preload.browser.js index 803ed2ac5b9..37bd1f53719 100644 --- a/packages/desktop-client/src/browser-preload.browser.js +++ b/packages/desktop-client/src/browser-preload.browser.js @@ -125,6 +125,7 @@ global.Actual = { applyAppUpdate: () => {}, updateAppMenu: isBudgetOpen => {}, + ipcConnect: () => {}, getServerSocket: async () => { return worker; }, diff --git a/packages/desktop-electron/index.js b/packages/desktop-electron/index.js index 0a07bfb34b2..07227d1eb40 100644 --- a/packages/desktop-electron/index.js +++ b/packages/desktop-electron/index.js @@ -2,14 +2,8 @@ // (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, @@ -18,6 +12,7 @@ const { dialog, shell, protocol, + utilityProcess, } = require('electron'); const promiseRetry = require('promise-retry'); @@ -30,15 +25,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 +49,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 +65,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 +84,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 +153,6 @@ async function createWindow() { } }); - win.webContents.on('did-finish-load', () => { - win.webContents.send('set-socket', { name: serverSocket }); - }); - // hit when middle-clicking buttons or with a target set to _blank // always deny, optionally redirect to browser win.webContents.setWindowOpenHandler(({ url }) => { @@ -304,6 +259,8 @@ app.on('ready', async () => { require('electron').powerMonitor.on('suspend', () => { console.log('Suspending', new Date()); }); + + createBackgroundProcess(); }); app.on('window-all-closed', () => { @@ -326,14 +283,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 +326,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; diff --git a/packages/desktop-electron/package.json b/packages/desktop-electron/package.json index 6e3e93b8968..8b4d265a85f 100644 --- a/packages/desktop-electron/package.json +++ b/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", diff --git a/packages/desktop-electron/preload.js b/packages/desktop-electron/preload.js index 8aac6545db5..2625c58a85b 100644 --- a/packages/desktop-electron/preload.js +++ b/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 => { diff --git a/packages/desktop-electron/server.js b/packages/desktop-electron/server.js index 109fb02336d..0747310123d 100644 --- a/packages/desktop-electron/server.js +++ b/packages/desktop-electron/server.js @@ -11,11 +11,9 @@ function getBackend() { if (process.argv[2] === '--subprocess') { const isDev = false; - // let version = process.argv[3]; - const socketName = process.argv[4]; // Start the app - getBackend().initApp(isDev, socketName); + getBackend().initApp(isDev); } else if (process.argv[2] === '--standalone') { require('source-map-support').install(); getBackend().initApp(true, 'actual-standalone'); diff --git a/packages/loot-core/package.json b/packages/loot-core/package.json index d6c155b264f..13d2a555df4 100644 --- a/packages/loot-core/package.json +++ b/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" } } diff --git a/packages/loot-core/src/platform/client/fetch/index.web.ts b/packages/loot-core/src/platform/client/fetch/index.web.ts index cdf9e2c5fdf..c37e18a26c5 100644 --- a/packages/loot-core/src/platform/client/fetch/index.web.ts +++ b/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; + + 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 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) { + replyHandlers.delete(id); - const handler = replyHandlers.get(id); - if (handler) { - replyHandlers.delete(id); + if (!mutated) { + undo.gc(undoTag); + } - if (!mutated) { - undo.gc(undoTag); + handler.resolve(result); } - - 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 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) { diff --git a/packages/loot-core/src/platform/server/connection/index.electron.ts b/packages/loot-core/src/platform/server/connection/index.electron.ts index 92206ff419c..4f974477fd3 100644 --- a/packages/loot-core/src/platform/server/connection/index.electron.ts +++ b/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 }); - - // websockets doesn't support sending objects so parse/stringify needed - wss.on('connection', function connection(ws) { - ws.on('error', console.error); - - ws.on('message', data => { - const msg = JSON.parse(data); - - if (ws.readyState !== 1) { - return; - } - - const { id, name, args, undoTag, catchErrors } = msg; +export const init: T.Init = function (_socketName, handlers) { + process.parentPort.on('message', ({ data }) => { + const { id, name, args, undoTag, catchErrors } = data; - if (handlers[name]) { - runHandler(handlers[name], args, { undoTag, name }).then( - result => { - if (ws.readyState !== 1) { - return; - } - if (catchErrors) { - result = { data: result, error: null }; - } + if (handlers[name]) { + runHandler(handlers[name], args, { undoTag, name }).then( + result => { + 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 }); }; diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index d16ec1093da..8d8da4116ec 100644 --- a/packages/loot-core/src/server/main.ts +++ b/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', }); diff --git a/packages/loot-core/webpack/webpack.desktop.config.js b/packages/loot-core/webpack/webpack.desktop.config.js index a0f9b2c6dc2..b1df8510040 100644 --- a/packages/loot-core/webpack/webpack.desktop.config.js +++ b/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/, diff --git a/yarn.lock b/yarn.lock index 03fe210bc28..4f82064c2a7 100644 --- a/yarn.lock +++ b/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,43 +21524,33 @@ __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" +"ws@npm:^7.3.1, ws@npm:^7.4.6": + version: 7.5.9 + resolution: "ws@npm:7.5.9" peerDependencies: bufferutil: ^4.0.1 - utf-8-validate: ">=5.0.2" + 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 + checksum: 171e35012934bd8788150a7f46f963e50bac43a4dc524ee714c20f258693ac4d3ba2abadb00838fdac42a47af9e958c7ae7e6f4bc56db047ba897b8a2268cf7c languageName: node linkType: hard -"ws@npm:^7.3.1, ws@npm:^7.4.6": - version: 7.5.9 - resolution: "ws@npm:7.5.9" +"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 + utf-8-validate: ">=5.0.2" peerDependenciesMeta: bufferutil: optional: true utf-8-validate: optional: true - checksum: 171e35012934bd8788150a7f46f963e50bac43a4dc524ee714c20f258693ac4d3ba2abadb00838fdac42a47af9e958c7ae7e6f4bc56db047ba897b8a2268cf7c + checksum: 1769532b6fdab9ff659f0b17810e7501831d34ecca23fd179ee64091dd93a51f42c59f6c7bb4c7a384b6c229aca8076fb312aa35626257c18081511ef62a161d languageName: node linkType: hard