fix(utils): isolated fatal logging teardown
Made fatal reporting bypass revoked stderr streams and armed a referenced forced-exit watchdog around bounded cleanup. Separated rotating log and audit namespaces by PID and disabled compression pipelines so concurrent TUI processes cannot race shared rotation state. Fixes #5716
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed orphaned TUI processes with revoked terminal descriptors remaining alive after a fatal error and amplifying shared log-rotation races into runaway memory, file-descriptor, swap, and disk consumption ([#5716](https://github.com/can1357/oh-my-pi/issues/5716)).
|
||||
|
||||
## [17.0.1] - 2026-07-16
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -241,7 +241,7 @@ export async function getLogText(): Promise<string> {
|
||||
return readLastLines(getLogPath(), MAX_LOG_LINES);
|
||||
}
|
||||
|
||||
const LOG_FILE_PATTERN = new RegExp(`^${APP_NAME}\\.(\\d{4}-\\d{2}-\\d{2})\\.log$`);
|
||||
const LOG_FILE_PATTERN = new RegExp(`^${APP_NAME}\\.(\\d{4}-\\d{2}-\\d{2})\\.\\d+\\.log$`);
|
||||
|
||||
export async function createDebugLogSource(): Promise<DebugLogSource> {
|
||||
const logsDir = getLogsDir();
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed fatal cleanup failing to reach `process.exit()` when terminal stderr is revoked, and isolated rotating log files/audit state per process to prevent concurrent OMP instances from racing compression and rotation ([#5716](https://github.com/can1357/oh-my-pi/issues/5716)).
|
||||
|
||||
## [17.0.1] - 2026-07-16
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -513,9 +513,9 @@ export function getLogsDir(): string {
|
||||
return dirs.rootSubdir("logs", "state");
|
||||
}
|
||||
|
||||
/** Get the path to a dated log file (~/.omp/logs/omp.YYYY-MM-DD.log). */
|
||||
export function getLogPath(date = new Date()): string {
|
||||
return path.join(getLogsDir(), `${APP_NAME}.${date.toISOString().slice(0, 10)}.log`);
|
||||
/** Get this process's dated log path (~/.omp/logs/omp.YYYY-MM-DD.PID.log). */
|
||||
export function getLogPath(date = new Date(), pid = process.pid): string {
|
||||
return path.join(getLogsDir(), `${APP_NAME}.${date.toISOString().slice(0, 10)}.${pid}.log`);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* Centralized logger for omp.
|
||||
*
|
||||
* Default: rotating `~/.omp/logs/omp.<DATE>.log`, no console output (writing
|
||||
* Default: rotating `~/.omp/logs/omp.<DATE>.<PID>.log`, no console output (writing
|
||||
* to stdout/stderr would corrupt the TUI). Long-running headless services
|
||||
* (the auth broker, etc.) call {@link setTransports} to swap in a console
|
||||
* transport so a process supervisor (pm2, journald, k8s) captures the logs.
|
||||
@@ -76,11 +76,11 @@ function getLogFormat(): winston.Logform.Format {
|
||||
function makeFileTransport(dir?: string): winston.transport {
|
||||
return new DailyRotateFile({
|
||||
dirname: ensureDir(dir ?? getLogsDir()),
|
||||
filename: "omp.%DATE%.log",
|
||||
filename: `omp.%DATE%.${process.pid}.log`,
|
||||
datePattern: "YYYY-MM-DD",
|
||||
maxSize: "10m",
|
||||
maxFiles: 5,
|
||||
zippedArchive: true,
|
||||
zippedArchive: false,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,8 @@
|
||||
* in response to process exit, signals, or fatal exceptions. It is intended to
|
||||
* allow reliably releasing resources or shutting down subprocesses, files, sockets, etc.
|
||||
*/
|
||||
|
||||
import * as fs from "node:fs";
|
||||
import inspector from "node:inspector";
|
||||
import { isMainThread } from "node:worker_threads";
|
||||
import { logger } from ".";
|
||||
@@ -68,7 +70,6 @@ function runCleanup(reason: Reason): Promise<void> {
|
||||
cleanupStage = "complete";
|
||||
deadline.resolve();
|
||||
}, CLEANUP_DEADLINE_MS);
|
||||
deadlineTimer.unref();
|
||||
cleanupPromise = Promise.race([cleanupSettled, deadline.promise]).finally(() => {
|
||||
clearTimeout(deadlineTimer);
|
||||
});
|
||||
@@ -175,6 +176,23 @@ function formatFatalError(label: string, err: Error): string {
|
||||
return `\n[${label}] ${name}: ${message}${formattedStack}\n`;
|
||||
}
|
||||
|
||||
async function exitAfterFatal(label: string, logMessage: string, err: Error, reason: Reason): Promise<void> {
|
||||
const forcedExit = setTimeout(() => process.exit(1), CLEANUP_DEADLINE_MS);
|
||||
try {
|
||||
restoreTerminalStderr();
|
||||
// A revoked terminal can make stream writes raise another fatal error. Use
|
||||
// the descriptor directly so failure stays synchronous and contained.
|
||||
try {
|
||||
fs.writeSync(2, formatFatalError(label, err));
|
||||
} catch {}
|
||||
logger.error(logMessage, { err });
|
||||
await runCleanup(reason);
|
||||
} finally {
|
||||
clearTimeout(forcedExit);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
if (isMainThread) {
|
||||
process
|
||||
.on("SIGINT", async () => {
|
||||
@@ -193,15 +211,7 @@ if (isMainThread) {
|
||||
logger.warn("Ignoring expected cleanup exception", { err });
|
||||
return;
|
||||
}
|
||||
// fd 2 may be redirected to the log while a TUI owns the terminal
|
||||
// (stderr-guard); re-point it at the real terminal so the fatal
|
||||
// report is visible. Terminal modes are restored moments later by
|
||||
// the terminal-restore cleanup callback inside runCleanup().
|
||||
restoreTerminalStderr();
|
||||
process.stderr.write(formatFatalError("Uncaught Exception", err));
|
||||
logger.error("Uncaught exception", { err });
|
||||
await runCleanup(Reason.UNCAUGHT_EXCEPTION);
|
||||
process.exit(1);
|
||||
await exitAfterFatal("Uncaught Exception", "Uncaught exception", err, Reason.UNCAUGHT_EXCEPTION);
|
||||
})
|
||||
.on("unhandledRejection", async reason => {
|
||||
const err = reason instanceof Error ? reason : new Error(String(reason));
|
||||
@@ -237,12 +247,7 @@ if (isMainThread) {
|
||||
});
|
||||
}
|
||||
}
|
||||
// See uncaughtException above: surface the report on the real stderr.
|
||||
restoreTerminalStderr();
|
||||
process.stderr.write(formatFatalError("Unhandled Rejection", err));
|
||||
logger.error("Unhandled rejection", { err });
|
||||
await runCleanup(Reason.UNHANDLED_REJECTION);
|
||||
process.exit(1);
|
||||
await exitAfterFatal("Unhandled Rejection", "Unhandled rejection", err, Reason.UNHANDLED_REJECTION);
|
||||
})
|
||||
.on("exit", async () => {
|
||||
void runCleanup(Reason.EXIT); // fire and forget (exit imminent)
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
|
||||
const loggerModuleUrl = pathToFileURL(path.join(import.meta.dir, "../src/logger.ts")).href;
|
||||
const roots: string[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map(root => fs.rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
async function makeProbe(logsDir: string): Promise<string> {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-logger-probe-"));
|
||||
roots.push(root);
|
||||
const probePath = path.join(root, "probe.ts");
|
||||
await Bun.write(
|
||||
probePath,
|
||||
`import { info, setTransports } from ${JSON.stringify(loggerModuleUrl)};\n` +
|
||||
`setTransports({ file: ${JSON.stringify(logsDir)} });\n` +
|
||||
`info("multiprocess probe");\n` +
|
||||
`setTransports({ file: false });\n`,
|
||||
);
|
||||
return probePath;
|
||||
}
|
||||
|
||||
async function waitForExit(proc: Bun.Subprocess): Promise<number> {
|
||||
const code = await proc.exited;
|
||||
return code;
|
||||
}
|
||||
|
||||
describe("multiprocess file logging", () => {
|
||||
it("gives concurrent processes independent rotation files and audit state", async () => {
|
||||
const logsDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-logger-output-"));
|
||||
roots.push(logsDir);
|
||||
const probePath = await makeProbe(logsDir);
|
||||
const processes = [
|
||||
Bun.spawn([process.execPath, probePath], { stdout: "ignore", stderr: "pipe" }),
|
||||
Bun.spawn([process.execPath, probePath], { stdout: "ignore", stderr: "pipe" }),
|
||||
];
|
||||
|
||||
expect(await Promise.all(processes.map(waitForExit))).toEqual([0, 0]);
|
||||
const entries = await fs.readdir(logsDir);
|
||||
const datedPrefix = `omp.${new Date().toISOString().slice(0, 10)}`;
|
||||
for (const proc of processes) {
|
||||
expect(entries).toContain(`${datedPrefix}.${proc.pid}.log`);
|
||||
}
|
||||
expect(entries.filter(name => name.endsWith("-audit.json"))).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
@@ -121,6 +121,24 @@ describe("postmortem expected cleanup errors", () => {
|
||||
expect(result.stderr).toContain("[Unhandled Rejection] Error: unexpected cleanup rejection");
|
||||
});
|
||||
|
||||
it("exits after an uncaught exception when terminal stderr is revoked", async () => {
|
||||
const result = await runPostmortemProbe(`
|
||||
import { spyOn } from "bun:test";
|
||||
import "${postmortemModuleUrl}";
|
||||
|
||||
spyOn(process.stderr, "write").mockImplementation(() => {
|
||||
throw Object.assign(new Error("terminal revoked"), { code: "EIO" });
|
||||
});
|
||||
queueMicrotask(() => {
|
||||
throw new Error("fatal after disconnect");
|
||||
});
|
||||
await Promise.withResolvers<void>().promise;
|
||||
`);
|
||||
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.stderr).toContain("[Uncaught Exception] Error: fatal after disconnect");
|
||||
});
|
||||
|
||||
it("releases manual cleanup at the deadline even when a callback never settles", async () => {
|
||||
const result = await runPostmortemProbe(`
|
||||
import { vi } from "bun:test";
|
||||
|
||||
Reference in New Issue
Block a user