Skip to content

refactor(backup): convert sync fs operations to async promises #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BillChirico merged 4 commits into from
Mar 3, 2026
Merged

refactor(backup): convert sync fs operations to async promises #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d9d6ad
refactor(backup): convert sync fs operations to async promises
Mar 3, 2026
fc7373a
fix(backup): await async operations in API routes
Mar 3, 2026
38e0829
fix(backup): await async operations in scheduled backup interval
Mar 3, 2026
5db6669
Merge main into backup-async branch
Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/api/routes/backup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ router.post(
router.get(
'/',
(req, res, next) => requireGlobalAdmin('Backup access', req, res, next),
(_req, res) => {
const backups = listBackups();
async (_req, res) => {
const backups = await listBackups();
res.json(backups);
},
);
Expand Down Expand Up @@ -229,9 +229,9 @@ router.get(
router.post(
'/',
(req, res, next) => requireGlobalAdmin('Backup access', req, res, next),
(_req, res) => {
async (_req, res) => {
try {
const meta = createBackup();
const meta = await createBackup();
return res.status(201).json({ id: meta.id, size: meta.size, createdAt: meta.createdAt });
} catch (err) {
return res.status(500).json({ error: 'Failed to create backup', details: err.message });
Expand Down Expand Up @@ -278,11 +278,11 @@ router.post(
router.get(
'/:id/download',
(req, res, next) => requireGlobalAdmin('Backup access', req, res, next),
(req, res) => {
async (req, res) => {
const { id } = req.params;

try {
const payload = readBackup(id);
const payload = await readBackup(id);
const filename = `${id}.json`;
res.setHeader('Content-Disposition', `attachment; filename="${filename}"`);
res.setHeader('Content-Type', 'application/json');
Expand Down Expand Up @@ -420,7 +420,7 @@ router.post(
router.post(
'/prune',
(req, res, next) => requireGlobalAdmin('Backup access', req, res, next),
(req, res) => {
async (req, res) => {
const retention = req.body ?? {};
const errors = [];

Expand All @@ -439,7 +439,7 @@ router.post(
return res.status(400).json({ error: 'Invalid prune options', details: errors });
}

const deleted = pruneBackups(retention);
const deleted = await pruneBackups(retention);
return res.json({ deleted, count: deleted.length });
},
);
Expand Down
87 changes: 40 additions & 47 deletions src/modules/backup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,7 @@
* @see https://github.com/VolvoxLLC/volvox-bot/issues/129
*/

import {
existsSync,
mkdirSync,
readdirSync,
readFileSync,
statSync,
unlinkSync,
writeFileSync,
} from 'node:fs';

// TODO: Consider switching to fs.promises for async operations to improve performance
// and avoid blocking the event loop with synchronous file system operations.
import { access, constants, mkdir, readdir, readFile, stat, unlink, writeFile } from 'node:fs/promises';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { SAFE_CONFIG_KEYS, SENSITIVE_FIELDS } from '../api/utils/configAllowlist.js';
Expand Down Expand Up @@ -45,13 +34,12 @@
* Get or create the backup directory.
*
* @param {string} [dir] - Override backup directory path
* @returns {string} The backup directory path
* @returns {Promise<string>} The backup directory path
*/
export function getBackupDir(dir) {
export async function getBackupDir(dir) {
const backupDir = dir ?? DEFAULT_BACKUP_DIR;
if (!existsSync(backupDir)) {
mkdirSync(backupDir, { recursive: true });
}
// Use recursive mkdir to avoid race condition between check and create
await mkdir(backupDir, { recursive: true });

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-7Apl7a'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.createBackup src/modules/backup.js:186:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-7Apl7a' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-fHzWv3'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.readBackup src/modules/backup.js:282:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-fHzWv3' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-OjBSih'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.readBackup src/modules/backup.js:282:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-OjBSih' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-aiS5qc'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.createBackup src/modules/backup.js:186:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-aiS5qc' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/empty-backup-vz2sPN'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.listBackups src/modules/backup.js:247:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/empty-backup-vz2sPN' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-BmSkWW'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.listBackups src/modules/backup.js:247:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-BmSkWW' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-BmSkWW'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.createBackup src/modules/backup.js:186:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-BmSkWW' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 42 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: ENOENT: no such file or directory, mkdir '/tmp/backup-test-IMZHEu'
 ❯ getBackupDir src/modules/backup.js:42:3
 ❯ Module.createBackup src/modules/backup.js:186:15

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { errno: -2, code: 'ENOENT', syscall: 'mkdir', path: '/tmp/backup-test-IMZHEu' }
This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "lists created backups sorted newest first". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
return backupDir;
}

Expand Down Expand Up @@ -192,28 +180,28 @@
* Create a timestamped backup of the current config in the backup directory.
*
* @param {string} [backupDir] - Override backup directory
* @returns {{id: string, path: string, size: number, createdAt: string}} Backup metadata
* @returns {Promise<{id: string, path: string, size: number, createdAt: string}>} Backup metadata
*/
export function createBackup(backupDir) {
const dir = getBackupDir(backupDir);
export async function createBackup(backupDir) {
const dir = await getBackupDir(backupDir);
const now = new Date();
const filename = makeBackupFilename(now);
const filePath = path.join(dir, filename);

const payload = exportConfig();
const json = JSON.stringify(payload, null, 2);

writeFileSync(filePath, json, 'utf8');
await writeFile(filePath, json, 'utf8');

const { size } = statSync(filePath);
const stats = await stat(filePath);
const id = filename.replace('.json', '');

info('Config backup created', { id, path: filePath, size });
info('Config backup created', { id, path: filePath, size: stats.size });

return {
id,
path: filePath,
size,
size: stats.size,
createdAt: now.toISOString(),
};
}
Expand All @@ -223,16 +211,17 @@
*
* @param {string} filename - Backup filename
* @param {string} dir - Directory containing the backup file
* @returns {{id: string, filename: string, createdAt: string, size: number} | null}
* @returns {Promise<{id: string, filename: string, createdAt: string, size: number} | null>}
*/
function parseBackupMeta(filename, dir) {
async function parseBackupMeta(filename, dir) {
const match = BACKUP_FILENAME_PATTERN.exec(filename);
if (!match) return null;

const filePath = path.join(dir, filename);
let size = 0;
try {
size = statSync(filePath).size;
const stats = await stat(filePath);
size = stats.size;
} catch {
return null;
}
Expand All @@ -252,20 +241,22 @@
* List all available backups, sorted newest first.
*
* @param {string} [backupDir] - Override backup directory
* @returns {Array<{id: string, filename: string, createdAt: string, size: number}>}
* @returns {Promise<Array<{id: string, filename: string, createdAt: string, size: number}>>}
*/
export function listBackups(backupDir) {
const dir = getBackupDir(backupDir);
export async function listBackups(backupDir) {
const dir = await getBackupDir(backupDir);

let files;
try {
files = readdirSync(dir);
files = await readdir(dir);
} catch {
return [];
}

const backups = files
.map((filename) => parseBackupMeta(filename, dir))
const backupMetaPromises = files.map((filename) => parseBackupMeta(filename, dir));
const results = await Promise.all(backupMetaPromises);

const backups = results
.filter(Boolean)
.sort((a, b) => new Date(b.createdAt) - new Date(a.createdAt));

Expand All @@ -277,26 +268,28 @@
*
* @param {string} id - Backup ID (filename without .json)
* @param {string} [backupDir] - Override backup directory
* @returns {Object} Parsed backup payload
* @returns {Promise<Object>} Parsed backup payload
* @throws {Error} If backup file not found or invalid
*/
export function readBackup(id, backupDir) {
export async function readBackup(id, backupDir) {
// Validate ID against strict pattern: backup-YYYY-MM-DDTHH-mm-ss-SSS-NNNN
const BACKUP_ID_PATTERN =
/^backup-[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}-[0-9]{2}-[0-9]{2}-[0-9]{3}-[0-9]{4}$/;
if (!BACKUP_ID_PATTERN.test(id)) {
throw new Error('Invalid backup ID');

Check failure on line 279 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: Invalid backup ID
 ❯ Module.readBackup src/modules/backup.js:279:11
 ❯ tests/modules/backup.test.js:237:18
 ❯ tests/modules/backup.test.js:237:55

This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 279 in src/modules/backup.js

View workflow job for this annotation

GitHub Actions / Test (Vitest Coverage)

Unhandled error 

Error: Invalid backup ID
 ❯ Module.readBackup src/modules/backup.js:279:11
 ❯ tests/modules/backup.test.js:225:21

This error originated in "tests/modules/backup.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws for invalid backup format". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
}

const dir = getBackupDir(backupDir);
const dir = await getBackupDir(backupDir);
const filename = `${id}.json`;
const filePath = path.join(dir, filename);

if (!existsSync(filePath)) {
try {
await access(filePath, constants.F_OK);
} catch {
throw new Error(`Backup not found: ${id}`);
}

const raw = readFileSync(filePath, 'utf8');
const raw = await readFile(filePath, 'utf8');
try {
return JSON.parse(raw);
} catch {
Expand All @@ -313,7 +306,7 @@
* @throws {Error} If backup not found or invalid
*/
export async function restoreBackup(id, backupDir) {
const payload = readBackup(id, backupDir);
const payload = await readBackup(id, backupDir);

const validationErrors = validateImportPayload(payload);
if (validationErrors.length > 0) {
Expand All @@ -338,12 +331,12 @@
*
* @param {{daily?: number, weekly?: number}} [retention] - Retention counts
* @param {string} [backupDir] - Override backup directory
* @returns {string[]} IDs of deleted backups
* @returns {Promise<string[]>} IDs of deleted backups
*/
export function pruneBackups(retention, backupDir) {
export async function pruneBackups(retention, backupDir) {
const { daily = DEFAULT_RETENTION.daily, weekly = DEFAULT_RETENTION.weekly } = retention ?? {};
const dir = getBackupDir(backupDir);
const all = listBackups(dir);
const dir = await getBackupDir(backupDir);
const all = await listBackups(dir);

if (all.length === 0) return [];

Expand Down Expand Up @@ -372,7 +365,7 @@
for (const backup of all) {
if (!toKeep.has(backup.id)) {
try {
unlinkSync(path.join(dir, backup.filename));
await unlink(path.join(dir, backup.filename));
deleted.push(backup.id);
info('Pruned old backup', { id: backup.id });
} catch (err) {
Expand Down Expand Up @@ -406,10 +399,10 @@

info('Starting scheduled config backups', { intervalMs });

scheduledBackupInterval = setInterval(() => {
scheduledBackupInterval = setInterval(async () => {
try {
createBackup(backupDir);
pruneBackups(retention, backupDir);
await createBackup(backupDir);
await pruneBackups(retention, backupDir);
} catch (err) {
logError('Scheduled backup failed', { error: err.message });
}
Expand Down
Loading