Logging: guard console settings recursion
This commit is contained in:
committed by
Peter Steinberger
parent
17f2a990a8
commit
3ba9821254
83
src/logging/console-settings.test.ts
Normal file
83
src/logging/console-settings.test.ts
Normal file
@@ -0,0 +1,83 @@
|
|||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
|
vi.mock("./config.js", () => ({
|
||||||
|
readLoggingConfig: () => undefined,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("./logger.js", () => ({
|
||||||
|
getLogger: () => ({
|
||||||
|
trace: () => {},
|
||||||
|
debug: () => {},
|
||||||
|
info: () => {},
|
||||||
|
warn: () => {},
|
||||||
|
error: () => {},
|
||||||
|
fatal: () => {},
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
let loadConfigCalls = 0;
|
||||||
|
vi.mock("node:module", async () => {
|
||||||
|
const actual = await vi.importActual<typeof import("node:module")>("node:module");
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
createRequire: (url: string | URL) => {
|
||||||
|
const realRequire = actual.createRequire(url);
|
||||||
|
return (specifier: string) => {
|
||||||
|
if (specifier.endsWith("config.js")) {
|
||||||
|
return {
|
||||||
|
loadConfig: () => {
|
||||||
|
loadConfigCalls += 1;
|
||||||
|
if (loadConfigCalls > 5) {
|
||||||
|
return {};
|
||||||
|
}
|
||||||
|
console.error("config load failed");
|
||||||
|
return {};
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return realRequire(specifier);
|
||||||
|
};
|
||||||
|
},
|
||||||
|
};
|
||||||
|
});
|
||||||
|
let originalIsTty: boolean | undefined;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
loadConfigCalls = 0;
|
||||||
|
vi.resetModules();
|
||||||
|
originalIsTty = process.stdout.isTTY;
|
||||||
|
Object.defineProperty(process.stdout, "isTTY", { value: false, configurable: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
Object.defineProperty(process.stdout, "isTTY", { value: originalIsTty, configurable: true });
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
async function loadLogging() {
|
||||||
|
const logging = await import("../logging.js");
|
||||||
|
const state = await import("./state.js");
|
||||||
|
state.loggingState.cachedConsoleSettings = null;
|
||||||
|
return { logging, state };
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("getConsoleSettings", () => {
|
||||||
|
it("does not recurse when loadConfig logs during resolution", async () => {
|
||||||
|
const { logging } = await loadLogging();
|
||||||
|
logging.setConsoleTimestampPrefix(true);
|
||||||
|
logging.enableConsoleCapture();
|
||||||
|
const { getConsoleSettings } = logging;
|
||||||
|
getConsoleSettings();
|
||||||
|
expect(loadConfigCalls).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips config fallback during re-entrant resolution", async () => {
|
||||||
|
const { logging, state } = await loadLogging();
|
||||||
|
state.loggingState.resolvingConsoleSettings = true;
|
||||||
|
logging.setConsoleTimestampPrefix(true);
|
||||||
|
logging.enableConsoleCapture();
|
||||||
|
logging.getConsoleSettings();
|
||||||
|
expect(loadConfigCalls).toBe(0);
|
||||||
|
state.loggingState.resolvingConsoleSettings = false;
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -35,6 +35,10 @@ function resolveConsoleSettings(): ConsoleSettings {
|
|||||||
let cfg: ClawdbotConfig["logging"] | undefined =
|
let cfg: ClawdbotConfig["logging"] | undefined =
|
||||||
(loggingState.overrideSettings as LoggerSettings | null) ?? readLoggingConfig();
|
(loggingState.overrideSettings as LoggerSettings | null) ?? readLoggingConfig();
|
||||||
if (!cfg) {
|
if (!cfg) {
|
||||||
|
if (loggingState.resolvingConsoleSettings) {
|
||||||
|
cfg = undefined;
|
||||||
|
} else {
|
||||||
|
loggingState.resolvingConsoleSettings = true;
|
||||||
try {
|
try {
|
||||||
const loaded = requireConfig("../config/config.js") as {
|
const loaded = requireConfig("../config/config.js") as {
|
||||||
loadConfig?: () => ClawdbotConfig;
|
loadConfig?: () => ClawdbotConfig;
|
||||||
@@ -42,6 +46,9 @@ function resolveConsoleSettings(): ConsoleSettings {
|
|||||||
cfg = loaded.loadConfig?.().logging;
|
cfg = loaded.loadConfig?.().logging;
|
||||||
} catch {
|
} catch {
|
||||||
cfg = undefined;
|
cfg = undefined;
|
||||||
|
} finally {
|
||||||
|
loggingState.resolvingConsoleSettings = false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const level = normalizeConsoleLevel(cfg?.consoleLevel);
|
const level = normalizeConsoleLevel(cfg?.consoleLevel);
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ export const loggingState = {
|
|||||||
forceConsoleToStderr: false,
|
forceConsoleToStderr: false,
|
||||||
consoleTimestampPrefix: false,
|
consoleTimestampPrefix: false,
|
||||||
consoleSubsystemFilter: null as string[] | null,
|
consoleSubsystemFilter: null as string[] | null,
|
||||||
|
resolvingConsoleSettings: false,
|
||||||
rawConsole: null as {
|
rawConsole: null as {
|
||||||
log: typeof console.log;
|
log: typeof console.log;
|
||||||
info: typeof console.info;
|
info: typeof console.info;
|
||||||
|
|||||||
Reference in New Issue
Block a user