fix: handle quit with proper cleanup (#223)

* wip: bring back changes from #191

* chore: remove unnecessary debug config

* fix: make sure getReadyQuit doesn't indefinitely hang if bg window dies out for some reason

Author: nsrCodes

src/lib/autoupdate/index.js

@@ -93,7 +93,7 @@ class AutoUpdate {
     ipcMain.handle("quit-and-install", () => {
       log.info("recieved quit and install")
       global.quitAndInstall = true;
-      let res = autoUpdater.quitAndInstall();
+      const res = autoUpdater.quitAndInstall();
       log.info("finished quit and install", res)
     });
   };

src/main/actions/cleanup.js

@@ -1,21 +1,31 @@
 const { app, ipcMain } = require("electron");
 
-export const cleanupAndQuit = () => {
-  cleanup();
-
-  if (global.backgroundWindow) {
-    ipcMain.on("shutdown-success", () => {
-      console.log("shudown sucess");
-      app.quit();
-    });
-  }
-};
-
 const cleanup = () => {
-  if (global.backgroundWindow) {
-    global.backgroundWindow.webContents.send("shutdown");
+  if(global.backgroundProcessStarted) {
+    if (global.backgroundWindow) {
+      global.backgroundWindow.webContents.send("shutdown");
+    } else {
+      // No backgroundWindow. Quit directly without cleanup
+      app.quit();
+    }
   } else {
-    // No backgroundWindow. Quit directly without cleanup
     app.quit();
   }
 };
+
+
+export const getReadyToQuitApp = async  () => {
+  return new Promise((resolve) => {
+    cleanup();
+  
+    if (global.backgroundWindow) {
+      ipcMain.once("shutdown-success", () => {
+        console.log("shudown sucess");
+        global.backgroundWindow?.close();
+        resolve()
+      });
+    } else {
+      resolve();
+    }
+  })
+};

src/main/actions/logNetworkRequest.js

@@ -1,17 +1,3 @@
-/**
- * @param {Array} array into which element will be pushed
- * @param {number} required max length of array
- * @param {*} element to push
- */
-const addToLog = (array, limit, element) => {
-  const arrayToModify = [...array];
-  if (arrayToModify.length >= limit) {
-    arrayToModify.shift();
-  }
-  arrayToModify.push(element);
-  return arrayToModify;
-};
-
 const logNetworkRequest = (event, message, webAppWindow) => {
   const newLog = message;
   if (webAppWindow) {

src/main/actions/logNetworkRequestV2.js

@@ -1,20 +1,6 @@
-/**
- * @param {Array} array into which element will be pushed
- * @param {number} required max length of array
- * @param {*} element to push
- */
-const addToLog = (array, limit, element) => {
-  const arrayToModify = [...array];
-  if (arrayToModify.length >= limit) {
-    arrayToModify.shift();
-  }
-  arrayToModify.push(element);
-  return arrayToModify;
-};
-
 const logNetworkRequestV2 = (event, message, webAppWindow) => {
   const newLog = message;
-  if (webAppWindow) {
+  if (webAppWindow && !webAppWindow.isDestroyed()) {
     webAppWindow.send("log-network-request-v2", newLog);
   }
 };

src/main/actions/startBackgroundProcess.js

@@ -79,14 +79,6 @@ const startBackgroundProcess = async () => {
       resolve(true);
     });
 
-    backgroundWindow.on("close", (event) => {
-      if (global.isQuitActionConfirmed) {
-        console.log("BG quitting");
-        return;
-      }
-
-      event.preventDefault();
-    });
   });
 };
 

src/main/events.js

@@ -76,6 +76,9 @@ function removeFileFromAccessRecords(filePath) {
 
 // These events do not require the browser window
 export const registerMainProcessEvents = () => {
+  ipcMain.on("background-process-started", () => {
+    global.backgroundProcessStarted = true;
+  });
   ipcMain.handle("start-background-process", startBackgroundProcess);
   /** Main Process State Management */
   ipcMain.handle("get-state", getState);
@@ -116,32 +119,32 @@ export const registerMainProcessEventsForWebAppWindow = (webAppWindow) => {
 
   // Open handle for async source detection
   ipcMain.handle("app-detected", async (event, payload) => {
-    webAppWindow.send("app-detected", payload);
+    webAppWindow?.send("app-detected", payload);
   });
 
   ipcMain.handle("browser-connected", async (event, payload) => {
-    webAppWindow.send("browser-connected", payload);
+    webAppWindow?.send("browser-connected", payload);
   });
 
   ipcMain.handle("browser-disconnected", async (event, payload) => {
-    webAppWindow.send("browser-disconnected", payload);
+    webAppWindow?.send("browser-disconnected", payload);
   });
 
   // Open handle for async browser close
   ipcMain.handle("browser-closed", async (event, payload) => {
-    webAppWindow.send("browser-closed", payload);
+    webAppWindow?.send("browser-closed", payload);
   });
 
   // Open handle for async browser close
   ipcMain.handle("proxy-restarted", async (event, payload) => {
     createOrUpdateAxiosInstance(payload);
-    webAppWindow.send("proxy-restarted", payload);
+    webAppWindow?.send("proxy-restarted", payload);
   });
 
   // hacky implementation for syncing addition and deletion
   const resendAllNetworkLogs = async () => {
     const res = await getAllNetworkSessions();
-    webAppWindow.send("network-sessions-updated", res);
+    webAppWindow?.send("network-sessions-updated", res);
   };
 
   ipcMain.handle("get-all-network-sessions", async () => {
@@ -264,7 +267,7 @@ export const registerMainProcessEventsForWebAppWindow = (webAppWindow) => {
   });
 
   ipcMain.handle("helper-server-hit", () => {
-    webAppWindow.send("helper-server-hit");
+    webAppWindow?.send("helper-server-hit");
   });
 };
 

src/main/main.ts

@@ -12,7 +12,7 @@
 import "core-js/stable";
 import "regenerator-runtime/runtime";
 import path from "path";
-import { app, BrowserWindow, shell, dialog, Tray, Menu, clipboard } from "electron";
+import { app, BrowserWindow, shell, dialog, Tray, Menu, clipboard, ipcMain } from "electron";
 import log from "electron-log";
 import MenuBuilder from "./menu";
 import {
@@ -24,9 +24,7 @@ import {
 /** Storage - State */
 import "./actions/initGlobalState";
 import AutoUpdate from "../lib/autoupdate";
-import { cleanupAndQuit } from "./actions/cleanup";
-import { trackEventViaWebApp } from "./actions/events";
-import EVENTS from "./actions/events/constants";
+import { getReadyToQuitApp } from "./actions/cleanup";
 import fs from "fs";
 import logger from "../utils/logger";
 import { setupIPCForwardingToWebApp } from "./actions/setupIPCForwarding";
@@ -85,7 +83,7 @@ export default function createTrayMenu(ip?: string, port?: number) {
     {
       label: "Show Requestly",
       click: () => {
-        if(webAppWindow) {
+        if(webAppWindow && !webAppWindow.isDestroyed()) {
           if(webAppWindow.isMinimized()) {
             webAppWindow.restore()
           }
@@ -151,7 +149,7 @@ export default function createTrayMenu(ip?: string, port?: number) {
     {
       label: "Quit",
       click: () => {
-        app.quit();
+        webAppWindow?.close();
       },
     },
   ]
@@ -170,7 +168,7 @@ export default function createTrayMenu(ip?: string, port?: number) {
   tray.setContextMenu(trayMenu);
 }
 
-
+let closingAccepted = false
 const createWindow = async () => {
   if (isDevelopment) {
     await installExtensions();
@@ -258,63 +256,19 @@ const createWindow = async () => {
     }
   });
 
-  // webAppWindow.on('closed', () => {
-  //   webAppWindow = null;
-  // });
-
-  webAppWindow.on("close", (e) => {
-    // Check if user has already asked to Quit app from here or somewhere else
-    // @ts-expect-error
-    if (global.isQuitActionConfirmed) {
-      saveCookies();
-      app.quit();
-      return;
-    }
-
-    if (webAppWindow) {
-      let message =
-        "Do you really want to quit? This would also stop the proxy server.";
-
-      // @ts-expect-error
-      if (global.quitAndInstall) {
-        message = "Confirm to restart & install update";
-        // @ts-expect-error
-        global.quitAndInstall = false;
-      }
-
-      const choice = dialog.showMessageBoxSync(webAppWindow, {
-        type: "question",
-        buttons: ["Yes, quit Requestly", "Minimize instead", "Cancel"],
-        title: "Quit Requestly",
-        message: message,
-      });
-
-      switch (choice) {
-        // If Quit is clicked
-        case 0:
-          // Set flag to check next iteration
-          trackEventViaWebApp(webAppWindow, EVENTS.QUIT_APP)
-          // @ts-expect-error
-          global.isQuitActionConfirmed = true;
-          // Calling app.quit() would again invoke this function
-          e.preventDefault();
-          cleanupAndQuit();
-          break;
-        // If Minimize is clicked
-        case 1:
-          webAppWindow.minimize();
-          e.preventDefault();
-          break;
-        // If cancel is clicked
-        case 2:
-          e.preventDefault();
-          break;
-        default:
-          break;
-      }
+  webAppWindow.on('close', async (event) => {
+    if(!closingAccepted) {
+      event.preventDefault();
+      webAppWindow?.webContents.send("initiate-app-close")
     }
-  });
+  })
 
+  webAppWindow.on('closed', async () => {
+    saveCookies();
+    await getReadyToQuitApp();
+    webAppWindow = null;
+    return;
+  })
   const enableBGWindowDebug = () => {
     // Show bg window and toggle the devtools
     try {
@@ -368,7 +322,7 @@ function handleCustomProtocolURL(urlString: string) {
 
 // custom protocol (requestly) handler
 app.on("open-url", (_event, rqUrl) => {
-  if(webAppWindow) {
+  if(webAppWindow && !webAppWindow.isDestroyed()) {
     handleCustomProtocolURL(rqUrl)
   } else {
     onWebAppReadyHandlers.push(() => handleCustomProtocolURL(rqUrl))
@@ -401,7 +355,7 @@ async function handleFileOpen(filePath: string, webAppWindow?: BrowserWindow) {
 
 app.on('open-file', async (event, filePath) => {
   event.preventDefault();
-  if(webAppWindow) {
+  if(webAppWindow && !webAppWindow.isDestroyed()) {
     handleFileOpen(filePath, webAppWindow);
   } else {
     logger.log("webAppWindow not ready")
@@ -492,3 +446,24 @@ app
   .catch((err) => {
     console.log(err);
   });
+
+ipcMain.handle("quit-app", (_event) => {
+  closingAccepted = true
+  webAppWindow?.close();
+})
+
+app.on("before-quit", () => {
+  // cleanup when quitting has been finalised
+  ipcMain.removeAllListeners();
+  webAppWindow?.removeAllListeners();
+  // @ts-expect-error BrowserWindow types are not being enforced for this variable
+  backgroundWindow?.removeAllListeners();
+
+  ipcMain.removeAllListeners();
+  process.on('uncaughtException', (err) => {
+    logger.error('Unhandled Exception while quitting:', err);
+  });
+  process.on('unhandledRejection', (err) => {
+    logger.error('Unhandled Rejection while quitting:', err);
+  });
+})

src/main/menu.ts

@@ -1,5 +1,4 @@
 import {
-  app,
   Menu,
   shell,
   BrowserWindow,
@@ -13,6 +12,7 @@ interface DarwinMenuItemConstructorOptions extends MenuItemConstructorOptions {
 
 export default class MenuBuilder {
   mainWindow: BrowserWindow;
+
   enableBGWindowDebug: Function;
 
   constructor(mainWindow: BrowserWindow, enableBGWindowDebug: Function) {
@@ -79,10 +79,7 @@ export default class MenuBuilder {
           label: "Quit",
           accelerator: "Command+Q",
           click: () => {
-            if (this.mainWindow) {
-              return this.mainWindow.close();
-            }
-            app.quit();
+            return this.mainWindow?.close();
           },
         },
       ],
@@ -289,10 +286,7 @@ export default class MenuBuilder {
             label: "&Close",
             accelerator: "Ctrl+W",
             click: () => {
-              if (this.mainWindow) {
-                return this.mainWindow.close();
-              }
-              app.quit();
+              return this.mainWindow?.close();
             },
           },
         ],