diff --git a/e2e/src/specs/maintenance/server/database-backups.e2e-spec.ts b/e2e/src/specs/maintenance/server/database-backups.e2e-spec.ts index 2b0f6ae61a..b69bd099ed 100644 --- a/e2e/src/specs/maintenance/server/database-backups.e2e-spec.ts +++ b/e2e/src/specs/maintenance/server/database-backups.e2e-spec.ts @@ -10,7 +10,9 @@ describe('/admin/database-backups', () => { beforeAll(async () => { await utils.resetDatabase(); - admin = await utils.adminSetup(); + admin = await utils.adminSetup({ + onboarding: false, + }); await utils.resetBackups(admin.accessToken); }); @@ -94,7 +96,9 @@ describe('/admin/database-backups', () => { ({ status, body }) => status === 200 && !body.maintenanceMode, ); - admin = await utils.adminSetup(); + admin = await utils.adminSetup({ + onboarding: false, + }); }); it.sequential('should not work when the server is configured', async () => { diff --git a/server/src/app.common.ts b/server/src/app.common.ts index 934c13343f..98161f69d1 100644 --- a/server/src/app.common.ts +++ b/server/src/app.common.ts @@ -4,7 +4,7 @@ import compression from 'compression'; import cookieParser from 'cookie-parser'; import { existsSync } from 'node:fs'; import sirv from 'sirv'; -import { excludePaths, serverVersion } from 'src/constants'; +import { IMMICH_SERVER_START, excludePaths, serverVersion } from 'src/constants'; import { MaintenanceWorkerService } from 'src/maintenance/maintenance-worker.service'; import { WebSocketAdapter } from 'src/middleware/websocket.adapter'; import { ConfigRepository } from 'src/repositories/config.repository'; @@ -83,5 +83,5 @@ export async function configureExpress( const server = await (host ? app.listen(port, host) : app.listen(port)); server.requestTimeout = 24 * 60 * 60 * 1000; - logger.log(`Immich Server is listening on ${await app.getUrl()} [v${serverVersion}] [${environment}] `); + logger.log(`${IMMICH_SERVER_START} on ${await app.getUrl()} [v${serverVersion}] [${environment}] `); } diff --git a/server/src/app.module.ts b/server/src/app.module.ts index 49b779ca18..b59317577f 100644 --- a/server/src/app.module.ts +++ b/server/src/app.module.ts @@ -29,6 +29,7 @@ import { ProcessRepository } from 'src/repositories/process.repository'; import { StorageRepository } from 'src/repositories/storage.repository'; import { SystemMetadataRepository } from 'src/repositories/system-metadata.repository'; import { teardownTelemetry, TelemetryRepository } from 'src/repositories/telemetry.repository'; +import { UserRepository } from 'src/repositories/user.repository'; import { WebsocketRepository } from 'src/repositories/websocket.repository'; import { services } from 'src/services'; import { AuthService } from 'src/services/auth.service'; @@ -111,6 +112,7 @@ export class ApiModule extends BaseModule {} StorageRepository, ProcessRepository, DatabaseRepository, + UserRepository, SystemMetadataRepository, AppRepository, MaintenanceHealthRepository, diff --git a/server/src/constants.ts b/server/src/constants.ts index e24057beba..4f8d9342c7 100644 --- a/server/src/constants.ts +++ b/server/src/constants.ts @@ -4,6 +4,8 @@ import { dirname, join } from 'node:path'; import { SemVer } from 'semver'; import { ApiTag, AudioCodec, DatabaseExtension, ExifOrientation, VectorIndex } from 'src/enum'; +export const IMMICH_SERVER_START = 'Immich Server is listening'; + export const ErrorMessages = { InconsistentMediaLocation: 'Detected an inconsistent media location. For more information, see https://docs.immich.app/errors#inconsistent-media-location', diff --git a/server/src/maintenance/maintenance-health.repository.ts b/server/src/maintenance/maintenance-health.repository.ts index aeef93ec51..6eab265677 100644 --- a/server/src/maintenance/maintenance-health.repository.ts +++ b/server/src/maintenance/maintenance-health.repository.ts @@ -1,6 +1,7 @@ import { Injectable } from '@nestjs/common'; import { fork } from 'node:child_process'; import { dirname, join } from 'node:path'; +import { IMMICH_SERVER_START } from 'src/constants'; @Injectable() export class MaintenanceHealthRepository { @@ -20,45 +21,27 @@ export class MaintenanceHealthRepository { stdio: ['ignore', 'pipe', 'ignore', 'ipc'], }); - async function checkHealth() { - try { - const response = await fetch('http://127.0.0.1:33001/api/server/config'); - const { isOnboarded } = await response.json(); - if (isOnboarded) { - resolve(); - } else { - reject(new Error('Server health check failed, no admin exists.')); - } - } catch (error) { - reject(error); - } finally { - if (worker.exitCode === null) { - worker.kill('SIGTERM'); - } - } - } - - let output = '', - alive = false; + let output = ''; worker.stdout?.on('data', (data) => { - if (alive) { + if (worker.exitCode !== null) { return; } output += data; - if (output.includes('Immich Server is listening')) { - alive = true; - void checkHealth(); + if (output.includes(IMMICH_SERVER_START)) { + resolve(); + worker.kill('SIGTERM'); } }); - worker.on('exit', reject); - worker.on('error', reject); + worker.on('exit', (code, signal) => reject(`Server health check failed, server exited with ${signal ?? code}`)); + worker.on('error', (error) => reject(`Server health check failed, process threw: ${error}`)); setTimeout(() => { if (worker.exitCode === null) { + reject('Server health check failed, took too long to start.'); worker.kill('SIGTERM'); } }, 20_000); diff --git a/server/src/services/database-backup.service.spec.ts b/server/src/services/database-backup.service.spec.ts index 429e60aede..37964e7b6f 100644 --- a/server/src/services/database-backup.service.spec.ts +++ b/server/src/services/database-backup.service.spec.ts @@ -27,6 +27,7 @@ describe(DatabaseBackupService.name, () => { mocks.systemMetadata as never, mocks.process, mocks.database as never, + mocks.user as never, mocks.cron as never, mocks.job as never, maintenanceHealthRepositoryMock as never, @@ -187,6 +188,7 @@ describe(DatabaseBackupService.name, () => { mocks.systemMetadata as never, mocks.process, mocks.database as never, + mocks.user as never, mocks.cron as never, mocks.job as never, void 0 as never, @@ -400,6 +402,7 @@ describe(DatabaseBackupService.name, () => { mocks.systemMetadata as never, mocks.process, mocks.database as never, + mocks.user as never, mocks.cron as never, mocks.job as never, void 0 as never, @@ -474,6 +477,7 @@ describe(DatabaseBackupService.name, () => { mocks.systemMetadata as never, mocks.process, mocks.database as never, + mocks.user as never, mocks.cron as never, mocks.job as never, void 0 as never, @@ -536,6 +540,7 @@ describe(DatabaseBackupService.name, () => { mocks.systemMetadata as never, mocks.process, mocks.database as never, + mocks.user as never, mocks.cron as never, mocks.job as never, void 0 as never, @@ -663,6 +668,7 @@ describe(DatabaseBackupService.name, () => { mocks.systemMetadata as never, mocks.process, mocks.database as never, + mocks.user as never, mocks.cron as never, mocks.job as never, maintenanceHealthRepositoryMock, @@ -678,6 +684,8 @@ describe(DatabaseBackupService.name, () => { it('should successfully restore a backup', async () => { let writtenToPsql = ''; + mocks.user.hasAdmin.mockResolvedValue(true); + mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', '')); mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', '')); mocks.process.spawnDuplexStream.mockImplementationOnce(() => { @@ -740,6 +748,8 @@ describe(DatabaseBackupService.name, () => { it('should generate pg_dumpall specific SQL instructions', async () => { let writtenToPsql = ''; + mocks.user.hasAdmin.mockResolvedValue(true); + mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', '')); mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', '')); mocks.process.spawnDuplexStream.mockImplementationOnce(() => { @@ -834,7 +844,24 @@ describe(DatabaseBackupService.name, () => { expect(mocks.process.spawnDuplexStream).toHaveBeenCalledTimes(4); }); + it('should rollback if there is no admin user', async () => { + mocks.user.hasAdmin.mockResolvedValue(false); + + const progress = vitest.fn(); + await expect( + sut.restoreDatabaseBackup('development-filename.sql', progress), + ).rejects.toThrowErrorMatchingInlineSnapshot(`[Error: Server health check failed, no admin exists.]`); + + expect(progress).toHaveBeenCalledWith('backup', 0.05); + expect(progress).toHaveBeenCalledWith('migrations', 0.9); + expect(progress).toHaveBeenCalledWith('rollback', 0); + + expect(mocks.user.hasAdmin).toHaveBeenCalled(); + expect(mocks.process.spawnDuplexStream).toHaveBeenCalledTimes(4); + }); + it('should rollback if API healthcheck fails', async () => { + mocks.user.hasAdmin.mockResolvedValue(true); maintenanceHealthRepositoryMock.checkApiHealth.mockRejectedValue(new Error('Health Error')); const progress = vitest.fn(); @@ -846,6 +873,7 @@ describe(DatabaseBackupService.name, () => { expect(progress).toHaveBeenCalledWith('migrations', 0.9); expect(progress).toHaveBeenCalledWith('rollback', 0); + expect(mocks.user.hasAdmin).toHaveBeenCalled(); expect(maintenanceHealthRepositoryMock.checkApiHealth).toHaveBeenCalled(); expect(mocks.process.spawnDuplexStream).toHaveBeenCalledTimes(4); }); diff --git a/server/src/services/database-backup.service.ts b/server/src/services/database-backup.service.ts index 3c964c950c..666ddcff0a 100644 --- a/server/src/services/database-backup.service.ts +++ b/server/src/services/database-backup.service.ts @@ -20,6 +20,7 @@ import { LoggingRepository } from 'src/repositories/logging.repository'; import { ProcessRepository } from 'src/repositories/process.repository'; import { StorageRepository } from 'src/repositories/storage.repository'; import { SystemMetadataRepository } from 'src/repositories/system-metadata.repository'; +import { UserRepository } from 'src/repositories/user.repository'; import { getConfig } from 'src/utils/config'; import { findDatabaseBackupVersion, @@ -40,6 +41,7 @@ export class DatabaseBackupService { private readonly systemMetadataRepository: SystemMetadataRepository, private readonly processRepository: ProcessRepository, private readonly databaseRepository: DatabaseRepository, + private readonly userRepository: UserRepository, @Optional() private readonly cronRepository: CronRepository, @Optional() @@ -405,7 +407,14 @@ export class DatabaseBackupService { try { progressCb?.('migrations', 0.9); + await this.databaseRepository.runMigrations(); + + const hasAdmin = await this.userRepository.hasAdmin(); + if (!hasAdmin) { + throw new Error('Server health check failed, no admin exists.'); + } + await this.maintenanceHealthRepository.checkApiHealth(); } catch (error) { progressCb?.('rollback', 0);