Fix rejected auto-updater promise handling

electron/updater-coordinator.mjs

@@ -0,0 +1,81 @@
+export function createUpdaterCoordinator(updater, setState) {
+  let checkOperation = null;
+  let downloadOperation = null;
+
+  function handleRejectedOperation(manual, error) {
+    if (!manual) {
+      setState({ status: "idle" });
+      return;
+    }
+    setState({ status: "error", message: String(error?.message ?? error) });
+  }
+
+  function checkOwnsState() {
+    return !downloadOperation && !checkOperation?.supersededByDownload;
+  }
+
+  updater.on("checking-for-update", () => {
+    if (checkOwnsState()) setState({ status: "checking" });
+  });
+  updater.on("update-available", (info) => {
+    if (checkOwnsState()) {
+      setState({ status: "available", version: info?.version, message: undefined });
+    }
+  });
+  updater.on("update-not-available", () => {
+    if (checkOwnsState()) setState({ status: "idle" });
+  });
+  updater.on("download-progress", (progress) =>
+    setState({ status: "downloading", percent: Math.round(progress?.percent ?? 0) }),
+  );
+  updater.on("update-downloaded", (info) =>
+    setState({ status: "downloaded", version: info?.version }),
+  );
+
+  function check(manual = false) {
+    if (checkOperation) {
+      // A manual caller upgrades the shared operation; a timer never downgrades it.
+      if (manual) checkOperation.manual = true;
+      return checkOperation.promise;
+    }
+
+    const operation = { manual, supersededByDownload: Boolean(downloadOperation), promise: null };
+    checkOperation = operation;
+    try {
+      operation.promise = Promise.resolve(updater.checkForUpdates())
+        .catch((error) => {
+          if (!operation.supersededByDownload) handleRejectedOperation(operation.manual, error);
+        })
+        .finally(() => {
+          if (checkOperation === operation) checkOperation = null;
+        });
+    } catch (error) {
+      if (!operation.supersededByDownload) handleRejectedOperation(operation.manual, error);
+      checkOperation = null;
+      operation.promise = Promise.resolve();
+    }
+    return operation.promise;
+  }
+
+  function download() {
+    if (checkOperation) checkOperation.supersededByDownload = true;
+    if (downloadOperation) return downloadOperation.promise;
+
+    const operation = { promise: null };
+    downloadOperation = operation;
+    try {
+      operation.promise = Promise.resolve(updater.downloadUpdate())
+        .catch((error) => handleRejectedOperation(true, error))
+        .finally(() => {
+          if (downloadOperation === operation) downloadOperation = null;
+        });
+    } catch (error) {
+      handleRejectedOperation(true, error);
+      downloadOperation = null;
+      operation.promise = Promise.resolve();
+    }
+    return operation.promise;
+  }
+
+  return { check, download };
+}

electron/updater-coordinator.node-test.mjs

