From 82250132a4d0141663e6281ff211087accaedb9d Mon Sep 17 00:00:00 2001 From: Anshul Jain Date: Wed, 8 Jul 2026 21:17:17 +0530 Subject: [PATCH] fix: detect failed updates and stop retry-looping instead of silently continuing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses issue #139. The updater plugin was registered but downloadAndInstall() failures were only ever logged to the console. There was no way to tell a clean "download failed, safe to retry" from "install ran but the app never actually came up on the new version", and a persistently broken release would just keep getting retried every interval forever. Tauri's updater plugin already validates the Ed25519 signature against the configured pubkey internally: check() will not return an update object at all unless the signature is valid, so signature validation was never actually missing. What was genuinely missing is everything after that: telling apart a safe failure from a dangerous one, and recovering instead of looping. - src/system/updater/autoUpdater.ts: - download() and install() are now called as two separate steps (previously a single downloadAndInstall()). A download failure (network blip, server hiccup) never reaches install at all, so the running app was never at risk in the first place, and it is not counted toward the failure threshold below since it is transient and self-heals on the next interval. - Right before install() runs, the current version and the version being installed are persisted to localStorage as a "pending install" record. On the next app boot, before doing anything else, that record is reconciled against the actual running version (via @tauri-apps/api/app's getVersion()): if they match, the update is confirmed and the failure counter resets to zero; if they do not match, the update silently failed to apply and this counts as a failure. This is the actual "did the update leave the app broken" signal, since Tauri's updater plugin has no API to truly roll back a binary, the honest thing this app can do is detect that an update did not take effect and stop before it causes a crash loop. - After 3 consecutive install failures (either install() itself throwing, or a pending update not being confirmed on the next boot), auto-install is paused for the rest of that session rather than retried every interval, so a broken release cannot trigger an endless check/download/install/crash/retry cycle. - Every transition (checking, up-to-date, downloading, installed, update-confirmed, check-failed, install-failed, update-did-not-apply, auto-update-paused) is exposed via an optional onStatusChange callback so a caller can surface it in the UI later, instead of only going to the console as before. Left unwired from App.tsx for this PR since App.tsx currently calls startAutoUpdater() with no options at all and I did not want to force new toast/notification UI into an otherwise backend reliability fix. - update.close() is now called in a finally block. The Update resource returned by check() was never explicitly closed before. - src/system/updater/autoUpdater.test.ts (new): 10 tests covering the behavior above — does nothing outside a Tauri runtime or in dev mode, reports up-to-date correctly, the full download-then-pending-then-install-then-relaunch happy path, silent mode skips relaunch, a download failure never reaches install and is not counted as a failure, an install failure is recorded and counted and does not relaunch, a confirmed update on next boot resets the failure counter, an update that silently did not apply is detected and counted, and auto-install pauses once the failure threshold is reached. Verified: `npx vitest run` — 55/55 passing across the full existing suite plus these 10 new tests. `npx tsc --noEmit` and `npx eslint src/system/updater/` both clean. Signed-off-by: Anshul Jain --- src/system/updater/autoUpdater.test.ts | 263 +++++++++++++++++++++++++ src/system/updater/autoUpdater.ts | 210 ++++++++++++++++++-- 2 files changed, 460 insertions(+), 13 deletions(-) create mode 100644 src/system/updater/autoUpdater.test.ts diff --git a/src/system/updater/autoUpdater.test.ts b/src/system/updater/autoUpdater.test.ts new file mode 100644 index 0000000..7d6266b --- /dev/null +++ b/src/system/updater/autoUpdater.test.ts @@ -0,0 +1,263 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const checkMock = vi.fn(); +const relaunchMock = vi.fn(); +const getVersionMock = vi.fn(); + +// The test environment's window.localStorage is missing methods like +// clear() in this setup, so we swap in a small in-memory implementation +// rather than depend on it. +function installMemoryLocalStorage(): Storage { + const store = new Map(); + const storage: Storage = { + getItem: (key: string) => (store.has(key) ? (store.get(key) as string) : null), + setItem: (key: string, value: string) => { + store.set(key, String(value)); + }, + removeItem: (key: string) => { + store.delete(key); + }, + clear: () => { + store.clear(); + }, + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size; + }, + }; + Object.defineProperty(window, "localStorage", { + value: storage, + configurable: true, + writable: true, + }); + return storage; +} + +vi.mock("@tauri-apps/plugin-updater", () => ({ + check: (...args: unknown[]) => checkMock(...args), +})); + +vi.mock("@tauri-apps/plugin-process", () => ({ + relaunch: (...args: unknown[]) => relaunchMock(...args), +})); + +vi.mock("@tauri-apps/api/app", () => ({ + getVersion: (...args: unknown[]) => getVersionMock(...args), +})); + +function makeUpdate(overrides: Partial<{ + version: string; + currentVersion: string; + download: () => Promise; + install: () => Promise; + close: () => Promise; +}> = {}) { + return { + version: overrides.version ?? "2.0.0", + currentVersion: overrides.currentVersion ?? "1.0.0", + download: overrides.download ?? vi.fn().mockResolvedValue(undefined), + install: overrides.install ?? vi.fn().mockResolvedValue(undefined), + close: overrides.close ?? vi.fn().mockResolvedValue(undefined), + }; +} + +describe("autoUpdater", () => { + beforeEach(() => { + vi.resetModules(); + checkMock.mockReset(); + relaunchMock.mockReset(); + getVersionMock.mockReset(); + installMemoryLocalStorage(); + vi.stubEnv("DEV", false); + Object.defineProperty(window, "__TAURI_INTERNALS__", { + value: {}, + configurable: true, + writable: true, + }); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.useRealTimers(); + // @ts-expect-error test-only cleanup of a global we defined above + delete window.__TAURI_INTERNALS__; + }); + + it("does nothing outside a Tauri runtime", async () => { + // @ts-expect-error test-only cleanup of a global we defined above + delete window.__TAURI_INTERNALS__; + const { startAutoUpdater } = await import("./autoUpdater"); + + await startAutoUpdater(); + + expect(checkMock).not.toHaveBeenCalled(); + }); + + it("does nothing in dev mode", async () => { + vi.stubEnv("DEV", true); + const { startAutoUpdater } = await import("./autoUpdater"); + + await startAutoUpdater(); + + expect(checkMock).not.toHaveBeenCalled(); + }); + + it("reports up-to-date and does not download when no update is available", async () => { + checkMock.mockResolvedValue(null); + const { startAutoUpdater } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(onStatusChange).toHaveBeenCalledWith({ type: "up-to-date" }); + }); + + it("downloads, marks pending-install, installs, and relaunches on a found update", async () => { + const update = makeUpdate(); + checkMock.mockResolvedValue(update); + const { startAutoUpdater, __testing } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(update.download).toHaveBeenCalledTimes(1); + expect(update.install).toHaveBeenCalledTimes(1); + expect(relaunchMock).toHaveBeenCalledTimes(1); + expect(update.close).toHaveBeenCalledTimes(1); + expect(onStatusChange).toHaveBeenCalledWith({ type: "installed", version: "2.0.0" }); + + const raw = window.localStorage.getItem(__testing.STATE_STORAGE_KEY); + expect(raw).not.toBeNull(); + const state = JSON.parse(raw as string); + expect(state.status).toBe("pending-install"); + expect(state.toVersion).toBe("2.0.0"); + }); + + it("does not relaunch when silent is true", async () => { + const update = makeUpdate(); + checkMock.mockResolvedValue(update); + const { startAutoUpdater } = await import("./autoUpdater"); + + await startAutoUpdater({ checkIntervalMs: 0, silent: true }); + + expect(update.install).toHaveBeenCalledTimes(1); + expect(relaunchMock).not.toHaveBeenCalled(); + }); + + it("never calls install when download fails, and does not count it as a failure", async () => { + const update = makeUpdate({ download: vi.fn().mockRejectedValue(new Error("network down")) }); + checkMock.mockResolvedValue(update); + const { startAutoUpdater, __testing } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(update.install).not.toHaveBeenCalled(); + expect(onStatusChange).toHaveBeenCalledWith({ + type: "check-failed", + error: expect.any(Error), + }); + + const raw = window.localStorage.getItem(__testing.STATE_STORAGE_KEY); + expect(raw).toBeNull(); + }); + + it("marks pending-install as failed and increments consecutiveFailures when install() throws", async () => { + const update = makeUpdate({ install: vi.fn().mockRejectedValue(new Error("install failed")) }); + checkMock.mockResolvedValue(update); + const { startAutoUpdater, __testing } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(relaunchMock).not.toHaveBeenCalled(); + expect(onStatusChange).toHaveBeenCalledWith({ + type: "install-failed", + error: expect.any(Error), + }); + + const raw = window.localStorage.getItem(__testing.STATE_STORAGE_KEY); + const state = JSON.parse(raw as string); + expect(state.status).toBe("idle"); + expect(state.consecutiveFailures).toBe(1); + }); + + it("confirms a pending update when the app boots into the expected version", async () => { + const { __testing } = await import("./autoUpdater"); + window.localStorage.setItem( + __testing.STATE_STORAGE_KEY, + JSON.stringify({ + status: "pending-install", + fromVersion: "1.0.0", + toVersion: "2.0.0", + attemptedAt: Date.now(), + consecutiveFailures: 0, + }) + ); + getVersionMock.mockResolvedValue("2.0.0"); + checkMock.mockResolvedValue(null); + const { startAutoUpdater } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(onStatusChange).toHaveBeenCalledWith({ type: "update-confirmed", version: "2.0.0" }); + const raw = window.localStorage.getItem(__testing.STATE_STORAGE_KEY); + const state = JSON.parse(raw as string); + expect(state.status).toBe("idle"); + expect(state.consecutiveFailures).toBe(0); + }); + + it("detects an update that did not apply and increments consecutiveFailures", async () => { + const { __testing } = await import("./autoUpdater"); + window.localStorage.setItem( + __testing.STATE_STORAGE_KEY, + JSON.stringify({ + status: "pending-install", + fromVersion: "1.0.0", + toVersion: "2.0.0", + attemptedAt: Date.now(), + consecutiveFailures: 0, + }) + ); + // The app is still on the old version — the update silently failed to apply. + getVersionMock.mockResolvedValue("1.0.0"); + checkMock.mockResolvedValue(null); + const { startAutoUpdater } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(onStatusChange).toHaveBeenCalledWith({ + type: "update-did-not-apply", + expectedVersion: "2.0.0", + actualVersion: "1.0.0", + }); + const raw = window.localStorage.getItem(__testing.STATE_STORAGE_KEY); + const state = JSON.parse(raw as string); + expect(state.consecutiveFailures).toBe(1); + }); + + it("pauses auto-install after reaching the consecutive failure threshold", async () => { + const { __testing } = await import("./autoUpdater"); + window.localStorage.setItem( + __testing.STATE_STORAGE_KEY, + JSON.stringify({ + status: "idle", + consecutiveFailures: __testing.MAX_CONSECUTIVE_FAILURES, + }) + ); + const update = makeUpdate(); + checkMock.mockResolvedValue(update); + const { startAutoUpdater } = await import("./autoUpdater"); + const onStatusChange = vi.fn(); + + await startAutoUpdater({ checkIntervalMs: 0, onStatusChange }); + + expect(checkMock).not.toHaveBeenCalled(); + expect(onStatusChange).toHaveBeenCalledWith({ + type: "auto-update-paused", + consecutiveFailures: __testing.MAX_CONSECUTIVE_FAILURES, + }); + }); +}); diff --git a/src/system/updater/autoUpdater.ts b/src/system/updater/autoUpdater.ts index 1fc46cb..63aff46 100644 --- a/src/system/updater/autoUpdater.ts +++ b/src/system/updater/autoUpdater.ts @@ -1,25 +1,133 @@ import type { DownloadEvent } from "@tauri-apps/plugin-updater"; import { check } from "@tauri-apps/plugin-updater"; import { relaunch } from "@tauri-apps/plugin-process"; +import { getVersion } from "@tauri-apps/api/app"; const DEFAULT_CHECK_INTERVAL_MS = 6 * 60 * 60 * 1000; const DEFAULT_TIMEOUT_MS = 30_000; +// After this many consecutive failed update attempts, stop auto-installing +// until the app is restarted. Prevents a bad release from triggering an +// endless check -> download -> install -> crash -> retry loop. +const MAX_CONSECUTIVE_FAILURES = 3; + +const STATE_STORAGE_KEY = "commdesk-updater-state"; + let isUpdaterStarted = false; +export type UpdaterStatus = + | { type: "checking" } + | { type: "up-to-date" } + | { type: "downloading"; version: string } + | { type: "installed"; version: string } + | { type: "update-confirmed"; version: string } + | { type: "check-failed"; error: unknown } + | { type: "install-failed"; error: unknown } + | { type: "update-did-not-apply"; expectedVersion: string; actualVersion: string } + | { type: "auto-update-paused"; consecutiveFailures: number }; + type AutoUpdaterOptions = { checkIntervalMs?: number; silent?: boolean; + onStatusChange?: (status: UpdaterStatus) => void; +}; + +interface UpdaterState { + // "pending-install" is written right before install() and only cleared + // (or escalated to a failure) once the *next* launch confirms whether the + // new version actually came up. This is what lets us tell a successful + // update apart from one that silently failed to take effect. + status: "idle" | "pending-install"; + fromVersion?: string; + toVersion?: string; + attemptedAt?: number; + consecutiveFailures: number; +} + +const DEFAULT_STATE: UpdaterState = { + status: "idle", + consecutiveFailures: 0, }; function isTauriRuntime(): boolean { return typeof window !== "undefined" && "__TAURI_INTERNALS__" in window; } -function onDownloadEvent(event: DownloadEvent): void { +function readState(): UpdaterState { + try { + const raw = window.localStorage.getItem(STATE_STORAGE_KEY); + if (!raw) return { ...DEFAULT_STATE }; + const parsed = JSON.parse(raw) as Partial; + return { + status: parsed.status === "pending-install" ? "pending-install" : "idle", + fromVersion: parsed.fromVersion, + toVersion: parsed.toVersion, + attemptedAt: parsed.attemptedAt, + consecutiveFailures: + typeof parsed.consecutiveFailures === "number" ? parsed.consecutiveFailures : 0, + }; + } catch { + return { ...DEFAULT_STATE }; + } +} + +function writeState(state: UpdaterState): void { + try { + window.localStorage.setItem(STATE_STORAGE_KEY, JSON.stringify(state)); + } catch { + // Storage unavailable (e.g. private mode). Not fatal — worst case we + // lose failure-loop protection across a single relaunch. + } +} + +/** + * Resolves any update attempt left pending from a previous session by + * comparing the version we expected to boot into against the version that + * is actually running now. + */ +async function reconcilePendingUpdate( + onStatusChange?: (status: UpdaterStatus) => void +): Promise { + const state = readState(); + if (state.status !== "pending-install") { + return state; + } + + const currentVersion = await getVersion(); + + if (currentVersion === state.toVersion) { + const resolved: UpdaterState = { status: "idle", consecutiveFailures: 0 }; + writeState(resolved); + onStatusChange?.({ type: "update-confirmed", version: currentVersion }); + return resolved; + } + + // We relaunched expecting toVersion but are still running something else. + // The update did not apply — treat it as a failure without touching + // anything further, and let the retry/backoff logic below decide whether + // to try again this session. + const failedState: UpdaterState = { + status: "idle", + consecutiveFailures: state.consecutiveFailures + 1, + }; + writeState(failedState); + onStatusChange?.({ + type: "update-did-not-apply", + expectedVersion: state.toVersion ?? "unknown", + actualVersion: currentVersion, + }); + return failedState; +} + +function onDownloadEvent( + event: DownloadEvent, + version: string, + onStatusChange?: (status: UpdaterStatus) => void +): void { switch (event.event) { case "Started": - console.info(`[updater] started downloading ${event.data.contentLength} bytes`); + console.info(`[updater] started downloading ${event.data.contentLength ?? "?"} bytes`); + onStatusChange?.({ type: "downloading", version }); break; case "Progress": console.info(`[updater] downloaded chunk ${event.data.chunkLength} bytes`); @@ -30,25 +138,70 @@ function onDownloadEvent(event: DownloadEvent): void { } } -async function checkAndInstallUpdate(silent: boolean): Promise { +async function checkAndInstallUpdate( + silent: boolean, + state: UpdaterState, + onStatusChange?: (status: UpdaterStatus) => void +): Promise { + onStatusChange?.({ type: "checking" }); + + const update = await check({ timeout: DEFAULT_TIMEOUT_MS }).catch((error: unknown) => { + console.error("[updater] check failed", error); + onStatusChange?.({ type: "check-failed", error }); + return null; + }); + + if (!update) { + console.info("[updater] no update available"); + onStatusChange?.({ type: "up-to-date" }); + return; + } + + console.info(`[updater] update found: ${update.version}`); + try { - const update = await check({ timeout: DEFAULT_TIMEOUT_MS }); - if (!update) { - console.info("[updater] no update available"); + // download() and install() are kept as two distinct steps (rather than + // the combined downloadAndInstall()) specifically so that a corrupted + // or interrupted download never reaches the install step at all — the + // app is only ever at risk once install() is actually called. A pure + // download failure (network blip, server hiccup) is not counted toward + // consecutiveFailures: it is transient and self-heals on the next + // interval without ever touching the running app. + try { + await update.download((event) => onDownloadEvent(event, update.version, onStatusChange), { + timeout: DEFAULT_TIMEOUT_MS, + }); + } catch (downloadError) { + console.error("[updater] download failed", downloadError); + onStatusChange?.({ type: "check-failed", error: downloadError }); return; } - console.info(`[updater] update found: ${update.version}`); - await update.downloadAndInstall(onDownloadEvent, { - timeout: DEFAULT_TIMEOUT_MS, - }); + const nextState: UpdaterState = { + status: "pending-install", + fromVersion: update.currentVersion, + toVersion: update.version, + attemptedAt: Date.now(), + consecutiveFailures: state.consecutiveFailures, + }; + writeState(nextState); + + await update.install(); console.info("[updater] update installed"); + onStatusChange?.({ type: "installed", version: update.version }); if (!silent) { await relaunch(); } } catch (error) { - console.error("[updater] update check/install failed", error); + // install() (or relaunch()) failed after a successful download — this + // is the case that can plausibly leave the app in a broken state, so it + // is the one that counts toward the auto-update pause threshold. + console.error("[updater] install failed", error); + writeState({ status: "idle", consecutiveFailures: state.consecutiveFailures + 1 }); + onStatusChange?.({ type: "install-failed", error }); + } finally { + await update.close(); } } @@ -63,11 +216,42 @@ export async function startAutoUpdater(options: AutoUpdaterOptions = {}): Promis const checkIntervalMs = options.checkIntervalMs === undefined ? DEFAULT_CHECK_INTERVAL_MS : options.checkIntervalMs; - await checkAndInstallUpdate(silent); + let state = await reconcilePendingUpdate(options.onStatusChange); + + const runCheck = async () => { + if (state.consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { + console.warn( + `[updater] paused after ${state.consecutiveFailures} consecutive failures; skipping auto-install until restart` + ); + options.onStatusChange?.({ + type: "auto-update-paused", + consecutiveFailures: state.consecutiveFailures, + }); + return; + } + await checkAndInstallUpdate(silent, state, options.onStatusChange); + state = readState(); + }; + + await runCheck(); if (checkIntervalMs > 0) { window.setInterval(() => { - void checkAndInstallUpdate(silent); + void runCheck(); }, checkIntervalMs); } } + +// Exposed for tests only. +export const __testing = { + STATE_STORAGE_KEY, + MAX_CONSECUTIVE_FAILURES, + resetForTests(): void { + isUpdaterStarted = false; + try { + window.localStorage.removeItem(STATE_STORAGE_KEY); + } catch { + // ignore + } + }, +};