Skip to content

Commit 97bc3d0

Browse files
charlesviencharlesvien
committed
Implement AppLifecycleService and refactor to fix auto updater
1 parent a841447 commit 97bc3d0

7 files changed

Lines changed: 262 additions & 22 deletions

File tree

‎apps/array/src/main/services/app-lifecycle/service.test.ts‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const { mockApp, mockAgentService, mockTrackAppEvent, mockShutdownPostHog } =
4+
vi.hoisted(() => ({
5+
mockApp: {
6+
exit: vi.fn(),
7+
},
8+
mockAgentService: {
9+
cleanupAll: vi.fn(() => Promise.resolve()),
10+
},
11+
mockTrackAppEvent: vi.fn(),
12+
mockShutdownPostHog: vi.fn(() => Promise.resolve()),
13+
}));
14+
15+
vi.mock("electron", () => ({
16+
app: mockApp,
17+
}));
18+
19+
vi.mock("../../lib/logger.js", () => ({
20+
logger: {
21+
scope: () => ({
22+
info: vi.fn(),
23+
error: vi.fn(),
24+
warn: vi.fn(),
25+
debug: vi.fn(),
26+
}),
27+
},
28+
}));
29+
30+
vi.mock("../posthog-analytics.js", () => ({
31+
trackAppEvent: mockTrackAppEvent,
32+
shutdownPostHog: mockShutdownPostHog,
33+
}));
34+
35+
vi.mock("../../di/tokens.js", () => ({
36+
MAIN_TOKENS: {
37+
AgentService: Symbol.for("AgentService"),
38+
},
39+
}));
40+
41+
vi.mock("../../../types/analytics.js", () => ({
42+
ANALYTICS_EVENTS: {
43+
APP_QUIT: "app_quit",
44+
},
45+
}));
46+
47+
import { AppLifecycleService } from "./service.js";
48+
49+
describe("AppLifecycleService", () => {
50+
let service: AppLifecycleService;
51+
52+
beforeEach(() => {
53+
vi.clearAllMocks();
54+
55+
service = new AppLifecycleService();
56+
(
57+
service as unknown as { agentService: typeof mockAgentService }
58+
).agentService = mockAgentService;
59+
});
60+
61+
describe("isQuittingForUpdate", () => {
62+
it("returns false by default", () => {
63+
expect(service.isQuittingForUpdate).toBe(false);
64+
});
65+
66+
it("returns true after setQuittingForUpdate is called", () => {
67+
service.setQuittingForUpdate();
68+
expect(service.isQuittingForUpdate).toBe(true);
69+
});
70+
});
71+
72+
describe("shutdown", () => {
73+
it("cleans up agents", async () => {
74+
await service.shutdown();
75+
expect(mockAgentService.cleanupAll).toHaveBeenCalled();
76+
});
77+
78+
it("tracks app quit event", async () => {
79+
await service.shutdown();
80+
expect(mockTrackAppEvent).toHaveBeenCalledWith("app_quit");
81+
});
82+
83+
it("shuts down PostHog", async () => {
84+
await service.shutdown();
85+
expect(mockShutdownPostHog).toHaveBeenCalled();
86+
});
87+
88+
it("calls cleanup steps in order", async () => {
89+
const callOrder: string[] = [];
90+
91+
mockAgentService.cleanupAll.mockImplementation(async () => {
92+
callOrder.push("cleanupAll");
93+
});
94+
mockTrackAppEvent.mockImplementation(() => {
95+
callOrder.push("trackAppEvent");
96+
});
97+
mockShutdownPostHog.mockImplementation(async () => {
98+
callOrder.push("shutdownPostHog");
99+
});
100+
101+
await service.shutdown();
102+
103+
expect(callOrder).toEqual([
104+
"cleanupAll",
105+
"trackAppEvent",
106+
"shutdownPostHog",
107+
]);
108+
});
109+
110+
it("continues shutdown if agent cleanup fails", async () => {
111+
mockAgentService.cleanupAll.mockRejectedValue(
112+
new Error("cleanup failed"),
113+
);
114+
115+
await service.shutdown();
116+
117+
expect(mockTrackAppEvent).toHaveBeenCalled();
118+
expect(mockShutdownPostHog).toHaveBeenCalled();
119+
});
120+
121+
it("continues shutdown if PostHog shutdown fails", async () => {
122+
mockShutdownPostHog.mockRejectedValue(new Error("posthog failed"));
123+
124+
// Should not throw
125+
await expect(service.shutdown()).resolves.toBeUndefined();
126+
});
127+
});
128+
129+
describe("shutdownAndExit", () => {
130+
it("calls shutdown before exit", async () => {
131+
const callOrder: string[] = [];
132+
133+
mockAgentService.cleanupAll.mockImplementation(async () => {
134+
callOrder.push("cleanupAll");
135+
});
136+
mockApp.exit.mockImplementation(() => {
137+
callOrder.push("exit");
138+
});
139+
140+
await service.shutdownAndExit();
141+
142+
expect(callOrder[0]).toBe("cleanupAll");
143+
expect(callOrder[callOrder.length - 1]).toBe("exit");
144+
});
145+
146+
it("exits with code 0", async () => {
147+
await service.shutdownAndExit();
148+
expect(mockApp.exit).toHaveBeenCalledWith(0);
149+
});
150+
});
151+
});