@@ -0,0 +1,272 @@
+import assert from "node:assert/strict";
+import { EventEmitter } from "node:events";
+import test from "node:test";
+
+import { createUpdaterCoordinator } from "./updater-coordinator.mjs";
+
+function deferred() {
+  let resolve;
+  let reject;
+  const promise = new Promise((res, rej) => {
+    resolve = res;
+    reject = rej;
+  });
+  return { promise, resolve, reject };
+}
+
+function harness() {
+  const updater = new EventEmitter();
+  // electron-updater has its own error listener; model that without routing it.
+  updater.on("error", () => {});
+  let state = { status: "idle" };
+  const states = [];
+  const coordinator = createUpdaterCoordinator(updater, (patch) => {
+    state = { ...state, ...patch };
+    states.push({ ...state });
+  });
+  return { updater, coordinator, states, getState: () => state };
+}
+
+function errorStates(states) {
+  return states.filter((entry) => entry.status === "error");
+}
+
+test("automatic check rejection is handled and returns to idle", async () => {
+  const { updater, coordinator, getState } = harness();
+  updater.checkForUpdates = () => Promise.reject(new Error("offline"));
+
+  await assert.doesNotReject(coordinator.check());
+
+  assert.equal(getState().status, "idle");
+});
+
+test("manual check rejection is handled as a user-visible error", async () => {
+  const { updater, coordinator, getState } = harness();
+  updater.checkForUpdates = () => Promise.reject(new Error("feed failed"));
+
+  await assert.doesNotReject(coordinator.check(true));
+
+  assert.deepEqual(getState(), { status: "error", message: "feed failed" });
+});
+
+test("download rejection is handled as a user-visible error", async () => {
+  const { updater, coordinator, getState } = harness();
+  updater.downloadUpdate = () => Promise.reject(new Error("download failed"));
+
+  await assert.doesNotReject(coordinator.download());
+
+  assert.deepEqual(getState(), { status: "error", message: "download failed" });
+});
+
+test("synchronous check and download throws are handled", async () => {
+  const { updater, coordinator, getState } = harness();
+  updater.checkForUpdates = () => {
+    throw new Error("check threw");
+  };
+
+  await assert.doesNotReject(coordinator.check(true));
+  assert.deepEqual(getState(), { status: "error", message: "check threw" });
+
+  updater.downloadUpdate = () => {
+    throw new Error("download threw");
+  };
+
+  await assert.doesNotReject(coordinator.download());
+  assert.deepEqual(getState(), { status: "error", message: "download threw" });
+});
+
+test("a concurrent background check cannot downgrade a manual check", async () => {
+  const { updater, coordinator, getState } = harness();
+  const pending = deferred();
+  let calls = 0;
+  updater.checkForUpdates = () => {
+    calls += 1;
+    return pending.promise;
+  };
+
+  const manual = coordinator.check(true);
+  const background = coordinator.check();
+  assert.strictEqual(background, manual);
+  assert.equal(calls, 1);
+
+  pending.reject(new Error("manual failure"));
+  await manual;
+
+  assert.deepEqual(getState(), { status: "error", message: "manual failure" });
+});
+
+test("a manual request during a background check preserves user-visible errors", async () => {
+  const { updater, coordinator, getState } = harness();
+  const pending = deferred();
+  let calls = 0;
+  updater.checkForUpdates = () => {
+    calls += 1;
+    return pending.promise;
+  };
+
+  const background = coordinator.check();
+  const manual = coordinator.check(true);
+  assert.strictEqual(manual, background);
+  assert.equal(calls, 1);
+
+  pending.reject(new Error("background request failed"));
+  await background;
+
+  assert.deepEqual(getState(), { status: "error", message: "background request failed" });
+});
+
+test("an active download state survives a later background check failure", async () => {
+  const { updater, coordinator, getState } = harness();
+  const downloadPending = deferred();
+  const checkPending = deferred();
+  updater.downloadUpdate = () => {
+    updater.emit("download-progress", { percent: 42 });
+    return downloadPending.promise;
+  };
+  updater.checkForUpdates = () => {
+    updater.emit("checking-for-update");
+    updater.emit("update-available", { version: "2.1.0" });
+    updater.emit("update-not-available");
+    return checkPending.promise;
+  };
+
+  const download = coordinator.download();
+  assert.deepEqual(getState(), { status: "downloading", percent: 42 });
+
+  const background = coordinator.check();
+  checkPending.reject(new Error("background check failed"));
+  await background;
+  assert.deepEqual(getState(), { status: "downloading", percent: 42 });
+
+  downloadPending.resolve();
+  await download;
+});
+
+test("a download error remains authoritative after a later background failure", async () => {
+  const { updater, coordinator, getState } = harness();
+  const downloadPending = deferred();
+  const checkPending = deferred();
+  updater.downloadUpdate = () => {
+    updater.emit("download-progress", { percent: 75 });
+    return downloadPending.promise;
+  };
+  updater.checkForUpdates = () => checkPending.promise;
+
+  const download = coordinator.download();
+  const background = coordinator.check();
+
+  const downloadError = new Error("download failed first");
+  downloadPending.reject(downloadError);
+  await download;
+  assert.deepEqual(getState(), {
+    status: "error",
+    percent: 75,
+    message: "download failed first",
+  });
+
+  updater.emit("checking-for-update");
+  updater.emit("update-available", { version: "2.1.0" });
+  updater.emit("update-not-available");
+  checkPending.reject(new Error("background check failed later"));
+  await background;
+
+  assert.deepEqual(getState(), {
+    status: "error",
+    percent: 75,
+    message: "download failed first",
+  });
+});
+
+test("a background failure stays silent before a later download failure", async () => {
+  const { updater, coordinator, getState, states } = harness();
+  const downloadPending = deferred();
+  const checkPending = deferred();
+  updater.checkForUpdates = () => checkPending.promise;
+  updater.downloadUpdate = () => {
+    updater.emit("download-progress", { percent: 18 });
+    return downloadPending.promise;
+  };
+
+  const background = coordinator.check();
+  const download = coordinator.download();
+
+  updater.emit("checking-for-update");
+  updater.emit("update-available", { version: "2.1.0" });
+  updater.emit("update-not-available");
+  checkPending.reject(new Error("background check failed first"));
+  await background;
+  assert.deepEqual(getState(), { status: "downloading", percent: 18 });
+  assert.equal(errorStates(states).length, 0);
+
+  const downloadError = new Error("download failed later");
+  downloadPending.reject(downloadError);
+  await download;
+
+  assert.deepEqual(getState(), {
+    status: "error",
+    percent: 18,
+    message: "download failed later",
+  });
+});
+
+test("available, not-available, progress, and downloaded events preserve success behavior", async () => {
+  const available = harness();
+  available.updater.checkForUpdates = () => {
+    available.updater.emit("checking-for-update");
+    queueMicrotask(() => available.updater.emit("update-available", { version: "2.0.0" }));
+    return Promise.resolve({ isUpdateAvailable: true });
+  };
+  await available.coordinator.check(true);
+  assert.equal(available.getState().status, "available");
+  assert.equal(available.getState().version, "2.0.0");
+
+  available.updater.downloadUpdate = () => {
+    available.updater.emit("download-progress", { percent: 42.4 });
+    return Promise.resolve().then(() => {
+      available.updater.emit("update-downloaded", { version: "2.0.0" });
+      return ["update.zip"];
+    });
+  };
+  await available.coordinator.download();
+  assert.deepEqual(available.getState(), {
+    status: "downloaded",
+    version: "2.0.0",
+    message: undefined,
+    percent: 42,
+  });
+
+  const notAvailable = harness();
+  notAvailable.updater.checkForUpdates = () => {
+    notAvailable.updater.emit("checking-for-update");
+    notAvailable.updater.emit("update-not-available");
+    return Promise.resolve({ isUpdateAvailable: false });
+  };
+  await notAvailable.coordinator.check();
+  assert.equal(notAvailable.getState().status, "idle");
+});
+
+test("an updater error event and rejected promise produce one deterministic state", async () => {
+  const check = harness();
+  const checkError = new Error("check failed once");
+  check.updater.checkForUpdates = () =>
+    Promise.reject(checkError).catch((error) => {
+      check.updater.emit("error", error);
+      throw error;
+    });
+
+  await check.coordinator.check(true);
+  assert.equal(errorStates(check.states).length, 1);
+  assert.deepEqual(check.getState(), { status: "error", message: "check failed once" });
+
+  const download = harness();
+  const downloadError = new Error("download failed once");
+  download.updater.downloadUpdate = () =>
+    Promise.reject(downloadError).catch((error) => {
+      download.updater.emit("error", error);
+      throw error;
+    });
+
+  await download.coordinator.download();
+  assert.equal(errorStates(download.states).length, 1);
+  assert.deepEqual(download.getState(), { status: "error", message: "download failed once" });
+});