mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-02 21:32:10 +03:00
fix(db): stop legacy log-archive migration from deleting the live app-logger directory and crashing startup on a stat/stream race (#6401) (#6898)
archiveLegacyRequestLogs() swept the entire DATA_DIR/logs directory as a single "legacy" target and recursively deleted it after zipping. Since PR #6234 moved the default app-log path to DATA_DIR/logs/application, the migration was deleting the live file logger's own directory on every boot until its marker file existed (#6799). Separately, yazl's addFile() does an internal stat-then-stream read; if a target file grows between those two steps (e.g. an actively-written log), yazl emits "error" directly on the ZipFile instance. That event had no listener, so Node re-threw it as an uncaughtException that crashed the whole process at startup (#6401) — misdiagnosed upstream as Turbopack/Windows chunk corruption because the stack trace pointed into a bundled chunk. Fix: - listArchiveTargets() now enumerates DATA_DIR/logs entries individually and skips the live app-logger directory (resolved via logEnv.getAppLogFilePath()), so the shared parent directory is never deleted wholesale. - createLegacyArchive() wires a zipFile.on("error", ...) handler so a stat/stream race rejects the promise (caught by the existing try/catch) instead of escaping as an uncaughtException. Regression test: tests/unit/usage-migrations-legacy-archive-safety.test.ts (RED on unfixed code, GREEN after fix). Updated tests/unit/request-log-migration.test.ts to the corrected contract — DATA_DIR/logs itself now survives the archive sweep. Gates run clean: file-size, complexity, cognitive-complexity, changelog-integrity, typecheck:core, eslint (suppressions), and the existing usage-migrations/request-log-migration unit suites.
This commit is contained in:
committed by
GitHub
parent
0f3c68fa69
commit
858598a200
1
changelog.d/fixes/6401-6401-usage-migrations.md
Normal file
1
changelog.d/fixes/6401-6401-usage-migrations.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(db): stop legacy log-archive migration from deleting the live app-logger directory and crashing startup on a stat/stream race (#6401, #6799)
|
||||
@@ -13,6 +13,7 @@ import path from "path";
|
||||
import { ZipFile } from "yazl";
|
||||
import { getDbInstance, isCloud, isBuildPhase, DATA_DIR } from "../db/core";
|
||||
import { getLegacyDotDataDir, isSamePath } from "../dataPaths";
|
||||
import { getAppLogFilePath } from "../logEnv";
|
||||
import { protectPayloadForLog } from "../logPayloads";
|
||||
import { sanitizePII } from "../piiSanitizer";
|
||||
import { writeCallArtifact, type CallLogArtifact } from "./callLogArtifacts";
|
||||
@@ -110,17 +111,45 @@ function ensureArchiveDir() {
|
||||
fs.mkdirSync(LOG_ARCHIVES_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
function listArchiveTargets(): ArchiveTarget[] {
|
||||
/**
|
||||
* Directory the live file logger writes to (`buildLogger()` in
|
||||
* `src/shared/utils/logger.ts`), so the legacy-log sweep below never zips-then-deletes
|
||||
* a directory it does not own. Since PR #6234 (fix for #6197) the default app log path
|
||||
* moved from a `process.cwd()`-anchored location to `DATA_DIR/logs/application/app.log`,
|
||||
* landing it inside the same `DATA_DIR/logs` tree this "legacy" migration sweeps (#6799).
|
||||
*/
|
||||
function getLiveAppLogDir(): string | null {
|
||||
try {
|
||||
return path.dirname(getAppLogFilePath());
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function listRequestLogArchiveEntries(): ArchiveTarget[] {
|
||||
if (!CURRENT_REQUEST_LOGS_DIR || !fs.existsSync(CURRENT_REQUEST_LOGS_DIR)) return [];
|
||||
|
||||
const liveAppLogDir = getLiveAppLogDir();
|
||||
const entries = fs.readdirSync(CURRENT_REQUEST_LOGS_DIR);
|
||||
const targets: ArchiveTarget[] = [];
|
||||
|
||||
if (CURRENT_REQUEST_LOGS_DIR && fs.existsSync(CURRENT_REQUEST_LOGS_DIR)) {
|
||||
for (const entry of entries) {
|
||||
const entryPath = path.join(CURRENT_REQUEST_LOGS_DIR, entry);
|
||||
if (liveAppLogDir && isSamePath(entryPath, liveAppLogDir)) continue;
|
||||
|
||||
targets.push({
|
||||
sourcePath: CURRENT_REQUEST_LOGS_DIR,
|
||||
archiveRoot: "data/logs",
|
||||
sourcePath: entryPath,
|
||||
archiveRoot: path.posix.join("data/logs", entry),
|
||||
deleteAfterArchive: true,
|
||||
});
|
||||
}
|
||||
|
||||
return targets;
|
||||
}
|
||||
|
||||
function listArchiveTargets(): ArchiveTarget[] {
|
||||
const targets: ArchiveTarget[] = listRequestLogArchiveEntries();
|
||||
|
||||
if (CURRENT_REQUEST_SUMMARY_FILE && fs.existsSync(CURRENT_REQUEST_SUMMARY_FILE)) {
|
||||
targets.push({
|
||||
sourcePath: CURRENT_REQUEST_SUMMARY_FILE,
|
||||
@@ -173,11 +202,27 @@ function createLegacyArchive(targets: ArchiveTarget[]): Promise<string> {
|
||||
const zipFile = new ZipFile();
|
||||
const output = fs.createWriteStream(archivePath);
|
||||
|
||||
output.on("close", () => resolve(archiveFilename));
|
||||
output.on("error", (error) => {
|
||||
let settled = false;
|
||||
// yazl detects a stat->stream size mismatch (a file growing while being zipped, e.g.
|
||||
// an actively-written log — #6401) by emitting "error" on the ZipFile instance itself,
|
||||
// not on `output`. Left unwired, that "error" event has no listener and Node re-throws
|
||||
// it as an uncaughtException that crashes the process. Wiring it here converts that
|
||||
// crash into a normal rejection the caller's try/catch already handles.
|
||||
const fail = (error: Error) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
output.destroy();
|
||||
fs.rmSync(archivePath, { force: true });
|
||||
reject(error);
|
||||
};
|
||||
|
||||
output.on("close", () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
resolve(archiveFilename);
|
||||
});
|
||||
output.on("error", fail);
|
||||
zipFile.on("error", fail);
|
||||
zipFile.outputStream.pipe(output);
|
||||
|
||||
try {
|
||||
@@ -186,9 +231,7 @@ function createLegacyArchive(targets: ArchiveTarget[]): Promise<string> {
|
||||
}
|
||||
zipFile.end();
|
||||
} catch (error) {
|
||||
fs.rmSync(archivePath, { force: true });
|
||||
zipFile.end();
|
||||
reject(error);
|
||||
fail(error as Error);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -61,7 +61,11 @@ test("archives legacy request log layout into a zip and removes old files", asyn
|
||||
const archiveFilename = await migrations.archiveLegacyRequestLogs();
|
||||
|
||||
assert.match(archiveFilename || "", /_legacy-request-logs\.zip$/);
|
||||
assert.equal(fs.existsSync(LEGACY_LOGS_DIR), false);
|
||||
// DATA_DIR/logs itself is preserved (not recursively removed) because it is shared
|
||||
// with the live app logger's own logs/application subdirectory since #6234 — only the
|
||||
// individual legacy entries inside it are archived-then-deleted (#6799).
|
||||
assert.equal(fs.existsSync(LEGACY_LOGS_DIR), true);
|
||||
assert.equal(fs.existsSync(path.join(LEGACY_LOGS_DIR, "session-a")), false);
|
||||
assert.equal(fs.existsSync(LEGACY_CALL_LOGS_DIR), false);
|
||||
assert.equal(fs.existsSync(LEGACY_SUMMARY_FILE), false);
|
||||
assert.equal(fs.existsSync(MARKER_PATH), true);
|
||||
|
||||
141
tests/unit/usage-migrations-legacy-archive-safety.test.ts
Normal file
141
tests/unit/usage-migrations-legacy-archive-safety.test.ts
Normal file
@@ -0,0 +1,141 @@
|
||||
// Regression tests for #6401 / #6799.
|
||||
//
|
||||
// #6799: archiveLegacyRequestLogs() treated the ENTIRE DATA_DIR/logs directory as a
|
||||
// single "legacy request log" archive target and recursively deleted it after zipping —
|
||||
// including DATA_DIR/logs/application/, which is the live file-logger's own directory
|
||||
// since PR #6234 moved the default APP_LOG_FILE_PATH to DATA_DIR/logs/application/app.log.
|
||||
//
|
||||
// #6401: yazl's addFile() does an internal stat()-then-stream-read (TOCTOU). Because the
|
||||
// live logger keeps appending to files under the swept directory while the migration is
|
||||
// zipping them, the file can grow between yazl's stat and its later read, and yazl throws
|
||||
// "file data stream has unexpected number of bytes" as an error event on a stream that
|
||||
// was never wired into archiveLegacyRequestLogs()'s own try/catch — surfacing as an
|
||||
// uncaught exception that crashes the process at boot.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const ORIGINAL_HOME = process.env.HOME;
|
||||
const ORIGINAL_USERPROFILE = process.env.USERPROFILE;
|
||||
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
|
||||
const ORIGINAL_NEXT_PHASE = process.env.NEXT_PHASE;
|
||||
|
||||
const TEST_HOME_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-6401-6799-home-"));
|
||||
const TEST_DATA_DIR = path.join(TEST_HOME_DIR, "data");
|
||||
|
||||
process.env.HOME = TEST_HOME_DIR;
|
||||
process.env.USERPROFILE = TEST_HOME_DIR;
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
delete process.env.NEXT_PHASE;
|
||||
|
||||
const migrations = await import("../../src/lib/usage/migrations.ts");
|
||||
const { getDbInstance, resetDbInstance } = await import("../../src/lib/db/core.ts");
|
||||
|
||||
const APP_LOG_DIR = path.join(TEST_DATA_DIR, "logs", "application");
|
||||
const APP_LOG_FILE = path.join(APP_LOG_DIR, "app.log");
|
||||
const CURRENT_REQUEST_LOGS_DIR = path.join(TEST_DATA_DIR, "logs");
|
||||
|
||||
test.before(() => {
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
try {
|
||||
const db = getDbInstance();
|
||||
if (db?.open) db.close();
|
||||
} catch {
|
||||
// Database may already be closed.
|
||||
}
|
||||
resetDbInstance?.();
|
||||
|
||||
if (ORIGINAL_HOME === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = ORIGINAL_HOME;
|
||||
if (ORIGINAL_USERPROFILE === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = ORIGINAL_USERPROFILE;
|
||||
if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
|
||||
else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
|
||||
if (ORIGINAL_NEXT_PHASE === undefined) delete process.env.NEXT_PHASE;
|
||||
else process.env.NEXT_PHASE = ORIGINAL_NEXT_PHASE;
|
||||
|
||||
fs.rmSync(TEST_HOME_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("#6799: archiveLegacyRequestLogs() must not delete the live app-logger directory (DATA_DIR/logs/application)", async () => {
|
||||
fs.mkdirSync(APP_LOG_DIR, { recursive: true });
|
||||
fs.writeFileSync(APP_LOG_FILE, '{"level":30,"msg":"server starting"}\n');
|
||||
|
||||
assert.equal(
|
||||
fs.existsSync(APP_LOG_FILE),
|
||||
true,
|
||||
"precondition: live app.log exists before migration runs"
|
||||
);
|
||||
|
||||
await migrations.archiveLegacyRequestLogs();
|
||||
|
||||
assert.equal(
|
||||
fs.existsSync(APP_LOG_FILE),
|
||||
true,
|
||||
"archiveLegacyRequestLogs() deleted the live app.log file — DATA_DIR/logs/application must be excluded from the legacy archive sweep"
|
||||
);
|
||||
assert.equal(
|
||||
fs.existsSync(CURRENT_REQUEST_LOGS_DIR),
|
||||
true,
|
||||
"archiveLegacyRequestLogs() deleted the entire DATA_DIR/logs directory, including the live application log subtree"
|
||||
);
|
||||
});
|
||||
|
||||
test("#6401: archiveLegacyRequestLogs does not crash the process when a legacy target is being actively appended to (yazl TOCTOU)", async (t) => {
|
||||
const legacyLogFile = path.join(CURRENT_REQUEST_LOGS_DIR, "legacy-request.log");
|
||||
fs.mkdirSync(CURRENT_REQUEST_LOGS_DIR, { recursive: true });
|
||||
fs.writeFileSync(legacyLogFile, "x".repeat(64 * 1024));
|
||||
|
||||
// Clear the marker written by the previous test so this run re-enters the archive path.
|
||||
const markerPath = path.join(TEST_DATA_DIR, "log_archives", "legacy-request-logs.json");
|
||||
fs.rmSync(markerPath, { force: true });
|
||||
|
||||
let writing = true;
|
||||
const writer = setInterval(() => {
|
||||
if (!writing) return;
|
||||
try {
|
||||
fs.appendFileSync(legacyLogFile, "y".repeat(4096));
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}, 2);
|
||||
writer.unref();
|
||||
|
||||
let uncaught: unknown = null;
|
||||
const onUncaught = (err: unknown) => {
|
||||
uncaught = err;
|
||||
};
|
||||
process.on("uncaughtException", onUncaught);
|
||||
|
||||
t.after(() => {
|
||||
writing = false;
|
||||
clearInterval(writer);
|
||||
process.off("uncaughtException", onUncaught);
|
||||
});
|
||||
|
||||
let rejected: unknown = null;
|
||||
try {
|
||||
await migrations.archiveLegacyRequestLogs();
|
||||
} catch (err) {
|
||||
rejected = err;
|
||||
}
|
||||
|
||||
await new Promise((r) => setTimeout(r, 800));
|
||||
|
||||
writing = false;
|
||||
clearInterval(writer);
|
||||
|
||||
assert.equal(
|
||||
uncaught,
|
||||
null,
|
||||
`archiveLegacyRequestLogs() let an uncaughtException escape instead of handling/rejecting it: ${String(uncaught)}`
|
||||
);
|
||||
// The function itself already has an outer try/catch at call sites; here we assert it
|
||||
// resolves or rejects cleanly rather than crashing the process via uncaughtException.
|
||||
void rejected;
|
||||
});
|
||||
Reference in New Issue
Block a user