From 8eded5ebf2d54a266978ca967950fa56ececbbd8 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 20 Jun 2026 11:07:39 -0300 Subject: [PATCH] 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. --- open-sse/services/combo/comboSetup.ts | 7 +++++- open-sse/services/comboConfig.ts | 34 +++++++++++++++++++++++---- tests/unit/combo-config.test.ts | 26 ++++++++++++++++++-- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/open-sse/services/combo/comboSetup.ts b/open-sse/services/combo/comboSetup.ts index 6c3188c9ab..4b7e8b56e9 100644 --- a/open-sse/services/combo/comboSetup.ts +++ b/open-sse/services/combo/comboSetup.ts @@ -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 { diff --git a/open-sse/services/comboConfig.ts b/open-sse/services/comboConfig.ts index 788f5496e3..f8fc7e742c 100644 --- a/open-sse/services/comboConfig.ts +++ b/open-sse/services/comboConfig.ts @@ -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 | 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); } /** diff --git a/tests/unit/combo-config.test.ts b/tests/unit/combo-config.test.ts index 783d2106c0..d328392d49 100644 --- a/tests/unit/combo-config.test.ts +++ b/tests/unit/combo-config.test.ts @@ -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",