[PM-8297] Improve updater behavior

apps/desktop/src/main.ts

@@ -201,7 +201,7 @@ export class Main {
       (win) => this.trayMain.setupWindowListeners(win),
     );
     this.messagingMain = new MessagingMain(this, this.desktopSettingsService);
-    this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain);
+    this.updaterMain = new UpdaterMain(this.i18nService, this.logService, this.windowMain);
 
     const messageSubject = new Subject<Message<Record<string, unknown>>>();
     this.messagingService = MessageSender.combine(

apps/desktop/src/main/updater.main.ts

@@ -1,8 +1,9 @@
 import { dialog, shell } from "electron";
 import log from "electron-log";
-import { autoUpdater } from "electron-updater";
+import { autoUpdater, UpdateDownloadedEvent, VerifyUpdateSupport } from "electron-updater";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 
 import { isAppImage, isDev, isMacAppStore, isWindowsPortable, isWindowsStore } from "../utils";
 
@@ -15,13 +16,18 @@ export class UpdaterMain {
   private doingUpdateCheck = false;
   private doingUpdateCheckWithFeedback = false;
   private canUpdate = false;
+  private updateDownloaded: UpdateDownloadedEvent = null;
+  private originalRolloutFunction: VerifyUpdateSupport;
 
   constructor(
     private i18nService: I18nService,
+    private logService: LogService,
     private windowMain: WindowMain,
   ) {
     autoUpdater.logger = log;
 
+    this.originalRolloutFunction = autoUpdater.isUserWithinRollout;
+
     const linuxCanUpdate = process.platform === "linux" && isAppImage();
     const windowsCanUpdate =
       process.platform === "win32" && !isWindowsStore() && !isWindowsPortable();
@@ -35,10 +41,16 @@ export class UpdaterMain {
     global.setInterval(async () => await this.checkForUpdate(), UpdaterCheckInterval);
 
     autoUpdater.on("checking-for-update", () => {
+      this.logService.debug("[Updater] Checking for update...");
       this.doingUpdateCheck = true;
     });
 
-    autoUpdater.on("update-available", async () => {
+    autoUpdater.on("update-available", async (info) => {
+      this.logService.debug(
+        `[Updater] Update available (with feedback: ${this.doingUpdateCheckWithFeedback})`,
+        info,
+      );
+
       if (this.doingUpdateCheckWithFeedback) {
         if (this.windowMain.win == null) {
           this.reset();
@@ -57,20 +69,18 @@ export class UpdaterMain {
         });
 
         if (result.response === 0) {
-          // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          autoUpdater.downloadUpdate();
+          await autoUpdater.downloadUpdate();
         } else {
           this.reset();
         }
       }
     });
 
-    autoUpdater.on("update-not-available", () => {
+    autoUpdater.on("update-not-available", async (info) => {
+      this.logService.debug("[Updater] No update available", info);
+
       if (this.doingUpdateCheckWithFeedback && this.windowMain.win != null) {
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        dialog.showMessageBox(this.windowMain.win, {
+        await dialog.showMessageBox(this.windowMain.win, {
           message: this.i18nService.t("noUpdatesAvailable"),
           buttons: [this.i18nService.t("ok")],
           defaultId: 0,
@@ -82,29 +92,19 @@ export class UpdaterMain {
     });
 
     autoUpdater.on("update-downloaded", async (info) => {
+      this.logService.debug("[Updater] Update downloaded", info);
+
       if (this.windowMain.win == null) {
         return;
       }
 
-      const result = await dialog.showMessageBox(this.windowMain.win, {
-        type: "info",
-        title: this.i18nService.t("bitwarden") + " - " + this.i18nService.t("restartToUpdate"),
-        message: this.i18nService.t("restartToUpdate"),
-        detail: this.i18nService.t("restartToUpdateDesc", info.version),
-        buttons: [this.i18nService.t("restart"), this.i18nService.t("later")],
-        cancelId: 1,
-        defaultId: 0,
-        noLink: true,
-      });
-
-      if (result.response === 0) {
-        // Quit and install have a different window logic, setting `isQuitting` just to be safe.
-        this.windowMain.isQuitting = true;
-        autoUpdater.quitAndInstall(true, true);
-      }
+      this.updateDownloaded = info;
+      await this.promptRestartUpdate(info);
     });
 
     autoUpdater.on("error", (error) => {
+      this.logService.error("[Updater] Error in auto-updater", error);
+
       if (this.doingUpdateCheckWithFeedback) {
         dialog.showErrorBox(
           this.i18nService.t("updateError"),
@@ -117,15 +117,23 @@ export class UpdaterMain {
   }
 
   async checkForUpdate(withFeedback = false) {
-    if (this.doingUpdateCheck || isDev()) {
+    if (isDev()) {
+      return;
+    }
+
+    if (this.updateDownloaded && withFeedback) {
+      await this.promptRestartUpdate(this.updateDownloaded);
+      return;
+    }
+
+    if (this.doingUpdateCheck) {
+      this.logService.debug("[Updater] Already checking for update");
       return;
     }
 
     if (!this.canUpdate) {
       if (withFeedback) {
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        shell.openExternal("https://github.com/bitwarden/clients/releases");
+        void shell.openExternal("https://github.com/bitwarden/clients/releases");
       }
 
       return;
@@ -134,6 +142,10 @@ export class UpdaterMain {
     this.doingUpdateCheckWithFeedback = withFeedback;
     if (withFeedback) {
       autoUpdater.autoDownload = false;
+
+      // If the user has explicitly checked for updates, we want to bypass
+      // the current staging rollout percentage
+      autoUpdater.isUserWithinRollout = (info) => true;
     }
 
     await autoUpdater.checkForUpdates();
@@ -141,7 +153,29 @@ export class UpdaterMain {
 
   private reset() {
     autoUpdater.autoDownload = true;
+    // Reset the rollout check to the default behavior
+    autoUpdater.isUserWithinRollout = this.originalRolloutFunction;
     this.doingUpdateCheck = false;
+    this.updateDownloaded = null;
+  }
+
+  private async promptRestartUpdate(info: UpdateDownloadedEvent) {
+    const result = await dialog.showMessageBox(this.windowMain.win, {
+      type: "info",
+      title: this.i18nService.t("bitwarden") + " - " + this.i18nService.t("restartToUpdate"),
+      message: this.i18nService.t("restartToUpdate"),
+      detail: this.i18nService.t("restartToUpdateDesc", info.version),
+      buttons: [this.i18nService.t("restart"), this.i18nService.t("later")],
+      cancelId: 1,
+      defaultId: 0,
+      noLink: true,
+    });
+
+    if (result.response === 0) {
+      // Quit and install have a different window logic, setting `isQuitting` just to be safe.
+      this.windowMain.isQuitting = true;
+      autoUpdater.quitAndInstall(true, true);
+    }
   }
 
   private userDisabledUpdates(): boolean {