From c1a3d83c27adf30d3e2cd9dfbec1e57a366f5395 Mon Sep 17 00:00:00 2001 From: Jan Leon Date: Sat, 18 Jul 2026 20:12:24 +0200 Subject: [PATCH] fix(combo): reject known context overflow without exhausting providers (#7177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix context-window exhaustion classification * fix(combo): keep chat.ts/comboStructure.ts under the file-size ratchet + fix context-overflow boundary bug - Extract getKnownContextOverflow (+ its KnownContextOverflow type) out of comboStructure.ts into a new open-sse/services/combo/knownContextOverflow.ts leaf, so the file-size ratchet (cap 800 for new files) passes. - Extract the skipConnectionDisable predicate out of handleSingleModelChat in chat.ts into open-sse/services/combo/comboPredicates.ts::shouldSkipConnDisable, and consolidate the new combo-failure-handling imports, to keep chat.ts under its frozen file-size baseline (1796) after the #7177 request-scoped-failure wiring. - Fix a real boundary bug in getKnownContextOverflow surfaced by the merge: estimateRequestInputTokens counted a caller-omitted `messages: []` (which some combo entrypoints default in) as real content, charging a few phantom "structural" JSON.stringify tokens toward the estimate. That was enough to falsely trip the new known-context-overflow rejection for a request with no real input when max_tokens exactly equals the target's context window (a common config where limit_input === limit_output === limit_context), regressing tests/unit/combo-routing-engine.test.ts's pre-existing #3587 "non-reasoning model does not get max_tokens buffer" case. Empty arrays/objects no longer count as estimable content. - Add a regression test for the exact-boundary empty-content case. Co-authored-by: Diego Rodrigues de Sa e Souza * refactor(combo): move overflow logic into knownContextOverflow module (file-size cap) comboStructure.ts is not frozen in the file-size baseline but is capped at 800 lines; this PR's net +29 on that file alone would push it over once merged. knownContextOverflow.ts already exists in this PR as the dedicated home for "known context limit" logic, so move the genuinely new pieces there instead of leaving them in comboStructure.ts: - hasEstimableContent (new): its own doc comment already frames it purely in terms of the known-context-overflow boundary check, so it belongs next to that check, not in the general request-compatibility file. - getKnownContextLimit (new, requestedOutputTokens-aware): this *is* the "how big is a target's known context window" primitive knownContextOverflow already consumes; hosting it there is a better fit than comboStructure.ts. - getLegacyKnownContextLimit: kept alongside its sibling rather than split across two files, since both are alternate implementations of the same concept (used only by comboStructure.ts's hasKnownCompatibleContextLimit). comboStructure.ts now imports all three back for its own internal callers (estimateRequestInputTokens, getTargetCompatibilityFailures, hasKnownCompatibleContextLimit). deriveRequestCompatibilityRequirements and the RequestCompatibilityRequirements type stay in comboStructure.ts exactly as this PR already has them (still consumed internally there), so knownContextOverflow.ts keeps importing those two, same as before. No behavior change — pure relocation, verified by the existing PR test suite (combo-context-window-filter, combo-breaker-429, combo-failure-log-message, combo-target-exhaustion, diagnostics) plus the pre-existing combo-vision-aware-routing/combo-context-requirements/combo-roundrobin-compat-fallback-6238 suites, all green. Net effect on open-sse/services/combo/comboStructure.ts vs. this PR's merge base: -2 lines (was +29). typecheck:core, lint, and the complexity ratchets (check:complexity, check:cognitive-complexity) are unchanged from this PR's current HEAD — the 4 pre-existing complexity/max-lines findings in valueContainsImagePart/filterTargetsByRequestCompatibility are untouched by this move (same violations, same total ratchet counts, just shifted line numbers). Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Diego Rodrigues de Sa e Souza Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- open-sse/handlers/chatCore.ts | 30 +++++- open-sse/services/combo.ts | 72 +++++++++++++- open-sse/services/combo/comboPredicates.ts | 44 ++++++++- open-sse/services/combo/comboStructure.ts | 7 +- .../services/combo/knownContextOverflow.ts | 97 +++++++++++++++++++ open-sse/services/combo/targetExhaustion.ts | 17 +++- open-sse/utils/diagnostics.ts | 22 +++++ src/sse/handlers/chat.ts | 15 +-- src/sse/handlers/comboFailureLogging.ts | 20 ++++ tests/unit/combo-breaker-429.test.ts | 11 +++ .../unit/combo-context-window-filter.test.ts | 89 ++++++++++++++++- tests/unit/combo-failure-log-message.test.ts | 29 ++++++ .../combo/combo-target-exhaustion.test.ts | 19 ++++ tests/unit/diagnostics.test.ts | 23 ++++- 14 files changed, 466 insertions(+), 29 deletions(-) create mode 100644 open-sse/services/combo/knownContextOverflow.ts create mode 100644 src/sse/handlers/comboFailureLogging.ts create mode 100644 tests/unit/combo-failure-log-message.test.ts diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 83a3eb7152..98f4c44681 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -125,7 +125,11 @@ import { formatProviderError, sanitizeErrorMessage, } from "../utils/error.ts"; -import { reportMalformed200, detectMalformedNonStream } from "../utils/diagnostics.ts"; +import { + reportMalformed200, + detectMalformedNonStream, + describeMalformedNonStream, +} from "../utils/diagnostics.ts"; import { checkTokenLimits, recordTokenUsage, @@ -3384,7 +3388,13 @@ export async function handleChatCore({ // otherwise degenerate into a 429 rate-limit storm). Connection stays // active since only the specific model is unavailable. (#6827) const notFoundCooldownMs = COOLDOWN_MS.notFound; - lockModel(provider, errorConnectionId, currentModel, "model_not_found", notFoundCooldownMs); + lockModel( + provider, + errorConnectionId, + currentModel, + "model_not_found", + notFoundCooldownMs + ); console.warn( `[provider] Node ${errorConnectionId} model not found (${statusCode}) for ${currentModel} - locking model for ${Math.ceil(notFoundCooldownMs / 1000)}s (connection stays active)` ); @@ -4070,7 +4080,11 @@ export async function handleChatCore({ connectionId, status: `FAILED ${HTTP_STATUS.BAD_GATEWAY}`, }).catch(() => {}); - const malformedMessage = `[${provider}/${model}] returned an empty response (no usable choices/output)`; + const malformed = describeMalformedNonStream(translatedResponse, malformedTranslatedReason); + const malformedMessage = `[${provider}/${model}] ${malformed.message}`; + const malformedClientBody = buildErrorBody(HTTP_STATUS.BAD_GATEWAY, malformedMessage); + malformedClientBody.error.code = malformed.code; + malformedClientBody.error.type = malformed.type; persistAttemptLogs({ status: HTTP_STATUS.BAD_GATEWAY, tokens: usage, @@ -4079,14 +4093,20 @@ export async function handleChatCore({ providerResponse: looksLikeSSE ? { _streamed: true, _format: "sse-json", summary: responseBody } : responseBody, - clientResponse: buildErrorBody(HTTP_STATUS.BAD_GATEWAY, malformedMessage), + clientResponse: malformedClientBody, claudeCacheMeta: claudePromptCacheLogMeta, claudeCacheUsageMeta: cacheUsageLogMeta, cacheSource: "upstream", }); persistFailureUsage(HTTP_STATUS.BAD_GATEWAY, "malformed_translated_response"); trackPendingRequest(model, provider, pendingConnId, false); - return createErrorResult(HTTP_STATUS.BAD_GATEWAY, malformedMessage); + return createErrorResult( + HTTP_STATUS.BAD_GATEWAY, + malformedMessage, + null, + malformed.code, + malformed.type + ); } // ── Phase 9.1: Cache store (non-streaming, temp=0) ── diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index e8b10406dd..375e47fb9b 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -138,6 +138,8 @@ import { clampComboDepth, shouldSkipForPredictedTtft, shouldRecordProviderBreakerFailure, + isRequestScopedUpstreamFailure, + shouldSkipConnDisable, resolveDelayMs, comboModelNotFoundResponse, isStreamReadinessFailureErrorBody, @@ -163,6 +165,7 @@ import { resolveWeightedTargets, resolveWeightedStepGroups, } from "./combo/comboStructure.ts"; +import { getKnownContextOverflow } from "./combo/knownContextOverflow.ts"; import { QUOTA_SOFT_DEPRIORITIZE_FACTOR, setCandidateQuotaSoftPenalty, @@ -200,10 +203,21 @@ export { QUOTA_SOFT_DEPRIORITIZE_FACTOR, setCandidateQuotaSoftPenalty }; export { scoreAutoTargets, expandAutoComboCandidatePool }; export type { SingleModelTarget, ResolvedComboTarget }; export { validateResponseQuality }; -export { clampComboDepth, shouldSkipForPredictedTtft, shouldRecordProviderBreakerFailure }; +export { + clampComboDepth, + shouldSkipForPredictedTtft, + shouldRecordProviderBreakerFailure, + isRequestScopedUpstreamFailure, + shouldSkipConnDisable, +}; export { resolveShadowTargets, scheduleShadowRouting }; export { preScreenTargets }; -export { resolveComboRuntimeUnits, resolveComboTargets, filterTargetsByRequestCompatibility }; +export { + resolveComboRuntimeUnits, + resolveComboTargets, + filterTargetsByRequestCompatibility, + getKnownContextOverflow, +}; export { getComboFromData, getComboModelsFromData, @@ -1152,6 +1166,31 @@ export async function handleComboChat({ orderedTargets = await applyRequestTagRouting(orderedTargets, body, log); + const knownContextOverflow = getKnownContextOverflow(orderedTargets, body); + if (knownContextOverflow) { + const { requiredContextTokens, maxKnownContextTokens } = knownContextOverflow; + log.warn( + "COMBO", + `Request context exceeds every known target limit (${requiredContextTokens} > ${maxKnownContextTokens} tokens)` + ); + return errorResponseWithComboDiagnostics( + 400, + `Request requires approximately ${requiredContextTokens} tokens, but the largest known context limit in this combo is ${maxKnownContextTokens} tokens. Reduce or compact the request context.`, + { + poolSize: orderedTargets.length, + attempted: 0, + excluded: orderedTargets.map((target) => ({ + provider: target.provider, + model: target.modelStr, + reason: "context_window", + })), + attemptOrder: [], + terminalReason: "context_length_exceeded", + }, + { code: "context_length_exceeded", type: "invalid_request_error" } + ); + } + if (strategy === "weighted") { log.info( "COMBO", @@ -2067,6 +2106,7 @@ export async function handleComboChat({ : undefined, } : undefined; + const requestScopedFailure = isRequestScopedUpstreamFailure(structuredError); const fallbackResult = checkFallbackError( result.status, errorText, @@ -2179,6 +2219,7 @@ export async function handleComboChat({ status: result.status, sameProviderNext, skipProviderBreaker: fallbackResult.skipProviderBreaker, + requestScopedFailure, }) ) { recordProviderFailure(provider, log, targetWithConnection.connectionId, profile); @@ -2204,7 +2245,7 @@ export async function handleComboChat({ // once the model is cooling down, retrying it would waste an upstream // call and extend the cooldown via exponential backoff. let lockoutRecorded = false; - if (provider && rawModel && retry === 0) { + if (provider && rawModel && retry === 0 && !requestScopedFailure) { const mlSettings = resolveModelLockoutSettings(settings); if (mlSettings.enabled && mlSettings.errorCodes.includes(result.status)) { recordModelLockoutFailure( @@ -2247,7 +2288,7 @@ export async function handleComboChat({ if (i > 0) fallbackCount++; // Wire combo failures into the resilience dashboard (model-level lockout) // alongside the provider-level cooldown below — they govern different scopes. - if (provider && rawModel) { + if (provider && rawModel && !requestScopedFailure) { const mlSettings = resolveModelLockoutSettings(settings); if (mlSettings.enabled && mlSettings.errorCodes.includes(result.status)) { recordModelLockoutFailure( @@ -2276,6 +2317,7 @@ export async function handleComboChat({ resilienceSettings.providerCooldown.enabled && provider && provider !== "unknown" && + !requestScopedFailure && !(result.status === 500 && hasPerModelQuota(provider, rawModel)) ) { recordProviderCooldown( @@ -2548,6 +2590,25 @@ async function handleRoundRobinCombo({ ); const tagFilteredTargets = await applyRequestTagRouting(orderedTargets, body, log); const evalRankedTargets = orderTargetsByEvalScores(tagFilteredTargets, config.evalRouting, log); + const knownContextOverflow = getKnownContextOverflow(evalRankedTargets, body); + if (knownContextOverflow) { + return errorResponseWithComboDiagnostics( + 400, + `Request requires approximately ${knownContextOverflow.requiredContextTokens} tokens, but the largest known context limit in this combo is ${knownContextOverflow.maxKnownContextTokens} tokens. Reduce or compact the request context.`, + { + poolSize: evalRankedTargets.length, + attempted: 0, + excluded: evalRankedTargets.map((target) => ({ + provider: target.provider, + model: target.modelStr, + reason: "context_window", + })), + attemptOrder: [], + terminalReason: "context_length_exceeded", + }, + { code: "context_length_exceeded", type: "invalid_request_error" } + ); + } const filteredTargets = filterTargetsByRequestCompatibility( evalRankedTargets, body, @@ -3044,6 +3105,7 @@ async function handleRoundRobinCombo({ : undefined, } : undefined; + const requestScopedFailure = isRequestScopedUpstreamFailure(structuredError); const fallbackResult = checkFallbackError( result.status, errorText, @@ -3095,6 +3157,7 @@ async function handleRoundRobinCombo({ if ( !isStreamReadinessFailure && !isTokenLimitBreach && + !requestScopedFailure && TRANSIENT_FOR_SEMAPHORE.includes(result.status) && cooldownMs > 0 ) { @@ -3137,6 +3200,7 @@ async function handleRoundRobinCombo({ resilienceSettings.providerCooldown.enabled && provider && provider !== "unknown" && + !requestScopedFailure && !( result.status === 500 && hasPerModelQuota(provider, parseModel(modelStr).model || modelStr) diff --git a/open-sse/services/combo/comboPredicates.ts b/open-sse/services/combo/comboPredicates.ts index 887709522f..39ba927640 100644 --- a/open-sse/services/combo/comboPredicates.ts +++ b/open-sse/services/combo/comboPredicates.ts @@ -8,6 +8,7 @@ import { errorResponse } from "../../utils/error.ts"; import { parseModel } from "../model.ts"; +import { isSelfInflictedUpstreamTimeout } from "../../handlers/chatCore/cooldownClassification.ts"; import type { ResolvedComboTarget } from "./types.ts"; // Status codes that should mark round-robin target semaphores as cooling down. @@ -150,12 +151,53 @@ export function shouldRecordProviderBreakerFailure(args: { status: number; sameProviderNext: boolean; skipProviderBreaker?: boolean; + requestScopedFailure?: boolean; }): boolean { return ( !args.isStreamReadinessFailure && PROVIDER_BREAKER_FAILURE_STATUSES.has(args.status) && !args.sameProviderNext && - !args.skipProviderBreaker + !args.skipProviderBreaker && + !args.requestScopedFailure + ); +} + +const REQUEST_SCOPED_UPSTREAM_ERROR_CODES = new Set([ + "context_length_exceeded", + "upstream_empty_response", + "upstream_response_failed", +]); + +/** Request/model-specific failures must not poison provider-wide resilience state. */ +export function isRequestScopedUpstreamFailure(error?: { + code?: string | null; + type?: string | null; +}): boolean { + const code = typeof error?.code === "string" ? error.code.toLowerCase() : ""; + const type = typeof error?.type === "string" ? error.type.toLowerCase() : ""; + return REQUEST_SCOPED_UPSTREAM_ERROR_CODES.has(code) || type === "context_length_exceeded"; +} + +/** + * #7177: whether handleSingleModelChat should skip the connection-level cooldown + * (markAccountUnavailable) for a failed attempt — client disconnects, a 401 when the + * connection has extra keys to rotate through, a known request-scoped upstream failure + * (e.g. context overflow — not a connection health signal), or our own self-inflicted + * timeout all mean the connection itself is healthy and should not be cooled down. + */ +export function shouldSkipConnDisable( + result: { status: number; errorCode?: string | null; errorType?: string | null }, + is401: boolean, + hasExtraKeys: boolean, + provider: string +): boolean { + return ( + result.status === 499 || + result.errorCode === "client_disconnected" || + result.errorType === "client_disconnected" || + (is401 && hasExtraKeys) || + isRequestScopedUpstreamFailure({ code: result.errorCode, type: result.errorType }) || + isSelfInflictedUpstreamTimeout(result.status, result.errorType, provider) ); } diff --git a/open-sse/services/combo/comboStructure.ts b/open-sse/services/combo/comboStructure.ts index 21b4242124..0b7f8cb0fa 100644 --- a/open-sse/services/combo/comboStructure.ts +++ b/open-sse/services/combo/comboStructure.ts @@ -18,6 +18,7 @@ import { getResolvedModelCapabilities } from "../modelCapabilities.ts"; import { parseModel } from "../model.ts"; import { dedupeTargetsByExecutionKey, isRecord } from "./comboData.ts"; import { getTargetProvider, MAX_COMBO_DEPTH } from "./comboPredicates.ts"; +import { hasEstimableContent } from "./knownContextOverflow.ts"; import { normalizeModelEntry, orderTargetsForWeightedFallback, @@ -409,7 +410,7 @@ export function getModelContextLimitForModelString(modelStr: string) { return getModelContextLimit(provider, model); } -type RequestCompatibilityRequirements = { +export type RequestCompatibilityRequirements = { requiresTools: boolean; requiresVision: boolean; requiresStructuredOutput: boolean; @@ -438,7 +439,7 @@ function requestRequiresStructuredOutput(body: Record): boolean function estimateRequestInputTokens(body: Record): number { const estimatePayload: Record = {}; for (const key of ["messages", "input", "tools", "functions", "response_format"]) { - if (body[key] !== undefined) estimatePayload[key] = body[key]; + if (hasEstimableContent(body[key])) estimatePayload[key] = body[key]; } return Object.keys(estimatePayload).length > 0 ? estimateTokens(estimatePayload) : 0; } @@ -460,7 +461,7 @@ function valueContainsImagePart(value: unknown, depth = 0): boolean { return Object.values(value).some((entry) => valueContainsImagePart(entry, depth + 1)); } -function deriveRequestCompatibilityRequirements( +export function deriveRequestCompatibilityRequirements( body: Record ): RequestCompatibilityRequirements { const estimatedInputTokens = estimateRequestInputTokens(body); diff --git a/open-sse/services/combo/knownContextOverflow.ts b/open-sse/services/combo/knownContextOverflow.ts new file mode 100644 index 0000000000..4416d1d451 --- /dev/null +++ b/open-sse/services/combo/knownContextOverflow.ts @@ -0,0 +1,97 @@ +/** + * Known context-overflow rejection, extracted from comboStructure.ts to keep + * that file under the file-size cap (#7177). + * + * Fixes: routing a request to a combo whose targets all have a KNOWN (not + * unknown/fail-open) context window too small for the request used to be + * discovered only after every target was tried and failed upstream — burning + * retries/cooldowns on a request that could never succeed. This lets the + * combo dispatcher reject it up front, before exhausting providers. + * + * getKnownContextLimit/hasEstimableContent also + * live here (moved from comboStructure.ts, same file-size-cap motivation): + * they are the "how big is a target's known context window" primitives, so + * they belong next to the overflow check that is their main consumer. + * comboStructure.ts's own compatibility filter now decides fit via its + * evaluateContextLimit (#7052); only hasEstimableContent is imported back. + */ + +import { getResolvedModelCapabilities } from "../modelCapabilities.ts"; +import { deriveRequestCompatibilityRequirements } from "./comboStructure.ts"; +import type { ResolvedComboTarget } from "./types.ts"; + +export type KnownContextOverflow = { + estimatedInputTokens: number; + requestedOutputTokens: number; + requiredContextTokens: number; + maxKnownContextTokens: number; + targetCount: number; +}; + +// #7177: an empty array/object (e.g. a default `messages: []` some combo entrypoints inject +// when the caller sent none) has no real content — counting it would charge a few phantom +// "structural" tokens (JSON.stringify braces/brackets) toward the estimate, which is enough +// to falsely trip the exact-boundary known-context-overflow check for a request that has no +// actual input at all. +export function hasEstimableContent(value: unknown): boolean { + if (value === undefined || value === null) return false; + if (Array.isArray(value)) return value.length > 0; + if (typeof value === "object") return Object.keys(value).length > 0; + return true; +} + +// #7177: known context limit that accounts for the request's own requested +// output tokens — a target whose input+output would together exceed +// maxInputTokens is exactly as incompatible as one whose contextWindow is too +// small, so both bounds go through the same min() so far the tightest wins. +export function getKnownContextLimit( + capabilities: { + maxInputTokens?: number | null; + contextWindow?: number | null; + }, + requestedOutputTokens = 0 +): number | null { + const limits: number[] = []; + if (capabilities.maxInputTokens != null) { + limits.push(capabilities.maxInputTokens + requestedOutputTokens); + } + if (capabilities.contextWindow != null) { + limits.push(capabilities.contextWindow); + } + return limits.length > 0 ? Math.min(...limits) : null; +} + + +/** + * Return a hard context-overflow decision only when every target has a known + * context limit and every one of those limits is too small for the request. + * Unknown metadata deliberately keeps the legacy fail-open behavior. + */ +export function getKnownContextOverflow( + targets: ResolvedComboTarget[], + body: Record +): KnownContextOverflow | null { + if (targets.length === 0) return null; + const requirements = deriveRequestCompatibilityRequirements(body); + if (requirements.requiredContextTokens <= 0) return null; + + const limits = targets.map((target) => + getKnownContextLimit( + getResolvedModelCapabilities(target.modelStr), + requirements.requestedOutputTokens + ) + ); + if (limits.some((limit) => limit === null)) return null; + + const knownLimits = limits as number[]; + const maxKnownContextTokens = Math.max(...knownLimits); + if (maxKnownContextTokens >= requirements.requiredContextTokens) return null; + + return { + estimatedInputTokens: requirements.estimatedInputTokens, + requestedOutputTokens: requirements.requestedOutputTokens, + requiredContextTokens: requirements.requiredContextTokens, + maxKnownContextTokens, + targetCount: targets.length, + }; +} diff --git a/open-sse/services/combo/targetExhaustion.ts b/open-sse/services/combo/targetExhaustion.ts index 7091b483ef..66f7ef5cd2 100644 --- a/open-sse/services/combo/targetExhaustion.ts +++ b/open-sse/services/combo/targetExhaustion.ts @@ -21,7 +21,7 @@ import { isProviderExhaustedReason, } from "../accountFallback.ts"; import { RateLimitReason } from "../../config/constants.ts"; -import { isProviderCircuitOpenResult } from "./comboPredicates.ts"; +import { isProviderCircuitOpenResult, isRequestScopedUpstreamFailure } from "./comboPredicates.ts"; import type { ComboLogger, ResolvedComboTarget } from "./types.ts"; // Connection-level failure statuses: the provider connection itself is likely bad (upstream @@ -104,7 +104,15 @@ export function applyComboTargetExhaustion( if (result.status === 429 && !isTokenLimitBreach && provider && provider !== "unknown") { transientRateLimitedProviders.add(provider); } - markConnectionLevelExhaustion(target, { result, errorText, sets, log, tag, rawModel }); + markConnectionLevelExhaustion(target, { + result, + errorText, + sets, + log, + tag, + rawModel, + structuredError, + }); } return providerExhausted; @@ -120,16 +128,17 @@ function markConnectionLevelExhaustion( target: ResolvedComboTarget, opts: Pick< ApplyComboTargetExhaustionOptions, - "result" | "errorText" | "sets" | "log" | "tag" | "rawModel" + "result" | "errorText" | "sets" | "log" | "tag" | "rawModel" | "structuredError" > ): void { - const { result, errorText, sets, log, tag, rawModel } = opts; + const { result, errorText, sets, log, tag, rawModel, structuredError } = opts; const provider = target.provider; if ( !provider || provider === "unknown" || !CONNECTION_LEVEL_ERROR_STATUSES.includes(result.status) || isProviderCircuitOpenResult(result, errorText) || + isRequestScopedUpstreamFailure(structuredError) || // #5085: empty-content 502 is a healthy connection returning no body — model-level, not // connection-level. Don't exhaust the provider; let the remaining legs (incl. same-provider) // be tried in-request. diff --git a/open-sse/utils/diagnostics.ts b/open-sse/utils/diagnostics.ts index 176d0f7843..39161e4137 100644 --- a/open-sse/utils/diagnostics.ts +++ b/open-sse/utils/diagnostics.ts @@ -284,5 +284,27 @@ export function detectMalformedNonStream(resp: unknown): MalformedReason | null return null; } +export function describeMalformedNonStream( + resp: unknown, + reason: MalformedReason +): { message: string; code: string; type: string } { + const body = resp && typeof resp === "object" ? (resp as Record) : null; + if (body?.object === "response" && body.status === "failed") { + return { + message: "upstream reported a failed response without usable output", + code: "upstream_response_failed", + type: "upstream_response_error", + }; + } + return { + message: + reason === "no_terminal" + ? "upstream response did not reach a terminal state" + : "upstream returned an empty response without usable output", + code: "upstream_empty_response", + type: "upstream_response_error", + }; +} + // ── Test-only export ───────────────────────────────────────────────────────── export const __test = { describeReason }; diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index cefc533561..3f63968159 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -22,9 +22,8 @@ import { resolveBareModelToConnectionDefault } from "@omniroute/open-sse/service import { errorResponse } from "@omniroute/open-sse/utils/error.ts"; import { getImageModelEntry } from "@omniroute/open-sse/config/imageRegistry.ts"; import { acceptHeaderForcesStream } from "@omniroute/open-sse/utils/aiSdkCompat.ts"; -import { isSelfInflictedUpstreamTimeout } from "@omniroute/open-sse/handlers/chatCore/cooldownClassification.ts"; import { applyNoThinkingAlias } from "@omniroute/open-sse/utils/noThinkingAlias.ts"; -import { handleComboChat } from "@omniroute/open-sse/services/combo.ts"; +import { handleComboChat, shouldSkipConnDisable } from "@omniroute/open-sse/services/combo.ts"; import { resolveRequestAutoControls } from "@omniroute/open-sse/services/autoCombo/requestControls.ts"; import { resolveComboConfig } from "@omniroute/open-sse/services/comboConfig.ts"; import { injectHandoffIntoBody } from "@omniroute/open-sse/services/contextHandoff.ts"; @@ -83,6 +82,7 @@ import { withCorrelationId, } from "./chatHelpers"; import { connectionHasExtraKeys } from "@omniroute/open-sse/services/apiKeyRotator.ts"; +import { getComboFailureLogError, isRequestScopedUpstreamFailure } from "./comboFailureLogging"; // Pipeline integration — wired modules import { classify429FromError, type FailureKind } from "@/shared/utils/classify429"; @@ -922,7 +922,7 @@ export async function handleChat( requestedModel: body?.model || resolvedModelStr, provider: "-", endpoint: clientRawRequest?.endpoint, - error: `[${response.status}] Combo "${combo.name}" failed — all targets exhausted`, + error: await getComboFailureLogError(response, combo.name), comboName: combo.name, apiKeyId: apiKeyInfo?.id ?? null, apiKeyName: apiKeyInfo?.name ?? null, @@ -1726,13 +1726,7 @@ async function handleSingleModelChat( ((credentials.providerSpecificData?.extraApiKeys as string[] | undefined) ?? []).length > 0 || connectionHasExtraKeys(credentials.connectionId); const is401 = result.status === 401; - // Our own timeout fired on a slow upstream; don't cool down a healthy account. - const skipConnectionDisable = - result.status === 499 || - result.errorCode === "client_disconnected" || - result.errorType === "client_disconnected" || - (is401 && hasExtraKeys) || - isSelfInflictedUpstreamTimeout(result.status, result.errorType, provider); + const skipConnectionDisable = shouldSkipConnDisable(result, is401, hasExtraKeys, provider); const { shouldFallback, cooldownMs } = skipConnectionDisable ? { shouldFallback: false, cooldownMs: 0 } @@ -1785,6 +1779,7 @@ async function handleSingleModelChat( if ( !forceLiveComboTest && !isCombo && + !isRequestScopedUpstreamFailure({ code: result.errorCode, type: result.errorType }) && PROVIDER_BREAKER_FAILURE_STATUSES.has(Number(result.status)) ) { breaker._onFailure(); diff --git a/src/sse/handlers/comboFailureLogging.ts b/src/sse/handlers/comboFailureLogging.ts new file mode 100644 index 0000000000..1ccf4b1fdb --- /dev/null +++ b/src/sse/handlers/comboFailureLogging.ts @@ -0,0 +1,20 @@ +// Re-exported here (rather than a separate import line in chat.ts, a frozen +// file-size chokepoint — see config/quality/file-size-baseline.json) so both +// combo-failure-handling helpers chat.ts needs share a single import line. +export { isRequestScopedUpstreamFailure } from "@omniroute/open-sse/services/combo/comboPredicates.ts"; + +export async function getComboFailureLogError( + response: Response, + comboName: string +): Promise { + const fallback = `[${response.status}] Combo "${comboName}" failed`; + try { + const body = await response.clone().json(); + const message = body?.error?.message; + return typeof message === "string" && message.trim() + ? `[${response.status}] ${message.trim()}` + : fallback; + } catch { + return fallback; + } +} diff --git a/tests/unit/combo-breaker-429.test.ts b/tests/unit/combo-breaker-429.test.ts index 98fbc966ed..988ebfb9c1 100644 --- a/tests/unit/combo-breaker-429.test.ts +++ b/tests/unit/combo-breaker-429.test.ts @@ -67,3 +67,14 @@ test("skipProviderBreaker:true suppresses recording regardless of status", () => ); } }); + +test("request-scoped synthetic 502 does NOT record a whole-provider breaker failure", () => { + assert.equal( + shouldRecordProviderBreakerFailure({ + ...OTHER_ARGS, + status: 502, + requestScopedFailure: true, + }), + false + ); +}); diff --git a/tests/unit/combo-context-window-filter.test.ts b/tests/unit/combo-context-window-filter.test.ts index 3dcf054cd1..342e360ce4 100644 --- a/tests/unit/combo-context-window-filter.test.ts +++ b/tests/unit/combo-context-window-filter.test.ts @@ -16,7 +16,8 @@ process.env.DATA_DIR = TEST_DATA_DIR; const core = await import("../../src/lib/db/core.ts"); const { saveModelsDevCapabilities, clearModelsDevCapabilities } = await import("../../src/lib/modelsDevSync.ts"); -const { filterTargetsByRequestCompatibility } = await import("../../open-sse/services/combo.ts"); +const { filterTargetsByRequestCompatibility, getKnownContextOverflow, handleComboChat } = + await import("../../open-sse/services/combo.ts"); test.after(() => { core.resetDbInstance(); @@ -181,6 +182,92 @@ test("all known-too-small context targets still fall back to strategy order", () ); }); +test("known context overflow reports the largest target limit", () => { + saveModelsDevCapabilities({ + "unit-known-context": { + tiny: capabilityEntry(8_000), + small: capabilityEntry(16_000), + }, + }); + + const overflow = getKnownContextOverflow( + [target("unit-known-context/tiny"), target("unit-known-context/small")], + largeContextBody() + ); + + assert.ok(overflow); + assert.ok(overflow.requiredContextTokens > overflow.maxKnownContextTokens); + assert.equal(overflow.maxKnownContextTokens, 16_000); + assert.equal(overflow.targetCount, 2); +}); + +test("#7177 an empty messages array is not counted as real content at an exact-boundary limit", () => { + // Regression: some combo entrypoints default a caller-omitted `messages` to `[]`. The + // estimator used to JSON.stringify whatever keys were merely *present* on the body, + // so an empty array still contributed a few phantom "structural" tokens (JSON braces/ + // brackets), which was enough to trip a false-positive overflow when max_tokens exactly + // equals the target's context window (a common config where limit_input === limit_output + // === limit_context) even though there is no real input to account for. + saveModelsDevCapabilities({ + "unit-known-context": { + exact: capabilityEntry(4_096), + }, + }); + + const overflow = getKnownContextOverflow([target("unit-known-context/exact")], { + messages: [], + max_tokens: 4_096, + }); + + assert.equal(overflow, null); +}); + +test("unknown context metadata keeps overflow detection fail-open", () => { + saveModelsDevCapabilities({ + "unit-known-context": { + tiny: capabilityEntry(8_000), + }, + }); + + const overflow = getKnownContextOverflow( + [target("unit-known-context/tiny"), target("unit-unknown-context/mystery")], + largeContextBody() + ); + + assert.equal(overflow, null); +}); + +test("combo rejects a known oversized request before upstream dispatch", async () => { + saveModelsDevCapabilities({ + "unit-known-context": { + tiny: capabilityEntry(8_000), + small: capabilityEntry(16_000), + }, + }); + let dispatches = 0; + + const response = await handleComboChat({ + body: largeContextBody(), + combo: { + name: "known-context-overflow", + strategy: "priority", + models: ["unit-known-context/tiny", "unit-known-context/small"], + }, + handleSingleModel: async () => { + dispatches += 1; + return new Response("unexpected", { status: 200 }); + }, + log: noopLog, + }); + + assert.equal(response.status, 400); + assert.equal(dispatches, 0); + const body = await response.json(); + assert.equal(body.error.code, "context_length_exceeded"); + assert.equal(body.diagnostics.terminalReason, "context_length_exceeded"); + assert.equal(body.diagnostics.attempted, 0); +}); + test("input-only maxInputTokens is not double-counted against the output reserve (#7039)", () => { // Faithful reproduction of #7039 (Codex gpt-5.5-xhigh): // maxInputTokens = 272_000, contextWindow = 400_000, maxOutputTokens = 128_000 diff --git a/tests/unit/combo-failure-log-message.test.ts b/tests/unit/combo-failure-log-message.test.ts new file mode 100644 index 0000000000..61775e76c8 --- /dev/null +++ b/tests/unit/combo-failure-log-message.test.ts @@ -0,0 +1,29 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +const { getComboFailureLogError } = await import("../../src/sse/handlers/comboFailureLogging.ts"); + +test("combo failure log preserves the concrete response error", async () => { + const response = new Response( + JSON.stringify({ + error: { + message: "Request context exceeds every known target limit", + code: "context_length_exceeded", + }, + }), + { status: 400, headers: { "Content-Type": "application/json" } } + ); + + assert.equal( + await getComboFailureLogError(response, "large-context-combo"), + "[400] Request context exceeds every known target limit" + ); +}); + +test("combo failure log uses a neutral fallback for an unreadable response", async () => { + const response = new Response("not-json", { status: 503 }); + assert.equal( + await getComboFailureLogError(response, "temporary-combo"), + '[503] Combo "temporary-combo" failed' + ); +}); diff --git a/tests/unit/combo/combo-target-exhaustion.test.ts b/tests/unit/combo/combo-target-exhaustion.test.ts index c6b0369187..4ac95bcbd3 100644 --- a/tests/unit/combo/combo-target-exhaustion.test.ts +++ b/tests/unit/combo/combo-target-exhaustion.test.ts @@ -93,6 +93,25 @@ test("connection-level 5xx with a connectionId poisons exhaustedConnections (#17 assert.equal(s.exhaustedProviders.size, 0); }); +test("request-scoped failed-response 502 does not exhaust the connection", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion(target(), { + ...baseOpts, + result: { status: 502, headers: null }, + fallbackResult: {}, + errorText: "upstream reported a failed response without usable output", + structuredError: { + code: "upstream_response_failed", + type: "upstream_response_error", + }, + sets: s, + }); + + assert.equal(exhausted, false); + assert.equal(s.exhaustedProviders.size, 0); + assert.equal(s.exhaustedConnections.size, 0); +}); + test("connection-level 5xx without a connectionId poisons exhaustedProviders (#1731)", () => { const s = sets(); applyComboTargetExhaustion(target({ connectionId: null }), { diff --git a/tests/unit/diagnostics.test.ts b/tests/unit/diagnostics.test.ts index 5be4614bbe..901feebff7 100644 --- a/tests/unit/diagnostics.test.ts +++ b/tests/unit/diagnostics.test.ts @@ -16,6 +16,7 @@ import { synthOpenAIErrorChunk, synthResponsesFailure, detectMalformedNonStream, + describeMalformedNonStream, } from "../../open-sse/utils/diagnostics.ts"; // ── (a) synthOpenAIErrorChunk shape ────────────────────────────────────────── @@ -88,6 +89,21 @@ test("detectMalformedNonStream returns 'empty_choices' for choices:[]", () => { assert.equal(detectMalformedNonStream({ choices: [] }), "empty_choices"); }); +test("failed Responses API body gets a request-scoped machine-readable classification", () => { + const failed = { + object: "response", + status: "failed", + output: [], + }; + const reason = detectMalformedNonStream(failed); + assert.equal(reason, "empty_choices"); + assert.deepEqual(describeMalformedNonStream(failed, reason), { + message: "upstream reported a failed response without usable output", + code: "upstream_response_failed", + type: "upstream_response_error", + }); +}); + test("detectMalformedNonStream returns 'empty_choices' when choice message has no content", () => { const body = { choices: [{ message: { content: "", tool_calls: null }, finish_reason: "stop" }], @@ -122,7 +138,12 @@ test("detectMalformedNonStream returns null for OpenAI choices whose content is test("detectMalformedNonStream returns 'empty_choices' for an OpenAI choice with an empty text-block array (#5559 guard)", () => { const body = { - choices: [{ message: { role: "assistant", content: [{ type: "text", text: "" }] }, finish_reason: "stop" }], + choices: [ + { + message: { role: "assistant", content: [{ type: "text", text: "" }] }, + finish_reason: "stop", + }, + ], }; assert.equal(detectMalformedNonStream(body), "empty_choices"); });