mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-17 20:52:15 +03:00
A combo target that stalls past comboTargetTimeoutMs is aborted by buildTargetTimeoutRunner, which swallows the resulting rejection behind its synthetic 524. Nothing marks the account unavailable — correctly, since a stall is not a quota/auth failure — so the #6219 eviction on the generic markAccountUnavailable -> shouldFallback path in chat.ts never ran. The session pin therefore survived its full TTL and every following request in that session was handed straight back to the account that had just stalled. Seen in production on combo "coding" [priority]: one codex account pinned for a 30-minute TTL, four consecutive requests, four 120s timeouts, "all targets exhausted" each time, while four sibling codex accounts stayed healthy and unused. Classify the abort reason (new dependency-free leaf comboAbortReasons.ts) and evict the connection-matched pin. Only a genuine per-model timeout evicts: a client disconnect or a hedge cancellation says nothing about account health, so those keep the pin and its prompt-cache locality. Eviction is best-effort and never breaks the dispatch path. The dispatch itself moves into a new seam, chatDispatch.ts, which merges the per-model abort signal into the outgoing request, runs executeChatWithBreaker, and owns the eviction on both the rejection and failed-result paths. Keeping that logic out of the frozen god-file leaves chat.ts one line SHORTER than before (1844 -> 1843). Co-authored-by: alexey.nazarov@softmg.ru <alexey.nazarov@softmg.ru> Co-authored-by: fenix007 <fenix007@users.noreply.github.com> Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
203 lines
7.0 KiB
TypeScript
203 lines
7.0 KiB
TypeScript
// #6219 follow-up — a COMBO per-model timeout must also evict the sticky session
|
|
// pin.
|
|
//
|
|
// Observed in production: combo "coding" [priority] pinned a session to one codex
|
|
// account. That account stalled past comboTargetTimeoutMs, the combo aborted the
|
|
// target and synthesized a 524, and — because a stall is not a quota/auth failure —
|
|
// nothing called markAccountUnavailable. The #6219 eviction only runs on that
|
|
// generic failover path, so the pin survived its full 30-minute TTL and every
|
|
// following request in the session was handed straight back to the stalled
|
|
// account: four consecutive requests, four 120s timeouts, "all targets exhausted"
|
|
// each time, while four sibling codex accounts sat healthy and unused.
|
|
//
|
|
// The fix classifies the abort reason (open-sse/services/combo/comboAbortReasons.ts)
|
|
// and evicts the connection-matched pin from the dispatch site in chat.ts.
|
|
|
|
import test 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-combo-timeout-affinity-"));
|
|
process.env.DATA_DIR = TEST_DATA_DIR;
|
|
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "combo-timeout-affinity-test-secret";
|
|
|
|
const core = await import("../../src/lib/db/core.ts");
|
|
const affinityDb = await import("../../src/lib/db/sessionAccountAffinity.ts");
|
|
const pin = await import("../../src/sse/services/sessionAffinityPin.ts");
|
|
const abortReasons = await import("../../open-sse/services/combo/comboAbortReasons.ts");
|
|
|
|
const PROVIDER = "codex";
|
|
const SESSION = "session-combo-timeout";
|
|
const STALLED = "conn-stalled";
|
|
const HEALTHY = "conn-healthy";
|
|
const TTL = 30 * 60_000;
|
|
|
|
function abortedWith(reason: unknown): AbortSignal {
|
|
const controller = new AbortController();
|
|
controller.abort(reason);
|
|
return controller.signal;
|
|
}
|
|
|
|
const timedOutSignal = () => abortedWith(new Error(abortReasons.COMBO_PER_MODEL_TIMEOUT_REASON));
|
|
|
|
test.beforeEach(() => {
|
|
core.resetDbInstance();
|
|
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
|
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
|
});
|
|
|
|
test.after(() => {
|
|
core.resetDbInstance();
|
|
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
|
});
|
|
|
|
test("evicts the pin when the combo per-model timeout abandons the pinned account", () => {
|
|
affinityDb.upsertSessionAccountAffinity(SESSION, PROVIDER, STALLED, Date.now(), TTL);
|
|
|
|
const evicted = pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: SESSION,
|
|
provider: PROVIDER,
|
|
connectionId: STALLED,
|
|
modelAbortSignal: timedOutSignal(),
|
|
});
|
|
|
|
assert.equal(evicted, true, "a timed-out pinned account must lose its pin");
|
|
assert.equal(
|
|
affinityDb.getSessionAccountAffinity(SESSION, PROVIDER, TTL),
|
|
null,
|
|
"the next request must be free to pick another account"
|
|
);
|
|
});
|
|
|
|
test("leaves the pin intact on a client disconnect", () => {
|
|
affinityDb.upsertSessionAccountAffinity(SESSION, PROVIDER, STALLED, Date.now(), TTL);
|
|
|
|
const evicted = pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: SESSION,
|
|
provider: PROVIDER,
|
|
connectionId: STALLED,
|
|
modelAbortSignal: abortedWith(new Error("request_signal_aborted")),
|
|
});
|
|
|
|
assert.equal(evicted, false, "a client hanging up says nothing about account health");
|
|
assert.equal(
|
|
affinityDb.getSessionAccountAffinity(SESSION, PROVIDER, TTL)?.connectionId,
|
|
STALLED,
|
|
"pin must survive so the session keeps its prompt-cache locality"
|
|
);
|
|
});
|
|
|
|
test("leaves the pin intact when a hedged sibling cancelled this target", () => {
|
|
affinityDb.upsertSessionAccountAffinity(SESSION, PROVIDER, STALLED, Date.now(), TTL);
|
|
|
|
const evicted = pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: SESSION,
|
|
provider: PROVIDER,
|
|
connectionId: STALLED,
|
|
modelAbortSignal: abortedWith(new Error(abortReasons.COMBO_HEDGE_CANCELLED_REASON)),
|
|
});
|
|
|
|
assert.equal(evicted, false, "losing a hedge race is not an account failure");
|
|
assert.equal(affinityDb.getSessionAccountAffinity(SESSION, PROVIDER, TTL)?.connectionId, STALLED);
|
|
});
|
|
|
|
test("leaves the pin intact when the dispatch was never aborted", () => {
|
|
affinityDb.upsertSessionAccountAffinity(SESSION, PROVIDER, STALLED, Date.now(), TTL);
|
|
|
|
const evicted = pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: SESSION,
|
|
provider: PROVIDER,
|
|
connectionId: STALLED,
|
|
modelAbortSignal: new AbortController().signal,
|
|
});
|
|
|
|
assert.equal(evicted, false);
|
|
assert.equal(affinityDb.getSessionAccountAffinity(SESSION, PROVIDER, TTL)?.connectionId, STALLED);
|
|
});
|
|
|
|
test("never evicts a pin that points at a different (healthy) account", () => {
|
|
affinityDb.upsertSessionAccountAffinity(SESSION, PROVIDER, HEALTHY, Date.now(), TTL);
|
|
|
|
const evicted = pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: SESSION,
|
|
provider: PROVIDER,
|
|
connectionId: STALLED,
|
|
modelAbortSignal: timedOutSignal(),
|
|
});
|
|
|
|
assert.equal(evicted, false, "connection-matched guard must hold");
|
|
assert.equal(affinityDb.getSessionAccountAffinity(SESSION, PROVIDER, TTL)?.connectionId, HEALTHY);
|
|
});
|
|
|
|
test("no-ops without a session key or connection id", () => {
|
|
const signal = timedOutSignal();
|
|
assert.equal(
|
|
pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: null,
|
|
provider: PROVIDER,
|
|
connectionId: STALLED,
|
|
modelAbortSignal: signal,
|
|
}),
|
|
false
|
|
);
|
|
assert.equal(
|
|
pin.evictSessionAffinityOnComboTimeout({
|
|
sessionKey: SESSION,
|
|
provider: PROVIDER,
|
|
connectionId: null,
|
|
modelAbortSignal: signal,
|
|
}),
|
|
false
|
|
);
|
|
});
|
|
|
|
test("isComboPerModelTimeoutAbort accepts a bare string abort reason", () => {
|
|
assert.equal(
|
|
abortReasons.isComboPerModelTimeoutAbort(
|
|
abortedWith(abortReasons.COMBO_PER_MODEL_TIMEOUT_REASON)
|
|
),
|
|
true
|
|
);
|
|
assert.equal(abortReasons.isComboPerModelTimeoutAbort(null), false);
|
|
});
|
|
|
|
test("the combo timeout runner aborts with the shared reason constant", () => {
|
|
const src = fs.readFileSync(
|
|
new URL("../../open-sse/services/combo/targetTimeoutRunner.ts", import.meta.url),
|
|
"utf8"
|
|
);
|
|
assert.match(
|
|
src,
|
|
/timeoutController\.abort\(new Error\(COMBO_PER_MODEL_TIMEOUT_REASON\)\)/,
|
|
"the runner must use the constant the eviction predicate matches on"
|
|
);
|
|
});
|
|
|
|
test("chat.ts routes its upstream dispatch through the eviction-aware seam", () => {
|
|
const src = fs.readFileSync(new URL("../../src/sse/handlers/chat.ts", import.meta.url), "utf8");
|
|
assert.match(
|
|
src,
|
|
/dispatchChatWithAffinityEviction\(/,
|
|
"chat.ts must dispatch through the seam that owns the eviction"
|
|
);
|
|
assert.doesNotMatch(
|
|
src,
|
|
/await executeChatWithBreaker\(/,
|
|
"chat.ts must not bypass the seam by calling executeChatWithBreaker directly"
|
|
);
|
|
});
|
|
|
|
test("the dispatch seam evicts when a dispatch is abandoned", () => {
|
|
const src = fs.readFileSync(
|
|
new URL("../../src/sse/handlers/chatDispatch.ts", import.meta.url),
|
|
"utf8"
|
|
);
|
|
assert.match(
|
|
src,
|
|
/evictSessionAffinityOnComboTimeout\(/,
|
|
"chatDispatch.ts must call the eviction"
|
|
);
|
|
});
|