fix: ack may not exist depending on caller

fix(maintenance): always ack messages
fix(maintenance): ensure Redis is connected first

server/src/maintenance/maintenance-websocket.repository.ts

@@ -37,7 +37,10 @@ export class MaintenanceWebsocketRepository implements OnGatewayConnection, OnGa
 
   afterInit(websocketServer: Server) {
     this.logger.log('Initialized websocket server');
-    websocketServer.on('AppRestart', () => this.appRepository.exitApp());
+    websocketServer.on('AppRestart', (_, ack?: (ok: 'ok') => void) => {
+      ack?.('ok');
+      this.appRepository.exitApp();
+    });
   }
 
   clientBroadcast<T extends keyof ClientEventMap>(event: T, ...data: ClientEventMap[T]) {

server/src/services/cli.service.spec.ts

@@ -5,6 +5,8 @@ import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 import { describe, it } from 'vitest';
 
+const mockSendRestart = vi.fn();
+
 describe(CliService.name, () => {
   let sut: CliService;
   let mocks: ServiceMocks;
@@ -85,7 +87,7 @@ describe(CliService.name, () => {
   describe('disableMaintenanceMode', () => {
     it('should not do anything if not in maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: false });
-      await expect(sut.disableMaintenanceMode()).resolves.toEqual({
+      await expect(sut.disableMaintenanceMode(mockSendRestart)).resolves.toEqual({
         alreadyDisabled: true,
       });
 
@@ -95,7 +97,7 @@ describe(CliService.name, () => {
 
     it('should disable maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: true, secret: 'secret' });
-      await expect(sut.disableMaintenanceMode()).resolves.toEqual({
+      await expect(sut.disableMaintenanceMode(mockSendRestart)).resolves.toEqual({
         alreadyDisabled: false,
       });
 
@@ -108,7 +110,7 @@ describe(CliService.name, () => {
   describe('enableMaintenanceMode', () => {
     it('should not do anything if in maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: true, secret: 'secret' });
-      await expect(sut.enableMaintenanceMode()).resolves.toEqual(
+      await expect(sut.enableMaintenanceMode(mockSendRestart)).resolves.toEqual(
         expect.objectContaining({
           alreadyEnabled: true,
         }),
@@ -120,7 +122,7 @@ describe(CliService.name, () => {
 
     it('should enable maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: false });
-      await expect(sut.enableMaintenanceMode()).resolves.toEqual(
+      await expect(sut.enableMaintenanceMode(mockSendRestart)).resolves.toEqual(
         expect.objectContaining({
           alreadyEnabled: false,
         }),
@@ -137,7 +139,7 @@ describe(CliService.name, () => {
     it('should return a valid login URL', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: true, secret: 'secret' });
 
-      const result = await sut.enableMaintenanceMode();
+      const result = await sut.enableMaintenanceMode(mockSendRestart);
 
       expect(result).toEqual(
         expect.objectContaining({

server/src/services/cli.service.ts

@@ -42,7 +42,7 @@ export class CliService extends BaseService {
     await this.updateConfig(config);
   }
 
-  async disableMaintenanceMode(): Promise<{ alreadyDisabled: boolean }> {
+  async disableMaintenanceMode(sendAppRestartCallback = sendOneShotAppRestart): Promise<{ alreadyDisabled: boolean }> {
     const currentState = await this.systemMetadataRepository
       .get(SystemMetadataKey.MaintenanceMode)
       .then((state) => state ?? { isMaintenanceMode: false as const });
@@ -56,14 +56,16 @@ export class CliService extends BaseService {
     const state = { isMaintenanceMode: false as const };
     await this.systemMetadataRepository.set(SystemMetadataKey.MaintenanceMode, state);
 
-    sendOneShotAppRestart(state);
+    await sendAppRestartCallback(state);
 
     return {
       alreadyDisabled: false,
     };
   }
 
-  async enableMaintenanceMode(): Promise<{ authUrl: string; alreadyEnabled: boolean }> {
+  async enableMaintenanceMode(
+    sendAppRestartCallback = sendOneShotAppRestart,
+  ): Promise<{ authUrl: string; alreadyEnabled: boolean }> {
     const { server } = await this.getConfig({ withCache: true });
     const baseUrl = getExternalDomain(server);
 
@@ -89,7 +91,7 @@ export class CliService extends BaseService {
       secret,
     });
 
-    sendOneShotAppRestart({
+    await sendAppRestartCallback({
       isMaintenanceMode: true,
     });
 

server/src/services/maintenance.service.ts

@@ -31,7 +31,8 @@ export class MaintenanceService extends BaseService {
   }
 
   @OnEvent({ name: 'AppRestart', server: true })
-  onRestart(): void {
+  onRestart(_: undefined, ack?: (ok: 'ok') => void): void {
+    ack?.('ok');
     this.appRepository.exitApp();
   }
 

server/src/utils/maintenance.ts

@@ -7,47 +7,25 @@ import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto';
 import { ConfigRepository } from 'src/repositories/config.repository';
 import { AppRestartEvent } from 'src/repositories/event.repository';
 
-export function sendOneShotAppRestart(state: AppRestartEvent): void {
+export async function sendOneShotAppRestart(state: AppRestartEvent): Promise<void> {
   const server = new SocketIO();
   const { redis } = new ConfigRepository().getEnv();
-  const pubClient = new Redis(redis);
+  const pubClient = new Redis({ ...redis, lazyConnect: true });
   const subClient = pubClient.duplicate();
+
+  await Promise.all([pubClient.connect(), subClient.connect()]);
+
   server.adapter(createAdapter(pubClient, subClient));
 
-  /**
-   * Keep trying until we manage to stop Immich
-   *
-   * Sometimes there appear to be communication
-   * issues between to the other servers.
-   *
-   * This issue only occurs with this method.
-   */
-  async function tryTerminate() {
-    while (true) {
-      try {
-        const responses = await server.serverSideEmitWithAck('AppRestart', state);
-        if (responses.length > 0) {
-          return;
-        }
-      } catch (error) {
-        console.error(error);
-        console.error('Encountered an error while telling Immich to stop.');
-      }
-
-      console.info(
-        "\nIt doesn't appear that Immich stopped, trying again in a moment.\nIf Immich is already not running, you can ignore this error.",
-      );
-
-      await new Promise((r) => setTimeout(r, 1e3));
-    }
-  }
-
   // => corresponds to notification.service.ts#onAppRestart
-  server.emit('AppRestartV1', state, () => {
-    void tryTerminate().finally(() => {
-      pubClient.disconnect();
-      subClient.disconnect();
-    });
+  server.emit('AppRestartV1', state, async () => {
+    const responses = await server.serverSideEmitWithAck('AppRestart', state);
+    if (responses.some((response) => response !== 'ok')) {
+      throw new Error("One or more node(s) returned a non-'ok' response to our restart request!");
+    }
+
+    pubClient.disconnect();
+    subClient.disconnect();
   });
 }