Skip to content

Commit 8e5ddd4

Browse files
Verify update installer launch before quitting (#260)
Co-authored-by: Owen McGirr <o.a.mcgirr@gmail.com>
1 parent 3b3d41a commit 8e5ddd4

15 files changed

Lines changed: 531 additions & 29 deletions

src/main/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,9 @@ if (!gotSingleInstanceLock) {
365365
currentVersion: app.getVersion(),
366366
isPackaged: app.isPackaged,
367367
platform: process.platform,
368-
autoUpdater
368+
resourcesPath: process.resourcesPath,
369+
autoUpdater,
370+
quitApp
369371
})
370372
);
371373
void bluetoothTransport.start();
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
import { join } from 'node:path';
2+
import { afterEach, describe, expect, it, vi } from 'vitest';
3+
import {
4+
launchWindowsUpdateInstaller,
5+
UPDATE_INSTALLER_ARGS,
6+
type SpawnInstaller,
7+
type SpawnProcess
8+
} from './update-installer-launcher';
9+
10+
class FakeSpawnProcess implements SpawnProcess {
11+
pid = 1234;
12+
readonly unref = vi.fn();
13+
private readonly errorListeners: Array<(error: Error) => void> = [];
14+
private readonly exitListeners: Array<(code: number | null, signal: NodeJS.Signals | null) => void> = [];
15+
16+
once(event: 'error', listener: (error: Error) => void): void;
17+
once(event: 'exit', listener: (code: number | null, signal: NodeJS.Signals | null) => void): void;
18+
once(
19+
event: 'error' | 'exit',
20+
listener: ((error: Error) => void) | ((code: number | null, signal: NodeJS.Signals | null) => void)
21+
): void {
22+
if (event === 'error') {
23+
this.errorListeners.push(listener as (error: Error) => void);
24+
return;
25+
}
26+
27+
this.exitListeners.push(listener as (code: number | null, signal: NodeJS.Signals | null) => void);
28+
}
29+
30+
emitError(error = new Error('failed')): void {
31+
for (const listener of this.errorListeners) {
32+
listener(error);
33+
}
34+
}
35+
36+
emitExit(code: number | null): void {
37+
for (const listener of this.exitListeners) {
38+
listener(code, null);
39+
}
40+
}
41+
}
42+
43+
describe('launchWindowsUpdateInstaller', () => {
44+
afterEach(() => {
45+
vi.useRealTimers();
46+
});
47+
48+
it('returns installer_unavailable when installer path is null', async () => {
49+
await expect(launch({ installerPath: null })).resolves.toEqual({
50+
ok: false,
51+
reason: 'installer_unavailable'
52+
});
53+
});
54+
55+
it('returns installer_unavailable when installer file is missing', async () => {
56+
await expect(
57+
launch({
58+
fileExists: (path) => path.endsWith('elevate.exe')
59+
})
60+
).resolves.toEqual({ ok: false, reason: 'installer_unavailable' });
61+
});
62+
63+
it('returns elevation_helper_unavailable when elevate.exe is missing', async () => {
64+
await expect(
65+
launch({
66+
fileExists: (path) => path.endsWith('installer.exe')
67+
})
68+
).resolves.toEqual({ ok: false, reason: 'elevation_helper_unavailable' });
69+
});
70+
71+
it('spawns elevate.exe with installer arguments', async () => {
72+
vi.useFakeTimers();
73+
const child = new FakeSpawnProcess();
74+
const spawnInstaller = vi.fn<SpawnInstaller>(() => child);
75+
76+
const resultPromise = launch({ child, spawnInstaller, settleMs: 10 });
77+
await vi.advanceTimersByTimeAsync(10);
78+
79+
await expect(resultPromise).resolves.toEqual({ ok: true, pid: 1234 });
80+
expect(spawnInstaller).toHaveBeenCalledWith(
81+
join('resources', 'elevate.exe'),
82+
[join('cache', 'installer.exe'), ...UPDATE_INSTALLER_ARGS],
83+
{
84+
detached: true,
85+
stdio: 'ignore',
86+
windowsHide: false
87+
}
88+
);
89+
expect(child.unref).toHaveBeenCalledTimes(1);
90+
});
91+
92+
it('returns installer_launch_failed if spawn throws', async () => {
93+
await expect(
94+
launch({
95+
spawnInstaller: () => {
96+
throw new Error('spawn failed');
97+
}
98+
})
99+
).resolves.toEqual({ ok: false, reason: 'installer_launch_failed' });
100+
});
101+
102+
it('returns installer_launch_failed if the child emits error before settling', async () => {
103+
vi.useFakeTimers();
104+
const child = new FakeSpawnProcess();
105+
const resultPromise = launch({ child, settleMs: 10 });
106+
107+
child.emitError();
108+
109+
await expect(resultPromise).resolves.toEqual({ ok: false, reason: 'installer_launch_failed' });
110+
expect(child.unref).not.toHaveBeenCalled();
111+
});
112+
113+
it('returns installer_launch_failed if the child exits non-zero before settling', async () => {
114+
vi.useFakeTimers();
115+
const child = new FakeSpawnProcess();
116+
const resultPromise = launch({ child, settleMs: 10 });
117+
118+
child.emitExit(1);
119+
120+
await expect(resultPromise).resolves.toEqual({ ok: false, reason: 'installer_launch_failed' });
121+
expect(child.unref).not.toHaveBeenCalled();
122+
});
123+
124+
it('returns success if the child exits cleanly before settling', async () => {
125+
vi.useFakeTimers();
126+
const child = new FakeSpawnProcess();
127+
const resultPromise = launch({ child, settleMs: 10 });
128+
129+
child.emitExit(0);
130+
131+
await expect(resultPromise).resolves.toEqual({ ok: true, pid: 1234 });
132+
expect(child.unref).toHaveBeenCalledTimes(1);
133+
});
134+
135+
it('returns success if the child survives the settle period', async () => {
136+
vi.useFakeTimers();
137+
const child = new FakeSpawnProcess();
138+
const resultPromise = launch({ child, settleMs: 10 });
139+
140+
await vi.advanceTimersByTimeAsync(10);
141+
142+
await expect(resultPromise).resolves.toEqual({ ok: true, pid: 1234 });
143+
expect(child.unref).toHaveBeenCalledTimes(1);
144+
});
145+
});
146+
147+
function launch({
148+
installerPath = join('cache', 'installer.exe'),
149+
resourcesPath = 'resources',
150+
settleMs = 0,
151+
child = new FakeSpawnProcess(),
152+
spawnInstaller = (() => child) as SpawnInstaller,
153+
fileExists = () => true
154+
}: {
155+
installerPath?: string | null;
156+
resourcesPath?: string;
157+
settleMs?: number;
158+
child?: FakeSpawnProcess;
159+
spawnInstaller?: SpawnInstaller;
160+
fileExists?: (path: string) => boolean;
161+
} = {}): Promise<ReturnType<typeof launchWindowsUpdateInstaller> extends Promise<infer Result> ? Result : never> {
162+
return launchWindowsUpdateInstaller({
163+
installerPath,
164+
resourcesPath,
165+
settleMs,
166+
spawnInstaller,
167+
fileExists
168+
});
169+
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { existsSync } from 'node:fs';
2+
import { join } from 'node:path';
3+
import { spawn } from 'node:child_process';
4+
5+
export type UpdateInstallerLaunchReason =
6+
| 'installer_unavailable'
7+
| 'elevation_helper_unavailable'
8+
| 'installer_launch_failed';
9+
10+
export type UpdateInstallerLaunchResult =
11+
| { ok: true; pid: number | null }
12+
| { ok: false; reason: UpdateInstallerLaunchReason };
13+
14+
export type SpawnProcess = {
15+
pid?: number;
16+
unref(): void;
17+
once(event: 'error', listener: (error: Error) => void): void;
18+
once(event: 'exit', listener: (code: number | null, signal: NodeJS.Signals | null) => void): void;
19+
};
20+
21+
export type SpawnInstaller = (
22+
command: string,
23+
args: string[],
24+
options: {
25+
detached: true;
26+
stdio: 'ignore';
27+
windowsHide: false;
28+
}
29+
) => SpawnProcess;
30+
31+
export const UPDATE_INSTALLER_ARGS = ['--updated', '--force-run'];
32+
export const INSTALLER_LAUNCH_SETTLE_MS = 1_500;
33+
34+
export async function launchWindowsUpdateInstaller({
35+
installerPath,
36+
resourcesPath,
37+
settleMs = INSTALLER_LAUNCH_SETTLE_MS,
38+
spawnInstaller = spawn as SpawnInstaller,
39+
fileExists = existsSync
40+
}: {
41+
installerPath: string | null;
42+
resourcesPath: string;
43+
settleMs?: number;
44+
spawnInstaller?: SpawnInstaller;
45+
fileExists?: (path: string) => boolean;
46+
}): Promise<UpdateInstallerLaunchResult> {
47+
if (!installerPath || !fileExists(installerPath)) {
48+
return { ok: false, reason: 'installer_unavailable' };
49+
}
50+
51+
const elevationHelperPath = join(resourcesPath, 'elevate.exe');
52+
if (!fileExists(elevationHelperPath)) {
53+
return { ok: false, reason: 'elevation_helper_unavailable' };
54+
}
55+
56+
let child: SpawnProcess;
57+
try {
58+
child = spawnInstaller(elevationHelperPath, [installerPath, ...UPDATE_INSTALLER_ARGS], {
59+
detached: true,
60+
stdio: 'ignore',
61+
windowsHide: false
62+
});
63+
} catch {
64+
return { ok: false, reason: 'installer_launch_failed' };
65+
}
66+
67+
return new Promise((resolve) => {
68+
let settled = false;
69+
const timer = setTimeout(() => {
70+
complete({ ok: true, pid: child.pid ?? null });
71+
}, settleMs);
72+
73+
const complete = (result: UpdateInstallerLaunchResult): void => {
74+
if (settled) return;
75+
settled = true;
76+
clearTimeout(timer);
77+
if (result.ok) {
78+
child.unref();
79+
}
80+
resolve(result);
81+
};
82+
83+
child.once('error', () => {
84+
complete({ ok: false, reason: 'installer_launch_failed' });
85+
});
86+
child.once('exit', (code) => {
87+
if (code === 0) {
88+
complete({ ok: true, pid: child.pid ?? null });
89+
return;
90+
}
91+
92+
complete({ ok: false, reason: 'installer_launch_failed' });
93+
});
94+
});
95+
}

src/main/updates/update-ipc.test.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type UpdateInstallConfirmation
88
} from './update-ipc';
99
import type { UpdateService } from './update-service';
10+
import type { UpdateInstallResult } from '../../shared/update';
1011

