mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-22 23:22:09 +03:00
fix(combo): guarantee combo loops terminate with an actionable error instead of hanging silently (#10463)
Guarantees combo/target loops always terminate with an actionable error instead of hanging when an upstream hangs — fixes 10 silent-stop gaps (G1–G10): combo-loop safety timer + abort on hang → 504, unexpected task throw → 502, round-robin safety net, chaos all-panel-failure visibility, ReDoS-safe eval regex, autoRefreshDaemon swallowed-error logging, batch-item wall-clock timeout. Validated in an isolated worktree boarded onto origin/release/v3.8.50 (2 real conflicts in combo.ts and chaosEngine.ts, both additive features from concurrently-merged PRs landing at the same insertion point — resolved by combining both, verified no variable-shadowing/scoping issues): - 10/10 new regression tests pass (combo-silent-stop-gaps.test.ts): hung upstream → 504, unexpected throw → 502, chaos all-fail logged, ReDoS regex rejected, batch timeout. - 163/163 broader focused tests pass (combo-routing-engine, combo-target-timeout-runner, pipeline-router, chaos-executor, batch-processor ×2, service-batch-processor, evalrunner-builtinsuites-split). - check-file-size, check-changelog-integrity: OK. - typecheck:core: clean. - check-complexity / check-cognitive-complexity: OK, both under baseline. Co-authored-by: herjarsa <herjarsa@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
cbf23772ec
commit
d098114fe5
@@ -180,7 +180,22 @@ function dispatchOnePanelModel(opts: {
|
||||
log?.info?.(
|
||||
`CHAOS panel ${index} (${model}) ok=${res.ok} status=${res.status} textLen=${text.length}`
|
||||
);
|
||||
const part: ChaosPart = { model, index, ok: true, text };
|
||||
// G5b: honor the upstream response status — a 4xx/5xx is a panel FAILURE,
|
||||
// not a success (previously ok:true was hardcoded, so an all-error panel
|
||||
// never reached the all-failed branch and the error text was streamed as
|
||||
// if it were a successful answer).
|
||||
if (res.ok) {
|
||||
const part: ChaosPart = { model, index, ok: true, text };
|
||||
await onResult?.(part);
|
||||
return part;
|
||||
}
|
||||
const part: ChaosPart = {
|
||||
model,
|
||||
index,
|
||||
ok: false,
|
||||
text: "",
|
||||
error: `upstream ${res.status}: ${text.slice(0, 200) || res.statusText || "error"}`,
|
||||
};
|
||||
await onResult?.(part);
|
||||
return part;
|
||||
} catch (err) {
|
||||
@@ -466,8 +481,17 @@ export async function handleChaosChat(opts: {
|
||||
}
|
||||
|
||||
if (successes.length === 0) {
|
||||
const errText = "All chaos panel models failed";
|
||||
await safeEnqueue(chatChunk(chunkId, panelToDispatch[0] ?? "", errText));
|
||||
// G5 (silent-stop fix): make an all-panel failure visible server-side.
|
||||
// The status stays 200 (SSE envelope must stay well-formed), but the
|
||||
// failure is now logged with the per-model errors so operators can see
|
||||
// why the chaos panel produced nothing.
|
||||
const modelErrors = allParts.map((p) => `${p.model}: ${p.error ?? "unknown"}`).join(" | ");
|
||||
log?.warn?.(
|
||||
"CHAOS",
|
||||
`All chaos panel models failed for ${comboName ?? "panel"}: ${modelErrors}`
|
||||
);
|
||||
const errText = `All chaos panel models failed — ${modelErrors}`;
|
||||
await safeEnqueue(chatChunk(chunkId, panelToDispatch[0] ?? panel[0] ?? "", errText));
|
||||
await safeEnqueue(SSE_DONE);
|
||||
await enqueueChain;
|
||||
closed = true;
|
||||
|
||||
@@ -343,6 +343,17 @@ export async function handlePipelineCombo({
|
||||
}
|
||||
}
|
||||
|
||||
// G6 (silent-stop fix): if the reflection loop burned its retry budget and the
|
||||
// verdict is still "fail", the fall-through below returns a FAILED result
|
||||
// indistinguishable from a first-attempt failure. Surface it loudly so the
|
||||
// caller (and operator logs) can tell "retries exhausted" apart.
|
||||
if (result.reflectVerdict === "fail" && reflectionCount > 0) {
|
||||
log.warn(
|
||||
"PIPELINE",
|
||||
`Reflection retries exhausted (${reflectionCount}/${maxReflectionLoops}) — pipeline verdict still "fail", returning the original failed result`
|
||||
);
|
||||
}
|
||||
|
||||
// ── Return result ─────────────────────────────────────────────────────────
|
||||
// Check if the last stage has a streaming Response
|
||||
const lastStage = result.stages[result.stages.length - 1];
|
||||
|
||||
@@ -125,8 +125,13 @@ class AutoRefreshDaemon {
|
||||
`[AutoRefreshDaemon] Credential expired for "${providerId}" (${config.displayName})`
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
// Network errors are non-fatal — retry next cycle
|
||||
} catch (err) {
|
||||
// Network errors are non-fatal — retry next cycle. G8: log which
|
||||
// provider failed so credential problems are not silently masked.
|
||||
console.warn(
|
||||
`[AutoRefreshDaemon] Network error validating credential for "${providerId}" — retry next cycle`,
|
||||
err instanceof Error ? err.message : err
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -165,8 +170,16 @@ class AutoRefreshDaemon {
|
||||
}
|
||||
|
||||
return true;
|
||||
} catch {
|
||||
// Network errors (timeout, DNS failure) don't mean the credential is bad
|
||||
} catch (err) {
|
||||
// Network errors (timeout, DNS failure) don't mean the credential is bad.
|
||||
// G8 (silent-stop fix): the previous bare `catch { return true; }` swallowed
|
||||
// the error entirely — operators could never tell a credential was failing
|
||||
// to validate due to network trouble. Log it (provider + reason) before
|
||||
// returning the fail-open result.
|
||||
console.warn(
|
||||
`[AutoRefreshDaemon] Network error validating credential for "${providerId}" — treated as valid (fail-open), will retry next cycle`,
|
||||
err instanceof Error ? err.message : err
|
||||
);
|
||||
return true;
|
||||
} finally {
|
||||
clearTimeout(timeout);
|
||||
|
||||
@@ -506,14 +506,46 @@ async function processSingleItemWithRetry(item: BatchRequestItem, apiKey: string
|
||||
}
|
||||
}
|
||||
|
||||
// G10 (silent-stop fix): individual batch-item dispatches can hang indefinitely
|
||||
// if the upstream route stalls (no signal/timeout plumbed through). Bound each
|
||||
// item with a wall-clock timeout so a stuck item fails fast (recorded as an item
|
||||
// error) instead of freezing the whole batch loop. The orphaned dispatch keeps
|
||||
// running in the background but can no longer block the batch.
|
||||
export const BATCH_ITEM_DISPATCH_TIMEOUT_MS = 120_000;
|
||||
|
||||
/**
|
||||
* G10: race a promise against a wall-clock deadline. Exported for unit testing
|
||||
* (batch dispatch is a module-internal import, so the timeout mechanism itself
|
||||
* is verified directly here).
|
||||
*/
|
||||
export function withItemDispatchTimeout<T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
label: string
|
||||
): Promise<T> {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeoutPromise = new Promise<never>((_, reject) => {
|
||||
timer = setTimeout(
|
||||
() => reject(new Error(`${label} timed out after ${timeoutMs}ms`)),
|
||||
timeoutMs
|
||||
);
|
||||
});
|
||||
return Promise.race([promise, timeoutPromise]).finally(() => {
|
||||
if (timer) clearTimeout(timer);
|
||||
});
|
||||
}
|
||||
|
||||
async function processSingleItem(item: BatchRequestItem, apiKey: string) {
|
||||
const body = buildRequestBody(item);
|
||||
|
||||
return await dispatch.dispatchBatchApiRequest({
|
||||
endpoint: item.url,
|
||||
body,
|
||||
apiKey,
|
||||
});
|
||||
return withItemDispatchTimeout(
|
||||
dispatch.dispatchBatchApiRequest({
|
||||
endpoint: item.url,
|
||||
body,
|
||||
apiKey,
|
||||
}),
|
||||
BATCH_ITEM_DISPATCH_TIMEOUT_MS,
|
||||
`Batch item dispatch (${item.url})`
|
||||
);
|
||||
}
|
||||
|
||||
export function buildRequestBody(item: BatchRequestItem) {
|
||||
|
||||
@@ -179,6 +179,8 @@ import {
|
||||
TRANSIENT_FOR_SEMAPHORE,
|
||||
MAX_FALLBACK_WAIT_MS,
|
||||
MAX_GLOBAL_ATTEMPTS,
|
||||
COMBO_LOOP_SAFETY_TIMEOUT_MS,
|
||||
COMBO_SAFETY_DRAIN_MS,
|
||||
isAllAccountsRateLimitedResponse,
|
||||
clampComboDepth,
|
||||
shouldSkipForPredictedTtft,
|
||||
@@ -1046,11 +1048,54 @@ async function handleComboChatInner({
|
||||
const globalPromise = new Promise<Response>((res) => {
|
||||
globalResolve = res;
|
||||
});
|
||||
|
||||
// G1 (silent-stop fix): the speculative loop's `Promise.race` waits on
|
||||
// `globalPromise`, which is ONLY resolved from inside a task (success or
|
||||
// fatal error). If a target hangs — e.g. the operator disabled the per-model
|
||||
// timeout (`targetTimeoutMs: 0`) and the upstream never settles — the race
|
||||
// never resolves and the request hangs forever with no response. This safety
|
||||
// promise force-resolves after the combo budget (comboTimeoutMs when set,
|
||||
// otherwise a hard ceiling) so the request ALWAYS terminates with an
|
||||
// actionable 504 instead of dying silently. `comboExpired` is flipped so the
|
||||
// target loop stops launching new work; the existing comboExpired branch
|
||||
// returns the aggregated 504.
|
||||
const loopSafetyMs =
|
||||
comboTimeoutMs > 0 ? comboTimeoutMs : COMBO_LOOP_SAFETY_TIMEOUT_MS;
|
||||
let loopSafetyFired = false;
|
||||
let loopSafetyTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
const loopSafetyPromise = new Promise<Response>((resolve) => {
|
||||
loopSafetyTimer = setTimeout(() => {
|
||||
loopSafetyFired = true;
|
||||
log.warn(
|
||||
"COMBO",
|
||||
`Combo loop safety timeout (${loopSafetyMs}ms) reached without a terminal response — force-terminating`
|
||||
);
|
||||
resolve(
|
||||
errorResponseWithComboDiagnostics(
|
||||
504,
|
||||
`Combo global timeout (${loopSafetyMs}ms) without a terminal response`,
|
||||
buildComboDiag("combo_timeout"),
|
||||
{ code: "COMBO_TIMEOUT", type: "server_error" }
|
||||
)
|
||||
);
|
||||
}, loopSafetyMs);
|
||||
loopSafetyTimer.unref?.();
|
||||
});
|
||||
const runningTasks = new Set<Promise<void>>();
|
||||
let anySuccess = false;
|
||||
// #10681: steps already recorded as dispatched (so per-target retries do not
|
||||
// duplicate the decision).
|
||||
const dispatchedTargets = new Set<string>();
|
||||
// G1: flip comboExpired as soon as the safety timer fires so the next loop
|
||||
// iteration breaks instead of launching more targets after the budget, and
|
||||
// abort every in-flight target so a hung upstream actually gets cancelled
|
||||
// (not just "response stops").
|
||||
const markLoopExpiredIfSafetyFired = () => {
|
||||
if (loopSafetyFired) {
|
||||
comboExpired = true;
|
||||
for (const [, ac] of abortControllers.entries()) ac.abort();
|
||||
}
|
||||
};
|
||||
const abortControllers = new Map<number, AbortController>();
|
||||
const zeroLatencyOptimizationsEnabled = config.zeroLatencyOptimizationsEnabled === true;
|
||||
const hasProtectedPriorityTarget =
|
||||
@@ -2363,6 +2408,17 @@ async function handleComboChatInner({
|
||||
})().catch((err) => {
|
||||
const logError = log.error ?? log.warn;
|
||||
logError("COMBO", `Speculative task error for target ${i}`, err);
|
||||
// G2 (silent-stop fix): never leave the speculative loop waiting on an
|
||||
// unresolved globalPromise. If a task throws unexpectedly (outside
|
||||
// executeTarget's error handling) and no other task succeeds, the post-loop
|
||||
// `Promise.race([globalPromise, ...])` would hang forever. Resolve with a
|
||||
// 502 so the request terminates with an actionable error.
|
||||
if (!anySuccess && globalResolve) {
|
||||
anySuccess = true;
|
||||
globalResolve(
|
||||
errorResponse(502, `Combo target ${i} failed with an unexpected error`)
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
runningTasks.add(task);
|
||||
@@ -2380,10 +2436,11 @@ async function handleComboChatInner({
|
||||
timeoutResolve = r;
|
||||
setTimeout(r, hedgeDelay);
|
||||
});
|
||||
await Promise.race([task, globalPromise, timeoutPromise]);
|
||||
await Promise.race([task, globalPromise, timeoutPromise, loopSafetyPromise]);
|
||||
} else {
|
||||
await Promise.race([task, globalPromise]);
|
||||
await Promise.race([task, globalPromise, loopSafetyPromise]);
|
||||
}
|
||||
markLoopExpiredIfSafetyFired();
|
||||
|
||||
// Global combo timeout check: after each target completes, stop trying
|
||||
// further targets if the total elapsed time exceeds comboTimeoutMs.
|
||||
@@ -2398,13 +2455,51 @@ async function handleComboChatInner({
|
||||
}
|
||||
|
||||
if (!anySuccess && runningTasks.size > 0) {
|
||||
await Promise.race([globalPromise, Promise.all([...runningTasks])]);
|
||||
// G1: include loopSafetyPromise so a hung last task (per-model timeout
|
||||
// disabled) cannot freeze this post-loop race forever.
|
||||
await Promise.race([globalPromise, Promise.all([...runningTasks]), loopSafetyPromise]);
|
||||
markLoopExpiredIfSafetyFired();
|
||||
}
|
||||
|
||||
// G1: if the safety timer won the race (request would otherwise hang), give
|
||||
// in-flight tasks a short drain window to land their per-model errors into
|
||||
// comboErrors so the 504 carries the same "tried: a (500)" summary the
|
||||
// regular comboExpired branch produces — then return the safety 504.
|
||||
if (loopSafetyFired && !anySuccess) {
|
||||
if (runningTasks.size > 0) {
|
||||
await Promise.race([
|
||||
Promise.allSettled([...runningTasks]),
|
||||
new Promise((resolve) => setTimeout(resolve, COMBO_SAFETY_DRAIN_MS)),
|
||||
]);
|
||||
}
|
||||
const summary = comboErrors
|
||||
.slice(0, 5)
|
||||
.map((e) => `${e.model} (${e.status})`)
|
||||
.join(", ");
|
||||
const msg =
|
||||
`Combo global timeout (${loopSafetyMs}ms) after ${recordedAttempts}/${orderedTargets.length} targets` +
|
||||
(comboErrors.length > 0
|
||||
? ` | tried: ${summary}${comboErrors.length > 5 ? `... (+${comboErrors.length - 5})` : ""}`
|
||||
: "") +
|
||||
" without a terminal response";
|
||||
return errorResponseWithComboDiagnostics(
|
||||
504,
|
||||
msg,
|
||||
buildComboDiag("combo_timeout"),
|
||||
{ code: "COMBO_TIMEOUT", type: "server_error" }
|
||||
);
|
||||
}
|
||||
|
||||
// #10681: finalize the decision trace (success).
|
||||
finalizeComboTrace(traceInvocationId, orderedTargets);
|
||||
finishComboTrace(traceInvocationId, { status: 200 });
|
||||
if (anySuccess) {
|
||||
// G1: clear the safety timer on the happy path so a successful combo does
|
||||
// not leave a 10-minute timer alive per request.
|
||||
if (loopSafetyTimer) {
|
||||
clearTimeout(loopSafetyTimer);
|
||||
loopSafetyTimer = null;
|
||||
}
|
||||
return await globalPromise;
|
||||
}
|
||||
|
||||
@@ -2923,6 +3018,33 @@ async function handleRoundRobinCombo({
|
||||
// and the "Done with this model" path below), mirroring handleComboChat.
|
||||
const rrOutcomes: Array<ComboErrorEntry> = [];
|
||||
|
||||
// G4 (silent-stop fix): round-robin has NO global timeout — a hung model
|
||||
// (per-model timeout disabled via targetTimeoutMs: 0) would freeze the request
|
||||
// forever with no response. Safety promise + timer bound the whole loop; when
|
||||
// it fires, rrExpired flips and every subsequent model attempt short-circuits
|
||||
// to the 504. Cleaned up in the loop's finally.
|
||||
const rrConfiguredTimeoutMs =
|
||||
(config as { comboTimeoutMs?: number }).comboTimeoutMs ?? 0;
|
||||
const rrLoopSafetyMs =
|
||||
rrConfiguredTimeoutMs > 0 ? rrConfiguredTimeoutMs : COMBO_LOOP_SAFETY_TIMEOUT_MS;
|
||||
let rrExpired = false;
|
||||
let rrLoopSafetyTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let rrResolveSafety: ((res: Response) => void) | null = null;
|
||||
const rrSafetyPromise = new Promise<Response>((resolve) => {
|
||||
rrResolveSafety = resolve;
|
||||
});
|
||||
rrLoopSafetyTimer = setTimeout(() => {
|
||||
rrExpired = true;
|
||||
log.warn(
|
||||
"COMBO-RR",
|
||||
`Round-robin loop exceeded ${rrLoopSafetyMs}ms without a terminal response — force-terminating`
|
||||
);
|
||||
rrResolveSafety?.(
|
||||
errorResponse(504, `Round-robin combo exceeded ${rrLoopSafetyMs}ms without a terminal response`)
|
||||
);
|
||||
}, rrLoopSafetyMs);
|
||||
rrLoopSafetyTimer.unref?.();
|
||||
|
||||
// #1731: Per-request in-memory set of providers whose quota is fully exhausted.
|
||||
// When a target returns a quota-exhausted 429, remaining targets from the same
|
||||
// provider are skipped to avoid the cascade through N same-provider targets.
|
||||
@@ -2931,8 +3053,11 @@ async function handleRoundRobinCombo({
|
||||
const transientRateLimitedProviders = new Set<string>();
|
||||
|
||||
// Try each model starting from the round-robin target
|
||||
for (let offset = 0; offset < modelCount; offset++) {
|
||||
const modelIndex = (rrStartIndex + offset) % modelCount;
|
||||
try {
|
||||
for (let offset = 0; offset < modelCount; offset++) {
|
||||
// G4: stop launching new work once the safety timer fired.
|
||||
if (rrExpired) break;
|
||||
const modelIndex = (rrStartIndex + offset) % modelCount;
|
||||
const target = filteredTargets[modelIndex];
|
||||
const modelStr = target.modelStr;
|
||||
const provider = target.provider;
|
||||
@@ -3077,11 +3202,15 @@ async function handleRoundRobinCombo({
|
||||
fingerprint: resolveTargetFingerprint(target) ?? "",
|
||||
});
|
||||
|
||||
const result = await handleSingleModel(attemptBody, modelStr, {
|
||||
...targetForAttempt,
|
||||
effectiveComboStrategy: "round-robin",
|
||||
failoverBeforeRetry: config.failoverBeforeRetry,
|
||||
});
|
||||
const result = await Promise.race([
|
||||
handleSingleModel(attemptBody, modelStr, {
|
||||
...targetForAttempt,
|
||||
effectiveComboStrategy: "round-robin",
|
||||
failoverBeforeRetry: config.failoverBeforeRetry,
|
||||
}),
|
||||
rrSafetyPromise,
|
||||
]);
|
||||
if (rrExpired) return result; // G4: safety timer won — stop everything
|
||||
|
||||
// Quota-aware scheduling: reserve the estimated budget for this
|
||||
// dispatch (opt-in, same env gate as the pre-request check). Best-effort
|
||||
@@ -3519,6 +3648,26 @@ async function handleRoundRobinCombo({
|
||||
release();
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
// G4: unexpected exception in the round-robin loop must never crash the
|
||||
// request silently — surface a 500 instead of hanging the client.
|
||||
log.error?.("COMBO-RR", "Unexpected error in round-robin loop", err);
|
||||
return errorResponse(500, "Unexpected error in round-robin combo");
|
||||
} finally {
|
||||
if (rrLoopSafetyTimer) {
|
||||
clearTimeout(rrLoopSafetyTimer);
|
||||
rrLoopSafetyTimer = null;
|
||||
}
|
||||
}
|
||||
|
||||
// G4: if the safety timer fired between iterations (no race captured it),
|
||||
// terminate with the actionable 504 instead of the generic exhaustion path.
|
||||
if (rrExpired) {
|
||||
return errorResponse(
|
||||
504,
|
||||
`Round-robin combo exceeded ${rrLoopSafetyMs}ms without a terminal response`
|
||||
);
|
||||
}
|
||||
|
||||
// All models exhausted
|
||||
const latencyMs = Date.now() - startTime;
|
||||
|
||||
@@ -18,6 +18,15 @@ import type { ResolvedComboTarget } from "./types.ts";
|
||||
|
||||
// Status codes that should mark round-robin target semaphores as cooling down.
|
||||
export const TRANSIENT_FOR_SEMAPHORE = [429, 502, 503, 504];
|
||||
// G1 (silent-stop fix): hard ceiling for the combo target loop when the operator
|
||||
// left comboTimeoutMs at 0 ("unlimited"). Without this, a hung upstream (per-model
|
||||
// timeout disabled) would freeze the request forever with no response. 10 minutes
|
||||
// is a generous bound for legitimate long-running fallback cascades.
|
||||
export const COMBO_LOOP_SAFETY_TIMEOUT_MS = 10 * 60 * 1000;
|
||||
// G1: after the safety timer fires, wait this long for in-flight targets to land
|
||||
// their per-model errors into comboErrors (so the 504 carries the same "tried:"
|
||||
// summary as the regular timeout path) before returning the safety response.
|
||||
export const COMBO_SAFETY_DRAIN_MS = 2000;
|
||||
// Patterns that signal all accounts for a provider are rate-limited / exhausted.
|
||||
// Used to detect 503 responses from handleNoCredentials so combo can fallback.
|
||||
export const ALL_ACCOUNTS_RATE_LIMITED_PATTERNS = [
|
||||
|
||||
@@ -106,6 +106,13 @@ export function buildTargetTimeoutRunner(deps: {
|
||||
target?: SingleModelTarget
|
||||
): Promise<Response> => {
|
||||
if (comboTargetTimeoutMs <= 0) {
|
||||
// G3 (silent-stop fix): a disabled per-model timeout means a hung upstream
|
||||
// stalls the target until the combo loop safety timer (COMBO_LOOP_SAFETY_TIMEOUT_MS)
|
||||
// force-terminates — surface that dependency instead of silently running bare.
|
||||
log.warn(
|
||||
"COMBO",
|
||||
`Per-model combo timeout is DISABLED (comboTargetTimeoutMs=${comboTargetTimeoutMs}) for ${modelStr} — a hung upstream will hang this target until the combo loop safety timeout`
|
||||
);
|
||||
return handleSingleModel(b, modelStr, target).catch((err) =>
|
||||
errorResponse(502, err?.message ?? "Upstream model error")
|
||||
);
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
*/
|
||||
|
||||
import { getCustomEvalSuite, listCustomEvalSuites } from "@/lib/db/evals";
|
||||
import safeRegex from "safe-regex";
|
||||
import {
|
||||
goldenSet,
|
||||
codingSuite,
|
||||
@@ -161,6 +162,16 @@ export function evaluateCase(evalCase: any, actualOutput: string) {
|
||||
details.error = "Regex pattern too large for safe evaluation.";
|
||||
break;
|
||||
}
|
||||
// G7 (silent-stop fix): a catastrophic regex (nested quantifiers like
|
||||
// `(a+)+$`) can hang the event loop for minutes on adversarial output —
|
||||
// the eval loop then "stops doing anything" with no error. safe-regex
|
||||
// statically rejects such patterns before test() runs.
|
||||
if (!safeRegex(regex)) {
|
||||
passed = false;
|
||||
details.error =
|
||||
"Regex pattern rejected as potentially unsafe (catastrophic backtracking risk). Simplify the pattern.";
|
||||
break;
|
||||
}
|
||||
passed = regex.test(actualOutput);
|
||||
details.pattern = String(expectedValue);
|
||||
break;
|
||||
|
||||
305
tests/unit/combo-silent-stop-gaps.test.ts
Normal file
305
tests/unit/combo-silent-stop-gaps.test.ts
Normal file
@@ -0,0 +1,305 @@
|
||||
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";
|
||||
|
||||
// Isolate DATA_DIR before any DB-touching import (combo.ts pulls in the
|
||||
// SQLite layer) — mirrors tests/unit/combo-routing-engine.test.ts.
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-silent-stop-gaps-"));
|
||||
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Silent-stop gap fixes — regression tests
|
||||
// ---------------------------------------------------------------------------
|
||||
// These lock the G1–G10 fixes for loops that could terminate silently:
|
||||
// G1 combo.ts globalPromise safety timer (hung target → 504, never hang)
|
||||
// G2 combo.ts task wrapper catch resolves globalPromise (unexpected throw → 502)
|
||||
// G3 targetTimeoutRunner warns when per-model timeout is disabled
|
||||
// G4 round-robin loop safety timer (hung model → 504)
|
||||
// G5 chaosEngine logs all-panel failures with per-model errors
|
||||
// G7 evalRunner rejects catastrophic regex (ReDoS) via safe-regex
|
||||
// G8 autoRefreshDaemon logs network errors per provider
|
||||
// G10 batchProcessor item dispatch timeout (hung item fails fast)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const { buildTargetTimeoutRunner } =
|
||||
await import("../../open-sse/services/combo/targetTimeoutRunner.ts");
|
||||
const { handleComboChat } = await import("../../open-sse/services/combo.ts");
|
||||
const { handleChaosChat } = await import("../../open-sse/services/autoCombo/chaosEngine.ts");
|
||||
const { evaluateCase } = await import("../../src/lib/evals/evalRunner.ts");
|
||||
const { withItemDispatchTimeout } = await import("../../open-sse/services/batchProcessor.ts");
|
||||
const { saveModelsDevCapabilities } = await import("../../src/lib/modelsDevSync.ts");
|
||||
|
||||
function capabilityEntry(limitContext: unknown, overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
tool_call: true,
|
||||
reasoning: false,
|
||||
attachment: false,
|
||||
structured_output: true,
|
||||
temperature: true,
|
||||
modalities_input: JSON.stringify(["text"]),
|
||||
modalities_output: JSON.stringify(["text"]),
|
||||
knowledge_cutoff: null,
|
||||
release_date: null,
|
||||
last_updated: null,
|
||||
status: null,
|
||||
family: null,
|
||||
open_weights: false,
|
||||
limit_context: limitContext,
|
||||
limit_input: limitContext,
|
||||
limit_output: 4096,
|
||||
interleaved_field: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function createLog() {
|
||||
const entries: Array<{ level: string; tag: string; msg: string }> = [];
|
||||
return {
|
||||
info: (tag: string, msg: string) => entries.push({ level: "info", tag, msg }),
|
||||
warn: (tag: string, msg: string) => entries.push({ level: "warn", tag, msg }),
|
||||
error: (tag: string, msg: string) => entries.push({ level: "error", tag, msg }),
|
||||
debug: (tag: string, msg: string) => entries.push({ level: "debug", tag, msg }),
|
||||
entries,
|
||||
};
|
||||
}
|
||||
|
||||
function okResponse(body: Record<string, unknown> = { choices: [{ message: { content: "ok" } }] }) {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
function errorResponse(status: number, message: string = `Error ${status}`) {
|
||||
return new Response(JSON.stringify({ error: { message } }), {
|
||||
status,
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
// ── G1: combo loop safety timer ─────────────────────────────────────────────
|
||||
// A hung target (per-model timeout disabled / upstream never settles) must NOT
|
||||
// freeze the request forever: the safety timer force-resolves with 504.
|
||||
test.after(async () => {
|
||||
if (ORIGINAL_DATA_DIR === undefined) {
|
||||
delete process.env.DATA_DIR;
|
||||
} else {
|
||||
process.env.DATA_DIR = ORIGINAL_DATA_DIR;
|
||||
}
|
||||
try {
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
});
|
||||
|
||||
test("G1: hung target with per-model timeout disabled → 504, not a silent hang", async () => {
|
||||
const log = createLog();
|
||||
const combo = {
|
||||
name: "g1-hang",
|
||||
models: ["openai/gpt-4o-mini"],
|
||||
config: {
|
||||
// Per-model timeout off + tiny global budget so the test runs in ms.
|
||||
targetTimeoutMs: 0,
|
||||
comboTimeoutMs: 100,
|
||||
},
|
||||
};
|
||||
saveModelsDevCapabilities({ openai: { "gpt-4o-mini": capabilityEntry(128000) } });
|
||||
const startedAt = Date.now();
|
||||
const result = await handleComboChat({
|
||||
body: {},
|
||||
combo,
|
||||
// Never settles — simulates an upstream that accepts the connection and
|
||||
// then stalls forever.
|
||||
handleSingleModel: () => new Promise<Response>(() => {}),
|
||||
isModelAvailable: async () => true,
|
||||
log,
|
||||
settings: null,
|
||||
allCombos: null,
|
||||
});
|
||||
const elapsed = Date.now() - startedAt;
|
||||
assert.equal(result.status, 504);
|
||||
assert.ok(elapsed < 10_000, `safety timeout took ${elapsed}ms — too slow`);
|
||||
assert.ok(
|
||||
log.entries.some((e) => e.level === "warn" && /safety timeout/i.test(String(e.msg))),
|
||||
"expected a warn about the combo loop safety timeout"
|
||||
);
|
||||
});
|
||||
|
||||
// ── G2: task wrapper catch resolves globalPromise ───────────────────────────
|
||||
// An unexpected throw inside executeTarget (e.g. isModelAvailable exploding)
|
||||
// must surface as 502, not leave the request hanging.
|
||||
test("G2: unexpected throw in a target surfaces 502 instead of hanging", async () => {
|
||||
const log = createLog();
|
||||
const combo = {
|
||||
name: "g2-throw",
|
||||
models: ["openai/gpt-4o-mini"],
|
||||
};
|
||||
const result = await handleComboChat({
|
||||
body: {},
|
||||
combo,
|
||||
handleSingleModel: async () => okResponse(),
|
||||
// Throw inside executeTarget before dispatch — the wrapper catch must
|
||||
// resolve globalPromise so the loop terminates.
|
||||
isModelAvailable: async () => {
|
||||
throw new Error("unexpected availability explosion");
|
||||
},
|
||||
log,
|
||||
settings: null,
|
||||
allCombos: null,
|
||||
});
|
||||
assert.equal(result.status, 502);
|
||||
const body = await result.text();
|
||||
assert.match(body, /unexpected error/i);
|
||||
assert.ok(
|
||||
log.entries.some((e) => e.level === "error" && /Speculative task error/i.test(String(e.msg))),
|
||||
"expected the speculative task error to be logged"
|
||||
);
|
||||
});
|
||||
|
||||
// ── G3: targetTimeoutRunner warns when per-model timeout is disabled ────────
|
||||
test("G3: targetTimeoutRunner warns when comboTargetTimeoutMs <= 0", async () => {
|
||||
const log = createLog();
|
||||
const runner = buildTargetTimeoutRunner({
|
||||
handleSingleModel: async () => new Response("ok"),
|
||||
comboTargetTimeoutMs: 0,
|
||||
log,
|
||||
});
|
||||
const res = await runner({}, "m");
|
||||
assert.equal(await res.text(), "ok");
|
||||
assert.ok(
|
||||
log.entries.some((e) => e.level === "warn" && /DISABLED|disabled/i.test(String(e.msg))),
|
||||
"expected a warn about the disabled per-model timeout"
|
||||
);
|
||||
});
|
||||
|
||||
// ── G4: round-robin loop safety timer ───────────────────────────────────────
|
||||
test("G4: round-robin hung model → 504 via loop safety timer", async () => {
|
||||
const log = createLog();
|
||||
const combo = {
|
||||
name: "g4-rr-hang",
|
||||
models: ["openai/gpt-4o-mini", "claude/sonnet"],
|
||||
strategy: "round-robin",
|
||||
config: {
|
||||
targetTimeoutMs: 0,
|
||||
comboTimeoutMs: 100,
|
||||
},
|
||||
};
|
||||
saveModelsDevCapabilities({
|
||||
openai: { "gpt-4o-mini": capabilityEntry(128000) },
|
||||
claude: { sonnet: capabilityEntry(200000) },
|
||||
});
|
||||
const startedAt = Date.now();
|
||||
const result = await handleComboChat({
|
||||
body: {},
|
||||
combo,
|
||||
handleSingleModel: () => new Promise<Response>(() => {}),
|
||||
isModelAvailable: async () => true,
|
||||
log,
|
||||
settings: null,
|
||||
allCombos: null,
|
||||
});
|
||||
const elapsed = Date.now() - startedAt;
|
||||
assert.equal(result.status, 504);
|
||||
assert.ok(elapsed < 10_000, `RR safety timeout took ${elapsed}ms — too slow`);
|
||||
assert.ok(
|
||||
log.entries.some((e) => e.level === "warn" && /Round-robin/i.test(String(e.msg))),
|
||||
"expected a warn about the round-robin safety timeout"
|
||||
);
|
||||
});
|
||||
|
||||
// ── G5: chaosEngine logs all-panel failures ─────────────────────────────────
|
||||
test("G5: chaos all-panel failure is logged with per-model errors", async () => {
|
||||
const log = createLog();
|
||||
const res = await handleChaosChat({
|
||||
body: {},
|
||||
models: ["openai/gpt-4o-mini", "claude/sonnet"],
|
||||
handleSingleModel: async () => errorResponse(503, "upstream down"),
|
||||
log,
|
||||
comboName: "g5-chaos",
|
||||
});
|
||||
assert.equal(res.status, 200); // SSE envelope stays well-formed
|
||||
const body = await res.text();
|
||||
assert.match(body, /All chaos panel models failed/);
|
||||
assert.match(body, /upstream down/);
|
||||
assert.ok(
|
||||
log.entries.some(
|
||||
(e) => e.level === "warn" && /All chaos panel models failed/i.test(String(e.msg))
|
||||
),
|
||||
"expected the all-failed warn with model errors"
|
||||
);
|
||||
});
|
||||
|
||||
// ── G7: evalRunner rejects catastrophic regex (ReDoS guard) ─────────────────
|
||||
test("G7: catastrophic regex is rejected instead of hanging the eval loop", () => {
|
||||
const startedAt = Date.now();
|
||||
const result = evaluateCase(
|
||||
{ id: "redos", name: "redos", expected: { strategy: "regex", value: "(a+)+$" } },
|
||||
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!"
|
||||
);
|
||||
const elapsed = Date.now() - startedAt;
|
||||
assert.equal(result.passed, false);
|
||||
assert.match(String(result.details?.error ?? ""), /unsafe|backtracking/i);
|
||||
assert.ok(elapsed < 1000, `regex eval took ${elapsed}ms — ReDoS guard failed`);
|
||||
});
|
||||
|
||||
test("G7: benign regex still evaluates normally", () => {
|
||||
const result = evaluateCase(
|
||||
{ id: "ok-regex", name: "ok-regex", expected: { strategy: "regex", value: "^hello" } },
|
||||
"hello world"
|
||||
);
|
||||
assert.equal(result.passed, true);
|
||||
});
|
||||
|
||||
// ── G8: autoRefreshDaemon logs network errors per provider ──────────────────
|
||||
test("G8: autoRefreshDaemon logs network errors instead of swallowing them", async () => {
|
||||
const { autoRefreshDaemon } = await import("../../open-sse/services/autoRefreshDaemon.ts");
|
||||
const originalFetch = globalThis.fetch;
|
||||
const originalWarn = console.warn;
|
||||
const warnings: string[] = [];
|
||||
console.warn = (...args: unknown[]) => warnings.push(args.map(String).join(" "));
|
||||
// Simulate a provider whose validation request blows up (network error).
|
||||
globalThis.fetch = (async () => {
|
||||
throw new Error("ECONNRESET");
|
||||
}) as typeof fetch;
|
||||
try {
|
||||
autoRefreshDaemon.registerCredential("claude-web", "cookie-value");
|
||||
await autoRefreshDaemon.check();
|
||||
assert.ok(
|
||||
warnings.some((w) => w.includes("claude-web") && w.includes("ECONNRESET")),
|
||||
"expected a warn naming the provider and the network error, got: " + warnings.join(" | ")
|
||||
);
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
console.warn = originalWarn;
|
||||
autoRefreshDaemon.unregisterCredential("claude-web");
|
||||
}
|
||||
});
|
||||
|
||||
// ── G10: batch item dispatch timeout ────────────────────────────────────────
|
||||
test("G10: hung batch item dispatch fails fast via wall-clock timeout", async () => {
|
||||
const startedAt = Date.now();
|
||||
await assert.rejects(
|
||||
withItemDispatchTimeout(
|
||||
new Promise<Response>(() => {}), // never settles
|
||||
50,
|
||||
"Batch item dispatch (/v1/chat/completions)"
|
||||
),
|
||||
/timed out after 50ms/
|
||||
);
|
||||
const elapsed = Date.now() - startedAt;
|
||||
assert.ok(elapsed < 5000, `timeout fired after ${elapsed}ms — too slow`);
|
||||
});
|
||||
|
||||
test("G10: fast dispatch wins the race untouched", async () => {
|
||||
const res = await withItemDispatchTimeout(
|
||||
Promise.resolve(new Response("ok", { status: 200 })),
|
||||
1000,
|
||||
"Batch item dispatch"
|
||||
);
|
||||
assert.equal(res.status, 200);
|
||||
assert.equal(await res.text(), "ok");
|
||||
});
|
||||
Reference in New Issue
Block a user