Skip to content

Commit b004cef

Browse files
committed
fix(logging): keep the active log when rotation fails
Renaming onto the previous generation replaces it only when that rename succeeds. Deleting that generation first, then truncating the active file on failure, drops every record but the last.
1 parent b38244f commit b004cef

2 files changed

Lines changed: 36 additions & 11 deletions

File tree

‎src/logging/sink.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,34 @@ describe("installFileLogSink", () => {
108108
}
109109
});
110110

111+
test("a failed rotation keeps already-written records in the active file", () => {
112+
const { dir, file } = tempLogFile();
113+
try {
114+
installFileLogSink(file, { maxBytes: 180 });
115+
const logger = getLogger(["logging", "rotate-fail"]);
116+
for (let i = 0; i < 30; i++) logger.info(`seed-line-${i}-xxxxxxxx`);
117+
flushFileLogSinkSync();
118+
const rotated = `${file}.1`;
119+
expect(fs.statSync(rotated).isFile()).toBe(true);
120+
fs.rmSync(rotated);
121+
fs.mkdirSync(rotated);
122+
const kept = fs.readFileSync(file, "utf8");
123+
expect(kept.length).toBeGreaterThan(0);
124+
logger.info("kept-tail-1");
125+
logger.info("kept-tail-2");
126+
logger.info("kept-tail-3");
127+
flushFileLogSinkSync();
128+
const active = fs.readFileSync(file, "utf8");
129+
expect(active.startsWith(kept)).toBe(true);
130+
expect(active).toContain("kept-tail-1");
131+
expect(active).toContain("kept-tail-2");
132+
expect(active).toContain("kept-tail-3");
133+
expect(fs.statSync(rotated).isDirectory()).toBe(true);
134+
} finally {
135+
fs.rmSync(dir, { recursive: true, force: true });
136+
}
137+
});
138+
111139
test("a crash or exit flush writes the pending tail", () => {
112140
const { dir, file } = tempLogFile();
113141
try {

‎src/logging/sink.ts‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,17 @@ function fileSize(path: string): number {
5858
}
5959

6060
function rotateActive(current: FileSinkState): void {
61-
const rotated = `${current.path}.1`;
61+
// Publish the filled file by renaming it onto `.1`. Do not remove `.1`
62+
// first, and do not truncate the active file if rename fails: a directory
63+
// or busy `.1` throws, and wiping the live log then leaves only the next
64+
// line. A failed rename leaves both paths untouched, so this cycle may
65+
// grow past the cap.
6266
try {
63-
fs.rmSync(rotated, { force: true });
64-
fs.renameSync(current.path, rotated);
65-
current.size = 0;
67+
fs.renameSync(current.path, `${current.path}.1`);
6668
} catch {
67-
try {
68-
// rename can fail (missing file, cross-device); truncating still caps growth.
69-
fs.truncateSync(current.path, 0);
70-
current.size = 0;
71-
} catch {
72-
// Keep `size` so a later flush retries instead of appending forever.
73-
}
69+
return;
7470
}
71+
current.size = 0;
7572
}
7673

7774
function writeBatch(

0 commit comments

Comments
 (0)