diff --git a/server/src/demo/demo-reset.ts b/server/src/demo/demo-reset.ts index d5dc70e3b..00ee128d6 100644 --- a/server/src/demo/demo-reset.ts +++ b/server/src/demo/demo-reset.ts @@ -3,7 +3,7 @@ import path from 'path'; import { RequestContext, type EntityManager } from '@mikro-orm/core'; import { readEnv } from '../app-config'; import { DemoRepository, type DemoAdminCredentialsRow, type DemoInstanceKeyRow } from '../db/repositories/DemoRepository'; -import type { DatabaseBackupStrategy } from '../nest/database/database-backup.interface'; +import { DatabaseConnectionLostError, type DatabaseBackupStrategy } from '../nest/database/database-backup.interface'; const dataDir = path.join(__dirname, '../../data'); const baselinePath = path.join(dataDir, 'travel-baseline.db'); @@ -84,11 +84,14 @@ async function resetDemoUser(database: DatabaseBackupStrategy): Promise { } // Close, swap the baseline in, reopen. The reopen runs even when the swap - // fails, so the instance is never left without a connection. + // fails, so the instance is never left without a connection. A failed swap + // alone is logged and the reset skipped; a failed swap whose reopen failed as + // well goes up to the tick's log line, because the connection is gone. let reopenError: unknown; try { ({ reopenError } = await database.replace(baselinePath)); } catch (e: unknown) { + if (e instanceof DatabaseConnectionLostError) throw e; console.error('[Demo Reset] Failed to restore baseline:', e instanceof Error ? e.message : e); return; } diff --git a/server/src/nest/backup/sqlite-database-backup.ts b/server/src/nest/backup/sqlite-database-backup.ts index 63e72fec4..d40686351 100644 --- a/server/src/nest/backup/sqlite-database-backup.ts +++ b/server/src/nest/backup/sqlite-database-backup.ts @@ -3,7 +3,11 @@ import path from 'path'; import { Injectable } from '@nestjs/common'; import { MaintenanceRepository } from '../../db/repositories/MaintenanceRepository'; import { DatabaseLifecycle } from '../database/database-lifecycle.service'; -import type { DatabaseBackupRefusal, DatabaseBackupStrategy } from '../database/database-backup.interface'; +import { + DatabaseConnectionLostError, + type DatabaseBackupRefusal, + type DatabaseBackupStrategy, +} from '../database/database-backup.interface'; import { logInfo, logWarn } from '../audit/audit-log.logger'; import { checkBackupDatabase } from './backup-archive'; @@ -81,7 +85,8 @@ export class SqliteDatabaseBackup implements DatabaseBackupStrategy { async replace(source: string): Promise<{ reopenError: unknown }> { const dest = this.lifecycle.file; this.lifecycle.close(); - let reopenError: unknown = null; + let swapFailed = false; + let swapError: unknown = null; try { // Copy to a temp file on the SAME filesystem, drop the old sidecars (they // belong to the database being replaced and would corrupt the new one), @@ -96,15 +101,25 @@ export class SqliteDatabaseBackup implements DatabaseBackupStrategy { } } fs.renameSync(tmp, dest); - } finally { - // Reopening must always run, even when the swap threw, so the process is - // never left without a connection. A reopen failure is reported, not - // thrown: the files already landed and the caller has to say "restart". - try { - await this.lifecycle.reopen(); - } catch (err) { - reopenError = err; - } + } catch (err) { + swapFailed = true; + swapError = err; + } + + // Reopening must always run, even when the swap threw, so the process is + // never left without a connection. After a swap that landed, a reopen + // failure is reported, not thrown: the files are in place and the caller + // has to say "restart". After a swap that failed, both errors go up + // together, because the swap's alone would hide that the connection is gone. + let reopenError: unknown = null; + try { + await this.lifecycle.reopen(); + } catch (err) { + reopenError = err; + } + if (swapFailed) { + if (reopenError) throw new DatabaseConnectionLostError(swapError, reopenError); + throw swapError; } return { reopenError }; } diff --git a/server/src/nest/database/database-backup.interface.ts b/server/src/nest/database/database-backup.interface.ts index 886c7b876..c30849a8d 100644 --- a/server/src/nest/database/database-backup.interface.ts +++ b/server/src/nest/database/database-backup.interface.ts @@ -58,7 +58,8 @@ export interface DatabaseBackupStrategy { * Puts `source` (a database this strategy wrote or verified) in place of the * live one, then reopens the connection, migrating it forward. The reopen * runs even when the swap throws, so the process is never left without a - * connection; the swap's error is rethrown after it. + * connection; the swap's error is rethrown after it. When that reopen fails + * as well, it throws a `DatabaseConnectionLostError` instead, carrying both. * * Resolves with `reopenError` set when the new database landed but the * connection could not be reopened: the caller reports "restart required". @@ -74,3 +75,25 @@ export interface DatabaseBackupRefusal { /** Injection token for the active `DatabaseBackupStrategy`. */ export const DATABASE_BACKUP = Symbol('DATABASE_BACKUP'); + +const describeError = (err: unknown): string => (err instanceof Error ? err.message : String(err)); + +/** + * Thrown by `replace` when the swap failed and the reopen that always follows + * failed too. The process is left without a database connection until a + * restart, so this must reach a log line rather than read as an ordinary failed + * swap. `swapError` is why the swap failed; `cause` is the reopen failure. + */ +export class DatabaseConnectionLostError extends Error { + constructor( + readonly swapError: unknown, + reopenError: unknown, + ) { + super( + `The database could not be replaced (${describeError(swapError)}) and the connection could not be reopened ` + + `(${describeError(reopenError)}). Restart the server.`, + { cause: reopenError }, + ); + this.name = 'DatabaseConnectionLostError'; + } +} diff --git a/server/tests/unit/demo/demo-reset-path.test.ts b/server/tests/unit/demo/demo-reset-path.test.ts index 39f38e67d..1a2a98126 100644 --- a/server/tests/unit/demo/demo-reset-path.test.ts +++ b/server/tests/unit/demo/demo-reset-path.test.ts @@ -32,6 +32,7 @@ import { SqliteDatabaseBackup } from '../../../src/nest/backup/sqlite-database-b import type { DatabaseLifecycle } from '../../../src/nest/database/database-lifecycle.service'; import type { MaintenanceRepository } from '../../../src/db/repositories/MaintenanceRepository'; import { resetDemoUser, saveBaseline } from '../../../src/demo/demo-reset'; +import { DatabaseConnectionLostError } from '../../../src/nest/database/database-backup.interface'; const LIVE_DB = path.join(path.sep, 'srv', 'trek', 'custom-name.db'); const BASELINE = path.resolve(__dirname, '..', '..', '..', 'data', 'travel-baseline.db'); @@ -128,4 +129,16 @@ describe('demo-reset DB path', () => { await expect(withCtx(() => resetDemoUser(database))).rejects.toThrow('database is locked'); }); + + it('DEMORESET-007: a swap that fails with a reopen that fails too is not swallowed either', async () => { + copyFileSync.mockImplementation(() => { throw new Error('EACCES'); }); + lifecycle.reopen.mockRejectedValueOnce(new Error('database is locked')); + + // The instance has no connection now: the tick has to log that, so the + // reset does not end as a plain "failed to restore" line. + await expect(withCtx(() => resetDemoUser(database))).rejects.toBeInstanceOf(DatabaseConnectionLostError); + + expect(console.error).not.toHaveBeenCalledWith('[Demo Reset] Failed to restore baseline:', expect.anything()); + expect(renameSync).not.toHaveBeenCalled(); + }); }); diff --git a/server/tests/unit/nest/sqlite-database-backup.test.ts b/server/tests/unit/nest/sqlite-database-backup.test.ts index 32bc041e1..b8d969b28 100644 --- a/server/tests/unit/nest/sqlite-database-backup.test.ts +++ b/server/tests/unit/nest/sqlite-database-backup.test.ts @@ -17,6 +17,7 @@ const logMock = vi.hoisted(() => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logWar vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock); import { SqliteDatabaseBackup } from '../../../src/nest/backup/sqlite-database-backup'; +import { DatabaseConnectionLostError } from '../../../src/nest/database/database-backup.interface'; import { MaintenanceRepository } from '../../../src/db/repositories/MaintenanceRepository'; import type { DatabaseLifecycle } from '../../../src/nest/database/database-lifecycle.service'; import { withRequestContext } from '../../../src/nest/database/request-context'; @@ -213,5 +214,36 @@ describe('SqliteDatabaseBackup', () => { expect(lifecycle.reopen).toHaveBeenCalledTimes(1); expect(readLabel(liveFile)).toBe('live'); }); + + it('SQLBK-013: a swap that fails and a reopen that fails too both go up, so the lost connection is not hidden', async () => { + writeDb(liveFile, 'live'); + const reopenFailure = new Error('database is locked'); + lifecycle.reopen.mockRejectedValueOnce(reopenFailure); + + const thrown = await portWith(maintenanceStub()) + .replace(path.join(dir, 'does-not-exist.db')) + .then( + () => null, + (err: unknown) => err, + ); + + expect(thrown).toBeInstanceOf(DatabaseConnectionLostError); + const lost = thrown as DatabaseConnectionLostError; + expect((lost.swapError as Error).message).toMatch(/ENOENT/); + expect(lost.cause).toBe(reopenFailure); + expect(lost.message).toMatch(/could not be replaced \(ENOENT.*\) and the connection could not be reopened \(database is locked\)\. Restart the server\.$/); + expect(lifecycle.reopen).toHaveBeenCalledTimes(1); + expect(readLabel(liveFile)).toBe('live'); + }); + }); + + it('SQLBK-014: the lost-connection error names values that are not Errors as they are', () => { + const lost = new DatabaseConnectionLostError('disk gone', 42); + expect(lost.name).toBe('DatabaseConnectionLostError'); + expect(lost.swapError).toBe('disk gone'); + expect(lost.cause).toBe(42); + expect(lost.message).toBe( + 'The database could not be replaced (disk gone) and the connection could not be reopened (42). Restart the server.', + ); }); });