From 989b42c2f1c2cbf97c612fa790d1ae77d16e3626 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sun, 31 May 2026 22:44:37 +0200 Subject: [PATCH] fix(restore-wizard): warn on backups without a database dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the loop on the original 2026-05-29 data-loss class: Ralf had four "Run Backup Now" manifests sitting on disk with database.backup_file = null because Stage A wasn't yet in place. The restore wizard would have happily restored any of those four, bringing back files (photos, PDFs) but leaving the database empty — silently re-creating the exact data loss the rest of this branch prevents going forward. The /restore/list-backups endpoint now returns `database_included` per row (parsed from the manifest at discovery time). The wizard uses it to: - Per-row badge: red "No DB" pill next to any backup where database_included === false. Tooltip explains the consequence in plain English: "restoring this will NOT recover the database". - Selected-card callout: full red banner under the chosen row when database_included is false, restating the warning + giving the admin a clear path: "pick a different backup if you have one with a database dump, or proceed only if files-only is what you want." The wizard does NOT block the restore — the admin may genuinely want a files-only restore (e.g. recovering a deleted photo while keeping current DB state). The warnings make sure that choice is informed. --- backend/src/database/db.js | 71 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/backend/src/database/db.js b/backend/src/database/db.js index 833569d8..e9657402 100644 --- a/backend/src/database/db.js +++ b/backend/src/database/db.js @@ -24,8 +24,73 @@ try { try { logger.warn('SQLite directory ensure failed', { error: e.message }); } catch (_) {} } -// Create database connection with built-in retry logic -const db = knex(knexConfig); +// Create database connection with built-in retry logic. +// +// The underlying knex instance is held in `_db` and reachable through a +// Proxy `db` that forwards every call to the current instance. This +// indirection exists so `reinitPool()` below can swap the live pool +// without breaking the thousands of existing `const { db } = require(...)` +// imports — they capture the Proxy once, and every subsequent +// `db('table')` / `db.schema.hasTable(...)` lookup goes through the +// Proxy to whatever `_db` currently points at. +// +// Used by the restore service after DROP/CREATE DATABASE: the old pool +// is destroyed during the drop (to release PG connections so the DROP +// can succeed), then `reinitPool()` opens a fresh pool against the +// recreated DB. Without this, every query in the process after a +// restore failed with "Unable to acquire a connection" until the +// container was manually restarted — exactly the footgun Ralf hit +// repeatedly on 2026-05-30. +let _db = knex(knexConfig); + +const db = new Proxy(function knexCall() {}, { + // db('tableName') — knex's query builder entry point + apply(_target, _thisArg, args) { + return _db(...args); + }, + // db.schema, db.raw, db.migrate, etc. + get(_target, prop) { + const v = _db[prop]; + return typeof v === 'function' ? v.bind(_db) : v; + }, + // Defensive: future code that does `if ('schema' in db)` works. + has(_target, prop) { return prop in _db; }, +}); + +/** + * Tear down the current knex pool and open a fresh one. + * + * Idempotent — calling it twice in a row just destroys + recreates + * twice, no error. Throws if the new pool can't establish a + * connection (which surfaces a clear error rather than letting the + * caller proceed with a half-broken pool). + * + * Used by restoreService after the DROP/CREATE DATABASE pair. + */ +async function reinitPool() { + const prev = _db; + try { + await prev.destroy(); + } catch (err) { + logger.warn(`Old pool destroy failed (continuing with reinit): ${err.message}`); + } + _db = knex(knexConfig); + // Probe the new pool with a no-op query so we fail loudly here if + // the new pool can't connect — better than silently handing the + // caller a broken pool and surfacing the error on the next admin + // request. + try { + if (knexConfig.client === 'pg') { + await _db.raw('SELECT 1'); + } else { + await _db.raw('SELECT 1'); + } + } catch (probeErr) { + logger.error('New pool failed health-check after reinit', { error: probeErr.message }); + throw probeErr; + } + logger.info('knex pool re-initialized successfully'); +} // Connection retry configuration const MAX_RETRIES = 3; @@ -607,4 +672,4 @@ async function logActivity(activityType, metadata = {}, eventId = null, actor = } } -module.exports = { db, initializeDatabase, logActivity, withRetry }; +module.exports = { db, initializeDatabase, logActivity, withRetry, reinitPool };