1112
type IpcHandler = (event: Electron.IpcMainInvokeEvent, ...args: unknown[]) => unknown;
1213

@@ -42,6 +43,20 @@ describe('registerUpdateIpc', () => {
4243
expect(updateService.installDownloadedUpdate).toHaveBeenCalledTimes(1);
4344
});
4445

46+
it('returns installer launch failures after confirmation', async () => {
47+
const updateService = createUpdateService({
48+
downloaded: true,
49+
installResult: { ok: false, reason: 'installer_launch_failed' }
50+
});
51+
const confirmInstallDownloadedUpdate = vi.fn<UpdateInstallConfirmation>(async () => true);
52+
53+
registerUpdateIpc(updateService, { confirmInstallDownloadedUpdate });
54+
55+
await expect(invokeInstall()).resolves.toEqual({ ok: false, reason: 'installer_launch_failed' });
56+
expect(confirmInstallDownloadedUpdate).toHaveBeenCalledTimes(1);
57+
expect(updateService.installDownloadedUpdate).toHaveBeenCalledTimes(1);
58+
});
59+
4560
it('does not install a downloaded update when confirmation is cancelled', async () => {
4661
const updateService = createUpdateService({ downloaded: true, installResult: { ok: true } });
4762
const confirmInstallDownloadedUpdate = vi.fn<UpdateInstallConfirmation>(async () => false);
@@ -91,7 +106,7 @@ function createUpdateService({
91106
installResult
92107
}: {
93108
downloaded: boolean;
94-
installResult: { ok: boolean; reason?: string };
109+
installResult: UpdateInstallResult;
95110
}): UpdateService {
96111
return {
97112
getState: vi.fn(() => ({
@@ -112,7 +127,7 @@ function createUpdateService({
112127
})),
113128
checkForUpdates: vi.fn(),
114129
downloadUpdate: vi.fn(),
115-
installDownloadedUpdate: vi.fn(() => installResult)
130+
installDownloadedUpdate: vi.fn(async () => installResult)
116131
} as unknown as UpdateService;
117132
}
118133

src/main/updates/update-ipc.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ export function registerUpdateIpc(
4545
ipcMain.handle(DOWNLOAD_UPDATE_CHANNEL, () => updateService.downloadUpdate());
4646
ipcMain.handle(INSTALL_DOWNLOADED_UPDATE_CHANNEL, async (event) => {
4747
if (updateService.getState().download.status !== 'downloaded') {
48-
return updateService.installDownloadedUpdate();
48+
return await updateService.installDownloadedUpdate();
4949
}
5050

5151
if (!(await confirmInstallDownloadedUpdate(event))) {
5252
return { ok: false, reason: 'cancelled' };
5353
}
5454

55-
return updateService.installDownloadedUpdate();
55+
return await updateService.installDownloadedUpdate();
5656
});
5757
}

0 commit comments

Comments
 (0)