Skip to content

Commit 9d8f6d7

Browse files
committed
fix(desktop): guard installUpdate against repeat clicks
MacUpdater.quitAndInstall() registers a fresh native-updater `update-downloaded` listener each call when Squirrel.Mac hasn't finished staging. Repeat clicks on the update button stacked listeners, then fanned out into parallel nativeUpdater.quitAndInstall() calls once Squirrel fired — racing to swap the binary and leaving the app on the old version. Matches the reporter's symptom (app quits, reopens on same version). Add an `isInstalling` guard + `status === READY` precondition so repeat clicks collapse to a single quitAndInstall, and clear the flag in the error handler so the user can retry if Squirrel surfaces an error instead of actually quitting. Closes #3507
1 parent 867ef87 commit 9d8f6d7

2 files changed

Lines changed: 108 additions & 0 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { beforeEach, describe, expect, mock, test } from "bun:test";
2+
import { EventEmitter } from "node:events";
3+
4+
class FakeAutoUpdater extends EventEmitter {
5+
autoDownload = false;
6+
autoInstallOnAppQuit = false;
7+
disableDifferentialDownload = false;
8+
allowDowngrade = false;
9+
setFeedURL = mock(() => {});
10+
checkForUpdates = mock(() => Promise.resolve(null));
11+
quitAndInstall = mock(() => {});
12+
}
13+
14+
const fakeAutoUpdater = new FakeAutoUpdater();
15+
16+
mock.module("electron-updater", () => ({
17+
autoUpdater: fakeAutoUpdater,
18+
}));
19+
20+
mock.module("electron", () => ({
21+
app: {
22+
getPath: mock(() => ""),
23+
getName: mock(() => "test-app"),
24+
getVersion: mock(() => "1.0.0"),
25+
getAppPath: mock(() => ""),
26+
isPackaged: false,
27+
isReady: mock(() => true),
28+
whenReady: mock(() => Promise.resolve()),
29+
},
30+
dialog: {
31+
showMessageBox: mock(() => Promise.resolve({ response: 0 })),
32+
},
33+
}));
34+
35+
mock.module("main/index", () => ({
36+
setSkipQuitConfirmation: mock(() => {}),
37+
}));
38+
39+
const autoUpdater = await import("./auto-updater");
40+
const { AUTO_UPDATE_STATUS } = await import("shared/auto-update");
41+
42+
describe("installUpdate", () => {
43+
beforeEach(() => {
44+
fakeAutoUpdater.removeAllListeners();
45+
fakeAutoUpdater.quitAndInstall.mockClear();
46+
fakeAutoUpdater.checkForUpdates.mockClear();
47+
fakeAutoUpdater.setFeedURL.mockClear();
48+
autoUpdater.setupAutoUpdater();
49+
// Error handler resets the module-level isInstalling guard so each
50+
// test starts from the same state even though the module is a singleton.
51+
fakeAutoUpdater.emit("error", new Error("reset"));
52+
});
53+
54+
test("ignores install requests when no update is ready", () => {
55+
expect(autoUpdater.getUpdateStatus().status).not.toBe(
56+
AUTO_UPDATE_STATUS.READY,
57+
);
58+
59+
autoUpdater.installUpdate();
60+
61+
expect(fakeAutoUpdater.quitAndInstall).not.toHaveBeenCalled();
62+
});
63+
64+
test("collapses repeat install clicks into a single quitAndInstall call", () => {
65+
fakeAutoUpdater.emit("update-downloaded", { version: "9.9.9" });
66+
expect(autoUpdater.getUpdateStatus().status).toBe(AUTO_UPDATE_STATUS.READY);
67+
68+
autoUpdater.installUpdate();
69+
autoUpdater.installUpdate();
70+
autoUpdater.installUpdate();
71+
72+
expect(fakeAutoUpdater.quitAndInstall).toHaveBeenCalledTimes(1);
73+
});
74+
75+
test("clears the in-flight guard when Squirrel surfaces an error", () => {
76+
fakeAutoUpdater.emit("update-downloaded", { version: "9.9.9" });
77+
autoUpdater.installUpdate();
78+
expect(fakeAutoUpdater.quitAndInstall).toHaveBeenCalledTimes(1);
79+
80+
fakeAutoUpdater.emit("error", new Error("squirrel failed"));
81+
fakeAutoUpdater.emit("update-downloaded", { version: "9.9.9" });
82+
autoUpdater.installUpdate();
83+
84+
expect(fakeAutoUpdater.quitAndInstall).toHaveBeenCalledTimes(2);
85+
});
86+
});

apps/desktop/src/main/lib/auto-updater.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ function isNetworkError(error: Error | string): boolean {
8282
let currentStatus: AutoUpdateStatus = AUTO_UPDATE_STATUS.IDLE;
8383
let currentVersion: string | undefined;
8484
let isDismissed = false;
85+
let isInstalling = false;
8586

8687
function emitStatus(
8788
status: AutoUpdateStatus,
@@ -111,6 +112,25 @@ export function installUpdate(): void {
111112
emitStatus(AUTO_UPDATE_STATUS.IDLE);
112113
return;
113114
}
115+
// MacUpdater.quitAndInstall() registers a fresh native-updater
116+
// `update-downloaded` listener each time it runs before Squirrel.Mac has
117+
// finished staging. Repeat clicks therefore fan out into multiple parallel
118+
// quitAndInstall calls once Squirrel fires, racing each other and leaving
119+
// the binary un-swapped. Gate on status + an in-flight flag to collapse
120+
// duplicates.
121+
if (isInstalling) {
122+
console.info(
123+
"[auto-updater] Install already in progress, ignoring duplicate request",
124+
);
125+
return;
126+
}
127+
if (currentStatus !== AUTO_UPDATE_STATUS.READY) {
128+
console.warn(
129+
`[auto-updater] Install ignored: update not ready (status=${currentStatus})`,
130+
);
131+
return;
132+
}
133+
isInstalling = true;
114134
setSkipQuitConfirmation();
115135
autoUpdater.quitAndInstall(false, true);
116136
}
@@ -242,6 +262,8 @@ export function setupAutoUpdater(): void {
242262
);
243263

244264
autoUpdater.on("error", (error) => {
265+
// Allow retry if Squirrel surfaces an error instead of actually quitting.
266+
isInstalling = false;
245267
if (isNetworkError(error)) {
246268
console.info("[auto-updater] Network unavailable, will retry later");
247269
emitStatus(AUTO_UPDATE_STATUS.IDLE);

0 commit comments

Comments
 (0)