diff --git a/apps/desktop/src/main/lib/auto-updater.test.ts b/apps/desktop/src/main/lib/auto-updater.test.ts new file mode 100644 index 00000000000..eabaa65c768 --- /dev/null +++ b/apps/desktop/src/main/lib/auto-updater.test.ts @@ -0,0 +1,93 @@ +import { beforeEach, describe, expect, mock, test } from "bun:test"; +import { EventEmitter } from "node:events"; + +// Minimal fake autoUpdater that behaves like electron-updater's +// instance for the code paths we exercise here. +class FakeAutoUpdater extends EventEmitter { + autoDownload = false; + autoInstallOnAppQuit = false; + disableDifferentialDownload = false; + allowDowngrade = false; + setFeedURL = mock(() => {}); + checkForUpdates = mock(() => Promise.resolve(null)); + quitAndInstall = mock(() => {}); +} + +const fakeAutoUpdater = new FakeAutoUpdater(); + +mock.module("electron-updater", () => ({ + autoUpdater: fakeAutoUpdater, +})); + +// The global test-setup's electron mock doesn't include app.isReady / +// app.whenReady; patch them here so setupAutoUpdater can run under bun:test. +mock.module("electron", () => ({ + app: { + getPath: mock(() => ""), + getName: mock(() => "test-app"), + getVersion: mock(() => "1.0.0"), + getAppPath: mock(() => ""), + isPackaged: false, + isReady: mock(() => true), + whenReady: mock(() => Promise.resolve()), + }, + dialog: { + showMessageBox: mock(() => Promise.resolve({ response: 0 })), + }, +})); + +mock.module("main/index", () => ({ + setSkipQuitConfirmation: mock(() => {}), +})); + +const autoUpdater = await import("./auto-updater"); +const { AUTO_UPDATE_STATUS } = await import("shared/auto-update"); + +describe("installUpdate", () => { + beforeEach(() => { + fakeAutoUpdater.removeAllListeners(); + fakeAutoUpdater.quitAndInstall.mockClear(); + fakeAutoUpdater.checkForUpdates.mockClear(); + fakeAutoUpdater.setFeedURL.mockClear(); + }); + + test("ignores install requests when no update is ready", () => { + autoUpdater.setupAutoUpdater(); + + // No update-downloaded emitted, so status is not READY. + expect(autoUpdater.getUpdateStatus().status).not.toBe( + AUTO_UPDATE_STATUS.READY, + ); + + autoUpdater.installUpdate(); + + // Calling quitAndInstall without a staged update leaves the user with + // no feedback and nothing to install. + expect(fakeAutoUpdater.quitAndInstall).not.toHaveBeenCalled(); + }); + + test("does not invoke quitAndInstall multiple times when the install button is clicked repeatedly", () => { + autoUpdater.setupAutoUpdater(); + + // Simulate electron-updater announcing the update is downloaded so the + // module transitions into READY and the UI would render an Install button. + fakeAutoUpdater.emit("update-downloaded", { version: "9.9.9" }); + expect(autoUpdater.getUpdateStatus().status).toBe(AUTO_UPDATE_STATUS.READY); + + // User clicks "Install" several times before the app has actually + // quit (Squirrel.Mac is still finalising the download in the + // background, so quitAndInstall is a no-op until it finishes). + autoUpdater.installUpdate(); + autoUpdater.installUpdate(); + autoUpdater.installUpdate(); + + // BUG (pre-fix): each click drove another quitAndInstall() call. On + // macOS each call re-registers a native-updater "update-downloaded" + // listener, so when Squirrel finally finishes the download we fire N + // concurrent quitAndInstall() calls against the native autoUpdater. + // That is the root cause of + // https://github.com/superset-sh/superset/issues/3507: the app + // closes but the version doesn't change. + expect(fakeAutoUpdater.quitAndInstall).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/desktop/src/main/lib/auto-updater.ts b/apps/desktop/src/main/lib/auto-updater.ts index 0171a91cddd..76e943bae68 100644 --- a/apps/desktop/src/main/lib/auto-updater.ts +++ b/apps/desktop/src/main/lib/auto-updater.ts @@ -82,6 +82,7 @@ function isNetworkError(error: Error | string): boolean { let currentStatus: AutoUpdateStatus = AUTO_UPDATE_STATUS.IDLE; let currentVersion: string | undefined; let isDismissed = false; +let isInstalling = false; function emitStatus( status: AutoUpdateStatus, @@ -111,6 +112,24 @@ export function installUpdate(): void { emitStatus(AUTO_UPDATE_STATUS.IDLE); return; } + // Guard against repeat clicks. On macOS, MacUpdater.quitAndInstall() adds + // a fresh `update-downloaded` listener every time it is called before + // Squirrel.Mac has finished downloading, so each extra click fans out into + // another nativeUpdater.quitAndInstall() once Squirrel finally fires — + // which in turn leaves the binary un-swapped (see issue #3507). + if (isInstalling) { + console.info( + "[auto-updater] Install already in progress, ignoring duplicate request", + ); + return; + } + if (currentStatus !== AUTO_UPDATE_STATUS.READY) { + console.warn( + `[auto-updater] Install ignored: update not ready (status=${currentStatus})`, + ); + return; + } + isInstalling = true; setSkipQuitConfirmation(); autoUpdater.quitAndInstall(false, true); } @@ -242,6 +261,9 @@ export function setupAutoUpdater(): void { ); autoUpdater.on("error", (error) => { + // Clear install-in-progress flag so the user can retry if Squirrel + // surfaces an error instead of actually quitting the app. + isInstalling = false; if (isNetworkError(error)) { console.info("[auto-updater] Network unavailable, will retry later"); emitStatus(AUTO_UPDATE_STATUS.IDLE);