fix(backup): report a lost connection when the swap and its reopen both fail

SqliteDatabaseBackup.replace reopens the connection in a finally block, so
it runs even when the swap throws. A reopen that failed there was stored
and then dropped: the swap's exception propagated past the return that
would have carried it. The demo reset caught that swap error, logged
"Failed to restore baseline" and returned normally, leaving the instance
without a database connection and nothing in the log to say so. Before
the port, the same branch awaited reinitialize() and its rejection reached
the tick's log line.

replace now throws a DatabaseConnectionLostError when both steps fail. It
carries the swap error and has the reopen failure as its cause, and its
message tells the operator to restart. The demo reset lets that error
through to DemoResetJob's tick, which logs it, and still logs and skips a
failed swap whose reopen succeeded. A restore that hits it answers the
same 500 as any other thrown restore error, with both failures in the
server log.
This commit is contained in:
Maurice
2026-10-08 15:40:00 +02:00
parent fd28ba7fd3
commit 175f5d96da
5 changed files with 100 additions and 14 deletions
+5 -2
View File
@@ -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<void> {
}
// 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;
}
@@ -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 };
}
@@ -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';
}
}
@@ -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();
});
});
@@ -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.',
);
});
});