Always install the newest eligible desktop update (#2149)
* Reapply "Always revalidate desktop updates before install"
This reverts commit 623c05aa4d.
* fix(desktop): make update revalidation safe on quit
Updater-triggered quits now bypass normal quit handling, and manifest revalidation gives up after five seconds without allowing a late install.
* fix(desktop): preserve fail-closed update installs
Keep cached updates deferred when quit-time validation is offline, and report validation timeouts separately from superseded releases.
* fix(desktop): preserve updater quit behavior
Automatic installs remain silent without relaunching, updater handoff is bounded, and background preparation failures remain visible without pinning manifest checks.
* fix(desktop): keep AppImage updates manual
Preserve the AppImage safety exemption by skipping ordinary quit-time installation while retaining the explicit Update now path.
* fix(desktop): serialize update preparation
* fix(desktop): recognize macOS updater quit handoff
This commit is contained in:
Mohamed Boudra
@@ -1,8 +1,8 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { DEFAULT_DESKTOP_SETTINGS } from "../settings/desktop-settings";
|
||||
import {
|
||||
createBeforeQuitHandler,
|
||||
createQuitLifecycle,
|
||||
shouldStopDesktopManagedDaemonOnQuit,
|
||||
stopDesktopManagedDaemonOnQuitIfNeeded,
|
||||
} from "./quit-lifecycle";
|
||||
@@ -16,6 +16,18 @@ const SETTINGS_STOP_ON_QUIT = {
|
||||
},
|
||||
};
|
||||
|
||||
function deferred<T>(): { promise: Promise<T>; resolve(value: T): void } {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((resolvePromise) => {
|
||||
resolve = resolvePromise;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
function waitForQuitLifecycle(): Promise<void> {
|
||||
return new Promise((resolve) => setImmediate(resolve));
|
||||
}
|
||||
|
||||
describe("quit-lifecycle", () => {
|
||||
it("only stops when keepRunningAfterQuit is explicitly disabled", () => {
|
||||
expect(shouldStopDesktopManagedDaemonOnQuit(SETTINGS_STOP_ON_QUIT)).toBe(true);
|
||||
@@ -23,96 +35,238 @@ describe("quit-lifecycle", () => {
|
||||
});
|
||||
|
||||
it("short-circuits without inspecting the daemon when keep-running is on", async () => {
|
||||
const isDesktopManagedDaemonRunning = vi.fn(() => true);
|
||||
const stopDaemon = vi.fn(async () => undefined);
|
||||
const showShutdownFeedback = vi.fn();
|
||||
const events: string[] = [];
|
||||
|
||||
const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({
|
||||
settingsStore: { get: async () => SETTINGS_KEEP_RUNNING },
|
||||
isDesktopManagedDaemonRunning,
|
||||
stopDaemon,
|
||||
showShutdownFeedback,
|
||||
isDesktopManagedDaemonRunning: () => {
|
||||
events.push("inspect");
|
||||
return true;
|
||||
},
|
||||
stopDaemon: async () => {
|
||||
events.push("stop");
|
||||
},
|
||||
showShutdownFeedback: () => {
|
||||
events.push("feedback");
|
||||
},
|
||||
});
|
||||
|
||||
expect(stopped).toBe(false);
|
||||
expect(isDesktopManagedDaemonRunning).not.toHaveBeenCalled();
|
||||
expect(stopDaemon).not.toHaveBeenCalled();
|
||||
expect(showShutdownFeedback).not.toHaveBeenCalled();
|
||||
expect(events).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not stop a manually started daemon on quit", async () => {
|
||||
const stopDaemon = vi.fn(async () => undefined);
|
||||
const showShutdownFeedback = vi.fn();
|
||||
const events: string[] = [];
|
||||
|
||||
const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({
|
||||
settingsStore: { get: async () => SETTINGS_STOP_ON_QUIT },
|
||||
isDesktopManagedDaemonRunning: () => false,
|
||||
stopDaemon,
|
||||
showShutdownFeedback,
|
||||
stopDaemon: async () => {
|
||||
events.push("stop");
|
||||
},
|
||||
showShutdownFeedback: () => {
|
||||
events.push("feedback");
|
||||
},
|
||||
});
|
||||
|
||||
expect(stopped).toBe(false);
|
||||
expect(stopDaemon).not.toHaveBeenCalled();
|
||||
expect(showShutdownFeedback).not.toHaveBeenCalled();
|
||||
expect(events).toEqual([]);
|
||||
});
|
||||
|
||||
it("shows feedback then stops a desktop-managed daemon", async () => {
|
||||
const stopDaemon = vi.fn(async () => undefined);
|
||||
const showShutdownFeedback = vi.fn();
|
||||
const events: string[] = [];
|
||||
|
||||
const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({
|
||||
settingsStore: { get: async () => SETTINGS_STOP_ON_QUIT },
|
||||
isDesktopManagedDaemonRunning: () => true,
|
||||
stopDaemon,
|
||||
showShutdownFeedback,
|
||||
stopDaemon: async () => {
|
||||
events.push("stop");
|
||||
},
|
||||
showShutdownFeedback: () => {
|
||||
events.push("feedback");
|
||||
},
|
||||
});
|
||||
|
||||
expect(stopped).toBe(true);
|
||||
expect(showShutdownFeedback).toHaveBeenCalledTimes(1);
|
||||
expect(stopDaemon).toHaveBeenCalledTimes(1);
|
||||
expect(showShutdownFeedback.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
stopDaemon.mock.invocationCallOrder[0],
|
||||
);
|
||||
expect(events).toEqual(["feedback", "stop"]);
|
||||
});
|
||||
|
||||
it("preventDefaults the first quit, runs the async stop decision, then exits hard", async () => {
|
||||
let resolveStopDecision: (() => void) | null = null;
|
||||
const app = { exit: vi.fn() };
|
||||
const closeTransportSessions = vi.fn();
|
||||
const onStopError = vi.fn();
|
||||
const preventDefault = vi.fn();
|
||||
const secondPreventDefault = vi.fn();
|
||||
it("revalidates updates after daemon shutdown before exiting", async () => {
|
||||
const stopDecision = deferred<boolean>();
|
||||
const updateDecision = deferred<boolean>();
|
||||
const events: string[] = [];
|
||||
|
||||
const handleBeforeQuit = createBeforeQuitHandler({
|
||||
app,
|
||||
closeTransportSessions,
|
||||
stopDesktopManagedDaemonIfNeeded: vi.fn(
|
||||
() =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
resolveStopDecision = () => resolve(false);
|
||||
}),
|
||||
),
|
||||
onStopError,
|
||||
const quitLifecycle = createQuitLifecycle({
|
||||
app: {
|
||||
exit: (code) => {
|
||||
events.push(`exit:${code}`);
|
||||
},
|
||||
},
|
||||
closeTransportSessions: () => {
|
||||
events.push("close-transports");
|
||||
},
|
||||
stopDesktopManagedDaemonIfNeeded: () => stopDecision.promise,
|
||||
installAppUpdateOnQuit: () => updateDecision.promise,
|
||||
createUpdateDeadlineSignal: () => new AbortController().signal,
|
||||
onStopError: () => {
|
||||
events.push("stop-error");
|
||||
},
|
||||
onUpdateError: () => {
|
||||
events.push("update-error");
|
||||
},
|
||||
});
|
||||
|
||||
handleBeforeQuit({ preventDefault });
|
||||
quitLifecycle.handleBeforeQuit({
|
||||
preventDefault: () => {
|
||||
events.push("prevent-default");
|
||||
},
|
||||
});
|
||||
|
||||
expect(preventDefault).toHaveBeenCalledTimes(1);
|
||||
expect(closeTransportSessions).toHaveBeenCalledTimes(1);
|
||||
expect(app.exit).not.toHaveBeenCalled();
|
||||
expect(resolveStopDecision).not.toBeNull();
|
||||
expect(events).toEqual(["close-transports", "prevent-default"]);
|
||||
|
||||
resolveStopDecision?.();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
events.push("daemon-stopped");
|
||||
stopDecision.resolve(false);
|
||||
await waitForQuitLifecycle();
|
||||
|
||||
expect(app.exit).toHaveBeenCalledWith(0);
|
||||
expect(onStopError).not.toHaveBeenCalled();
|
||||
expect(events).toEqual(["close-transports", "prevent-default", "daemon-stopped"]);
|
||||
|
||||
handleBeforeQuit({ preventDefault: secondPreventDefault });
|
||||
events.push("update-checked");
|
||||
updateDecision.resolve(false);
|
||||
await waitForQuitLifecycle();
|
||||
|
||||
expect(secondPreventDefault).not.toHaveBeenCalled();
|
||||
expect(closeTransportSessions).toHaveBeenCalledTimes(2);
|
||||
expect(app.exit).toHaveBeenCalledTimes(1);
|
||||
expect(events).toEqual([
|
||||
"close-transports",
|
||||
"prevent-default",
|
||||
"daemon-stopped",
|
||||
"update-checked",
|
||||
"exit:0",
|
||||
]);
|
||||
|
||||
quitLifecycle.handleBeforeQuit({
|
||||
preventDefault: () => {
|
||||
events.push("second-prevent-default");
|
||||
},
|
||||
});
|
||||
|
||||
expect(events.at(-1)).toBe("close-transports");
|
||||
expect(events).not.toContain("second-prevent-default");
|
||||
});
|
||||
|
||||
it("lets the updater own process exit when a validated update is installing", async () => {
|
||||
const exits: number[] = [];
|
||||
const quitLifecycle = createQuitLifecycle({
|
||||
app: { exit: (code) => exits.push(code) },
|
||||
closeTransportSessions: () => {},
|
||||
stopDesktopManagedDaemonIfNeeded: async () => false,
|
||||
installAppUpdateOnQuit: async () => true,
|
||||
createUpdateDeadlineSignal: () => new AbortController().signal,
|
||||
onStopError: () => {},
|
||||
onUpdateError: () => {},
|
||||
});
|
||||
|
||||
quitLifecycle.handleBeforeQuit({ preventDefault: () => {} });
|
||||
await waitForQuitLifecycle();
|
||||
quitLifecycle.handleBeforeQuitForUpdate();
|
||||
await waitForQuitLifecycle();
|
||||
|
||||
expect(exits).toEqual([]);
|
||||
});
|
||||
|
||||
it("recognizes a repeated quit as updater handoff", async () => {
|
||||
const exits: number[] = [];
|
||||
let preventedQuitCount = 0;
|
||||
const quitLifecycle = createQuitLifecycle({
|
||||
app: { exit: (code) => exits.push(code) },
|
||||
closeTransportSessions: () => {},
|
||||
stopDesktopManagedDaemonIfNeeded: async () => false,
|
||||
installAppUpdateOnQuit: async () => true,
|
||||
createUpdateDeadlineSignal: () => new AbortController().signal,
|
||||
onStopError: () => {},
|
||||
onUpdateError: () => {},
|
||||
});
|
||||
|
||||
quitLifecycle.handleBeforeQuit({ preventDefault: () => preventedQuitCount++ });
|
||||
await waitForQuitLifecycle();
|
||||
quitLifecycle.handleBeforeQuit({ preventDefault: () => preventedQuitCount++ });
|
||||
await waitForQuitLifecycle();
|
||||
|
||||
expect(preventedQuitCount).toBe(1);
|
||||
expect(exits).toEqual([]);
|
||||
});
|
||||
|
||||
it("exits when the updater does not take ownership before its deadline", async () => {
|
||||
const revalidationDeadline = new AbortController();
|
||||
const handoffDeadline = new AbortController();
|
||||
let deadlineCount = 0;
|
||||
const exits: number[] = [];
|
||||
const quitLifecycle = createQuitLifecycle({
|
||||
app: { exit: (code) => exits.push(code) },
|
||||
closeTransportSessions: () => {},
|
||||
stopDesktopManagedDaemonIfNeeded: async () => false,
|
||||
installAppUpdateOnQuit: async () => true,
|
||||
createUpdateDeadlineSignal: () =>
|
||||
deadlineCount++ === 0 ? revalidationDeadline.signal : handoffDeadline.signal,
|
||||
onStopError: () => {},
|
||||
onUpdateError: () => {},
|
||||
});
|
||||
|
||||
quitLifecycle.handleBeforeQuit({ preventDefault: () => {} });
|
||||
await waitForQuitLifecycle();
|
||||
handoffDeadline.abort();
|
||||
await waitForQuitLifecycle();
|
||||
|
||||
expect(exits).toEqual([0]);
|
||||
});
|
||||
|
||||
it("does not intercept a quit started by a manual update", () => {
|
||||
const events: string[] = [];
|
||||
const quitLifecycle = createQuitLifecycle({
|
||||
app: { exit: (code) => events.push(`exit:${code}`) },
|
||||
closeTransportSessions: () => events.push("close-transports"),
|
||||
stopDesktopManagedDaemonIfNeeded: async () => {
|
||||
events.push("stop-daemon");
|
||||
return false;
|
||||
},
|
||||
installAppUpdateOnQuit: async () => {
|
||||
events.push("revalidate-update");
|
||||
return false;
|
||||
},
|
||||
createUpdateDeadlineSignal: () => new AbortController().signal,
|
||||
onStopError: () => events.push("stop-error"),
|
||||
onUpdateError: () => events.push("update-error"),
|
||||
});
|
||||
|
||||
quitLifecycle.handleBeforeQuitForUpdate();
|
||||
quitLifecycle.handleBeforeQuit({
|
||||
preventDefault: () => events.push("prevent-default"),
|
||||
});
|
||||
|
||||
expect(events).toEqual(["close-transports"]);
|
||||
});
|
||||
|
||||
it("exits when update revalidation reaches its deadline", async () => {
|
||||
const deadline = new AbortController();
|
||||
const updateDecision = deferred<boolean>();
|
||||
const exits: number[] = [];
|
||||
const quitLifecycle = createQuitLifecycle({
|
||||
app: { exit: (code) => exits.push(code) },
|
||||
closeTransportSessions: () => {},
|
||||
stopDesktopManagedDaemonIfNeeded: async () => false,
|
||||
installAppUpdateOnQuit: () => updateDecision.promise,
|
||||
createUpdateDeadlineSignal: () => deadline.signal,
|
||||
onStopError: () => {},
|
||||
onUpdateError: () => {},
|
||||
});
|
||||
|
||||
quitLifecycle.handleBeforeQuit({ preventDefault: () => {} });
|
||||
await waitForQuitLifecycle();
|
||||
deadline.abort();
|
||||
await waitForQuitLifecycle();
|
||||
|
||||
expect(exits).toEqual([0]);
|
||||
|
||||
updateDecision.resolve(true);
|
||||
await waitForQuitLifecycle();
|
||||
expect(exits).toEqual([0]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,6 +14,16 @@ interface BeforeQuitApp {
|
||||
exit(code: number): void;
|
||||
}
|
||||
|
||||
interface QuitLifecycle {
|
||||
handleBeforeQuit(event: BeforeQuitEvent): void;
|
||||
handleBeforeQuitForUpdate(): void;
|
||||
}
|
||||
|
||||
interface DeferredUpdateQuit {
|
||||
promise: Promise<boolean>;
|
||||
resolve(): void;
|
||||
}
|
||||
|
||||
export interface StopOnQuitDeps {
|
||||
settingsStore: Pick<DesktopSettingsStore, "get">;
|
||||
isDesktopManagedDaemonRunning: () => boolean;
|
||||
@@ -42,36 +52,95 @@ export async function stopDesktopManagedDaemonOnQuitIfNeeded(
|
||||
return true;
|
||||
}
|
||||
|
||||
export function createBeforeQuitHandler({
|
||||
function waitForUpdateDeadline(signal: AbortSignal): Promise<boolean> {
|
||||
if (signal.aborted) {
|
||||
return Promise.resolve(false);
|
||||
}
|
||||
|
||||
return new Promise((resolve) => {
|
||||
signal.addEventListener("abort", () => resolve(false), { once: true });
|
||||
});
|
||||
}
|
||||
|
||||
function createDeferredUpdateQuit(): DeferredUpdateQuit {
|
||||
let resolvePromise!: (started: boolean) => void;
|
||||
const promise = new Promise<boolean>((resolve) => {
|
||||
resolvePromise = resolve;
|
||||
});
|
||||
return { promise, resolve: () => resolvePromise(true) };
|
||||
}
|
||||
|
||||
export function createQuitLifecycle({
|
||||
app,
|
||||
closeTransportSessions,
|
||||
stopDesktopManagedDaemonIfNeeded,
|
||||
installAppUpdateOnQuit,
|
||||
createUpdateDeadlineSignal,
|
||||
onStopError,
|
||||
onUpdateError,
|
||||
}: {
|
||||
app: BeforeQuitApp;
|
||||
closeTransportSessions: () => void;
|
||||
stopDesktopManagedDaemonIfNeeded: () => Promise<boolean>;
|
||||
installAppUpdateOnQuit: (signal: AbortSignal) => Promise<boolean>;
|
||||
createUpdateDeadlineSignal: () => AbortSignal;
|
||||
onStopError: (error: unknown) => void;
|
||||
}): (event: BeforeQuitEvent) => void {
|
||||
// We always preventDefault on first quit so we can run the async stop
|
||||
// decision, then call app.exit(0) — which bypasses Electron's
|
||||
// close → window-all-closed → will-quit chain. The window-all-closed
|
||||
// listener is a darwin no-op (macOS convention) and would otherwise
|
||||
// veto a re-fired app.quit().
|
||||
onUpdateError: (error: unknown) => void;
|
||||
}): QuitLifecycle {
|
||||
// The first quit waits for daemon shutdown and update revalidation. A validated
|
||||
// update re-fires app.quit(); otherwise app.exit(0) bypasses Electron's macOS
|
||||
// window-all-closed handler, which would veto that second quit.
|
||||
let quitting = false;
|
||||
let quittingForUpdate = false;
|
||||
const updateQuit = createDeferredUpdateQuit();
|
||||
|
||||
return (event) => {
|
||||
function handleBeforeQuit(event: BeforeQuitEvent): void {
|
||||
closeTransportSessions();
|
||||
if (quitting) return;
|
||||
if (quittingForUpdate) return;
|
||||
if (quitting) {
|
||||
// MacUpdater's no-relaunch path calls app.quit() without emitting
|
||||
// before-quit-for-update. A second quit is equivalent handoff evidence.
|
||||
updateQuit.resolve();
|
||||
return;
|
||||
}
|
||||
quitting = true;
|
||||
event.preventDefault();
|
||||
|
||||
void stopDesktopManagedDaemonIfNeeded()
|
||||
.catch((error) => {
|
||||
void (async () => {
|
||||
try {
|
||||
await stopDesktopManagedDaemonIfNeeded();
|
||||
} catch (error) {
|
||||
onStopError(error);
|
||||
})
|
||||
.finally(() => {
|
||||
app.exit(0);
|
||||
}
|
||||
|
||||
const signal = createUpdateDeadlineSignal();
|
||||
const updateInstallation = installAppUpdateOnQuit(signal).catch((error) => {
|
||||
onUpdateError(error);
|
||||
return false;
|
||||
});
|
||||
const installingUpdate = await Promise.race([
|
||||
updateInstallation,
|
||||
waitForUpdateDeadline(signal),
|
||||
]);
|
||||
if (installingUpdate) {
|
||||
const handoffStarted = await Promise.race([
|
||||
updateQuit.promise,
|
||||
waitForUpdateDeadline(createUpdateDeadlineSignal()),
|
||||
]);
|
||||
if (handoffStarted) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
app.exit(0);
|
||||
})();
|
||||
}
|
||||
|
||||
return {
|
||||
handleBeforeQuit,
|
||||
handleBeforeQuitForUpdate() {
|
||||
quittingForUpdate = true;
|
||||
updateQuit.resolve();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user