‎apps/array/src/main/services/app-lifecycle/service.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { app } from "electron";
2+
import { inject, injectable } from "inversify";
3+
import { ANALYTICS_EVENTS } from "../../../types/analytics.js";
4+
import { MAIN_TOKENS } from "../../di/tokens.js";
5+
import { logger } from "../../lib/logger.js";
6+
import type { AgentService } from "../agent/service.js";
7+
import { shutdownPostHog, trackAppEvent } from "../posthog-analytics.js";
8+
9+
const log = logger.scope("app-lifecycle");
10+
11+
@injectable()
12+
export class AppLifecycleService {
13+
@inject(MAIN_TOKENS.AgentService)
14+
private agentService!: AgentService;
15+
16+
private _isQuittingForUpdate = false;
17+
18+
get isQuittingForUpdate(): boolean {
19+
return this._isQuittingForUpdate;
20+
}
21+
22+
setQuittingForUpdate(): void {
23+
this._isQuittingForUpdate = true;
24+
}
25+
26+
async shutdown(): Promise<void> {
27+
log.info("Performing graceful shutdown...");
28+
29+
try {
30+
await this.agentService.cleanupAll();
31+
} catch (error) {
32+
log.error("Error cleaning up agents during shutdown", error);
33+
}
34+
35+
trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
36+
37+
try {
38+
await shutdownPostHog();
39+
} catch (error) {
40+
log.error("Error shutting down PostHog", error);
41+
}
42+
43+
log.info("Graceful shutdown complete");
44+
}
45+
46+
async shutdownAndExit(): Promise<void> {
47+
await this.shutdown();
48+
app.exit(0);
49+
}
50+
}

‎apps/twig/src/main/di/container.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import "reflect-metadata";
22
import { Container } from "inversify";
33
import { AgentService } from "../services/agent/service.js";
4+
import { AppLifecycleService } from "../services/app-lifecycle/service.js";
45
import { ConnectivityService } from "../services/connectivity/service.js";
56
import { ContextMenuService } from "../services/context-menu/service.js";
67
import { DeepLinkService } from "../services/deep-link/service.js";
@@ -23,6 +24,7 @@ export const container = new Container({
2324
});
2425

2526
container.bind(MAIN_TOKENS.AgentService).to(AgentService);
27+
container.bind(MAIN_TOKENS.AppLifecycleService).to(AppLifecycleService);
2628
container.bind(MAIN_TOKENS.ConnectivityService).to(ConnectivityService);
2729
container.bind(MAIN_TOKENS.ContextMenuService).to(ContextMenuService);
2830
container.bind(MAIN_TOKENS.DeepLinkService).to(DeepLinkService);

