fix(sse): default combo per-target timeout to 120s for fast failover (#4365)

Combo per-target timeout inherited the full FETCH_TIMEOUT_MS (600s) when a
combo did not set its own targetTimeoutMs, so a single hung/slow target stalled
the whole combo for up to 10 minutes before falling through to the next model.

Introduce DEFAULT_COMBO_TARGET_TIMEOUT_MS (120s) as the unset-default in
resolveComboTargetTimeoutMs (new 3rd arg) and wire it in phaseComboSetup. The
upstream ceiling (600s) and per-combo opt-out (targetTimeoutMs, up to the
ceiling) are preserved; single non-combo requests are unchanged. For streaming
requests this only bounds time-to-first-headers, so token generation is not cut
short.

TDD: failing-then-passing unit test in tests/unit/combo-config.test.ts.
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-20 11:07:39 -03:00
committed by GitHub
parent f628135e3c
commit 8eded5ebf2
3 changed files with 59 additions and 8 deletions

View File

@@ -20,6 +20,7 @@ import { applyComboAgentMiddleware } from "../comboAgentMiddleware.ts";
import {
resolveComboSetupConfig,
resolveComboTargetTimeoutMs,
DEFAULT_COMBO_TARGET_TIMEOUT_MS,
} from "../comboConfig.ts";
import { resolveResilienceSettings } from "../../../src/lib/resilience/settings";
import { FETCH_TIMEOUT_MS } from "../../config/constants.ts";
@@ -112,7 +113,11 @@ export function phaseComboSetup(ctx: ComboContext): ComboSetup {
// Use config cascade before dispatch so all strategies, pinned context routes,
// and round-robin targets share the same timeout policy.
const config = resolveComboSetupConfig(combo, settings);
const comboTargetTimeoutMs = resolveComboTargetTimeoutMs(config, FETCH_TIMEOUT_MS);
const comboTargetTimeoutMs = resolveComboTargetTimeoutMs(
config,
FETCH_TIMEOUT_MS,
DEFAULT_COMBO_TARGET_TIMEOUT_MS
);
const reasoningTokenBufferEnabled = config.reasoningTokenBufferEnabled !== false;
return {

View File

@@ -13,6 +13,18 @@ import { MAX_TIMER_TIMEOUT_MS } from "../../src/shared/utils/runtimeTimeouts.ts"
*/
export const PRE_SCREEN_CONCURRENCY = 5;
/**
* Default per-target timeout for combo fallback when a combo does not set its own
* `targetTimeoutMs`. Combos exist to fail over fast, so inheriting the full upstream
* request timeout (FETCH_TIMEOUT_MS, 600s by default) made a single hung target stall
* the whole combo for up to 10 minutes before falling through to the next model
* (escalated cmqlrhd7c). For STREAMING requests this only bounds the time-to-first-headers
* — token generation streams after the response resolves, so it is NOT cut short. Operators
* can still raise it per-combo via `targetTimeoutMs` (capped at the upstream ceiling), or set
* a longer value for slow non-streaming reasoning combos.
*/
export const DEFAULT_COMBO_TARGET_TIMEOUT_MS = 120_000;
const DEFAULT_COMBO_CONFIG = {
strategy: "priority",
maxRetries: 1,
@@ -111,16 +123,28 @@ function normalizePositiveTimeoutMs(value: unknown): number {
export function resolveComboTargetTimeoutMs(
config: Record<string, unknown> | null | undefined,
upstreamTimeoutMs: number
upstreamTimeoutMs: number,
defaultTimeoutMs: number = 0
): number {
const inheritedTimeoutMs = normalizePositiveTimeoutMs(upstreamTimeoutMs);
const ceilingTimeoutMs = normalizePositiveTimeoutMs(upstreamTimeoutMs);
const configuredTimeoutMs = isRecord(config)
? normalizePositiveTimeoutMs(config.targetTimeoutMs)
: 0;
if (configuredTimeoutMs <= 0) return inheritedTimeoutMs;
if (inheritedTimeoutMs <= 0) return configuredTimeoutMs;
return Math.min(configuredTimeoutMs, inheritedTimeoutMs);
// Explicit per-combo config: honour it, but never extend past the upstream ceiling.
if (configuredTimeoutMs > 0) {
if (ceilingTimeoutMs <= 0) return configuredTimeoutMs;
return Math.min(configuredTimeoutMs, ceilingTimeoutMs);
}
// Unset config: fall back to the saner combo default (when provided) so a hung target
// fails over fast instead of inheriting the full upstream timeout. Never exceed the
// ceiling. When no default is given OR the upstream timeout is disabled (0 = unbounded),
// preserve the legacy "inherit the upstream ceiling" behavior.
const fallbackDefaultMs = normalizePositiveTimeoutMs(defaultTimeoutMs);
if (ceilingTimeoutMs <= 0) return ceilingTimeoutMs;
if (fallbackDefaultMs <= 0) return ceilingTimeoutMs;
return Math.min(fallbackDefaultMs, ceilingTimeoutMs);
}
/**

View File

@@ -1,8 +1,12 @@
import test from "node:test";
import assert from "node:assert/strict";
const { resolveComboConfig, getDefaultComboConfig, resolveComboTargetTimeoutMs } =
await import("../../open-sse/services/comboConfig.ts");
const {
resolveComboConfig,
getDefaultComboConfig,
resolveComboTargetTimeoutMs,
DEFAULT_COMBO_TARGET_TIMEOUT_MS,
} = await import("../../open-sse/services/comboConfig.ts");
const { createComboSchema, updateComboDefaultsSchema } =
await import("../../src/shared/validation/schemas.ts");
const { MAX_TIMER_TIMEOUT_MS } = await import("../../src/shared/utils/runtimeTimeouts.ts");
@@ -257,6 +261,24 @@ test("resolveComboTargetTimeoutMs inherits the upstream timeout and only shorten
assert.equal(resolveComboTargetTimeoutMs({}, 999999999999), MAX_TIMER_TIMEOUT_MS);
});
test("resolveComboTargetTimeoutMs falls back to the saner combo default when unset", () => {
// The combo default is the documented 120s fallback-latency cap.
assert.equal(DEFAULT_COMBO_TARGET_TIMEOUT_MS, 120000);
// Unset config → use the default (capped at the ceiling), NOT the full upstream ceiling.
// This is what shortens a hung-target failover from 600s to 120s (escalated cmqlrhd7c).
assert.equal(resolveComboTargetTimeoutMs({}, 600000, 120000), 120000);
// Operators can still extend beyond the default, up to the ceiling.
assert.equal(resolveComboTargetTimeoutMs({ targetTimeoutMs: 300000 }, 600000, 120000), 300000);
// Explicit config above the ceiling is still capped at the ceiling.
assert.equal(resolveComboTargetTimeoutMs({ targetTimeoutMs: 900000 }, 600000, 120000), 600000);
// A default larger than the ceiling is clamped to the ceiling.
assert.equal(resolveComboTargetTimeoutMs({}, 100000, 120000), 100000);
// Backward-compat: omitting the default arg keeps the legacy inherit-the-ceiling behavior.
assert.equal(resolveComboTargetTimeoutMs({}, 600000), 600000);
// Disabled upstream timeout (0 = unbounded) stays unbounded even with a default present.
assert.equal(resolveComboTargetTimeoutMs({}, 0, 120000), 0);
});
test("combo timeout schema rejects values beyond the safe timer limit", () => {
const result = createComboSchema.safeParse({
name: "unsafe-timeout",