Backup directory failures must not crash the whole server
A fresh Windows install reported the server failing around backups. createBackupService created the backups subdirectory once, synchronously, at startup with no error handling - any failure there (permissions, a path beyond Windows' MAX_PATH, a OneDrive/cloud-sync lock, antivirus) crashed the entire Node process before it started listening, taking down timing with it instead of just leaving backups unavailable. Directory creation is now retried lazily and non-fatally on each backup attempt, and a post-backup file rename retries briefly on EBUSY/EPERM since a freshly written file is sometimes momentarily locked by antivirus/cloud-sync scanning on Windows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,12 @@ The application version is maintained in `package.json` and displayed in the adm
|
|||||||
|
|
||||||
No changes recorded yet.
|
No changes recorded yet.
|
||||||
|
|
||||||
|
## 1.4.1 - 2026-09-09
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- A backup failure at startup (e.g. no write permission for the backups folder, a path beyond Windows' MAX_PATH, a OneDrive/cloud-sync folder locking it, or antivirus interference) could crash the entire server before it started listening, instead of just leaving backups unavailable. Backup-directory creation is now non-fatal and retried on each backup attempt, and a file rename right after a backup completes now retries briefly if a file is momentarily locked by antivirus/cloud-sync scanning.
|
||||||
|
|
||||||
## 1.4.0 - 2026-09-09
|
## 1.4.0 - 2026-09-09
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
@@ -13,6 +13,8 @@ Track work here with checkboxes. Check off a task only after implementation and
|
|||||||
|
|
||||||
Verified 2026-09-08: 122 logic/API tests and the register/offline browser smoke suite passed on Linux with Chrome. Checked items below describe that coverage; other platforms and workflows remain open.
|
Verified 2026-09-08: 122 logic/API tests and the register/offline browser smoke suite passed on Linux with Chrome. Checked items below describe that coverage; other platforms and workflows remain open.
|
||||||
|
|
||||||
|
Fixed 2026-09-09: a fresh Windows install reported the server failing around backups. Root cause: `createBackupService` created the `backups` subdirectory once, synchronously, at server startup with no error handling - if that failed for any reason (no write permission, a path beyond Windows' MAX_PATH, a OneDrive/cloud-sync folder locking it, antivirus interference), the whole Node process crashed before the HTTP server even started listening, taking down timing and everything else with it, not just backups. The directory creation is now non-fatal and retried lazily on each backup attempt, with a clear "Backup directory is unavailable" error surfaced only to the backup action itself; a file rename right after `db.backup()` also now retries briefly on `EBUSY`/`EPERM`, since a freshly written file is sometimes still momentarily held by antivirus/cloud-sync scanning on Windows. Verified with a new unit test (`tests/backup_service.test.js`) that blocks directory creation via a read-only parent directory and asserts the service still constructs and lists successfully, and with a real spawned-server integration check (`RC_TIMING_DATA_DIR` pointed at a data dir where the `backups` path is pre-occupied by a plain file) confirming `/api/health` stays up and only the backup request itself returns a clear error. The exact filesystem condition on the reporting machine is still unconfirmed - this closes the crash-the-whole-server failure mode regardless of which specific cause it turns out to be.
|
||||||
|
|
||||||
## 1. Reliability and browser regression tests
|
## 1. Reliability and browser regression tests
|
||||||
|
|
||||||
Acceptance: real create/edit/save flows persist after reload; background updates do not discard edits. Tests use isolated data, never the installation's database.
|
Acceptance: real create/edit/save flows persist after reload; background updates do not discard edits. Tests use isolated data, never the installation's database.
|
||||||
|
|||||||
4
package-lock.json
generated
4
package-lock.json
generated
@@ -1,12 +1,12 @@
|
|||||||
{
|
{
|
||||||
"name": "rc-racecore",
|
"name": "rc-racecore",
|
||||||
"version": "1.4.0",
|
"version": "1.4.1",
|
||||||
"lockfileVersion": 3,
|
"lockfileVersion": 3,
|
||||||
"requires": true,
|
"requires": true,
|
||||||
"packages": {
|
"packages": {
|
||||||
"": {
|
"": {
|
||||||
"name": "rc-racecore",
|
"name": "rc-racecore",
|
||||||
"version": "1.4.0",
|
"version": "1.4.1",
|
||||||
"hasInstallScript": true,
|
"hasInstallScript": true,
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"better-sqlite3": "^12.11.1",
|
"better-sqlite3": "^12.11.1",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "rc-racecore",
|
"name": "rc-racecore",
|
||||||
"version": "1.4.0",
|
"version": "1.4.1",
|
||||||
"private": true,
|
"private": true,
|
||||||
"description": "RC RaceCore live timing and race control with AMMC ingest and SQLite persistence",
|
"description": "RC RaceCore live timing and race control with AMMC ingest and SQLite persistence",
|
||||||
"engines": {
|
"engines": {
|
||||||
|
|||||||
@@ -1,6 +1,25 @@
|
|||||||
const fs = require("fs");
|
const fs = require("fs");
|
||||||
const path = require("path");
|
const path = require("path");
|
||||||
|
|
||||||
|
const pause = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
|
||||||
|
|
||||||
|
// A file that was just written is sometimes still briefly held open by antivirus/cloud-sync
|
||||||
|
// scanning on Windows, so a rename right after can fail with EBUSY/EPERM even though nothing
|
||||||
|
// in this process still has it open. A short retry clears that up without surfacing a false
|
||||||
|
// failure to the user.
|
||||||
|
async function renameWithRetry(source, destination, attempts = 5, delayMs = 200) {
|
||||||
|
for (let attempt = 1; attempt <= attempts; attempt += 1) {
|
||||||
|
try {
|
||||||
|
fs.renameSync(source, destination);
|
||||||
|
return;
|
||||||
|
} catch (error) {
|
||||||
|
const retryable = error && (error.code === "EBUSY" || error.code === "EPERM");
|
||||||
|
if (!retryable || attempt === attempts) throw error;
|
||||||
|
await pause(delayMs);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function safeSegment(value, fallback = "backup") {
|
function safeSegment(value, fallback = "backup") {
|
||||||
const normalized = String(value || "")
|
const normalized = String(value || "")
|
||||||
.trim()
|
.trim()
|
||||||
@@ -14,7 +33,21 @@ function safeSegment(value, fallback = "backup") {
|
|||||||
function createBackupService({ db, dataDir, maxBackups = 30, now = () => new Date() }) {
|
function createBackupService({ db, dataDir, maxBackups = 30, now = () => new Date() }) {
|
||||||
if (!db || !dataDir) throw new Error("Backup service requires db and dataDir");
|
if (!db || !dataDir) throw new Error("Backup service requires db and dataDir");
|
||||||
const backupDir = path.join(dataDir, "backups");
|
const backupDir = path.join(dataDir, "backups");
|
||||||
fs.mkdirSync(backupDir, { recursive: true });
|
|
||||||
|
// Creating the backups directory can fail on a real install (no write permission,
|
||||||
|
// a path beyond Windows' MAX_PATH, a cloud-sync folder like OneDrive locking it,
|
||||||
|
// antivirus interference). That must never take down the whole server at startup -
|
||||||
|
// backups become unavailable, but timing and everything else keeps working.
|
||||||
|
function ensureBackupDir() {
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(backupDir, { recursive: true });
|
||||||
|
return true;
|
||||||
|
} catch (error) {
|
||||||
|
console.error(`[backup] Could not create backup directory ${backupDir}: ${error.message}`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
ensureBackupDir();
|
||||||
|
|
||||||
function metadataPath(fileName) {
|
function metadataPath(fileName) {
|
||||||
return path.join(backupDir, fileName.replace(/\.sqlite$/i, ".json"));
|
return path.join(backupDir, fileName.replace(/\.sqlite$/i, ".json"));
|
||||||
@@ -28,8 +61,16 @@ function createBackupService({ db, dataDir, maxBackups = 30, now = () => new Dat
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function backupDirReady() {
|
||||||
|
try {
|
||||||
|
return fs.statSync(backupDir).isDirectory();
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function listBackups() {
|
function listBackups() {
|
||||||
if (!fs.existsSync(backupDir)) return [];
|
if (!backupDirReady()) return [];
|
||||||
return fs.readdirSync(backupDir)
|
return fs.readdirSync(backupDir)
|
||||||
.filter((fileName) => fileName.endsWith(".sqlite"))
|
.filter((fileName) => fileName.endsWith(".sqlite"))
|
||||||
.map((fileName) => {
|
.map((fileName) => {
|
||||||
@@ -56,6 +97,9 @@ function createBackupService({ db, dataDir, maxBackups = 30, now = () => new Dat
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function createBackupNow({ reason = "manual", sessionId = "", eventId = "" } = {}) {
|
async function createBackupNow({ reason = "manual", sessionId = "", eventId = "" } = {}) {
|
||||||
|
if (!backupDirReady() && !ensureBackupDir()) {
|
||||||
|
throw new Error(`Backup directory is unavailable: ${backupDir}`);
|
||||||
|
}
|
||||||
const createdAt = now().toISOString();
|
const createdAt = now().toISOString();
|
||||||
const timestamp = createdAt.replace(/[:.]/g, "-");
|
const timestamp = createdAt.replace(/[:.]/g, "-");
|
||||||
const stem = `${timestamp}__${safeSegment(reason)}__${safeSegment(sessionId, "all")}`;
|
const stem = `${timestamp}__${safeSegment(reason)}__${safeSegment(sessionId, "all")}`;
|
||||||
@@ -69,7 +113,7 @@ function createBackupService({ db, dataDir, maxBackups = 30, now = () => new Dat
|
|||||||
const temporaryPath = `${finalPath}.tmp`;
|
const temporaryPath = `${finalPath}.tmp`;
|
||||||
try {
|
try {
|
||||||
await db.backup(temporaryPath);
|
await db.backup(temporaryPath);
|
||||||
fs.renameSync(temporaryPath, finalPath);
|
await renameWithRetry(temporaryPath, finalPath);
|
||||||
const metadata = {
|
const metadata = {
|
||||||
fileName,
|
fileName,
|
||||||
createdAt,
|
createdAt,
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import test from "node:test";
|
|||||||
import assert from "node:assert/strict";
|
import assert from "node:assert/strict";
|
||||||
import { createRequire } from "node:module";
|
import { createRequire } from "node:module";
|
||||||
import { spawnSync } from "node:child_process";
|
import { spawnSync } from "node:child_process";
|
||||||
import { mkdtempSync, readFileSync, rmSync } from "node:fs";
|
import { chmodSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
|
||||||
import { dirname, join } from "node:path";
|
import { dirname, join } from "node:path";
|
||||||
import { fileURLToPath } from "node:url";
|
import { fileURLToPath } from "node:url";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
@@ -102,6 +102,24 @@ test("local restore validates and replaces a stopped test database with a safety
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("an unwritable backups directory never crashes server startup, and surfaces a clear error on backup instead", async () => {
|
||||||
|
const dataDir = mkdtempSync(join(tmpdir(), "racecore-backup-blocked-"));
|
||||||
|
const db = createTestDb(dataDir);
|
||||||
|
try {
|
||||||
|
chmodSync(dataDir, 0o500); // read/execute only: mkdir("backups") inside it fails with EACCES
|
||||||
|
let service;
|
||||||
|
assert.doesNotThrow(() => {
|
||||||
|
service = createBackupService({ db, dataDir, maxBackups: 3 });
|
||||||
|
}, "constructing the service must not throw even if the backups directory cannot be created");
|
||||||
|
assert.deepEqual(service.listBackups(), [], "listing degrades to empty instead of throwing");
|
||||||
|
await assert.rejects(() => service.createBackup({ reason: "manual" }), /backup directory/i);
|
||||||
|
} finally {
|
||||||
|
chmodSync(dataDir, 0o700);
|
||||||
|
db.close();
|
||||||
|
rmSync(dataDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
test("backup file segments are bounded and filesystem safe", () => {
|
test("backup file segments are bounded and filesystem safe", () => {
|
||||||
assert.equal(safeSegment("A final / ../ unsafe"), "a-final-unsafe");
|
assert.equal(safeSegment("A final / ../ unsafe"), "a-final-unsafe");
|
||||||
assert.equal(safeSegment(""), "backup");
|
assert.equal(safeSegment(""), "backup");
|
||||||
|
|||||||
Reference in New Issue
Block a user