‎apps/twig/src/main/di/tokens.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
export const MAIN_TOKENS = Object.freeze({
88
// Services
99
AgentService: Symbol.for("Main.AgentService"),
10+
AppLifecycleService: Symbol.for("Main.AppLifecycleService"),
1011
ConnectivityService: Symbol.for("Main.ConnectivityService"),
1112
ContextMenuService: Symbol.for("Main.ContextMenuService"),
1213
DockBadgeService: Symbol.for("Main.DockBadgeService"),

‎apps/twig/src/main/index.ts‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,18 @@ import "./lib/logger";
2727
import { ANALYTICS_EVENTS } from "../types/analytics.js";
2828
import { container } from "./di/container.js";
2929
import { MAIN_TOKENS } from "./di/tokens.js";
30-
import type { AgentService } from "./services/agent/service.js";
3130
import type { DockBadgeService } from "./services/dock-badge/service.js";
3231
import type { UIService } from "./services/ui/service.js";
3332
import { setMainWindowGetter } from "./trpc/context.js";
3433
import { trpcRouter } from "./trpc/index.js";
3534

3635
import "./services/index.js";
36+
import type { AppLifecycleService } from "./services/app-lifecycle/service.js";
3737
import type { DeepLinkService } from "./services/deep-link/service.js";
3838
import type { ExternalAppsService } from "./services/external-apps/service.js";
3939
import type { OAuthService } from "./services/oauth/service.js";
4040
import {
4141
initializePostHog,
42-
shutdownPostHog,
4342
trackAppEvent,
4443
} from "./services/posthog-analytics.js";
4544
import type { TaskLinkService } from "./services/task-link/service";
@@ -361,19 +360,26 @@ app.whenReady().then(() => {
361360

362361
app.on("window-all-closed", async () => {
363362
if (process.platform !== "darwin") {
364-
trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
365-
await shutdownPostHog();
363+
const lifecycleService = container.get<AppLifecycleService>(
364+
MAIN_TOKENS.AppLifecycleService,
365+
);
366+
await lifecycleService.shutdown();
366367
app.quit();
367368
}
368369
});
369370

370371
app.on("before-quit", async (event) => {
372+
const lifecycleService = container.get<AppLifecycleService>(
373+
MAIN_TOKENS.AppLifecycleService,
374+
);
375+
376+
// If quitting to install an update, don't block - let the updater handle it
377+
if (lifecycleService.isQuittingForUpdate) {
378+
return;
379+
}
380+
371381
event.preventDefault();
372-
const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
373-
await agentService.cleanupAll();
374-
trackAppEvent(ANALYTICS_EVENTS.APP_QUIT);
375-
await shutdownPostHog();
376-
app.exit(0);
382+
await lifecycleService.shutdownAndExit();
377383
});
378384

379385
app.on("activate", () => {
@@ -385,8 +391,10 @@ app.on("activate", () => {
385391
// Handle process signals to ensure clean shutdown
386392
const handleShutdownSignal = async (_signal: string) => {
387393
try {
388-
const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
389-
await agentService.cleanupAll();
394+
const lifecycleService = container.get<AppLifecycleService>(
395+
MAIN_TOKENS.AppLifecycleService,
396+
);
397+
await lifecycleService.shutdown();
390398
} catch (_err) {}
391399
process.exit(0);
392400
};
@@ -398,16 +406,20 @@ process.on("SIGHUP", () => handleShutdownSignal("SIGHUP"));
398406
// Handle uncaught exceptions to attempt cleanup before crash
399407
process.on("uncaughtException", async (_error) => {
400408
try {
401-
const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
402-
await agentService.cleanupAll();
409+
const lifecycleService = container.get<AppLifecycleService>(
410+
MAIN_TOKENS.AppLifecycleService,
411+
);
412+
await lifecycleService.shutdown();
403413
} catch (_cleanupErr) {}
404414
process.exit(1);
405415
});
406416

407417
process.on("unhandledRejection", async (_reason) => {
408418
try {
409-
const agentService = container.get<AgentService>(MAIN_TOKENS.AgentService);
410-
await agentService.cleanupAll();
419+
const lifecycleService = container.get<AppLifecycleService>(
420+
MAIN_TOKENS.AppLifecycleService,
421+
);
422+
await lifecycleService.shutdown();
411423
} catch (_cleanupErr) {}
412424
process.exit(1);
413425
});

‎apps/twig/src/main/services/updates/service.test.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import { UpdatesEvent } from "./schemas.js";
33

44
// Use vi.hoisted to ensure mocks are available when vi.mock is hoisted
5-
const { mockApp, mockAutoUpdater } = vi.hoisted(() => ({
5+
const { mockApp, mockAutoUpdater, mockLifecycleService } = vi.hoisted(() => ({
66
mockAutoUpdater: {
77
setFeedURL: vi.fn(),
88
checkForUpdates: vi.fn(),
@@ -15,6 +15,10 @@ const { mockApp, mockAutoUpdater } = vi.hoisted(() => ({
1515
on: vi.fn(),
1616
whenReady: vi.fn(() => Promise.resolve()),
1717
},
18+
mockLifecycleService: {
19+
shutdown: vi.fn(() => Promise.resolve()),
20+
setQuittingForUpdate: vi.fn(),
21+
},
1822
}));
1923

2024
vi.mock("electron", () => ({
@@ -33,6 +37,12 @@ vi.mock("../../lib/logger.js", () => ({
3337
},
3438
}));
3539

40+
vi.mock("../../di/tokens.js", () => ({
41+
MAIN_TOKENS: {
42+
AppLifecycleService: Symbol.for("AppLifecycleService"),
43+
},
44+
}));
45+
3646
// Import the service after mocks are set up
3747
import { UpdatesService } from "./service.js";
3848

@@ -72,6 +82,10 @@ describe("UpdatesService", () => {
7282
delete process.env.ELECTRON_DISABLE_AUTO_UPDATE;
7383

7484
service = new UpdatesService();
85+
// Manually inject the mock lifecycle service (normally done by DI container)
86+
(
87+
service as unknown as { lifecycleService: typeof mockLifecycleService }
88+
).lifecycleService = mockLifecycleService;
7589
});
7690

7791
afterEach(() => {
@@ -292,8 +306,8 @@ describe("UpdatesService", () => {
292306
});
293307

294308
describe("installUpdate", () => {
295-
it("returns false when no update is ready", () => {
296-
const result = service.installUpdate();
309+
it("returns false when no update is ready", async () => {
310+
const result = await service.installUpdate();
297311
expect(result).toEqual({ installed: false });
298312
});
299313

@@ -309,8 +323,10 @@ describe("UpdatesService", () => {
309323
updateDownloadedHandler({}, "Release notes", "v2.0.0");
310324
}
311325

312-
const result = service.installUpdate();
326+
const result = await service.installUpdate();
313327
expect(result).toEqual({ installed: true });
328+
expect(mockLifecycleService.shutdown).toHaveBeenCalled();
329+
expect(mockLifecycleService.setQuittingForUpdate).toHaveBeenCalled();
314330
expect(mockAutoUpdater.quitAndInstall).toHaveBeenCalled();
315331
});
316332

@@ -330,7 +346,7 @@ describe("UpdatesService", () => {
330346
throw new Error("Failed to install");
331347
});
332348

333-
const result = service.installUpdate();
349+
const result = await service.installUpdate();
334350
expect(result).toEqual({ installed: false });
335351
});
336352
});

0 commit comments

Comments
 (0)