diff --git a/apps/desktop/src/main/services/updates/autoUpdateService.test.ts b/apps/desktop/src/main/services/updates/autoUpdateService.test.ts index fc593cae0..37fed4a16 100644 --- a/apps/desktop/src/main/services/updates/autoUpdateService.test.ts +++ b/apps/desktop/src/main/services/updates/autoUpdateService.test.ts @@ -447,9 +447,17 @@ describe("createAutoUpdateService", () => { await Promise.resolve(); expect(updater.checkForUpdates).not.toHaveBeenCalled(); - updater.checkForUpdates.mockImplementation(async () => ({ - updateInfo: { version: "1.2.63" }, - })); + // electron-updater emits these BEFORE checkForUpdates() resolves. A mock + // that only resolves cannot see the bug this test exists for: the + // update-available handler is what supersedes the staged update, deletes + // the finished download, and frees the resolve path to start a new one. + const downloadUpdate = vi.fn(async () => null); + (updater as unknown as { downloadUpdate: unknown }).downloadUpdate = downloadUpdate; + updater.checkForUpdates.mockImplementation(async () => { + updater.emit("checking-for-update"); + updater.emit("update-available", { version: "1.2.63" }); + return { updateInfo: { version: "1.2.63" } }; + }); service.checkForUpdates({ userInitiated: true }); await vi.waitFor(() => { @@ -461,6 +469,31 @@ describe("createAutoUpdateService", () => { latestKnownVersion: "1.2.63", }); }); + // Asking what the newest version is must not throw away the update the + // user already downloaded and is one restart away from installing. + expect(downloadUpdate).not.toHaveBeenCalled(); + + // Let the first check's promise chain settle: while `checkPromise` is + // still set, a second call is swallowed by the in-flight guard and would + // assert nothing. + await new Promise((resolve) => setTimeout(resolve, 0)); + + // ...and neither must a check that fails. A feed error here means "nothing + // new to tell you", not "discard the finished download". + updater.checkForUpdates.mockImplementation(async () => { + updater.emit("checking-for-update"); + updater.emit("error", new Error("net::ERR_INTERNET_DISCONNECTED")); + throw new Error("net::ERR_INTERNET_DISCONNECTED"); + }); + service.checkForUpdates({ userInitiated: true }); + + await vi.waitFor(() => expect(updater.checkForUpdates).toHaveBeenCalledTimes(2)); + expect(service.getSnapshot()).toMatchObject({ + status: "ready", + version: "1.2.61", + latestKnownVersion: "1.2.63", + }); + expect(downloadUpdate).not.toHaveBeenCalled(); service.dispose(); }); diff --git a/apps/desktop/src/main/services/updates/autoUpdateService.ts b/apps/desktop/src/main/services/updates/autoUpdateService.ts index 29b80e434..9a5105bf8 100644 --- a/apps/desktop/src/main/services/updates/autoUpdateService.ts +++ b/apps/desktop/src/main/services/updates/autoUpdateService.ts @@ -513,6 +513,12 @@ export function createAutoUpdateService({ let installReadySnapshot: AutoUpdateSnapshot | null = null; let ignoredDownloadVersion: string | null = null; let readyRefreshInProgress = false; + /** + * A user-initiated check while an update is already staged. The answer they + * want is "what is the newest version" — throwing away the update they have + * already downloaded in order to answer it is not a service. + */ + let readyMetadataRefreshInProgress = false; const readyRefreshFailure: { current: { error: unknown; phase: AutoUpdatePhase } | null; } = { current: null }; @@ -815,6 +821,20 @@ export function createAutoUpdateService({ }); return; } + if (readyMetadataRefreshInProgress) { + // electron-updater emits `update-available` BEFORE checkForUpdates() + // resolves, so without this the newer version would supersede the + // staged one here — deleting the finished download from the cache and + // leaving the resolve path free to start a fresh one — purely because + // someone pressed a button to ask a question. + ignoredDownloadVersion = info.version; + patchSnapshot({ latestKnownVersion: info.version }); + logger.info("autoUpdate.update_available_metadata_only", { + version: info.version, + readyVersion: snapshot.version, + }); + return; + } if (!readyRefreshInProgress) { cleanupUpdaterCacheDir({ updaterCacheDir, @@ -933,6 +953,19 @@ export function createAutoUpdateService({ phase: classified.phase, }); ignoredDownloadVersion = null; + if (readyMetadataRefreshInProgress) { + // The user asked what the newest version is and the feed did not answer. + // That is a reason to tell them nothing new, not a reason to throw away + // the update they already downloaded: setErrorSnapshot would replace the + // `ready` snapshot and, with currentPhase already moved to "download" by + // `checking-for-update`, take the finished download with it. + logger.warn("autoUpdate.metadata_refresh_failed", { + message, + kind: classified.kind, + readyVersion: snapshot.version, + }); + return; + } if (readyRefreshInProgress) { readyRefreshFailure.current = { error: err, @@ -964,7 +997,9 @@ export function createAutoUpdateService({ updater.on("update-cancelled", onUpdateCancelled); updater.on("error", onError); - async function runUpdateCheck(args: { allowReady?: boolean } = {}): Promise { + async function runUpdateCheck( + args: { allowReady?: boolean; metadataOnlyWhenReady?: boolean } = {}, + ): Promise { if (checkPromise) { await checkPromise; return; @@ -984,6 +1019,8 @@ export function createAutoUpdateService({ preservedDownloadRetry = reusableDownloadedVersion ? { version: reusableDownloadedVersion, releaseNotesUrl: snapshot.releaseNotesUrl } : null; + readyMetadataRefreshInProgress = args.metadataOnlyWhenReady === true + && snapshot.status === "ready"; checkPromise = updater.checkForUpdates() .then(async (result) => { const updateInfo = isUpdateCheckResultLike(result) ? result.updateInfo : undefined; @@ -1024,6 +1061,15 @@ export function createAutoUpdateService({ } }) .catch((error) => { + // Same reasoning as onError: a metadata-only refresh must leave the + // staged update exactly as it found it, however it fails. + if (readyMetadataRefreshInProgress) { + logger.warn("autoUpdate.metadata_refresh_failed", { + message: formatErrorMessage(error), + readyVersion: snapshot.version, + }); + return; + } if (readyRefreshInProgress) { readyRefreshFailure.current = { error, @@ -1046,6 +1092,7 @@ export function createAutoUpdateService({ .finally(() => { checkPromise = null; preservedDownloadRetry = null; + readyMetadataRefreshInProgress = false; }); await checkPromise; } @@ -1062,7 +1109,8 @@ export function createAutoUpdateService({ * refreshes the newest known version without disturbing the staged download. */ function checkForUpdates(options: { userInitiated?: boolean } = {}): void { - void runUpdateCheck({ allowReady: options.userInitiated === true }); + const userInitiated = options.userInitiated === true; + void runUpdateCheck({ allowReady: userInitiated, metadataOnlyWhenReady: userInitiated }); } async function refreshReadyUpdateBeforeInstall(): Promise {