From b8124bbc2e0f9152f4890253ceb3c584d784d511 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Thu, 8 Aug 2024 11:55:44 -0700 Subject: [PATCH] Clean up the Updater class and fix a runaway interval (#207) --- emain/emain.ts | 1 + emain/updater.ts | 96 +++++++++++++++++++++--------------------------- 2 files changed, 43 insertions(+), 54 deletions(-) diff --git a/emain/emain.ts b/emain/emain.ts index 66484a24d..637e3756e 100644 --- a/emain/emain.ts +++ b/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"); diff --git a/emain/updater.ts b/emain/updater.ts index 8216d0680..c46dcd7ca 100644 --- a/emain/updater.ts +++ b/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. - * @param [runCheckNow=true] Determines whether the update check should run immediately. Defaults to true. - * @returns The timeout object for the auto update checker. + * Check for updates and start the background update check, if configured. */ - startInterval(runCheckNow = true): NodeJS.Timeout { - // check for updates right away and keep checking later - if (runCheckNow) this.checkForUpdates(false); - return setInterval(() => { - this.checkForUpdates(false); - }, 600000); // intervals are unreliable when an app is suspended so we will check every 10 mins if the interval has passed. + async start() { + if (this.autoCheckEnabled) { + console.log("starting updater"); + this.autoCheckInterval = setInterval(() => { + 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. + 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.lastUpdateCheck || - Math.abs(now.getTime() - this.lastUpdateCheck.getTime()) > autoUpdateOpts.intervalms)) + (this.autoCheckInterval && + (!this.lastUpdateCheck || Math.abs(now.getTime() - this.lastUpdateCheck.getTime()) > this.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(); - autoUpdater.autoInstallOnAppQuit = autoUpdateOpts.installonquit; + const autoUpdateOpts = (await services.FileService.GetSettingsConfig()).autoupdate; + 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; }