mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 21:32:20 +03:00
* fix(compression): fall back in-process when the compression worker fails (#13145) The worker pool resolved every worker fault with the *uncompressed* body instead of reporting it. `PendingJob` had no reject path at all, so a thread error, a worker exit, a dispatch timeout, or an engine error posted back as `type: "error"` all resolved as `{ compressed: false, stats: null }`. `applyCompressionAsync` then treated that as a legitimate "nothing to compress" result and returned it as-is, so the request reached the provider uncompressed while the response header still announced the selected plan ("stacked") — the header is emitted before the pipeline runs. Nothing was logged at any level, and `compression_analytics` stayed empty because rows are only written when a compressed result is reported. The net effect was compression silently disabled for every worker-eligible request. The worker is a throughput optimisation, not a behavioural variant, so a worker fault must degrade to the in-process pipeline rather than to no compression: - `PendingJob` gains `reject`; `fail()` delegates to a new `abort()` that clears the slot timeout and rejects with a diagnostic cause (thread error, exit code, or timeout budget). - An `error` message from the worker is propagated instead of being swallowed. - `applyCompressionAsync` catches the rejection and falls through to the in-process path, logging the cause. The logger is imported lazily and defensively: `compressionWorker.ts` imports this module, so a static import would pull the logger into the worker bundle, and a logging failure must never be able to break compression itself. `close()` keeps resolving with the unchanged body — shutdown is not a fault. The regression test drives a real worker fault via `OMNI_COMPRESSION_WORKER_TIMEOUT_MS` rather than mocking the module, since this project's tsx/ESM + node:test setup has no `mock.module()` support. Its options are fully populated on purpose: `runCompressionAsync` forwards them into `workerOptions`, and `isStrictlySerializable` rejects an object holding `undefined` values — which would route the test through the in-process path and assert nothing. Production requests always carry all of those fields, which is why the worker path is taken there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGJQT3E6iJZq4zNGwfjkPs * fix(compression): keep the timeout path uncompressed, retry only fast worker faults (#13145) Review follow-up: the in-process fallthrough ran the full pipeline on the main event loop for *every* worker fault, including a dispatch timeout. A timeout means the worker already spent its whole budget on that body, so re-running the same CPU-bound work inline would stall other in-flight requests — strictly worse than not compressing on a shared gateway. Faults are now typed by whether recovery is cheap: - `CompressionWorkerError.retryInProcess` distinguishes fast faults (thread error, worker exit, engine throw — no work was done, so the in-process path costs what the worker would have) from a dispatch timeout. - Timeouts keep the original degrade-to-uncompressed behaviour, but are now reported. The defect this PR fixes is the silent swallow, not the degrade. Also strips `reject` from the structured-clone wire job. It is a function, so leaving it on the object handed to `postMessage` threw `DataCloneError` before the worker ever saw the job — turning every dispatch into an immediate fault. Adds the missing `changelog.d/fixes/` fragment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(compression): narrow the worker thread-error type for typecheck:core @types/node 26 types the Worker "error" event payload as unknown, not Error, so `error?.message` failed typecheck:core (TS2339). Narrow with an instanceof check before reading .message. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: marcs7 <marcs7@users.noreply.github.com>
227 lines
8.4 KiB
TypeScript
227 lines
8.4 KiB
TypeScript
import assert from "node:assert/strict";
|
|
import { after, describe, it } from "node:test";
|
|
import { Worker } from "node:worker_threads";
|
|
import {
|
|
isCompressionWorkerEligible,
|
|
isStrictlySerializable,
|
|
} from "../../../open-sse/services/compression/compressionWorkerProtocol.ts";
|
|
import {
|
|
closeCompressionWorkerPoolForTests,
|
|
CompressionWorkerPool,
|
|
} from "../../../open-sse/services/compression/compressionWorkerPool.ts";
|
|
import {
|
|
applyCompression,
|
|
applyCompressionAsync,
|
|
} from "../../../open-sse/services/compression/strategySelector.ts";
|
|
import type { CompressionConfig } from "../../../open-sse/services/compression/types.ts";
|
|
|
|
const body = {
|
|
model: "gpt-test",
|
|
messages: [
|
|
{ role: "system", content: "Answer accurately." },
|
|
{
|
|
role: "user",
|
|
content:
|
|
"Please basically actually simply carefully help with this very important task. ".repeat(
|
|
80
|
|
),
|
|
},
|
|
],
|
|
};
|
|
const config = {
|
|
enabled: true,
|
|
defaultMode: "stacked",
|
|
autoTriggerTokens: 1,
|
|
cacheMinutes: 0,
|
|
preserveSystemPrompt: true,
|
|
stackedPipeline: [{ engine: "rtk" }, { engine: "caveman" }],
|
|
} as CompressionConfig;
|
|
|
|
function comparable<T extends { stats: { durationMs?: number; timestamp: number } | null }>(
|
|
result: T
|
|
) {
|
|
if (!result.stats) return result;
|
|
const {
|
|
durationMs: _duration,
|
|
timestamp: _timestamp,
|
|
engineBreakdown,
|
|
...stats
|
|
} = result.stats as T["stats"] & {
|
|
engineBreakdown?: Array<Record<string, unknown>>;
|
|
};
|
|
const stableBreakdown = engineBreakdown?.map(({ durationMs: _stepDuration, ...step }) => step);
|
|
return {
|
|
...result,
|
|
stats: {
|
|
...stats,
|
|
...(stableBreakdown ? { engineBreakdown: stableBreakdown } : {}),
|
|
},
|
|
};
|
|
}
|
|
|
|
after(() => closeCompressionWorkerPoolForTests());
|
|
|
|
describe("compression worker eligibility", () => {
|
|
it("accepts only standard, rtk, and approved rtk+caveman stacks", () => {
|
|
assert.equal(isCompressionWorkerEligible(body, "standard", { config }), true);
|
|
assert.equal(isCompressionWorkerEligible(body, "rtk", { config }), true);
|
|
assert.equal(isCompressionWorkerEligible(body, "stacked", { config }), true);
|
|
for (const mode of ["off", "lite", "aggressive", "ultra", "omniglyph"] as const) {
|
|
assert.equal(isCompressionWorkerEligible(body, mode, { config }), false);
|
|
}
|
|
for (const engine of ["llmlingua", "omniglyph", "ccr", "session-dedup", "ultra"]) {
|
|
assert.equal(
|
|
isCompressionWorkerEligible(body, "stacked", {
|
|
config: { ...config, stackedPipeline: [{ engine }] } as CompressionConfig,
|
|
}),
|
|
false
|
|
);
|
|
}
|
|
});
|
|
|
|
it("rejects functions, symbols, classes, special objects, cycles, and non-finite numbers", () => {
|
|
for (const value of [
|
|
() => undefined,
|
|
Symbol("x"),
|
|
new Date(),
|
|
new Map(),
|
|
new Set(),
|
|
/x/,
|
|
NaN,
|
|
Infinity,
|
|
]) {
|
|
assert.equal(isStrictlySerializable(value), false);
|
|
}
|
|
const cyclic: Record<string, unknown> = {};
|
|
cyclic.self = cyclic;
|
|
assert.equal(isStrictlySerializable(cyclic), false);
|
|
});
|
|
|
|
it("#13154: does not misread a shared (non-cyclic) sub-object referenced by two sibling branches as a cycle", () => {
|
|
// Original bug: a single `seen` set shared across the whole recursion tree (never
|
|
// backtracked) meant visiting the SAME object twice via two different, non-cyclic
|
|
// paths (e.g. two messages both pointing at the same cached template object) was
|
|
// indistinguishable from a real cycle. Path-based tracking (add before descending,
|
|
// delete after) must treat this as eligible.
|
|
const shared = { nested: true };
|
|
const sharedBody = { messages: [shared, shared] };
|
|
assert.equal(isStrictlySerializable(sharedBody), true);
|
|
assert.equal(isCompressionWorkerEligible(sharedBody, "standard", { config }), true);
|
|
});
|
|
|
|
it("still rejects a body with a genuine cycle before it ever reaches postMessage", () => {
|
|
const cyclicMessage: Record<string, unknown> = { role: "user" };
|
|
cyclicMessage.self = cyclicMessage;
|
|
const cyclicBody = { messages: [cyclicMessage] };
|
|
assert.equal(isStrictlySerializable(cyclicBody), false);
|
|
assert.equal(isCompressionWorkerEligible(cyclicBody, "standard", { config }), false);
|
|
});
|
|
});
|
|
|
|
describe("compression worker execution", () => {
|
|
it("matches the synchronous body and stats except timing fields", async () => {
|
|
const sync = applyCompression(body, "stacked", { config });
|
|
const async = await applyCompressionAsync(body, "stacked", { config });
|
|
assert.deepEqual(comparable(async), comparable(sync));
|
|
});
|
|
|
|
it("preserves Responses bodies and hard-budget results", async () => {
|
|
const responsesBody = {
|
|
model: "gpt-test",
|
|
input: [{ role: "user", content: [{ type: "input_text", text: "word ".repeat(600) }] }],
|
|
};
|
|
const hardBudgetConfig = { ...config, targetTokens: 100 };
|
|
const sync = applyCompression(responsesBody, "stacked", { config: hardBudgetConfig });
|
|
const async = await applyCompressionAsync(responsesBody, "stacked", {
|
|
config: hardBudgetConfig,
|
|
});
|
|
assert.deepEqual(comparable(async), comparable(sync));
|
|
});
|
|
|
|
it("relays per-engine progress from the worker", async () => {
|
|
const steps: string[] = [];
|
|
await applyCompressionAsync(body, "stacked", {
|
|
config,
|
|
onEngineStep: (step) => steps.push(step.engine),
|
|
});
|
|
assert.deepEqual(steps, ["rtk", "caveman"]);
|
|
});
|
|
|
|
it("reports a timeout as a non-retryable fault instead of silently failing open (#13145)", async () => {
|
|
// The pool no longer swallows a dispatch timeout: it rejects with a typed fault
|
|
// whose retryInProcess=false tells the caller (strategySelector) that the worker
|
|
// already burned its budget, so the caller ships the body uncompressed and LOGS
|
|
// the fault rather than re-running the same heavy pipeline on the event loop.
|
|
const pool = new CompressionWorkerPool({ size: 1, timeoutMs: 1, idleMs: 100 });
|
|
try {
|
|
await assert.rejects(
|
|
() => pool.run(body, "stacked", { config }),
|
|
(err: unknown) =>
|
|
err instanceof Error &&
|
|
err.name === "CompressionWorkerError" &&
|
|
(err as { retryInProcess?: boolean }).retryInProcess === false &&
|
|
/timed out/.test(err.message)
|
|
);
|
|
} finally {
|
|
await pool.close();
|
|
}
|
|
});
|
|
|
|
it("terminates an idle worker instead of only dropping it from the pool", async () => {
|
|
const spawned = new Set<Worker>();
|
|
const terminated: Promise<number>[] = [];
|
|
const originalPostMessage = Worker.prototype.postMessage;
|
|
const originalTerminate = Worker.prototype.terminate;
|
|
Worker.prototype.postMessage = function (this: Worker, ...args) {
|
|
spawned.add(this);
|
|
return originalPostMessage.apply(this, args);
|
|
};
|
|
Worker.prototype.terminate = function (this: Worker) {
|
|
const exit = originalTerminate.call(this);
|
|
terminated.push(exit);
|
|
return exit;
|
|
};
|
|
const messagePorts = () =>
|
|
process.getActiveResourcesInfo().filter((resource) => resource === "MessagePort").length;
|
|
const portsBefore = messagePorts();
|
|
const pool = new CompressionWorkerPool({ size: 1, idleMs: 50 });
|
|
try {
|
|
await pool.run(body, "stacked", { config });
|
|
await new Promise((resolve) => setTimeout(resolve, 300));
|
|
assert.equal(spawned.size, 1);
|
|
assert.equal(terminated.length, 1, "idle eviction must terminate the worker thread");
|
|
await Promise.all(terminated);
|
|
assert.ok(messagePorts() <= portsBefore, "idle eviction must not retain the worker's port");
|
|
} finally {
|
|
Worker.prototype.postMessage = originalPostMessage;
|
|
Worker.prototype.terminate = originalTerminate;
|
|
await pool.close();
|
|
// Reap anything the pool forgot so a regression fails instead of hanging the runner.
|
|
await Promise.all([...spawned].map((worker) => worker.terminate().catch(() => undefined)));
|
|
}
|
|
});
|
|
|
|
it("keeps the parent event loop responsive while two workers overlap", async () => {
|
|
const largeBody = {
|
|
messages: Array.from({ length: 400 }, (_, index) => ({
|
|
role: "user",
|
|
content: `message ${index} ` + "basically actually simply ".repeat(400),
|
|
})),
|
|
};
|
|
let ticked = false;
|
|
const tick = new Promise<void>((resolve) =>
|
|
setTimeout(() => {
|
|
ticked = true;
|
|
resolve();
|
|
}, 0)
|
|
);
|
|
const jobs = Promise.all([
|
|
applyCompressionAsync(largeBody, "standard", { config }),
|
|
applyCompressionAsync(largeBody, "standard", { config }),
|
|
]);
|
|
await tick;
|
|
assert.equal(ticked, true);
|
|
await jobs;
|
|
});
|
|
});
|