mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-18 04:42:30 +03:00
fix(compression): log warnings for unreadable settings rows (#13522)
* fix(compression): log warnings for unreadable settings rows getCompressionSettings() silently skipped non-string (BLOB) and invalid-JSON settings rows, making it impossible to diagnose config drift between the panel and the runtime. Now logs a warn-level message for each unreadable row, including the key name and a remediation hint (re-save from the Storage panel). Also warns when the 'engines' row exists but yields no valid toggles, so operators know their panel-configured engines map is being silently replaced by the legacy fallback. Fixes #13456 * test(compression): cover getCompressionSettings warnings for unreadable rows The test for #13456 only asserted a stubbed console.warn recorded a message and never called getCompressionSettings(), so it never exercised the production change. Seed a BLOB row, an invalid-JSON row, and an 'engines' row that isn't a usable object, and assert the resulting warnings; also assert a legitimately empty (but valid) 'engines' map does not warn. Also stop warning on a valid-but-empty 'engines' row: parseStoredEnginesMap returns null both for an unreadable row and for a well-formed {} (an operator who deliberately disabled every engine), so only warn when the stored value isn't a usable object at all. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Koosha Pari <koosha@phenotype.ai> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
cdcf1d2589
commit
5455740faa
@@ -656,9 +656,27 @@ export async function getCompressionSettings(): Promise<CompressionConfig> {
|
||||
const record = toRecord(row);
|
||||
const key = typeof record.key === "string" ? record.key : null;
|
||||
const rawValue = typeof record.value === "string" ? record.value : null;
|
||||
if (!key || rawValue === null) continue;
|
||||
if (!key || rawValue === null) {
|
||||
// #13456: non-string values (BLOB from backup/restore/migration tooling) are
|
||||
// silently ignored — log so operators can diagnose config drift.
|
||||
if (key && typeof record.value !== "string" && record.value !== null) {
|
||||
console.warn(
|
||||
`[COMPRESSION] Settings row '${key}' has non-string value type ` +
|
||||
`(${typeof record.value}); skipping. This may indicate a backup/restore ` +
|
||||
`issue — re-save the setting from the Storage panel to fix.`
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const parsed = parseJsonSafe(rawValue);
|
||||
if (parsed === undefined) continue;
|
||||
if (parsed === undefined) {
|
||||
// #13456: invalid JSON is also silently ignored — log it.
|
||||
console.warn(
|
||||
`[COMPRESSION] Settings row '${key}' has unparseable JSON value; skipping. ` +
|
||||
`Re-save the setting from the Storage panel to fix.`
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
switch (key) {
|
||||
case "enabled":
|
||||
@@ -768,6 +786,16 @@ export async function getCompressionSettings(): Promise<CompressionConfig> {
|
||||
break;
|
||||
case "engines":
|
||||
storedEngines = parseStoredEnginesMap(parsed);
|
||||
// #13456: only warn when the row itself isn't a usable object — a valid object
|
||||
// that simply yields zero toggles (e.g. `{}`, an operator deliberately disabling
|
||||
// every engine) is legitimate config, not a parse failure, and must not warn.
|
||||
if (storedEngines === null && (!parsed || typeof parsed !== "object")) {
|
||||
console.warn(
|
||||
`[COMPRESSION] 'engines' settings row is present but unreadable; ` +
|
||||
`falling back to legacy settings. Re-save the engines map from the ` +
|
||||
`Storage panel to fix.`
|
||||
);
|
||||
}
|
||||
break;
|
||||
case "activeComboId":
|
||||
config.activeComboId = typeof parsed === "string" && parsed.trim() ? parsed.trim() : null;
|
||||
@@ -917,7 +945,10 @@ let proactiveRatioCache: { value: number; readAt: number } | null = null;
|
||||
|
||||
export function getProactiveCompressionRatio(): number {
|
||||
const now = Date.now();
|
||||
if (proactiveRatioCache && now - proactiveRatioCache.readAt < PROACTIVE_COMPRESSION_CACHE_TTL_MS) {
|
||||
if (
|
||||
proactiveRatioCache &&
|
||||
now - proactiveRatioCache.readAt < PROACTIVE_COMPRESSION_CACHE_TTL_MS
|
||||
) {
|
||||
return proactiveRatioCache.value;
|
||||
}
|
||||
let ratio = PROACTIVE_COMPRESSION_DEFAULT_RATIO;
|
||||
|
||||
115
tests/unit/compression/compression-settings-warn.test.ts
Normal file
115
tests/unit/compression/compression-settings-warn.test.ts
Normal file
@@ -0,0 +1,115 @@
|
||||
/**
|
||||
* Tests for #13456: compression settings row warnings for unreadable values.
|
||||
*
|
||||
* Before the fix, non-string (BLOB) and invalid-JSON settings rows were
|
||||
* silently ignored. Operators had no way to diagnose config drift from the
|
||||
* panel vs. the runtime.
|
||||
*/
|
||||
import { test, after, beforeEach } 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 TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-compression-warn-"));
|
||||
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const { getDbInstance, resetDbInstance } = await import("../../../src/lib/db/core.ts");
|
||||
const { getCompressionSettings } = await import("../../../src/lib/db/compression.ts");
|
||||
|
||||
const warnings: string[] = [];
|
||||
const originalWarn = console.warn;
|
||||
|
||||
function freshDir() {
|
||||
resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
warnings.length = 0;
|
||||
console.warn = (...args: unknown[]) => {
|
||||
warnings.push(args.join(" "));
|
||||
};
|
||||
});
|
||||
|
||||
after(() => {
|
||||
console.warn = originalWarn;
|
||||
resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
|
||||
if (ORIGINAL_DATA_DIR === undefined) {
|
||||
delete process.env.DATA_DIR;
|
||||
} else {
|
||||
process.env.DATA_DIR = ORIGINAL_DATA_DIR;
|
||||
}
|
||||
});
|
||||
|
||||
test("warns when a settings row is stored as a non-string (BLOB) value", async () => {
|
||||
freshDir();
|
||||
const db = getDbInstance();
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
"compression",
|
||||
"cacheMinutes",
|
||||
Buffer.from("corrupt-blob")
|
||||
);
|
||||
|
||||
await getCompressionSettings();
|
||||
|
||||
assert.ok(
|
||||
warnings.some((w) => w.includes("cacheMinutes") && w.includes("non-string value type")),
|
||||
`expected a non-string-value warning for 'cacheMinutes', got: ${JSON.stringify(warnings)}`
|
||||
);
|
||||
});
|
||||
|
||||
test("warns when a settings row has unparseable JSON", async () => {
|
||||
freshDir();
|
||||
const db = getDbInstance();
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
"compression",
|
||||
"cacheMinutes",
|
||||
"{not valid json"
|
||||
);
|
||||
|
||||
await getCompressionSettings();
|
||||
|
||||
assert.ok(
|
||||
warnings.some((w) => w.includes("cacheMinutes") && w.includes("unparseable JSON")),
|
||||
`expected an unparseable-JSON warning for 'cacheMinutes', got: ${JSON.stringify(warnings)}`
|
||||
);
|
||||
});
|
||||
|
||||
test("warns when the 'engines' row is not a usable object", async () => {
|
||||
freshDir();
|
||||
const db = getDbInstance();
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
"compression",
|
||||
"engines",
|
||||
"42"
|
||||
);
|
||||
|
||||
await getCompressionSettings();
|
||||
|
||||
assert.ok(
|
||||
warnings.some((w) => w.includes("'engines'") && w.includes("unreadable")),
|
||||
`expected an unreadable-engines warning, got: ${JSON.stringify(warnings)}`
|
||||
);
|
||||
});
|
||||
|
||||
test("does NOT warn when the 'engines' row is a valid but empty object", async () => {
|
||||
freshDir();
|
||||
const db = getDbInstance();
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
"compression",
|
||||
"engines",
|
||||
"{}"
|
||||
);
|
||||
|
||||
await getCompressionSettings();
|
||||
|
||||
assert.equal(
|
||||
warnings.some((w) => w.includes("'engines'")),
|
||||
false,
|
||||
`a deliberately empty (but valid) engines map must not warn, got: ${JSON.stringify(warnings)}`
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user