Clean up the Updater class and fix a runaway interval (#207)

emain/emain.ts

@@ -756,6 +756,7 @@ electronApp.on("window-all-closed", () => {
 });
 electronApp.on("before-quit", () => {
     globalIsQuitting = true;
+    updater?.stop();
 });
 process.on("SIGINT", () => {
     console.log("Caught SIGINT, shutting down");

emain/updater.ts

@@ -7,13 +7,25 @@ import { fireAndForget } from "../frontend/util/util";
 export let updater: Updater;
 
 export class Updater {
-    interval: NodeJS.Timeout | null;
+    autoCheckInterval: NodeJS.Timeout | null;
+    intervalms: number;
+    autoCheckEnabled: boolean;
     availableUpdateReleaseName: string | null;
     availableUpdateReleaseNotes: string | null;
     private _status: UpdaterStatus;
     lastUpdateCheck: Date;
 
-    constructor() {
+    constructor(options: AutoUpdateOpts) {
+        this.intervalms = options.intervalms;
+        this.autoCheckEnabled = options.enabled;
+
+        this._status = "up-to-date";
+        this.lastUpdateCheck = new Date(0);
+        this.autoCheckInterval = null;
+        this.availableUpdateReleaseName = null;
+
+        autoUpdater.autoInstallOnAppQuit = options.installonquit;
+
         autoUpdater.removeAllListeners();
 
         autoUpdater.on("error", (err) => {
@@ -51,21 +63,15 @@ export class Updater {
             });
             updateNotification.show();
         });
-
-        this._status = "up-to-date";
-        this.lastUpdateCheck = new Date(0);
-        this.interval = null;
-        this.availableUpdateReleaseName = null;
     }
 
+    /**
+     * The status of the Updater.
+     */
     get status(): UpdaterStatus {
         return this._status;
     }
 
-    /**
-     * Sets the app update status and sends it to the main window
-     * @param value The status to set
-     */
     private set status(value: UpdaterStatus) {
         this._status = value;
         electron.BrowserWindow.getAllWindows().forEach((window) => {
@@ -74,16 +80,27 @@ export class Updater {
     }
 
     /**
-     * Starts the auto update check interval.
+     * Check for updates and start the background update check, if configured.
-     * @param [runCheckNow=true] Determines whether the update check should run immediately. Defaults to true.
-     * @returns The timeout object for the auto update checker.
      */
-    startInterval(runCheckNow = true): NodeJS.Timeout {
+    async start() {
-        // check for updates right away and keep checking later
+        if (this.autoCheckEnabled) {
-        if (runCheckNow) this.checkForUpdates(false);
+            console.log("starting updater");
-        return setInterval(() => {
+            this.autoCheckInterval = setInterval(() => {
-            this.checkForUpdates(false);
+                fireAndForget(() => this.checkForUpdates(false));
-        }, 600000); // intervals are unreliable when an app is suspended so we will check every 10 mins if the interval has passed.
+            }, 600000); // intervals are unreliable when an app is suspended so we will check every 10 mins if the interval has passed.
+            await this.checkForUpdates(false);
+        }
+    }
+
+    /**
+     * Stop the background update check, if configured.
+     */
+    stop() {
+        console.log("stopping updater");
+        if (this.autoCheckInterval) {
+            clearInterval(this.autoCheckInterval);
+            this.autoCheckInterval = null;
+        }
     }
 
     /**
@@ -91,26 +108,13 @@ export class Updater {
      * @param userInput Whether the user is requesting this. If so, an alert will report the result of the check.
      */
     async checkForUpdates(userInput: boolean) {
-        console.log("checkForUpdates");
-        const autoUpdateOpts = (await services.FileService.GetSettingsConfig()).autoupdate;
-
-        // If there's an active update check interval, check that the user still has auto update checks enabled. If not, remove the interval.
-        if (this.interval && !autoUpdateOpts.enabled) {
-            console.log("Auto update is disabled in settings. Removing the auto update interval.");
-            clearInterval(this.interval);
-            this.interval = null;
-        } else if (!this.interval && autoUpdateOpts.enabled) {
-            this.startInterval(false);
-        }
-
         const now = new Date();
 
         // Run an update check always if the user requests it, otherwise only if there's an active update check interval and enough time has elapsed.
         if (
             userInput ||
-            (this.interval &&
+            (this.autoCheckInterval &&
-                (!this.lastUpdateCheck ||
+                (!this.lastUpdateCheck || Math.abs(now.getTime() - this.lastUpdateCheck.getTime()) > this.intervalms))
-                    Math.abs(now.getTime() - this.lastUpdateCheck.getTime()) > autoUpdateOpts.intervalms))
         ) {
             const result = await autoUpdater.checkForUpdates();
 
@@ -172,7 +176,6 @@ let autoUpdateLock = false;
 
 /**
  * Configures the auto-updater based on the user's preference
- * @param enabled Whether the auto-updater should be enabled
  */
 export async function configureAutoUpdater() {
     if (isDev()) {
@@ -185,31 +188,16 @@ export async function configureAutoUpdater() {
         console.log("auto-update configuration already in progress, skipping");
         return;
     }
-
-    console.log("configureAutoUpdater");
     autoUpdateLock = true;
 
-    const autoUpdateOpts = (await services.FileService.GetSettingsConfig()).autoupdate;
-
     try {
         console.log("Configuring updater");
-        updater = new Updater();
+        const autoUpdateOpts = (await services.FileService.GetSettingsConfig()).autoupdate;
-        autoUpdater.autoInstallOnAppQuit = autoUpdateOpts.installonquit;
+        updater = new Updater(autoUpdateOpts);
+        await updater.start();
     } catch (e) {
         console.warn("error configuring updater", e.toString());
     }
 
-    if (autoUpdateOpts.enabled && updater?.interval == null) {
-        try {
-            console.log("configuring auto update interval");
-            updater?.startInterval();
-        } catch (e) {
-            console.log("error configuring auto update interval", e.toString());
-        }
-    } else if (!autoUpdateOpts.enabled && updater?.interval != null) {
-        console.log("disabling auto updater");
-        clearInterval(updater.interval);
-        updater = null;
-    }
     autoUpdateLock = false;
 }