Config: dedupe repeated config warning logs
This commit is contained in:
Vincent Koc
parent
38f46e80b0
commit
c026374fb1
105
src/config/io.logging.test.ts
Normal file
105
src/config/io.logging.test.ts
Normal file
@ -0,0 +1,105 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createConfigIO } from "./io.js";
|
||||
|
||||
async function withTempConfig(
|
||||
run: (params: {
|
||||
home: string;
|
||||
configPath: string;
|
||||
logger: { warn: ReturnType<typeof vi.fn>; error: ReturnType<typeof vi.fn> };
|
||||
loadConfig: () => Record<string, unknown>;
|
||||
}) => Promise<void>,
|
||||
): Promise<void> {
|
||||
const home = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-log-"));
|
||||
const configDir = path.join(home, ".openclaw");
|
||||
const configPath = path.join(configDir, "openclaw.json");
|
||||
await fs.mkdir(configDir, { recursive: true });
|
||||
const logger = { warn: vi.fn(), error: vi.fn() };
|
||||
const io = createConfigIO({
|
||||
env: {} as NodeJS.ProcessEnv,
|
||||
homedir: () => home,
|
||||
logger,
|
||||
});
|
||||
try {
|
||||
await run({
|
||||
home,
|
||||
configPath,
|
||||
logger,
|
||||
loadConfig: () => io.loadConfig() as Record<string, unknown>,
|
||||
});
|
||||
} finally {
|
||||
await fs.rm(home, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("config io warning/error logging", () => {
|
||||
it("dedupes identical warning payloads until the config changes", async () => {
|
||||
await withTempConfig(async ({ configPath, logger, loadConfig }) => {
|
||||
await fs.writeFile(
|
||||
configPath,
|
||||
JSON.stringify(
|
||||
{
|
||||
plugins: {
|
||||
entries: {
|
||||
discord: { enabled: false, config: {} },
|
||||
},
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
loadConfig();
|
||||
loadConfig();
|
||||
expect(logger.warn).toHaveBeenCalledTimes(1);
|
||||
|
||||
await fs.writeFile(configPath, JSON.stringify({}, null, 2));
|
||||
loadConfig();
|
||||
|
||||
await fs.writeFile(
|
||||
configPath,
|
||||
JSON.stringify(
|
||||
{
|
||||
plugins: {
|
||||
entries: {
|
||||
slack: { enabled: false, config: {} },
|
||||
},
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
loadConfig();
|
||||
expect(logger.warn).toHaveBeenCalledTimes(2);
|
||||
expect(logger.warn.mock.calls[1]?.[0]).toContain("plugins.entries.slack");
|
||||
});
|
||||
});
|
||||
|
||||
it("dedupes identical invalid config errors until the config becomes valid again", async () => {
|
||||
await withTempConfig(async ({ configPath, logger, loadConfig }) => {
|
||||
await fs.writeFile(
|
||||
configPath,
|
||||
JSON.stringify({ gateway: { port: "not-a-number" } }, null, 2),
|
||||
);
|
||||
|
||||
expect(loadConfig()).toEqual({});
|
||||
expect(loadConfig()).toEqual({});
|
||||
expect(logger.error).toHaveBeenCalledTimes(1);
|
||||
|
||||
await fs.writeFile(configPath, JSON.stringify({ gateway: { port: 18789 } }, null, 2));
|
||||
expect(loadConfig()).toMatchObject({ gateway: { port: 18789 } });
|
||||
|
||||
await fs.writeFile(configPath, JSON.stringify({ gateway: { port: "still-bad" } }, null, 2));
|
||||
expect(loadConfig()).toEqual({});
|
||||
expect(logger.error).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
114
src/config/io.ts
114
src/config/io.ts
@ -81,7 +81,12 @@ const OPEN_DM_POLICY_ALLOW_FROM_RE =
|
||||
/^(?<policyPath>[a-z0-9_.-]+)\s*=\s*"open"\s+requires\s+(?<allowPath>[a-z0-9_.-]+)(?:\s+\(or\s+[a-z0-9_.-]+\))?\s+to include "\*"$/i;
|
||||
|
||||
const CONFIG_AUDIT_LOG_FILENAME = "config-audit.jsonl";
|
||||
const loggedInvalidConfigs = new Set<string>();
|
||||
type ConfigLogFingerprintState = {
|
||||
invalid?: string;
|
||||
warnings?: string;
|
||||
};
|
||||
|
||||
const configLogFingerprintByPath = new Map<string, ConfigLogFingerprintState>();
|
||||
|
||||
type ConfigWriteAuditResult = "rename" | "copy-fallback" | "failed";
|
||||
|
||||
@ -544,19 +549,63 @@ export type ConfigIoDeps = {
|
||||
logger?: Pick<typeof console, "error" | "warn">;
|
||||
};
|
||||
|
||||
function warnOnConfigMiskeys(raw: unknown, logger: Pick<typeof console, "warn">): void {
|
||||
if (!raw || typeof raw !== "object") {
|
||||
function getConfigLogFingerprintState(configPath: string): ConfigLogFingerprintState {
|
||||
let state = configLogFingerprintByPath.get(configPath);
|
||||
if (!state) {
|
||||
state = {};
|
||||
configLogFingerprintByPath.set(configPath, state);
|
||||
}
|
||||
return state;
|
||||
}
|
||||
|
||||
function logConfigMessageOnce(params: {
|
||||
configPath: string;
|
||||
kind: keyof ConfigLogFingerprintState;
|
||||
message: string;
|
||||
logger: Pick<typeof console, "error" | "warn">;
|
||||
}): void {
|
||||
const state = getConfigLogFingerprintState(params.configPath);
|
||||
const fingerprint = hashConfigRaw(`${params.kind}:${params.message}`);
|
||||
if (state[params.kind] === fingerprint) {
|
||||
return;
|
||||
}
|
||||
state[params.kind] = fingerprint;
|
||||
if (params.kind === "invalid") {
|
||||
params.logger.error(params.message);
|
||||
return;
|
||||
}
|
||||
params.logger.warn(params.message);
|
||||
}
|
||||
|
||||
function clearConfigMessageFingerprint(
|
||||
configPath: string,
|
||||
kind: keyof ConfigLogFingerprintState,
|
||||
): void {
|
||||
const state = configLogFingerprintByPath.get(configPath);
|
||||
if (!state) {
|
||||
return;
|
||||
}
|
||||
delete state[kind];
|
||||
if (!state.invalid && !state.warnings) {
|
||||
configLogFingerprintByPath.delete(configPath);
|
||||
}
|
||||
}
|
||||
|
||||
function getConfigMiskeyWarnings(raw: unknown): string[] {
|
||||
const warnings: string[] = [];
|
||||
if (!raw || typeof raw !== "object") {
|
||||
return warnings;
|
||||
}
|
||||
const gateway = (raw as Record<string, unknown>).gateway;
|
||||
if (!gateway || typeof gateway !== "object") {
|
||||
return;
|
||||
return warnings;
|
||||
}
|
||||
if ("token" in (gateway as Record<string, unknown>)) {
|
||||
logger.warn(
|
||||
warnings.push(
|
||||
'Config uses "gateway.token". This key is ignored; use "gateway.auth.token" instead.',
|
||||
);
|
||||
}
|
||||
return warnings;
|
||||
}
|
||||
|
||||
function stampConfigVersion(cfg: OpenClawConfig): OpenClawConfig {
|
||||
@ -571,20 +620,19 @@ function stampConfigVersion(cfg: OpenClawConfig): OpenClawConfig {
|
||||
};
|
||||
}
|
||||
|
||||
function warnIfConfigFromFuture(cfg: OpenClawConfig, logger: Pick<typeof console, "warn">): void {
|
||||
function getFutureVersionWarning(cfg: OpenClawConfig): string | null {
|
||||
const touched = cfg.meta?.lastTouchedVersion;
|
||||
if (!touched) {
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
const cmp = compareOpenClawVersions(VERSION, touched);
|
||||
if (cmp === null) {
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
if (cmp < 0) {
|
||||
logger.warn(
|
||||
`Config was last written by a newer OpenClaw (${touched}); current version is ${VERSION}.`,
|
||||
);
|
||||
return `Config was last written by a newer OpenClaw (${touched}); current version is ${VERSION}.`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function resolveConfigPathForDeps(deps: Required<ConfigIoDeps>): string {
|
||||
@ -683,6 +731,8 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
try {
|
||||
maybeLoadDotEnvForConfig(deps.env);
|
||||
if (!deps.fs.existsSync(configPath)) {
|
||||
clearConfigMessageFingerprint(configPath, "invalid");
|
||||
clearConfigMessageFingerprint(configPath, "warnings");
|
||||
if (shouldEnableShellEnvFallback(deps.env) && !shouldDeferShellEnvFallback(deps.env)) {
|
||||
loadShellEnvFallback({
|
||||
enabled: true,
|
||||
@ -700,8 +750,10 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
resolveConfigIncludesForRead(parsed, configPath, deps),
|
||||
deps.env,
|
||||
);
|
||||
warnOnConfigMiskeys(resolvedConfig, deps.logger);
|
||||
const warningMessages = getConfigMiskeyWarnings(resolvedConfig);
|
||||
if (typeof resolvedConfig !== "object" || resolvedConfig === null) {
|
||||
clearConfigMessageFingerprint(configPath, "invalid");
|
||||
clearConfigMessageFingerprint(configPath, "warnings");
|
||||
return {};
|
||||
}
|
||||
const preValidationDuplicates = findDuplicateAgentDirs(resolvedConfig as OpenClawConfig, {
|
||||
@ -716,22 +768,39 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
const details = validated.issues
|
||||
.map((iss) => `- ${iss.path || "<root>"}: ${iss.message}`)
|
||||
.join("\n");
|
||||
if (!loggedInvalidConfigs.has(configPath)) {
|
||||
loggedInvalidConfigs.add(configPath);
|
||||
deps.logger.error(`Invalid config at ${configPath}:\\n${details}`);
|
||||
}
|
||||
clearConfigMessageFingerprint(configPath, "warnings");
|
||||
logConfigMessageOnce({
|
||||
configPath,
|
||||
kind: "invalid",
|
||||
message: `Invalid config at ${configPath}:\\n${details}`,
|
||||
logger: deps.logger,
|
||||
});
|
||||
const error = new Error(`Invalid config at ${configPath}:\n${details}`);
|
||||
(error as { code?: string; details?: string }).code = "INVALID_CONFIG";
|
||||
(error as { code?: string; details?: string }).details = details;
|
||||
throw error;
|
||||
}
|
||||
clearConfigMessageFingerprint(configPath, "invalid");
|
||||
if (validated.warnings.length > 0) {
|
||||
const details = validated.warnings
|
||||
.map((iss) => `- ${iss.path || "<root>"}: ${iss.message}`)
|
||||
.join("\n");
|
||||
deps.logger.warn(`Config warnings:\\n${details}`);
|
||||
warningMessages.push(`Config warnings:\\n${details}`);
|
||||
}
|
||||
const futureVersionWarning = getFutureVersionWarning(validated.config);
|
||||
if (futureVersionWarning) {
|
||||
warningMessages.push(futureVersionWarning);
|
||||
}
|
||||
if (warningMessages.length > 0) {
|
||||
logConfigMessageOnce({
|
||||
configPath,
|
||||
kind: "warnings",
|
||||
message: warningMessages.join("\n"),
|
||||
logger: deps.logger,
|
||||
});
|
||||
} else {
|
||||
clearConfigMessageFingerprint(configPath, "warnings");
|
||||
}
|
||||
warnIfConfigFromFuture(validated.config, deps.logger);
|
||||
const cfg = applyTalkConfigNormalization(
|
||||
applyModelDefaults(
|
||||
applyCompactionDefaults(
|
||||
@ -948,7 +1017,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
warnIfConfigFromFuture(validated.config, deps.logger);
|
||||
const futureVersionWarning = getFutureVersionWarning(validated.config);
|
||||
const snapshotConfig = normalizeConfigPaths(
|
||||
applyTalkApiKey(
|
||||
applyTalkConfigNormalization(
|
||||
@ -974,7 +1043,12 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
config: snapshotConfig,
|
||||
hash,
|
||||
issues: [],
|
||||
warnings: validated.warnings,
|
||||
warnings: futureVersionWarning
|
||||
? [
|
||||
...validated.warnings,
|
||||
{ path: "meta.lastTouchedVersion", message: futureVersionWarning },
|
||||
]
|
||||
: validated.warnings,
|
||||
legacyIssues,
|
||||
},
|
||||
envSnapshotForRestore: readResolution.envSnapshotForRestore,
|
||||
|
||||
@ -415,4 +415,56 @@ describe("startGatewayConfigReloader", () => {
|
||||
await reloader.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("dedupes unchanged invalid snapshots and re-logs when the invalid payload changes", async () => {
|
||||
const readSnapshot = vi
|
||||
.fn<() => Promise<ConfigFileSnapshot>>()
|
||||
.mockResolvedValueOnce(
|
||||
makeSnapshot({
|
||||
valid: false,
|
||||
issues: [
|
||||
{ path: "plugins.entries.discord", message: "plugin disabled but config is present" },
|
||||
],
|
||||
hash: "invalid-1",
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
makeSnapshot({
|
||||
valid: false,
|
||||
issues: [
|
||||
{ path: "plugins.entries.discord", message: "plugin disabled but config is present" },
|
||||
],
|
||||
hash: "invalid-1",
|
||||
}),
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
makeSnapshot({
|
||||
valid: false,
|
||||
issues: [
|
||||
{ path: "plugins.entries.slack", message: "plugin disabled but config is present" },
|
||||
],
|
||||
hash: "invalid-2",
|
||||
}),
|
||||
);
|
||||
const { watcher, log, reloader } = createReloaderHarness(readSnapshot);
|
||||
|
||||
watcher.emit("change");
|
||||
await vi.runOnlyPendingTimersAsync();
|
||||
watcher.emit("change");
|
||||
await vi.runOnlyPendingTimersAsync();
|
||||
watcher.emit("change");
|
||||
await vi.runOnlyPendingTimersAsync();
|
||||
|
||||
expect(log.warn).toHaveBeenCalledTimes(2);
|
||||
expect(log.warn).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"config reload skipped (invalid config): plugins.entries.discord: plugin disabled but config is present",
|
||||
);
|
||||
expect(log.warn).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"config reload skipped (invalid config): plugins.entries.slack: plugin disabled but config is present",
|
||||
);
|
||||
|
||||
await reloader.stop();
|
||||
});
|
||||
});
|
||||
|
||||
@ -89,6 +89,7 @@ export function startGatewayConfigReloader(opts: {
|
||||
let stopped = false;
|
||||
let restartQueued = false;
|
||||
let missingConfigRetries = 0;
|
||||
let invalidSnapshotFingerprint: string | null = null;
|
||||
|
||||
const scheduleAfter = (wait: number) => {
|
||||
if (stopped) {
|
||||
@ -140,10 +141,15 @@ export function startGatewayConfigReloader(opts: {
|
||||
|
||||
const handleInvalidSnapshot = (snapshot: ConfigFileSnapshot): boolean => {
|
||||
if (snapshot.valid) {
|
||||
invalidSnapshotFingerprint = null;
|
||||
return false;
|
||||
}
|
||||
const issues = formatConfigIssueLines(snapshot.issues, "").join(", ");
|
||||
opts.log.warn(`config reload skipped (invalid config): ${issues}`);
|
||||
const fingerprint = snapshot.hash ?? issues;
|
||||
if (invalidSnapshotFingerprint !== fingerprint) {
|
||||
invalidSnapshotFingerprint = fingerprint;
|
||||
opts.log.warn(`config reload skipped (invalid config): ${issues}`);
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user