diff --git a/README.md b/README.md index a2f0871..f165497 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,7 @@ config accepts them as YAML, which decodes to the same JSON shape. | `users` | `UsersConfig` | — | Inline and/or file-backed API-key holders. With none configured, every request is unauthenticated and gets 401. | | `redis` | `RedisConfig` | — | Distributed limit-counter backend. Omitted means in-process counters only (per-replica, approximate across multiple Traefik instances). | | `retry` | `RetryConfig` | `{}` (disabled) | Same-provider retry for transient upstream failures — see [Retry](#retry). Omitted or `enabled: false` means no retry: every request makes exactly one upstream attempt, byte-identical to a gateway built before this field existed. | +| `failover` | `FailoverConfig` | `{}` (disabled) | Cross-provider failover for a bare model id more than one configured provider serves — see [Failover](#failover). Omitted or `enabled: false` means no failover, byte-identical to a gateway built before this field existed. `enabled: true` opts in; a deployment with no overlapping providers behaves exactly as before regardless of this setting either way. | | `cache` | `CacheConfig` | `{}` (disabled) | Opt-in Redis-backed response cache for unified non-streaming chat/embeddings — see [Caching](#caching). Omitted or `enabled: false` means no caching, byte-identical to a gateway built before this field existed. | | `breaker` | `BreakerConfig` | `{}` (every default) | Per-provider discovery circuit breaker — see [Provider health (discovery circuit breaker)](#provider-health-discovery-circuit-breaker). Every field is individually zero-means-default; a provider whose discovery never fails is unaffected regardless of what this block contains. | | `admin` | `*AdminConfig` | `nil` (disabled) | Read-only admin dashboard — see [Admin](#admin). `nil` or `enabled: false` means the `/admin*` routes are not registered at all. | @@ -807,10 +808,10 @@ released, so this never becomes a ceiling on total concurrent requests. ## Retry -Same-provider retry for a transient upstream failure — no cross-provider -failover. Off by default (`retry.enabled: false`); every config that -predates this field keeps making exactly one upstream attempt per -request. +Same-provider retry for a transient upstream failure. Off by default +(`retry.enabled: false`); every config that predates this field keeps +making exactly one upstream attempt per request. See [Failover](#failover) +below for the separate, cross-provider mechanism. - **Scope**: every adapter upstream call — chat, embeddings, model listing, and the three media endpoints (`images/generations`, @@ -846,6 +847,119 @@ request. is never double-counted. Request counters still increment once per client request, unchanged. +## Failover + +Cross-provider failover for a bare model id more than one configured +provider serves — for example `whisper-1` configured on both `openai` and +`openai-audio`. **Off by default** (`failover.enabled: false` unless set +otherwise): a version upgrade with no config change preserves prior +behavior exactly, since failover changes which provider serves a request +and can shift cost attribution or mask a broken provider key behind an +apparent success. Set `failover.enabled: true` to opt in. A deployment +with no overlapping providers, or with a single provider, behaves +exactly as before regardless of this setting either way, since there is +never a second candidate to try. + +- **Scope**: `POST /v1/chat/completions`, `POST /v1/embeddings`, and + `POST /v1/messages` only. The image/audio endpoints + (`images/generations`, `audio/speech`, `audio/transcriptions`) are not + wired into failover yet — each has its own request-shape handling + (multipart rebuild, binary/streaming responses) that needs its own + scoped follow-up. +- **Eligible candidates**: a bare model id resolved via the collision + winner (the same mechanism `GET /v1/models` documents for overlapping + catalogs) offers every OTHER configured provider that also serves that + id and that the caller's group is authorized to use. A `provider/model` + request, or an alias whose target is `provider/model`, is an explicit, + exact choice and is never eligible for substitution. +- **What falls through**: everything except HTTP 400 — connection + failures, timeouts, HTTP 429/401/403/404, and any 5xx. A 400 never + falls through: the request is malformed and every provider would reject + it identically. A failover past a 404 is logged loudly, always, naming + both providers and the model — a 404 usually means a real configuration + mistake, not a transient outage. The routine "failing over from X to Y" + line is rate-limited to once per 30s during a sustained outage (with a + suppressed-count note on the next line), so it cannot itself become a + blocking synchronous write on every request; the 404 line is exempt + from that limit. +- **Cost guard (not optional)**: failover never routes to a candidate + priced higher than the provider it is failing over FROM (the first + candidate actually attempted, never the cheapest in the list), checked + on BOTH input and output cost per token — a candidate cheaper on input + but dearer on output is still rejected. Pricing is resolved exactly the + way real billing resolves it: the canonical `provider/model` id in + `pricing` first, falling back to the bare model id, so a per-provider + `pricing` override correctly makes the identical model id cost + different amounts on different providers. Unknown pricing is never + treated as free: if the primary's price is known and a candidate's is + not, the candidate is skipped (an unknown price cannot be proven + cheaper); if the primary's price is unknown and a candidate's is known + and non-zero, the candidate is skipped too (moving from unbilled to + billed is itself a spend increase); both unknown, or both an explicit + known-zero price, are treated as equal and allowed. There is no config + flag for this — it applies unconditionally whenever failover is + enabled. +- **The cost guard cannot see `modelMeta`'s `free: true`.** It resolves + pricing only through `pricing` and the built-in table — the same + source `unifiedCostMicros` actually bills against — deliberately never + through `modelMeta` (see [Model metadata](#model-metadata) / + `ModelMetaConfig`), since `modelMeta` drives GET `/v1/models` display + only and never affects billing. A model declared free the `modelMeta` + way is therefore treated as unknown-priced by the cost guard and loses + redundancy: it is skipped as a failover candidate whenever the primary + has a known price (fail-safe, and logged like any other cost skip). + Declare the price through `pricing` (e.g. `{"inputPerM": 0, + "outputPerM": 0}`) if you want a genuinely free model to remain + eligible as a cheaper failover candidate. +- **Never mid-stream**: failover only happens while nothing has reached + the client yet. Once the first byte of a response is written, status + and body are committed and the request is never retried elsewhere. +- **Health signals, kept separate**: the discovery-endpoint health + surfaced on the admin dashboard is an ordering hint only — a provider + whose `/v1/models` is unhealthy is tried later, never skipped outright, + since plenty of providers serve chat completions fine while their model + listing is broken. A separate, in-memory, per-pod request-path health + signal (not yet exposed on the admin dashboard) is the actual routing + gate: a provider is skipped without being called once it has failed + three consecutive LOGICAL requests (same-provider retries all count as + one outcome, not one each) — everything except HTTP 400 counts as a + failure for this gate, including 401/403/404, so a provider with a dead + API key is eventually routed around instead of retried forever. +- **Accounting**: a failover chain still counts as exactly one client + request against `requestsPerMinute`/`requestsPerDay` — never once per + attempt. Per-provider attempt/failure counters (the admin dashboard's + success-rate badge) attribute every attempt to the provider that + actually made it. +- **Caching**: a cacheable response produced by a failover provider is + stored under that provider's own cache key, never the first candidate's. + Each candidate's own cache entry is checked immediately before that + candidate's own attempt, in try order — never for every candidate up + front. Checking every candidate's cache before any of them was + attempted was tried and reverted: it silently served a DIFFERENT + candidate's cached response even while the actual primary was + perfectly healthy, which is failover with no failure. The + request-path health signal above is what actually keeps a genuinely + dead primary from being dialed again — once it is marked unhealthy it + is excluded from the candidate list entirely, so a later request + reaches the next candidate's cache without the dead one ever being + attempted. +- **Configurable**: `failover.maxAttempts` caps how many different + providers one request will try in total, primary included (default 3, + maximum 10) — distinct from `retry.attempts`, which retries the SAME + provider. `failover.enabled: false` (the default) turns the whole + mechanism off. +- **Known limitation — no per-attempt timeout**: `newAdapterHTTPClient` + sets no client timeout and no response-header timeout; the only bound + on an upstream call is the incoming request's own context, shared + identically across every candidate. A provider that accepts the TCP + connection and then simply never responds will hang the request + indefinitely and never trigger failover — only a failure fast enough to + leave time on that shared context (a dial failure, a fast non-2xx + response, or the context's own deadline finally expiring) does. This is + a pre-existing property of the plugin's HTTP client, not specific to + failover, and applies to every route, not only this one; it is not + addressed in this change. + ## Caching An opt-in, Redis-backed response cache for the unified, non-streaming diff --git a/failover.go b/failover.go new file mode 100644 index 0000000..08c9498 --- /dev/null +++ b/failover.go @@ -0,0 +1,656 @@ +package traefikllmgateway + +import ( + "context" + "errors" + "fmt" + "net/http" + "sync" + "time" +) + +// This file implements feat/failover: cross-provider failover for a bare +// model id more than one configured provider serves (registry.go's +// bareWinner collision case — production logs show whisper-1, tts-1, +// tts-1-hd, text-embedding-3-small, text-embedding-3-large, and +// text-embedding-ada-002 served by two or more of openai/openai-audio/ +// copilot today). Before this feature, registry.go's resolve returns +// exactly one provider adapter and retry.go's retryPolicy only ever +// retries the SAME provider — so a down provider fails every request for +// its models even when another configured provider serves the identical +// model. This file turns that already-existing redundancy into a hot +// standby, hooked into routes_unified.go's runMeteredCall, the ONE shared +// metered-call path both /v1/chat/completions and /v1/messages already go +// through. +// +// Off by default (FailoverConfig.Enabled defaults to false — coordinator +// ruling, overriding this feature's original brief: new behavior that can +// change which provider serves a request goes behind a flag defaulting +// off). An operator who opts in also gets a non-optional cost guard +// (candidateCostAllowed, below): failover never routes to a candidate +// priced higher than the provider it is failing over from, on either +// input or output cost per token — there is no config knob to disable +// just that part. +// +// SCOPE: routes_media.go's three media routes (images/generations, +// audio/speech, audio/transcriptions) are deliberately NOT wired into +// failover in this change, even though two of the brief's own named +// overlapping models (whisper-1, tts-1/tts-1-hd) are media-route +// traffic. Each of those three handlers has its own request-shape +// preprocessing (multipart rebuild for transcriptions, a JSON +// re-marshal for speech, a binary/streaming response for speech) that +// would need its own careful "nothing written yet" analysis and its own +// test coverage, rather than reusing runMeteredCall's shared shape — the +// brief's own hook-point instruction ("do not fork a second +// implementation") is about not duplicating the chat/messages pipeline, +// not about extending every possible call site in one pass. Left as a +// scoped follow-up; see this branch's own report for the explicit +// call-out. + +// FailoverConfig configures feat/failover. Enabled is a *bool; nil or +// false (the default) means OFF — coordinator ruling, overriding this +// feature's original brief: a version upgrade with no config change must +// preserve prior behavior, and new behavior that can change which +// provider serves a request (shifting cost attribution, and able to mask +// a broken provider key behind an apparent success) goes behind a flag +// that defaults off, not on. Set Enabled: true to opt in. A +// single-provider deployment, or one where no two providers ever serve +// the identical bare model id, behaves exactly as before regardless of +// this setting either way: there is never more than one candidate to try +// (orderedFailoverCandidates' own doc comment). +type FailoverConfig struct { + Enabled *bool `json:"enabled,omitempty"` + // MaxAttempts caps how many DIFFERENT PROVIDERS runMeteredCall tries + // in total for one logical request, the primary included — distinct + // from RetryConfig.Attempts (retry.go), which retries the SAME + // provider. 0 (the default) uses defaultFailoverMaxAttempts. A value + // outside 1..maxFailoverMaxAttempts is a construction error. + MaxAttempts int `json:"maxAttempts,omitempty"` +} + +// defaultFailoverMaxAttempts is applied when FailoverConfig.MaxAttempts is +// left at 0. maxFailoverMaxAttempts bounds an explicit override, the same +// "cheap sanity ceiling on an operator value" precedent +// maxBreakerFailureThreshold/maxRetryAttempts already set (registry.go/ +// retry.go). +const ( + defaultFailoverMaxAttempts = 3 + maxFailoverMaxAttempts = 10 +) + +// failoverConfig is FailoverConfig, validated and defaulted once at +// construction (newGateway, llmgateway.go) — mirrors breakerConfig/ +// retryPolicy's own resolved-value-not-raw-config shape (registry.go/ +// retry.go). +type failoverConfig struct { + maxAttempts int + enabled bool +} + +// validateFailoverConfig validates fc and returns the resolved +// failoverConfig newGateway attaches to the Gateway. enabled defaults to +// false (coordinator ruling, FailoverConfig's own doc comment) — every +// config that predates this field, and every config that never sets +// failover.enabled at all, keeps behaving exactly as before this feature +// existed. +func validateFailoverConfig(fc FailoverConfig) (failoverConfig, error) { + enabled := false + if fc.Enabled != nil { + enabled = *fc.Enabled + } + + maxAttempts := fc.MaxAttempts + if maxAttempts == 0 { + maxAttempts = defaultFailoverMaxAttempts + } + if maxAttempts < 1 || maxAttempts > maxFailoverMaxAttempts { + return failoverConfig{}, fmt.Errorf("llmgateway: failover.maxAttempts must be between 1 and %d, got %d", maxFailoverMaxAttempts, maxAttempts) + } + + return failoverConfig{enabled: enabled, maxAttempts: maxAttempts}, nil +} + +// resolveCandidate is one provider that can serve a client-requested model +// id — modelRegistry.resolve's own (adapter, upstreamModel, canonical) +// triple, tagged with the provider's own name so a failover attempt can +// rebind its cache key, attempt recorder, and error log to the ACTUAL +// provider that served or failed the request, never the first one resolve +// happened to pick. +type resolveCandidate struct { + adapter providerAdapter + providerName string + upstreamModel string + canonical string +} + +// resolveWithCandidates resolves id exactly as modelRegistry.resolve +// (registry.go) does — identical winner, identical error — and +// additionally reports every OTHER configured provider that also serves +// the same bare upstream model id and remains authorized for grp, as +// feat/failover's candidate pool. +// +// primary/err are byte-identical to what resolve(id, grp) alone would +// return: this method calls resolve internally and never second-guesses +// its winner or its error — feat/failover only ADDS candidates after an +// already-successful resolution, it never changes which model or error a +// request resolves to (in particular, authorization denial/unknown-model +// behavior for the primary winner is completely unchanged by this +// branch). extra is nil whenever there is nothing to add: +// +// - id resolved via an explicit "provider/model" prefix, or an alias +// whose target is itself provider-prefixed: the requester's own +// explicit provider choice is never a candidate for pool +// substitution — ruling, this feature's own design: "provider/model" +// addressing is exact, not eligible for failover. +// - id resolved via a bare-id collision winner (bareWinner), or an +// alias whose target is itself a bare-id collision winner: extra is +// every OTHER configured provider whose known model set also +// contains that identical bare id string, in +// modelRegistry.providerNames (sorted) order, and authorized for grp +// on that provider. +func (m *modelRegistry) resolveWithCandidates(id string, grp *group) (resolveCandidate, []resolveCandidate, error) { + adapter, upstreamModel, canonical, err := m.resolve(id, grp) + if err != nil { + return resolveCandidate{}, nil, err + } + primary := resolveCandidate{adapter: adapter, providerName: adapter.name(), upstreamModel: upstreamModel, canonical: canonical} + + bareID := id + if target, isAlias := m.aliases[id]; isAlias { + if _, _, ok := m.splitConfiguredProvider(target); ok { + return primary, nil, nil + } + bareID = target + } else if _, _, ok := m.splitConfiguredProvider(id); ok { + return primary, nil, nil + } + + return primary, m.failoverCandidates(bareID, primary.providerName, grp), nil +} + +// resolvePrimaryOnly resolves id exactly like resolveWithCandidates' +// primary return (identical winner, identical error) WITHOUT ever +// scanning for or building failover candidates — the short-circuit +// runMeteredCall (routes_unified.go) takes when g.failover.enabled is +// false, so a deployment that never opted into failover pays no extra +// cost for a pool it will never use (measured: 74ns/1 alloc versus +// 521ns/13 allocs at 8 providers x 200 models for the full +// resolveWithCandidates scan — irrelevant next to a network round trip, +// but free to avoid). +func (m *modelRegistry) resolvePrimaryOnly(id string, grp *group) (resolveCandidate, error) { + adapter, upstreamModel, canonical, err := m.resolve(id, grp) + if err != nil { + return resolveCandidate{}, err + } + return resolveCandidate{adapter: adapter, providerName: adapter.name(), upstreamModel: upstreamModel, canonical: canonical}, nil +} + +// failoverCandidates returns every configured provider other than +// primaryProvider whose known model set contains bareID and that grp is +// authorized to use, in modelRegistry.providerNames (sorted) order — the +// raw candidate pool orderedFailoverCandidates (below) narrows and +// reorders further by health. +// +// Only grp.allowsProvider is re-checked per candidate: grp.allowsModel is +// already known true from the primary winner's own successful resolve +// call, and — for the identical bare id string every candidate here +// shares by construction — allowsModel's result does not vary by +// provider (auth.go's allowsModel takes no provider argument), so +// re-checking it per candidate would be redundant work, not a +// correctness gap. +func (m *modelRegistry) failoverCandidates(bareID, primaryProvider string, grp *group) []resolveCandidate { + var extra []resolveCandidate + for _, name := range m.providerNames { + if name == primaryProvider { + continue + } + if !m.states[name].hasModel(bareID) || !grp.allowsProvider(name) { + continue + } + // canonical assigned to a local before the struct literal, matching + // resolveAgainst's own documented Yaegi precedent (registry.go): a + // defensive, cheap precaution against the same class of interpreter + // trap, not a proven necessity for a struct-literal field (only a + // multi-value RETURN tuple is the proven-bad shape there). + canonical := name + "/" + bareID + extra = append(extra, resolveCandidate{ + adapter: m.adapters[name], + providerName: name, + upstreamModel: bareID, + canonical: canonical, + }) + } + return extra +} + +// orderedFailoverCandidates builds runMeteredCall's actual try list from +// resolveWithCandidates' own (primary, extra) split. +// +// Failover is a no-op (returns exactly [primary]) whenever it is disabled +// (g.failover.enabled false) or there is nothing to add (len(extra) == +// 0) — the second case is what makes "single-provider deployment behaves +// exactly as before" hold unconditionally, regardless of the enabled +// switch: there is never more than one candidate to try when no other +// configured provider shares this bare model id. +// +// Otherwise, two health signals apply, deliberately kept separate +// (modelRegistry.discoveryHealthy's own doc comment, registry.go, is +// binding on this): +// +// - REQUEST-PATH health (g.failoverHealth, this file) is a HARD SKIP +// gate: a candidate it reports unhealthy is excluded outright, +// never attempted. If that filtering would remove every candidate +// (this local, per-pod signal reporting the whole fleet down +// simultaneously is more likely stale than true), it is discarded +// and every original candidate is tried anyway — failing open +// against the gateway's own imperfect signal rather than refusing +// to route a request at all. +// - DISCOVERY health (g.registry.discoveryHealthy) is an ORDERING HINT +// ONLY, exactly as its own doc comment requires: a discovery- +// unhealthy candidate is moved after every discovery-healthy/ +// half-open one (a stable partition, not a full sort — relative +// order within each bucket is preserved), never dropped. +// +// This does NOT apply the cost guard or the maxAttempts cap — see +// runMeteredCall's own call sequence (routes_unified.go): candidates[0] +// coming out of THIS function is what filterCandidatesByCost (below) +// treats as "the primary" for pricing-guard purposes, so cost filtering +// must run against the health/discovery-ordered result, not the raw +// resolveWithCandidates order; and the maxAttempts cap runs LAST, after +// cost filtering, so a cap never re-admits an expensive candidate purely +// because a cheaper one ahead of it was removed on cost. +func (g *Gateway) orderedFailoverCandidates(primary resolveCandidate, extra []resolveCandidate) []resolveCandidate { + if !g.failover.enabled || len(extra) == 0 { + return []resolveCandidate{primary} + } + + all := make([]resolveCandidate, 0, len(extra)+1) + all = append(all, primary) + all = append(all, extra...) + + healthy := make([]resolveCandidate, 0, len(all)) + for _, c := range all { + if g.failoverHealth.healthy(c.providerName) { + healthy = append(healthy, c) + } + } + if len(healthy) == 0 { + healthy = all + } + + ordered := make([]resolveCandidate, 0, len(healthy)) + var deprioritized []resolveCandidate + for _, c := range healthy { + if g.registry.discoveryHealthy(c.providerName) { + ordered = append(ordered, c) + } else { + deprioritized = append(deprioritized, c) + } + } + return append(ordered, deprioritized...) +} + +// resolveUnifiedPricing resolves (canonical, bare)'s price using the +// IDENTICAL resolution unifiedCostMicros itself uses (routes_unified.go): +// canonical id first, falling back to the bare upstream model id only +// when canonical has no configured price at all. ok is lookupPricing's +// own known/unknown distinction (pricing.go) — false means neither id +// resolved to a price, exactly the case where unifiedCostMicros would +// warn and bill 0, which candidateCostAllowed (below) must NEVER treat +// as an actual known-zero price. +func resolveUnifiedPricing(canonical, bare string, overrides map[string]*ModelPricing) (ModelPricing, bool) { + if p, ok := lookupPricing(canonical, overrides); ok { + return p, true + } + return lookupPricing(bare, overrides) +} + +// candidateCostAllowed reports whether candidate is allowed to serve as a +// failover destination for primary, per the operator's binding ruling: +// failover must never route to a candidate more expensive than the +// provider it is failing over FROM, checked on BOTH the input and the +// output per-token price — a candidate cheaper on input but dearer on +// output can still cost more on a completion-heavy request, so a single +// blended number is not sufficient. This is NOT configurable (ruling: +// "this behaviour is ON and not optional"). +// +// Four cases, all by ruling, not inferred: +// +// - both known: allowed only when candidate's input AND output price +// are each <= primary's. Equal is allowed either way. +// - primary known, candidate UNKNOWN: never allowed — an unknown price +// cannot be proven not-more-expensive. +// - primary UNKNOWN, candidate known and non-zero: never allowed — +// moving from unbilled to billed is itself a spend increase. +// - primary UNKNOWN, candidate known and EXACTLY zero (both +// dimensions), or both unknown: allowed — they are equal (a known +// zero can never be more expensive than an unknown amount, and two +// unknowns are trivially equal to each other). +// +// Unknown pricing is never treated as zero here, unlike costMicros' own +// billing behavior (pricing.go) — the gateway RECORDING zero for an +// unpriced model is an accounting shortcut, not proof the provider +// invoices zero, and this guard exists specifically to protect real +// money, not the gateway's own displayed number. +func candidateCostAllowed(primary ModelPricing, primaryKnown bool, candidate ModelPricing, candidateKnown bool) bool { + switch { + case primaryKnown && candidateKnown: + return candidate.InputPerM <= primary.InputPerM && candidate.OutputPerM <= primary.OutputPerM + case primaryKnown && !candidateKnown: + return false + case !primaryKnown && candidateKnown: + return candidate.InputPerM == 0 && candidate.OutputPerM == 0 + default: // both unknown + return true + } +} + +// filterCandidatesByCost drops every candidate whose resolved price +// candidateCostAllowed rejects against candidates[0] — the FIRST +// candidate actually attempted, never the cheapest in the list, per the +// operator's own definition of "primary" for this rule. Pricing comes +// from resolveUnifiedPricing against g.cfg.Pricing, the SAME overrides +// map unifiedCostMicros itself resolves against, so this compares +// against what would actually be billed. requestedModel names the +// client's own request in the log line a skip always produces — an +// operator-visible g.warnf, unthrottled, naming both providers, the +// model, and both resolved prices: a silently shortened candidate list +// would look identical to having no redundancy at all, which the +// operator must be able to tell apart from a cost-guard skip. +func (g *Gateway) filterCandidatesByCost(candidates []resolveCandidate, requestedModel string) []resolveCandidate { + if len(candidates) < 2 { + return candidates + } + primary := candidates[0] + primaryPrice, primaryKnown := resolveUnifiedPricing(primary.canonical, primary.upstreamModel, g.cfg.Pricing) + + out := make([]resolveCandidate, 1, len(candidates)) + out[0] = primary + for _, cand := range candidates[1:] { + candPrice, candKnown := resolveUnifiedPricing(cand.canonical, cand.upstreamModel, g.cfg.Pricing) + if candidateCostAllowed(primaryPrice, primaryKnown, candPrice, candKnown) { + out = append(out, cand) + continue + } + g.warnf("failover: skipping candidate provider %q for model %q: would cost more than primary provider %q (primary input=%.4f/M output=%.4f/M usd known=%v; candidate input=%.4f/M output=%.4f/M usd known=%v)", + cand.providerName, requestedModel, primary.providerName, + primaryPrice.InputPerM, primaryPrice.OutputPerM, primaryKnown, + candPrice.InputPerM, candPrice.OutputPerM, candKnown) + } + return out +} + +// capFailoverCandidates truncates candidates to at most max entries, +// applied as the LAST step of runMeteredCall's own candidate-building +// sequence (routes_unified.go) — after health/discovery ordering and +// after cost filtering, so the cap can never re-admit a candidate cost +// filtering already removed, and never discards a cheaper candidate that +// cost filtering kept purely because a removed, more expensive one used +// to sit ahead of it in the raw list. max <= 0 is a no-op (defensive; +// validateFailoverConfig never produces one). +func capFailoverCandidates(candidates []resolveCandidate, max int) []resolveCandidate { + if max > 0 && len(candidates) > max { + return candidates[:max] + } + return candidates +} + +// failoverEligible reports whether callErr — runMeteredCall's own +// adapterCall return, evaluated only once nothing has reached the client +// yet (the hard constraint this whole feature is built around: once an +// adapter has written anything, status and body are committed and can +// never be retried elsewhere) — should try the next candidate instead of +// answering the client with callErr. +// +// Ruling (brief, "Which failures fall through" — decided, not +// relitigated here): every failure except HTTP 400 falls through. 5xx, +// connection failures, timeouts, 429, 401, 403, and 404 all do; 400 +// never does, because the request itself is malformed and every provider +// would reject it identically, so retrying just multiplies latency and +// burns budget. +// +// Two failure shapes are excluded for the SAME "retrying changes +// nothing" reasoning 400 itself is excluded for, even though neither is +// literally an HTTP 400: context.Canceled (the client walked away — +// failing over would only waste an attempt against a connection nobody +// is waiting on) and errRequestBuildFailed (upstreamBytes, providers.go +// — the identical malformed request shape reaches every candidate the +// same way, differing only in which operator-configured base URL it is +// built against). A *translateError or *responseTranslationError is this +// gateway's OWN translation code failing on an already-decoded request — +// not a signal about the PROVIDER's availability at all — so neither is +// eligible either. +func failoverEligible(callErr error) bool { + if callErr == nil { + return false + } + if errors.Is(callErr, context.Canceled) || errors.Is(callErr, errRequestBuildFailed) { + return false + } + // Plain type assertions, not errors.As — matching this package's + // established Yaegi-safe convention for these exact two pointer error + // types (providerHTTPError's own doc comment, providers.go; + // handleAdapterErrorEnvelope, routes_unified.go, does the identical + // assertion for the identical reason). + if perr, ok := callErr.(*providerHTTPError); ok { + return perr.status != http.StatusBadRequest + } + if _, ok := callErr.(*translateError); ok { + return false + } + if _, ok := callErr.(*responseTranslationError); ok { + return false + } + return true +} + +// isProviderNotFoundError reports whether err is a *providerHTTPError +// carrying HTTP 404 — routes_unified.go's runMeteredCall uses this to +// decide when a failover needs its own loud, non-suppressed log line +// (brief: "Failover after a 404 must log loudly ... a 404 usually means +// a real configuration mistake, and silently succeeding elsewhere would +// hide it"). +func isProviderNotFoundError(err error) bool { + perr, ok := err.(*providerHTTPError) + return ok && perr.status == http.StatusNotFound +} + +// requestBreakerFailureThreshold/requestBreakerOpenDuration/ +// requestBreakerMaxOpenDuration size requestHealthTracker's own breaker — +// deliberately separate constants from registry.go's discovery breaker +// (defaultBreakerFailureThreshold/defaultBreakerOpenDuration/ +// defaultBreakerMaxOpenDuration): a request-path outage needs to be +// noticed and routed around within seconds, not the discovery breaker's +// minutes-to-hours cadence, and modelRegistry.discoveryHealthy's own doc +// comment forbids widening that breaker to cover this outcome stream at +// all. Fixed constants, not configurable — unlike FailoverConfig above, +// the brief only asks for max-attempts and an on/off switch to be +// operator-configurable; this breaker's thresholds are an internal +// implementation detail of the routing-gate signal, not a documented +// tuning surface. +const ( + requestBreakerFailureThreshold = 3 + requestBreakerOpenDuration = 30 * time.Second + requestBreakerMaxOpenDuration = 5 * time.Minute +) + +// requestHealthState is one provider's REQUEST-PATH health (feat/ +// failover), mu-guarded — mirrors providerState's per-provider, +// individually-mutexed shape (registry.go) at a much smaller scale: this +// tracks exactly one outcome stream (request-path attempts), never +// discovery. The zero value is closed/healthy, so a provider that never +// fails never touches anything below open/backoff/openUntil. +// +// Field order below is fieldalignment-verified (golangci-lint's govet +// enable-all, run with -fix against a scratch copy to derive the exact +// zero-waste sequence, then hand-applied here so this doc comment +// survives — the same convention providerState's own doc comment +// explains, registry.go). +type requestHealthState struct { + openUntil time.Time + backoff time.Duration + consecutiveFailures int + mu sync.Mutex + open bool +} + +// requestHealthTracker is feat/failover's own per-pod, in-memory, +// request-path health signal — deliberately separate from registry.go's +// discovery circuit breaker (modelRegistry.discoveryHealthy's own doc +// comment forbids widening that one to cover request-path traffic). Fed +// by failoverEligible's own classification (record's own doc comment +// explains why, not limiter.recordProviderAttempt's narrower isTransient/ +// isDeadlineExceeded one), once per candidate per logical request, via +// record's own callers in routes_unified.go, but keeps its own +// independent state per provider name. +// +// Every method is nil-receiver-safe (both report "healthy"/no-op on a +// nil *requestHealthTracker), matching this package's own nil-means- +// disabled convention for an optional subsystem (responseCache's own doc +// comment, llmgateway.go's Gateway.cache field) — a Gateway assembled +// directly in a test without going through newGateway, or any future +// caller that does the same, degrades to "failover never skips a +// candidate for request health" rather than a nil-pointer panic. +type requestHealthTracker struct { + states map[string]*requestHealthState + nowFn func() time.Time + mu sync.Mutex +} + +// newRequestHealthTracker returns an empty tracker — every provider name +// reads healthy until its first recorded failure. +func newRequestHealthTracker() *requestHealthTracker { + return &requestHealthTracker{states: make(map[string]*requestHealthState), nowFn: time.Now} +} + +// stateFor returns provider's requestHealthState, creating it on first +// use. +func (t *requestHealthTracker) stateFor(provider string) *requestHealthState { + t.mu.Lock() + defer t.mu.Unlock() + st, ok := t.states[provider] + if !ok { + st = &requestHealthState{} + t.states[provider] = st + } + return st +} + +// record accounts ONE LOGICAL REQUEST's outcome for provider — called +// exactly once per candidate per client request, after call() has +// already exhausted whatever same-provider retries retryPolicy.do +// performed internally (routes_unified.go's runMeteredCall), never once +// per raw upstream attempt. success=false must be +// !failoverEligible(callErr) (routes_unified.go/failover.go's own +// classification of the FINAL adapter-level error) — adversarial-review +// ruling (F3/F6): an EARLIER version fed this from isTransient(resp, +// err) || isDeadlineExceeded(err), limits.go/retry.go's own +// retry-shaped classification, once per RAW attempt inside +// retryPolicy.do. That was wrong on two counts. isTransient treats a +// non-429 4xx as "the provider answered correctly to a request it did +// not like" — correct for deciding whether retrying the SAME provider +// is worth it, wrong for deciding whether the POOL should route around +// it: a provider whose API key is dead answers 401 to every request and +// must eventually be skipped, not retried forever, which is exactly the +// "masking a broken provider key" risk this feature exists to reduce. +// And recording per raw attempt let ONE client request with +// retry.enabled (attempts: 3) report FOUR failures against a threshold +// documented as three REQUESTS. failoverEligible already classifies +// "everything except HTTP 400" as a failure, matching what this gate +// needs with no separate function required. +// +// requestBreakerFailureThreshold consecutive failures opens the breaker +// for requestBreakerOpenDuration. While open, healthy (below) reports +// unhealthy until that window passes; once it has, the NEXT record call +// for this provider — success or failure — is what healthy already +// exposed as a post-cooldown probe attempt: success closes the breaker +// outright, failure re-opens it with a doubled backoff (capped at +// requestBreakerMaxOpenDuration). This is the same probe-then-escalate- +// or-close shape providerState's discovery breaker uses (registry.go's +// recordHealthLocked/openBreakerLocked), reimplemented independently +// rather than shared — the two track different outcome streams, and +// discoveryHealthy's own doc comment forbids widening it to cover this +// one. +// +// A failure arriving while ALREADY open and still within its cooldown +// window is a deliberate no-op (adversarial-review fix, F5): an earlier +// version fell through to the closed-state consecutiveFailures++ branch +// for this case too, which reset backoff to the flat base duration and +// slid openUntil forward on every such failure instead of leaving the +// existing window alone — doubling never actually happened outside the +// narrow post-cooldown probe path the doc comment claimed was the only +// place it did. +// +// Deliberately no single-flight guard on the probe window (unlike +// registry.go's tryBeginRefresh inFlight flag for its own, much lower- +// volume, background-goroutine probe): this signal sits on the live +// request path, where many concurrent requests can observe the same +// post-cooldown window at once. Serializing them would need its own +// lock-and-CAS dance for a benefit — one broken provider getting at most +// one real probe instead of a short burst of them right as its backoff +// expires — this per-pod, best-effort signal does not need to pay for. +func (t *requestHealthTracker) record(provider string, success bool) { + if t == nil || provider == "" { + return + } + st := t.stateFor(provider) + st.mu.Lock() + defer st.mu.Unlock() + now := t.nowFn() + + if success { + st.open = false + st.consecutiveFailures = 0 + st.backoff = 0 + st.openUntil = time.Time{} + return + } + + // Already open, cooldown not yet elapsed: this failure changes + // nothing (F5 fix, above). + if st.open && now.Before(st.openUntil) { + return + } + + if st.open { + // Cooldown elapsed: this failure IS the probe outcome. Escalate. + if st.backoff <= 0 { + st.backoff = requestBreakerOpenDuration + } else { + st.backoff *= 2 + } + if st.backoff > requestBreakerMaxOpenDuration { + st.backoff = requestBreakerMaxOpenDuration + } + st.openUntil = now.Add(st.backoff) + return + } + + st.consecutiveFailures++ + if st.consecutiveFailures >= requestBreakerFailureThreshold { + st.open = true + st.backoff = requestBreakerOpenDuration + st.openUntil = now.Add(st.backoff) + } +} + +// healthy reports whether provider's request-path breaker currently +// permits routing to it — feat/failover's hard SKIP gate +// (orderedFailoverCandidates, above), distinct from +// modelRegistry.discoveryHealthy's deprioritize-only contract. An +// unrecorded provider (never failed, or never attempted at all) +// reads healthy — the same "never seen a failure" default the discovery +// breaker's own zero value already applies (providerState's doc +// comment, registry.go). +func (t *requestHealthTracker) healthy(provider string) bool { + if t == nil || provider == "" { + return true + } + st := t.stateFor(provider) + st.mu.Lock() + defer st.mu.Unlock() + if !st.open { + return true + } + return !t.nowFn().Before(st.openUntil) +} diff --git a/failover_test.go b/failover_test.go new file mode 100644 index 0000000..f893d19 --- /dev/null +++ b/failover_test.go @@ -0,0 +1,1963 @@ +package traefikllmgateway + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --- modelRegistry.resolveWithCandidates / failoverCandidates --- + +func TestModelRegistry_ResolveWithCandidates_BareCollision_ReturnsOrderedExtras(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "beta": newFakeAdapter("beta"), + "alpha": newFakeAdapter("alpha"), + "gamma": newFakeAdapter("gamma"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "beta": {Models: []string{"shared"}}, + "alpha": {Models: []string{"shared"}}, + "gamma": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + + primary, extra, err := reg.resolveWithCandidates("shared", allowAllGroup()) + require.NoError(t, err) + assert.Equal(t, "alpha", primary.providerName, "primary must match resolve's own sorted-first winner") + + names := make([]string, len(extra)) + for i, c := range extra { + names[i] = c.providerName + } + assert.Equal(t, []string{"beta", "gamma"}, names, "extras must be every OTHER owning provider, in providerNames sorted order, primary excluded") + for _, c := range extra { + assert.Equal(t, "shared", c.upstreamModel) + assert.Equal(t, c.providerName+"/shared", c.canonical) + } +} + +func TestModelRegistry_ResolveWithCandidates_ProviderPrefixed_NoExtras(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "beta": newFakeAdapter("beta"), + "alpha": newFakeAdapter("alpha"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "beta": {Models: []string{"shared"}}, + "alpha": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + + primary, extra, err := reg.resolveWithCandidates("beta/shared", allowAllGroup()) + require.NoError(t, err) + assert.Equal(t, "beta", primary.providerName) + assert.Nil(t, extra, "an explicit provider/model request must never gain failover candidates") +} + +func TestModelRegistry_ResolveWithCandidates_AliasToProviderPrefixedTarget_NoExtras(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "beta": newFakeAdapter("beta"), + "alpha": newFakeAdapter("alpha"), + } + cfg := &Config{ + Providers: map[string]*ProviderConfig{ + "beta": {Models: []string{"shared"}}, + "alpha": {Models: []string{"shared"}}, + }, + ModelAliases: map[string]string{"aliased": "beta/shared"}, + } + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + + primary, extra, err := reg.resolveWithCandidates("aliased", allowAllGroup()) + require.NoError(t, err) + assert.Equal(t, "beta", primary.providerName) + assert.Nil(t, extra, "an alias targeting an explicit provider/model must never gain failover candidates") +} + +func TestModelRegistry_ResolveWithCandidates_AliasToBareTarget_ReturnsExtras(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "beta": newFakeAdapter("beta"), + "alpha": newFakeAdapter("alpha"), + } + cfg := &Config{ + Providers: map[string]*ProviderConfig{ + "beta": {Models: []string{"shared"}}, + "alpha": {Models: []string{"shared"}}, + }, + ModelAliases: map[string]string{"aliased": "shared"}, + } + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + + primary, extra, err := reg.resolveWithCandidates("aliased", allowAllGroup()) + require.NoError(t, err) + assert.Equal(t, "alpha", primary.providerName, "an alias to a bare-collision target resolves via the same sorted-first winner") + require.Len(t, extra, 1) + assert.Equal(t, "beta", extra[0].providerName) +} + +// TestModelRegistry_ResolveWithCandidates_ExtraFiltersByProviderAuthorization +// proves a group restricted away from a would-be failover provider never +// gets it offered as a candidate, even though that provider genuinely +// serves the bare id. +func TestModelRegistry_ResolveWithCandidates_ExtraFiltersByProviderAuthorization(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "beta": newFakeAdapter("beta"), + "alpha": newFakeAdapter("alpha"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "beta": {Models: []string{"shared"}}, + "alpha": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + + grp := &group{name: "restricted", providers: []string{"alpha"}} + primary, extra, err := reg.resolveWithCandidates("shared", grp) + require.NoError(t, err) + assert.Equal(t, "alpha", primary.providerName) + assert.Empty(t, extra, "beta is denied to this group, so it must never appear as a failover candidate") +} + +// TestModelRegistry_ResolveWithCandidates_ResolveError_PropagatesUnchanged +// proves feat/failover never changes resolve's own error/status contract: +// an unknown or denied model still resolves to the identical error, with +// no candidates offered. +func TestModelRegistry_ResolveWithCandidates_ResolveError_PropagatesUnchanged(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{"alpha": newFakeAdapter("alpha")} + cfg := &Config{Providers: map[string]*ProviderConfig{"alpha": {Models: []string{"gpt-test"}}}} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + + _, extra, err := reg.resolveWithCandidates("no-such-model", allowAllGroup()) + assert.ErrorIs(t, err, errModelUnknown) + assert.Nil(t, extra) + + grp := &group{name: "denied", models: []string{"something-else"}} + _, extra, err = reg.resolveWithCandidates("gpt-test", grp) + assert.ErrorIs(t, err, errModelDenied) + assert.Nil(t, extra) +} + +// --- requestHealthTracker --- + +func TestRequestHealthTracker_HealthyByDefault(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + assert.True(t, tr.healthy("never-seen")) +} + +func TestRequestHealthTracker_NilSafe(t *testing.T) { + t.Parallel() + var tr *requestHealthTracker + assert.True(t, tr.healthy("anything"), "a nil tracker must report healthy, never panic") + assert.NotPanics(t, func() { tr.record("anything", false) }) +} + +// TestRequestHealthTracker_OpensAfterThreshold_ThenSkips is the direct +// regression test for "a provider marked unhealthy by REQUEST-path health +// is skipped without being called": would FAIL if record() never opened +// the breaker (a reverted feature would leave healthy() returning true +// forever). +func TestRequestHealthTracker_OpensAfterThreshold_ThenSkips(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + now := time.Now() + tr.nowFn = func() time.Time { return now } + + for i := 0; i < requestBreakerFailureThreshold-1; i++ { + tr.record("p", false) + assert.True(t, tr.healthy("p"), "must stay healthy before the threshold is reached") + } + tr.record("p", false) + assert.False(t, tr.healthy("p"), "must open exactly at the failure threshold") +} + +func TestRequestHealthTracker_SuccessBeforeThreshold_ResetsCounter(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + for i := 0; i < requestBreakerFailureThreshold-1; i++ { + tr.record("p", false) + } + tr.record("p", true) + // The counter must have reset to zero, not merely "not yet open": one + // more failure alone must not open it. + tr.record("p", false) + assert.True(t, tr.healthy("p"), "a success below threshold must reset the failure count, not just pause it") +} + +// TestRequestHealthTracker_RecoversAfterCooldown_ThenCloses proves the +// breaker admits exactly one probe once its cooldown elapses, and a +// successful probe closes it. +func TestRequestHealthTracker_RecoversAfterCooldown_ThenCloses(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + now := time.Now() + tr.nowFn = func() time.Time { return now } + + for i := 0; i < requestBreakerFailureThreshold; i++ { + tr.record("p", false) + } + require.False(t, tr.healthy("p")) + + now = now.Add(requestBreakerOpenDuration - time.Millisecond) + assert.False(t, tr.healthy("p"), "must stay unhealthy until the cooldown fully elapses") + + now = now.Add(2 * time.Millisecond) + assert.True(t, tr.healthy("p"), "must admit exactly one probe once the cooldown elapses") + + tr.record("p", true) + assert.True(t, tr.healthy("p")) + // A single further failure alone must not reopen it — closeBreaker + // resets the failure counter to zero, mirroring registry.go's own + // discovery breaker. + tr.record("p", false) + assert.True(t, tr.healthy("p")) +} + +// TestRequestHealthTracker_FailedProbe_ReopensWithDoubledBackoff proves a +// failed post-cooldown probe re-opens the breaker with a LONGER backoff, +// not the same one — would FAIL if reverted to a fixed-duration reopen. +func TestRequestHealthTracker_FailedProbe_ReopensWithDoubledBackoff(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + now := time.Now() + tr.nowFn = func() time.Time { return now } + + for i := 0; i < requestBreakerFailureThreshold; i++ { + tr.record("p", false) + } + now = now.Add(requestBreakerOpenDuration + time.Millisecond) + require.True(t, tr.healthy("p"), "cooldown elapsed, must admit the probe") + tr.record("p", false) // the probe itself fails + + assert.False(t, tr.healthy("p"), "a failed probe must reopen the breaker") + // The new backoff is requestBreakerOpenDuration*2: advancing only + // past the ORIGINAL window must still read unhealthy. + now = now.Add(requestBreakerOpenDuration + time.Millisecond) + assert.False(t, tr.healthy("p"), "a failed probe must double the backoff, not repeat the original window") + + now = now.Add(requestBreakerOpenDuration + 2*time.Millisecond) + assert.True(t, tr.healthy("p"), "must recover once the doubled window has fully elapsed") +} + +// TestRequestHealthTracker_FailureWhileOpenBeforeCooldown_IsANoOp is the +// direct regression test for adversarial-review finding F5: a failure +// arriving while the breaker is ALREADY open and its cooldown has not +// yet elapsed must change nothing — not the backoff, not openUntil. An +// earlier version fell through to the closed-state consecutiveFailures++ +// branch for this case, which reset backoff to the flat base duration +// and slid openUntil forward on every such failure, so doubling never +// actually happened outside the narrow post-cooldown probe path. +func TestRequestHealthTracker_FailureWhileOpenBeforeCooldown_IsANoOp(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + now := time.Now() + tr.nowFn = func() time.Time { return now } + + for i := 0; i < requestBreakerFailureThreshold; i++ { + tr.record("p", false) + } + st := tr.stateFor("p") + st.mu.Lock() + openUntilAfterTrip := st.openUntil + backoffAfterTrip := st.backoff + st.mu.Unlock() + require.Equal(t, requestBreakerOpenDuration, backoffAfterTrip) + + // Well before the cooldown elapses: several more failures arrive + // (e.g. other in-flight requests against the same dead provider). + now = now.Add(requestBreakerOpenDuration / 2) + for i := 0; i < 5; i++ { + require.False(t, tr.healthy("p")) + tr.record("p", false) + } + + st.mu.Lock() + openUntilAfterExtraFailures := st.openUntil + backoffAfterExtraFailures := st.backoff + failuresAfterExtra := st.consecutiveFailures + st.mu.Unlock() + assert.Equal(t, openUntilAfterTrip, openUntilAfterExtraFailures, "openUntil must not slide forward from failures arriving before the cooldown elapses") + assert.Equal(t, backoffAfterTrip, backoffAfterExtraFailures, "backoff must stay at the base duration, not reset by every failure while already open") + assert.Equal(t, requestBreakerFailureThreshold, failuresAfterExtra, "consecutiveFailures must not keep growing once the breaker is already open") +} + +func TestRequestHealthTracker_BackoffCapsAtMax(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + now := time.Now() + tr.nowFn = func() time.Time { return now } + + for i := 0; i < requestBreakerFailureThreshold; i++ { + tr.record("p", false) + } + // Fail every probe repeatedly — backoff must never exceed + // requestBreakerMaxOpenDuration, so this loop must always terminate + // within a bounded number of doublings. + for i := 0; i < 20; i++ { + now = now.Add(requestBreakerMaxOpenDuration + time.Millisecond) + require.True(t, tr.healthy("p")) + tr.record("p", false) + } + st := tr.stateFor("p") + st.mu.Lock() + backoff := st.backoff + st.mu.Unlock() + assert.LessOrEqual(t, backoff, requestBreakerMaxOpenDuration) +} + +// TestRequestHealthTracker_IndependentPerProvider proves one provider's +// failures never affect another's health — the per-pod signal is keyed by +// provider name, not shared global state. +func TestRequestHealthTracker_IndependentPerProvider(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + for i := 0; i < requestBreakerFailureThreshold; i++ { + tr.record("broken", false) + } + assert.False(t, tr.healthy("broken")) + assert.True(t, tr.healthy("fine"), "an unrelated provider's health must be untouched") +} + +// TestRequestHealthTracker_ConcurrentRecord_NoRace exercises record/healthy +// from many goroutines at once — the whole point of this signal is to sit +// on the live request path. +func TestRequestHealthTracker_ConcurrentRecord_NoRace(t *testing.T) { + t.Parallel() + tr := newRequestHealthTracker() + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + tr.record("p", i%3 != 0) + tr.healthy("p") + }(i) + } + wg.Wait() +} + +// --- failoverEligible / isProviderNotFoundError --- + +func TestFailoverEligible(t *testing.T) { + t.Parallel() + deadlineExceededErr := fmt.Errorf("%w: %w", errUpstream, fmt.Errorf("dial tcp: %w", context.DeadlineExceeded)) + + tests := []struct { + err error + name string + want bool + }{ + {name: "nil error", err: nil, want: false}, + {name: "400 never falls through", err: &providerHTTPError{status: http.StatusBadRequest}, want: false}, + {name: "500 falls through", err: &providerHTTPError{status: http.StatusInternalServerError}, want: true}, + {name: "502 falls through", err: &providerHTTPError{status: http.StatusBadGateway}, want: true}, + {name: "429 falls through", err: &providerHTTPError{status: http.StatusTooManyRequests}, want: true}, + {name: "401 falls through", err: &providerHTTPError{status: http.StatusUnauthorized}, want: true}, + {name: "403 falls through", err: &providerHTTPError{status: http.StatusForbidden}, want: true}, + {name: "404 falls through", err: &providerHTTPError{status: http.StatusNotFound}, want: true}, + {name: "connection failure falls through", err: fmt.Errorf("%w: dial tcp: connection refused", errUpstream), want: true}, + {name: "deadline exceeded (timeout) falls through", err: deadlineExceededErr, want: true}, + {name: "client canceled does not fall through", err: fmt.Errorf("%w: %w", errUpstream, context.Canceled), want: false}, + {name: "request build failure does not fall through", err: fmt.Errorf("%w: %w: build request", errUpstream, errRequestBuildFailed), want: false}, + {name: "translateError does not fall through", err: &translateError{msg: "bad field"}, want: false}, + {name: "responseTranslationError does not fall through", err: &responseTranslationError{err: errors.New("boom")}, want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, failoverEligible(tt.err)) + }) + } +} + +func TestIsProviderNotFoundError(t *testing.T) { + t.Parallel() + assert.True(t, isProviderNotFoundError(&providerHTTPError{status: http.StatusNotFound})) + assert.False(t, isProviderNotFoundError(&providerHTTPError{status: http.StatusInternalServerError})) + assert.False(t, isProviderNotFoundError(errors.New("plain error"))) + assert.False(t, isProviderNotFoundError(nil)) +} + +// --- validateFailoverConfig --- + +func TestValidateFailoverConfig_Defaults(t *testing.T) { + t.Parallel() + fc, err := validateFailoverConfig(FailoverConfig{}) + require.NoError(t, err) + assert.False(t, fc.enabled, "nil Enabled must default to false — coordinator ruling: new behavior that can change which provider serves a request goes behind a flag defaulting off") + assert.Equal(t, defaultFailoverMaxAttempts, fc.maxAttempts) +} + +func TestValidateFailoverConfig_ExplicitEnable(t *testing.T) { + t.Parallel() + yes := true + fc, err := validateFailoverConfig(FailoverConfig{Enabled: &yes}) + require.NoError(t, err) + assert.True(t, fc.enabled) +} + +func TestValidateFailoverConfig_ExplicitDisable(t *testing.T) { + t.Parallel() + no := false + fc, err := validateFailoverConfig(FailoverConfig{Enabled: &no}) + require.NoError(t, err) + assert.False(t, fc.enabled) +} + +func TestValidateFailoverConfig_MaxAttemptsBounds(t *testing.T) { + t.Parallel() + _, err := validateFailoverConfig(FailoverConfig{MaxAttempts: -1}) + assert.Error(t, err) + + _, err = validateFailoverConfig(FailoverConfig{MaxAttempts: maxFailoverMaxAttempts + 1}) + assert.Error(t, err) + + fc, err := validateFailoverConfig(FailoverConfig{MaxAttempts: maxFailoverMaxAttempts}) + require.NoError(t, err) + assert.Equal(t, maxFailoverMaxAttempts, fc.maxAttempts) +} + +// --- Gateway.orderedFailoverCandidates --- + +// testGatewayForCandidates builds a minimal *Gateway sufficient for +// orderedFailoverCandidates: a real modelRegistry (for discoveryHealthy) +// and a fresh requestHealthTracker, without going through newGateway/New. +func testGatewayForCandidates(t *testing.T, providerNames []string, fc failoverConfig) *Gateway { + t.Helper() + adapters := make(map[string]providerAdapter, len(providerNames)) + providers := make(map[string]*ProviderConfig, len(providerNames)) + for _, name := range providerNames { + adapters[name] = newFakeAdapter(name) + providers[name] = &ProviderConfig{Models: []string{"shared"}} + } + cfg := &Config{Providers: providers} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + require.NoError(t, err) + return &Gateway{registry: reg, failoverHealth: newRequestHealthTracker(), failover: fc, cfg: cfg, name: "test"} +} + +func candNames(cands []resolveCandidate) []string { + out := make([]string, len(cands)) + for i, c := range cands { + out[i] = c.providerName + } + return out +} + +func TestOrderedFailoverCandidates_Disabled_ReturnsPrimaryOnly(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha", "beta"}, failoverConfig{enabled: false, maxAttempts: 3}) + primary := resolveCandidate{providerName: "alpha"} + extra := []resolveCandidate{{providerName: "beta"}} + got := g.orderedFailoverCandidates(primary, extra) + assert.Equal(t, []string{"alpha"}, candNames(got), "disabled failover must never try a second provider") +} + +func TestOrderedFailoverCandidates_NoExtras_ReturnsPrimaryOnly_RegardlessOfEnabled(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha"}, failoverConfig{enabled: true, maxAttempts: 3}) + primary := resolveCandidate{providerName: "alpha"} + got := g.orderedFailoverCandidates(primary, nil) + assert.Equal(t, []string{"alpha"}, candNames(got), "single-provider deployment must behave exactly as before regardless of the enabled switch") +} + +func TestOrderedFailoverCandidates_RequestUnhealthy_HardSkipped(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha", "beta"}, failoverConfig{enabled: true, maxAttempts: 3}) + for i := 0; i < requestBreakerFailureThreshold; i++ { + g.failoverHealth.record("alpha", false) + } + primary := resolveCandidate{providerName: "alpha"} + extra := []resolveCandidate{{providerName: "beta"}} + got := g.orderedFailoverCandidates(primary, extra) + assert.Equal(t, []string{"beta"}, candNames(got), "a request-unhealthy provider must be excluded outright, never merely deprioritized") +} + +func TestOrderedFailoverCandidates_AllRequestUnhealthy_FailsOpenToFullList(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha", "beta"}, failoverConfig{enabled: true, maxAttempts: 3}) + for _, name := range []string{"alpha", "beta"} { + for i := 0; i < requestBreakerFailureThreshold; i++ { + g.failoverHealth.record(name, false) + } + } + primary := resolveCandidate{providerName: "alpha"} + extra := []resolveCandidate{{providerName: "beta"}} + got := g.orderedFailoverCandidates(primary, extra) + assert.ElementsMatch(t, []string{"alpha", "beta"}, candNames(got), "every candidate unhealthy must fail OPEN to the full list, not refuse to route at all") +} + +func TestOrderedFailoverCandidates_DiscoveryUnhealthy_DeprioritizedNotDropped(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha", "beta"}, failoverConfig{enabled: true, maxAttempts: 3}) + // Force alpha's discovery breaker open directly. + st := g.registry.states["alpha"] + st.mu.Lock() + st.health = breakerOpen + st.openUntil = time.Now().Add(time.Hour) + st.mu.Unlock() + require.False(t, g.registry.discoveryHealthy("alpha")) + + primary := resolveCandidate{providerName: "alpha"} + extra := []resolveCandidate{{providerName: "beta"}} + got := g.orderedFailoverCandidates(primary, extra) + assert.Equal(t, []string{"beta", "alpha"}, candNames(got), "discovery-unhealthy must be deprioritized (moved after), never dropped from the list") +} + +// TestOrderedFailoverCandidates_DoesNotCap proves orderedFailoverCandidates +// itself no longer applies maxAttempts — capFailoverCandidates does, as the +// LAST step of runMeteredCall's own sequence, after cost filtering. +func TestOrderedFailoverCandidates_DoesNotCap(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha", "beta", "gamma"}, failoverConfig{enabled: true, maxAttempts: 2}) + primary := resolveCandidate{providerName: "alpha"} + extra := []resolveCandidate{{providerName: "beta"}, {providerName: "gamma"}} + got := g.orderedFailoverCandidates(primary, extra) + assert.Equal(t, []string{"alpha", "beta", "gamma"}, candNames(got), "orderedFailoverCandidates must return the full health/discovery-ordered list uncapped") +} + +func TestCapFailoverCandidates(t *testing.T) { + t.Parallel() + all := []resolveCandidate{{providerName: "alpha"}, {providerName: "beta"}, {providerName: "gamma"}} + + got := capFailoverCandidates(all, 2) + assert.Equal(t, []string{"alpha", "beta"}, candNames(got), "maxAttempts must cap the total candidate list, primary included") + + got = capFailoverCandidates(all, 10) + assert.Len(t, got, 3, "a cap larger than the list must not truncate anything") + + got = capFailoverCandidates(all, 0) + assert.Len(t, got, 3, "max <= 0 must be a defensive no-op") +} + +// --- resolveUnifiedPricing / candidateCostAllowed / filterCandidatesByCost --- + +func TestResolveUnifiedPricing_CanonicalFirst_ThenBareFallback(t *testing.T) { + t.Parallel() + overrides := map[string]*ModelPricing{ + "beta/shared": {InputPerM: 1, OutputPerM: 2}, + "shared": {InputPerM: 5, OutputPerM: 6}, + } + p, ok := resolveUnifiedPricing("beta/shared", "shared", overrides) + require.True(t, ok) + assert.Equal(t, ModelPricing{InputPerM: 1, OutputPerM: 2}, p, "an override keyed on the canonical id must win over the bare fallback — the exact case unifiedCostMicros itself resolves this way") +} + +func TestResolveUnifiedPricing_FallsBackToBare(t *testing.T) { + t.Parallel() + overrides := map[string]*ModelPricing{"shared": {InputPerM: 5, OutputPerM: 6}} + p, ok := resolveUnifiedPricing("beta/shared", "shared", overrides) + require.True(t, ok) + assert.Equal(t, ModelPricing{InputPerM: 5, OutputPerM: 6}, p) +} + +func TestResolveUnifiedPricing_Unknown(t *testing.T) { + t.Parallel() + _, ok := resolveUnifiedPricing("beta/shared", "shared", nil) + assert.False(t, ok, "neither the canonical nor the bare id has a price — this must never be silently treated as zero") +} + +func TestResolveUnifiedPricing_BuiltinTable(t *testing.T) { + t.Parallel() + p, ok := resolveUnifiedPricing("openai/gpt-5", "gpt-5", nil) + require.True(t, ok, "the built-in pricing table must still resolve with no operator overrides configured") + assert.Equal(t, builtinPricing["gpt-5"], p) +} + +// TestCandidateCostAllowed is the direct regression test for the +// operator's binding ruling: table-driven across every named case, +// including the two-dimension requirement (cheap on one axis, dear on +// the other, must still be rejected). +func TestCandidateCostAllowed(t *testing.T) { + t.Parallel() + cheap := ModelPricing{InputPerM: 1, OutputPerM: 2} + dear := ModelPricing{InputPerM: 3, OutputPerM: 4} + mixedCheapInputDearOutput := ModelPricing{InputPerM: 0.5, OutputPerM: 10} + zero := ModelPricing{} + + tests := []struct { + name string + primary, candidate ModelPricing + primaryKnown, candidateKnown bool + want bool + }{ + {name: "both known, candidate strictly cheaper on both", primary: dear, primaryKnown: true, candidate: cheap, candidateKnown: true, want: true}, + {name: "both known, identical price", primary: cheap, primaryKnown: true, candidate: cheap, candidateKnown: true, want: true}, + {name: "both known, candidate strictly dearer on both", primary: cheap, primaryKnown: true, candidate: dear, candidateKnown: true, want: false}, + {name: "both known, cheaper input but dearer output must still reject", primary: cheap, primaryKnown: true, candidate: mixedCheapInputDearOutput, candidateKnown: true, want: false}, + {name: "both known, dearer input but cheaper output must still reject", primary: mixedCheapInputDearOutput, primaryKnown: true, candidate: cheap, candidateKnown: true, want: false}, + {name: "primary known, candidate unknown: never allowed", primary: dear, primaryKnown: true, candidate: ModelPricing{}, candidateKnown: false, want: false}, + {name: "primary known-zero, candidate unknown: never allowed", primary: zero, primaryKnown: true, candidate: ModelPricing{}, candidateKnown: false, want: false}, + {name: "primary unknown, candidate known non-zero: never allowed", primary: ModelPricing{}, primaryKnown: false, candidate: cheap, candidateKnown: true, want: false}, + {name: "primary unknown, candidate known but only input non-zero: never allowed", primary: ModelPricing{}, primaryKnown: false, candidate: ModelPricing{InputPerM: 0.01}, candidateKnown: true, want: false}, + {name: "primary unknown, candidate known-zero: allowed", primary: ModelPricing{}, primaryKnown: false, candidate: zero, candidateKnown: true, want: true}, + {name: "both unknown: allowed", primary: ModelPricing{}, primaryKnown: false, candidate: ModelPricing{}, candidateKnown: false, want: true}, + {name: "both known-zero: allowed", primary: zero, primaryKnown: true, candidate: zero, candidateKnown: true, want: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := candidateCostAllowed(tt.primary, tt.primaryKnown, tt.candidate, tt.candidateKnown) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFilterCandidatesByCost_SkipsPricierKeepsCheaper(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha", "beta", "gamma"}, failoverConfig{enabled: true, maxAttempts: 3}) + g.cfg.Pricing = map[string]*ModelPricing{ + "alpha/shared": {InputPerM: 2, OutputPerM: 4}, // primary + "beta/shared": {InputPerM: 3, OutputPerM: 4}, // dearer input -> rejected + "gamma/shared": {InputPerM: 2, OutputPerM: 4}, // equal -> allowed + } + candidates := []resolveCandidate{ + {providerName: "alpha", upstreamModel: "shared", canonical: "alpha/shared"}, + {providerName: "beta", upstreamModel: "shared", canonical: "beta/shared"}, + {providerName: "gamma", upstreamModel: "shared", canonical: "gamma/shared"}, + } + got := g.filterCandidatesByCost(candidates, "shared") + assert.Equal(t, []string{"alpha", "gamma"}, candNames(got)) +} + +func TestFilterCandidatesByCost_FewerThanTwoCandidates_NoOp(t *testing.T) { + t.Parallel() + g := testGatewayForCandidates(t, []string{"alpha"}, failoverConfig{enabled: true, maxAttempts: 3}) + candidates := []resolveCandidate{{providerName: "alpha", upstreamModel: "shared", canonical: "alpha/shared"}} + got := g.filterCandidatesByCost(candidates, "shared") + assert.Equal(t, candidates, got) +} + +// ===================================================================== +// End-to-end HTTP tests +// ===================================================================== + +// twoProviderFailoverConfig builds a Config with two openai-type providers, +// "alpha" and "beta" (alpha sorted first — the bareWinner/primary +// candidate), both configured with the identical bare model id "shared", +// pointed at srvAlpha/srvBeta respectively, one group with no +// restrictions, and one user "alice" with unlimited limits. +func twoProviderFailoverConfig(srvAlpha, srvBeta *httptest.Server) *Config { + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "alpha": {Type: "openai", BaseURL: srvAlpha.URL, APIKey: "k", Models: []string{"shared"}}, + "beta": {Type: "openai", BaseURL: srvBeta.URL, APIKey: "k", Models: []string{"shared"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{ + {Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}, + }} + // Explicitly opt in: FailoverConfig.Enabled now defaults to false + // (coordinator ruling) — every test in this file that exercises actual + // failover behavior must opt in on purpose, the same way an operator + // would, rather than relying on a default this feature no longer ships + // with. + yes := true + cfg.Failover = FailoverConfig{Enabled: &yes} + return cfg +} + +const successRespBody = `{"id":"chatcmpl-ok","object":"chat.completion","model":"shared","choices":[{"index":0,"message":{"role":"assistant","content":"hi"},"finish_reason":"stop"}],"usage":{"prompt_tokens":3,"completion_tokens":2}}` + +func jsonServer(status int, body string) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) +} + +func countingServer(status int, body string, calls *int64) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(calls, 1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) +} + +// TestHandleChat_Failover_FallsThroughAndServes is table-driven across +// every failure shape the brief names as a fallthrough case (5xx, +// connection failure, 429, 401, 403, 404) — proving alpha's failure is +// classified eligible AND that beta's real response reaches the client +// each time. Would FAIL if failoverEligible or the candidate loop's +// hasNext wiring were reverted (alpha's error would just be returned to +// the client instead of falling through). +func TestHandleChat_Failover_FallsThroughAndServes(t *testing.T) { + tests := []struct { + name string + alphaStatus int + unreachable bool + }{ + {name: "5xx", alphaStatus: http.StatusInternalServerError}, + {name: "connection_failure", unreachable: true}, + {name: "429", alphaStatus: http.StatusTooManyRequests}, + {name: "401", alphaStatus: http.StatusUnauthorized}, + {name: "403", alphaStatus: http.StatusForbidden}, + {name: "404", alphaStatus: http.StatusNotFound}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var alphaURL string + if tt.unreachable { + // A closed listener: connections are refused immediately, + // deterministic and fast, unlike a black-holed address. + dead := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + alphaURL = dead.URL + dead.Close() + } else { + alphaSrv := jsonServer(tt.alphaStatus, `{"error":{"message":"alpha down","type":"server_error"}}`) + defer alphaSrv.Close() + alphaURL = alphaSrv.URL + } + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "alpha": {Type: "openai", BaseURL: alphaURL, APIKey: "k", Models: []string{"shared"}}, + "beta": {Type: "openai", BaseURL: betaSrv.URL, APIKey: "k", Models: []string{"shared"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + yes := true + cfg.Failover = FailoverConfig{Enabled: &yes} + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + origStderr := captureStderrStart() + body := map[string]any{"model": "shared", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + logOutput := captureStderrStop(origStderr) + + require.Equal(t, http.StatusOK, rec.Code, "must fall through to beta and serve its response, body=%s", rec.Body.String()) + assert.Equal(t, successRespBody, rec.Body.String()) + + if tt.alphaStatus == http.StatusNotFound { + assert.Contains(t, logOutput, "404", "a 404 failover must log loudly") + assert.Contains(t, logOutput, "alpha") + assert.Contains(t, logOutput, "beta") + } + }) + } +} + +// TestHandleChat_Failover_404LogLine_NeverRateLimited is the direct +// regression test for adversarial-review round 2, finding 5: nothing +// pinned the ruling that the 404 loud-log line is exempt from +// failoverLogGate — every OTHER test only ever drives one 404 event +// against a freshly constructed Gateway, whose gate's first call always +// logs regardless of whether it is rate-limited, so a version that +// wrapped the 404 line in failoverLogGate.shouldLog would still pass +// every one of them. Three providers here all serve the SAME bare model +// id and the first TWO both 404, so this drives two 404-failover events +// through the SAME Gateway (and so the SAME failoverLogGate instance) +// within milliseconds of each other — well inside the gate's own 30s +// window — and asserts BOTH loud lines appear. +func TestHandleChat_Failover_404LogLine_NeverRateLimited(t *testing.T) { + alphaSrv := jsonServer(http.StatusNotFound, `{"error":{"message":"not found","type":"invalid_request_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusNotFound, `{"error":{"message":"not found","type":"invalid_request_error"}}`) + defer betaSrv.Close() + gammaSrv := jsonServer(http.StatusOK, successRespBody) + defer gammaSrv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "alpha": {Type: "openai", BaseURL: alphaSrv.URL, APIKey: "k", Models: []string{"shared"}}, + "beta": {Type: "openai", BaseURL: betaSrv.URL, APIKey: "k", Models: []string{"shared"}}, + "gamma": {Type: "openai", BaseURL: gammaSrv.URL, APIKey: "k", Models: []string{"shared"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + yes := true + cfg.Failover = FailoverConfig{Enabled: &yes} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + origStderr := captureStderrStart() + body := map[string]any{"model": "shared", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + logOutput := captureStderrStop(origStderr) + + require.Equal(t, http.StatusOK, rec.Code, "body=%s", rec.Body.String()) + alphaLine := `provider "alpha" returned 404` + betaLine := `provider "beta" returned 404` + assert.Contains(t, logOutput, alphaLine, "alpha's own 404 must log, unsuppressed") + assert.Contains(t, logOutput, betaLine, "beta's own 404 must ALSO log, not suppressed by the generic failing-over rate limit even though it fired milliseconds after alpha's") +} + +// TestHandleChat_Failover_400_DoesNotFallThrough is the regression test +// for the brief's one hard exception: a malformed request must never +// retry against a second provider. Would FAIL if failoverEligible treated +// 400 the same as every other status. +func TestHandleChat_Failover_400_DoesNotFallThrough(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusBadRequest, `{"error":{"message":"bad request","type":"invalid_request_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code, "a 400 must be returned as-is, never retried") + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls), "beta must never be called after alpha's 400") +} + +// TestHandleChat_Failover_MidStream_DoesNotFailOver_NoCorruption is the +// regression test for THE HARD CONSTRAINT: once anything has reached the +// client, failover must never happen and the response must never be +// corrupted with a second envelope. Would FAIL if the loop checked +// failoverEligible before sw.wroteHeader. +func TestHandleChat_Failover_MidStream_DoesNotFailOver_NoCorruption(t *testing.T) { + const firstChunk = "data: {\"id\":\"c1\",\"object\":\"chat.completion.chunk\",\"choices\":[{\"delta\":{\"content\":\"Hi\"}}]}\n\n" + + var betaCalls int64 + alphaSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + fl, ok := w.(http.Flusher) + require.True(t, ok) + _, _ = w.Write([]byte(firstChunk)) + fl.Flush() + hj, ok := w.(http.Hijacker) + require.True(t, ok) + conn, _, hjErr := hj.Hijack() + if hjErr == nil { + _ = conn.Close() + } + })) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "stream": true, "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code, "headers already committed before the drop") + assert.Equal(t, firstChunk, rec.Body.String(), "no trailing envelope, no failover content appended") + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls), "beta must never be called once alpha has started writing") +} + +// TestHandleChat_Failover_RequestUnhealthy_SkippedWithoutBeingCalled is the +// end-to-end regression test for the request-path health hard-skip gate: +// after enough consecutive failures, alpha must be excluded from routing +// entirely — its upstream server must receive no further hits — while beta +// still serves. +func TestHandleChat_Failover_RequestUnhealthy_SkippedWithoutBeingCalled(t *testing.T) { + var alphaCalls, betaCalls int64 + alphaSrv := countingServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`, &alphaCalls) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "messages": []any{}} + for i := 0; i < requestBreakerFailureThreshold; i++ { + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code, "every one of these must still fail over to beta and succeed") + } + require.Equal(t, int64(requestBreakerFailureThreshold), atomic.LoadInt64(&alphaCalls)) + + // alpha's request-path breaker must now be open — one more request + // must skip it entirely. + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, int64(requestBreakerFailureThreshold), atomic.LoadInt64(&alphaCalls), "alpha must NOT receive a further hit once request-unhealthy") + assert.Equal(t, int64(requestBreakerFailureThreshold+1), atomic.LoadInt64(&betaCalls)) +} + +// TestHandleChat_Failover_DiscoveryUnhealthy_StillTried proves discovery +// health is never a HARD SKIP: a provider whose DISCOVERY breaker is +// open must still be attempted, and can still serve the response, when +// beta (the only other candidate) fails. Would FAIL if discoveryHealthy +// were used to exclude a candidate outright, but does NOT independently +// prove ordering (deleting the deprioritization partition in +// orderedFailoverCandidates leaves this test passing too, since beta +// fails either way and alpha is tried regardless of position) — the +// ordering claim itself is covered only by +// TestOrderedFailoverCandidates_DiscoveryUnhealthy_DeprioritizedNotDropped +// (adversarial-review correction: this comment previously overstated +// what this test proves). +func TestHandleChat_Failover_DiscoveryUnhealthy_StillTried(t *testing.T) { + alphaSrv := jsonServer(http.StatusOK, successRespBody) // alpha's CHAT endpoint works fine + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + // Force alpha's DISCOVERY breaker open directly (feat/provider-health's + // own signal) — its chat-completions endpoint is untouched and stays + // healthy; only discovery is marked broken, per discoveryHealthy's own + // documented scope. + st := gw.registry.states["alpha"] + st.mu.Lock() + st.health = breakerOpen + st.openUntil = time.Now().Add(time.Hour) + st.mu.Unlock() + require.False(t, gw.registry.discoveryHealthy("alpha")) + + // beta is healthy by every signal but its own chat endpoint 500s, so + // the request can only succeed if alpha is tried at some point in + // the chain despite its open discovery breaker. + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, "alpha must still be tried and must serve, body=%s", rec.Body.String()) + assert.Equal(t, successRespBody, rec.Body.String()) +} + +// TestHandleChat_Failover_TenantRequestCount_IncrementsOnceAcrossFailover +// is the regression test for "never double-count the request": a +// tenant's own req/min counter must move by exactly 1 across a whole +// failover chain, never once per attempt. +func TestHandleChat_Failover_TenantRequestCount_IncrementsOnceAcrossFailover(t *testing.T) { + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + now := time.Now() + reqCount, ok := gw.limiter.getCounter("user", "alice", metricReq, windowMin, now) + require.True(t, ok) + assert.Equal(t, int64(1), reqCount, "one client-facing request must count as exactly one, regardless of how many providers it took") + + totalReq, ok := gw.limiter.getCounter(totalScopeKind, totalScopeID, metricReq, windowMin, now) + require.True(t, ok) + assert.Equal(t, int64(1), totalReq) +} + +// TestHandleChat_Failover_PerProviderAttemptCounters_AttributeCorrectly is +// the regression test for "rebind the attempt recorder per attempt": alpha +// must show exactly one attempt/one failure, and beta exactly one +// attempt/zero failures — never both attributed to alpha (the closure's +// original, fixed-at-construction bug this feature explicitly fixes). +func TestHandleChat_Failover_PerProviderAttemptCounters_AttributeCorrectly(t *testing.T) { + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + // Deterministic: run the fire-and-forget counter write synchronously + // (mirrors TestHandleChat_ContextDeadlineExceeded_RecordsProviderFailure). + gw.limiter.spawn = func(f func()) { f() } + + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + now := time.Now() + alphaAttempts, _ := gw.limiter.getCounter(kindProvider, "alpha", metricProvAttempt, windowDay, now) + alphaFails, _ := gw.limiter.getCounter(kindProvider, "alpha", metricProvFail, windowDay, now) + betaAttempts, _ := gw.limiter.getCounter(kindProvider, "beta", metricProvAttempt, windowDay, now) + betaFails, _ := gw.limiter.getCounter(kindProvider, "beta", metricProvFail, windowDay, now) + + assert.Equal(t, int64(1), alphaAttempts, "alpha attempts") + assert.Equal(t, int64(1), alphaFails, "alpha fails — a reverted 'rebind per attempt' bug would misattribute this to whichever provider the closure was first bound to") + assert.Equal(t, int64(1), betaAttempts, "beta attempts") + assert.Equal(t, int64(0), betaFails, "beta fails") +} + +// TestHandleChat_Failover_CachedUnderWinningProviderKey is the regression +// test for cache-key correctness: a response produced by the FAILOVER +// provider (beta) must be retrievable via a cache key built with BETA's +// own name, not alpha's — would FAIL if cacheKeyStr were still computed +// once, before the loop, from the primary candidate alone. +func TestHandleChat_Failover_CachedUnderWinningProviderKey(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + redisLn := newBehavioralRedisServer(t) + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + cfg.Redis = &RedisConfig{Address: redisLn.Addr().String()} + cfg.Cache = CacheConfig{Enabled: true, TTL: "1m", MaxBodyBytes: 1 << 20} + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + reqBody := map[string]any{"model": "shared", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", reqBody) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + require.Equal(t, "miss", rec.Header().Get("X-Llmgw-Cache")) + require.Equal(t, int64(1), atomic.LoadInt64(&betaCalls)) + + // Reconstruct the exact key runMeteredCall would compute for BETA's + // own candidate — upstreamModel is "shared" (the bare id every + // candidate shares in this fixture), requestedModel is the client's + // own "shared" string, endpoint is cacheEndpointChat, and req is the + // decoded body with "model" rewritten (exactly what the gateway + // hashes) — see cacheKey's own doc comment, cache.go. + hashed := map[string]any{"model": "shared", "messages": reqBody["messages"]} + wantKey := cacheKey("beta", "shared", "shared", cacheEndpointChat, hashed) + cached, hit := gw.cache.lookup(wantKey) + require.True(t, hit, "the response must be stored under beta's own cache key") + assert.Contains(t, string(cached.Body), `"id":"chatcmpl-ok"`) + + // A second, identical request must now be served from cache without + // beta's real backend being hit again. + req2 := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", reqBody) + rec2 := httptest.NewRecorder() + h.ServeHTTP(rec2, req2) + assert.Equal(t, http.StatusOK, rec2.Code) + assert.Equal(t, int64(1), atomic.LoadInt64(&betaCalls), "the second request must hit cache under beta's key, not call beta's backend again") +} + +// --- cost guard (operator ruling): never fail over to a pricier candidate --- + +// costGuardConfig builds a two-provider (alpha, beta) config exactly like +// twoProviderFailoverConfig but WITHOUT setting cfg.Pricing, so each test +// configures its own overrides for its own case. +func costGuardConfig(srvAlpha, srvBeta *httptest.Server) *Config { + cfg := twoProviderFailoverConfig(srvAlpha, srvBeta) + cfg.Providers["alpha"].Models = []string{"shared"} + cfg.Providers["beta"].Models = []string{"shared"} + return cfg +} + +// driveCostGuardRequest posts one chat-completion request for "shared" +// through h and returns the recorder and everything logged to stderr +// while it ran. +func driveCostGuardRequest(t *testing.T, h http.Handler) (*httptest.ResponseRecorder, string) { + t.Helper() + origStderr := captureStderrStart() + body := map[string]any{"model": "shared", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec, captureStderrStop(origStderr) +} + +// TestHandleChat_Failover_CostGuard_SkipsPricierCandidate is the direct +// regression test for the operator's requirement: a candidate priced +// higher than the primary must never serve. Would FAIL if the cost guard +// were reverted (mutation-tested in this branch's own history). +func TestHandleChat_Failover_CostGuard_SkipsPricierCandidate(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) + cfg.Pricing = map[string]*ModelPricing{ + "alpha/shared": {InputPerM: 1, OutputPerM: 2}, + "beta/shared": {InputPerM: 1, OutputPerM: 3}, // dearer on output only — still must be rejected + } + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, logOutput := driveCostGuardRequest(t, h) + + assert.Equal(t, http.StatusInternalServerError, rec.Code, "beta is dearer, so alpha's own failure must be returned as-is, body=%s", rec.Body.String()) + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls), "beta must never be called once the cost guard rejects it") + assert.Contains(t, logOutput, "alpha") + assert.Contains(t, logOutput, "beta") + assert.Contains(t, logOutput, "shared") +} + +// TestHandleChat_Failover_CostGuard_AllowsCheaperOrEqualCandidate is the +// mirror positive case: a candidate priced at or below the primary must +// still serve normally. +func TestHandleChat_Failover_CostGuard_AllowsCheaperOrEqualCandidate(t *testing.T) { + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) + cfg.Pricing = map[string]*ModelPricing{ + "alpha/shared": {InputPerM: 2, OutputPerM: 4}, + "beta/shared": {InputPerM: 2, OutputPerM: 4}, // exactly equal — allowed + } + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, _ := driveCostGuardRequest(t, h) + assert.Equal(t, http.StatusOK, rec.Code, "beta is equally priced, so failover must still serve it, body=%s", rec.Body.String()) + assert.Equal(t, successRespBody, rec.Body.String()) +} + +// TestHandleChat_Failover_CostGuard_PrimaryKnown_CandidateUnknown_Skips +// covers the operator's first unknown-pricing ruling: primary priced, +// candidate unpriced — never allowed, since an unknown price cannot be +// proven not-more-expensive. +func TestHandleChat_Failover_CostGuard_PrimaryKnown_CandidateUnknown_Skips(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) + cfg.Pricing = map[string]*ModelPricing{ + "alpha/shared": {InputPerM: 1, OutputPerM: 2}, + // beta has no override, and "shared"/"beta/shared" is not in the + // built-in table either — its price is genuinely unknown. + } + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, _ := driveCostGuardRequest(t, h) + assert.Equal(t, http.StatusInternalServerError, rec.Code) + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls)) +} + +// TestHandleChat_Failover_CostGuard_PrimaryUnknown_CandidateKnownNonZero_Skips +// covers the operator's second unknown-pricing ruling: moving from +// unbilled to billed is itself a spend increase. +func TestHandleChat_Failover_CostGuard_PrimaryUnknown_CandidateKnownNonZero_Skips(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) + cfg.Pricing = map[string]*ModelPricing{ + "beta/shared": {InputPerM: 1, OutputPerM: 2}, // alpha has no override at all: unknown + } + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, _ := driveCostGuardRequest(t, h) + assert.Equal(t, http.StatusInternalServerError, rec.Code) + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls)) +} + +// TestHandleChat_Failover_CostGuard_PrimaryUnknown_CandidateKnownZero_Allows +// covers the ruling's own stated exception: a candidate that is known, +// explicit free can never be more expensive than an unknown amount. +func TestHandleChat_Failover_CostGuard_PrimaryUnknown_CandidateKnownZero_Allows(t *testing.T) { + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) + cfg.Pricing = map[string]*ModelPricing{ + "beta/shared": {InputPerM: 0, OutputPerM: 0}, // explicit, known-zero override + } + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, _ := driveCostGuardRequest(t, h) + assert.Equal(t, http.StatusOK, rec.Code, "a known-zero candidate can never be more expensive than an unknown primary, body=%s", rec.Body.String()) +} + +// TestHandleChat_Failover_CostGuard_BothUnknown_Allows and +// TestHandleChat_Failover_CostGuard_BothKnownZero_Allows cover the +// ruling's "equal, so allowed" cases. +func TestHandleChat_Failover_CostGuard_BothUnknown_Allows(t *testing.T) { + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) // no cfg.Pricing at all + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, _ := driveCostGuardRequest(t, h) + assert.Equal(t, http.StatusOK, rec.Code, "both unknown must be treated as equal, body=%s", rec.Body.String()) +} + +func TestHandleChat_Failover_CostGuard_BothKnownZero_Allows(t *testing.T) { + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := costGuardConfig(alphaSrv, betaSrv) + cfg.Pricing = map[string]*ModelPricing{ + "alpha/shared": {InputPerM: 0, OutputPerM: 0}, + "beta/shared": {InputPerM: 0, OutputPerM: 0}, + } + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + rec, _ := driveCostGuardRequest(t, h) + assert.Equal(t, http.StatusOK, rec.Code, "both known-zero must be treated as equal, body=%s", rec.Body.String()) +} + +// TestHandleChat_Failover_CostGuard_PerProviderPricingOverride_RealShape is +// the requested production-shape case: the SAME bare model id +// (a real, built-in-priced model — "gpt-5") served by two providers, +// where alpha has NO override (resolves via the built-in table) and beta +// has an operator-set per-provider override making it dearer on the +// canonical "beta/gpt-5" key. Proves the guard resolves pricing through +// the exact canonical-then-bare path unifiedCostMicros itself uses, not +// a parallel, simplified notion of price — the whole point of ruling 1. +func TestHandleChat_Failover_CostGuard_PerProviderPricingOverride_RealShape(t *testing.T) { + const realModel = "gpt-5" + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "alpha": {Type: "openai", BaseURL: alphaSrv.URL, APIKey: "k", Models: []string{realModel}}, + "beta": {Type: "openai", BaseURL: betaSrv.URL, APIKey: "k", Models: []string{realModel}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + yes := true + cfg.Failover = FailoverConfig{Enabled: &yes} + // Only beta gets a per-provider override — alpha resolves gpt-5's + // price from builtinPricing instead, exactly the real production + // shape a per-provider Pricing override produces. + builtin := builtinPricing[realModel] + cfg.Pricing = map[string]*ModelPricing{ + "beta/" + realModel: {InputPerM: builtin.InputPerM * 10, OutputPerM: builtin.OutputPerM * 10}, + } + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + origStderr := captureStderrStart() + body := map[string]any{"model": realModel, "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + logOutput := captureStderrStop(origStderr) + + assert.Equal(t, http.StatusInternalServerError, rec.Code, "beta's per-provider override makes it 10x alpha's built-in price, so it must be rejected, body=%s", rec.Body.String()) + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls)) + assert.Contains(t, logOutput, "alpha") + assert.Contains(t, logOutput, "beta") + assert.Contains(t, logOutput, realModel) +} + +// --- adversarial-review regressions (F1-F4, F6, F7) --- + +// TestHandleChat_Failover_F1_StreamOptionsNotLeakedAcrossCandidates is the +// direct regression test for adversarial-review finding F1: an +// openai-type adapter mutates req in place (chatCompletion forces +// stream_options.include_usage=true onto it BEFORE it ever calls the +// upstream, provider_openai.go), so reusing one shared req map across +// candidates let a later candidate read back an earlier candidate's own +// forced injection and misreport clientAskedUsage=true to a client that +// never asked for streamed usage — leaking the gateway's own +// deliberately-suppressed usage-only SSE chunk. alpha fails before ever +// reaching the network (its own mutation to its OWN req copy still +// happens first, exactly like the real bug), beta serves a real SSE +// stream containing a usage-only chunk the client never asked for. +func TestHandleChat_Failover_F1_StreamOptionsNotLeakedAcrossCandidates(t *testing.T) { + const contentChunk = "data: {\"id\":\"c1\",\"object\":\"chat.completion.chunk\",\"choices\":[{\"delta\":{\"content\":\"Hi\"}}]}\n\n" + const usageOnlyChunk = "data: {\"id\":\"c1\",\"object\":\"chat.completion.chunk\",\"choices\":[],\"usage\":{\"prompt_tokens\":5,\"completion_tokens\":2}}\n\n" + const doneMarker = "data: [DONE]\n\n" + + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + fl, ok := w.(http.Flusher) + require.True(t, ok) + for _, chunk := range []string{contentChunk, usageOnlyChunk, doneMarker} { + _, _ = w.Write([]byte(chunk)) + fl.Flush() + } + })) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + // The client explicitly does NOT set stream_options at all — it + // never asked for usage in the stream. + body := map[string]any{"model": "shared", "stream": true, "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rw := newRecordingWriter() + h.ServeHTTP(rw, req) + + require.Equal(t, http.StatusOK, rw.status) + assert.Contains(t, rw.buf.String(), `"content":"Hi"`, "the real content chunk must still be forwarded") + assert.NotContains(t, rw.buf.String(), `"prompt_tokens":5`, "a client that never asked for stream usage must never receive it, even though ALPHA's own (failed, discarded) attempt forced include_usage=true on ITS OWN copy of the request") +} + +// TestHandleChat_Failover_F2_StaleSSEHeadersDoNotSurviveToPlainResponse is +// the direct regression test for adversarial-review finding F2: +// newSSEWriter (sse.go) stages Content-Type/Cache-Control/ +// X-Accel-Buffering on sw.Header() the moment a streaming attempt starts, +// before any byte is written. alpha enters that streaming path (a real +// 200 + text/event-stream response) but its connection is severed before +// any complete SSE event is dispatched — an error, satisfying THE HARD +// CONSTRAINT (nothing was actually WRITTEN to the client) — so failover +// proceeds to beta, which answers a plain, non-streaming JSON body. +func TestHandleChat_Failover_F2_StaleSSEHeadersDoNotSurviveToPlainResponse(t *testing.T) { + alphaSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + hj, ok := w.(http.Hijacker) + require.True(t, ok) + conn, _, hjErr := hj.Hijack() + if hjErr == nil { + // Closing a hijacked chunked-encoding connection with no + // terminating chunk produces a genuine "unexpected EOF" on + // the client's read side (verified empirically), not a + // clean end-of-stream — readSSE returns that as a real + // error without ever dispatching an event, so + // sw.writeData/sw.WriteHeader is never reached even though + // newSSEWriter already staged its three headers. + _ = conn.Close() + } + })) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "stream": true, "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, "body=%s", rec.Body.String()) + assert.Equal(t, successRespBody, rec.Body.String()) + assert.Equal(t, "application/json", rec.Header().Get("Content-Type"), "beta's own Content-Type must win, not alpha's stale text/event-stream") + assert.Empty(t, rec.Header().Get("Cache-Control"), "alpha's staged SSE header must never survive into beta's plain JSON response") + assert.Empty(t, rec.Header().Get("X-Accel-Buffering"), "alpha's staged SSE header must never survive into beta's plain JSON response") +} + +// TestHandleChat_Failover_DefaultPath_PreservesPreexistingResponseHeaders is +// the direct regression test for adversarial-review round 2's MUST-FIX +// finding: the round-1 F2 header clear ran unconditionally, including on +// i == 0 — the ONLY iteration the default, failover-disabled path ever +// reaches — deleting response headers a DIFFERENT, earlier Traefik +// middleware had already staged on the shared ResponseWriter before this +// plugin ever ran (CORS, HSTS, request-id, ...), contradicting this +// package's own "byte-identical to a gateway built before this field +// existed" claim. Covers both a cache miss (the real upstream call path) +// and a cache hit (the round-1 harness found headers survived a hit but +// were dropped on a miss on the SAME endpoint for the SAME client). +func TestHandleChat_Failover_DefaultPath_PreservesPreexistingResponseHeaders(t *testing.T) { + t.Run("cache miss", func(t *testing.T) { + srv := jsonServer(http.StatusOK, successRespBody) + defer srv.Close() + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "openai": {Type: "openai", BaseURL: srv.URL, APIKey: "k", Models: []string{"gpt-test"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "gpt-test", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + // Simulates an EARLIER Traefik middleware in the chain (CORS, + // HSTS, request-id, ...) that already staged headers on the + // SAME underlying ResponseWriter before this plugin ever ran. + rec.Header().Set("Access-Control-Allow-Origin", "*") + rec.Header().Set("X-Preexisting-Mw", "keepme") + h.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, "body=%s", rec.Body.String()) + assert.Equal(t, "*", rec.Header().Get("Access-Control-Allow-Origin"), "a header staged by an earlier middleware must survive the default (single-candidate) path") + assert.Equal(t, "keepme", rec.Header().Get("X-Preexisting-Mw")) + }) + + t.Run("cache hit", func(t *testing.T) { + srv := jsonServer(http.StatusOK, successRespBody) + defer srv.Close() + redisLn := newBehavioralRedisServer(t) + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "openai": {Type: "openai", BaseURL: srv.URL, APIKey: "k", Models: []string{"gpt-test"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + cfg.Redis = &RedisConfig{Address: redisLn.Addr().String()} + cfg.Cache = CacheConfig{Enabled: true, TTL: "1m", MaxBodyBytes: 1 << 20} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "gpt-test", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + req1 := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec1 := httptest.NewRecorder() + h.ServeHTTP(rec1, req1) + require.Equal(t, http.StatusOK, rec1.Code) + require.Equal(t, "miss", rec1.Header().Get("X-Llmgw-Cache")) + + req2 := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec2 := httptest.NewRecorder() + rec2.Header().Set("Access-Control-Allow-Origin", "*") + rec2.Header().Set("X-Preexisting-Mw", "keepme") + h.ServeHTTP(rec2, req2) + + require.Equal(t, http.StatusOK, rec2.Code) + require.Equal(t, "hit", rec2.Header().Get("X-Llmgw-Cache")) + assert.Equal(t, "*", rec2.Header().Get("Access-Control-Allow-Origin"), "a header staged by an earlier middleware must survive a cache hit too") + assert.Equal(t, "keepme", rec2.Header().Get("X-Preexisting-Mw")) + }) +} + +// TestHandleChat_Failover_F3_401IsAFailure_OpensRequestHealthBreaker is the +// direct regression test for adversarial-review finding F3: a provider +// answering 401 to every request (a dead API key — exactly the "masking +// a broken provider key" risk this feature exists to reduce) must +// eventually be skipped, not retried forever. Would FAIL under the +// reverted isTransient-based classification, since isTransient returns +// false for any non-429 4xx. +func TestHandleChat_Failover_F3_401IsAFailure_OpensRequestHealthBreaker(t *testing.T) { + var alphaCalls, betaCalls int64 + alphaSrv := countingServer(http.StatusUnauthorized, `{"error":{"message":"invalid api key","type":"authentication_error"}}`, &alphaCalls) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "messages": []any{}} + for i := 0; i < requestBreakerFailureThreshold; i++ { + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code, "every one of these must still fail over to beta and succeed") + } + require.Equal(t, int64(requestBreakerFailureThreshold), atomic.LoadInt64(&alphaCalls)) + + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, int64(requestBreakerFailureThreshold), atomic.LoadInt64(&alphaCalls), "alpha must be skipped after 3 consecutive 401s, not called a 4th time") + assert.Equal(t, int64(requestBreakerFailureThreshold+1), atomic.LoadInt64(&betaCalls)) +} + +// TestHandleChat_Failover_F4_SharedDeadlineExhausted_NeverMisattributedToNextCandidate +// is the direct regression test for adversarial-review finding F4: alpha +// burns the ENTIRE shared request context sleeping past its deadline; +// beta must never be dialed at all (an attempt after the deadline is +// already spent would fail near-instantly for a reason that has nothing +// to do with beta's own health), never be recorded as having failed, and +// the client-facing error must name alpha, not beta. +func TestHandleChat_Failover_F4_SharedDeadlineExhausted_NeverMisattributedToNextCandidate(t *testing.T) { + alphaSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(300 * time.Millisecond) // well past the request's own 50ms deadline below + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(successRespBody)) + })) + defer alphaSrv.Close() + var betaCalls int64 + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + gw.limiter.spawn = func(f func()) { f() } + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", + strings.NewReader(`{"model":"shared","messages":[{"role":"user","content":"hi"}]}`)).WithContext(ctx) + req.Header.Set("Authorization", "Bearer sk-alice") + req.Header.Set("Content-Type", "application/json") + + origStderr := captureStderrStart() + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + logOutput := captureStderrStop(origStderr) + + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls), "beta must never be dialed once the shared deadline is already spent") + assert.True(t, gw.failoverHealth.healthy("beta"), "beta must never be marked unhealthy for a timeout that was alpha's own fault") + + now := time.Now() + betaAttempts, _ := gw.limiter.getCounter(kindProvider, "beta", metricProvAttempt, windowDay, now) + assert.Equal(t, int64(0), betaAttempts, "beta's own dashboard attempt counter must not move either") + + assert.Contains(t, logOutput, "alpha", "the error must name the provider that actually failed") +} + +// TestHandleChat_Failover_F6_RetryAttemptsDoNotOpenBreakerOnOneRequest is +// the direct regression test for adversarial-review finding F6: with +// retry.enabled and attempts:3, ONE client request against a +// permanently-failing provider must count as ONE outcome for the +// request-health gate, not one per raw retry attempt — otherwise a +// single request alone could reach requestBreakerFailureThreshold (3) +// purely from ITS OWN internal retries. +func TestHandleChat_Failover_F6_RetryAttemptsDoNotOpenBreakerOnOneRequest(t *testing.T) { + var alphaCalls int64 + alphaSrv := countingServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`, &alphaCalls) + defer alphaSrv.Close() + betaSrv := jsonServer(http.StatusOK, successRespBody) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + cfg.Retry = RetryConfig{Enabled: true, Attempts: 3, Backoff: "1ms"} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code, "must still fail over to beta after alpha's own retries are exhausted") + + // retry.attempts=3 means 4 raw HTTP attempts against alpha for this + // ONE client request (retryPolicy's own doc comment, retry.go). + require.Equal(t, int64(4), atomic.LoadInt64(&alphaCalls)) + // The request-health gate must still read this as exactly ONE + // failure, not four — well below requestBreakerFailureThreshold (3). + assert.True(t, gw.failoverHealth.healthy("alpha"), "one client request's internal retries must count as one outcome for the request-health gate, not one per raw attempt") +} + +// TestHandleChat_Failover_F7_CachedFailoverResponse_NeverHitsDeadPrimary is +// the direct regression test for adversarial-review finding F7: even +// while alpha stays permanently broken, a SECOND identical request must +// be served from beta's cache entry WITHOUT alpha's dead backend being +// hit again on that second request — proving cache lookups run for +// every candidate BEFORE any of them is attempted, not interleaved with +// each candidate's own turn. +// TestHandleChat_Failover_F7_CachedFailoverResponse_NeverHitsDeadPrimary is +// the regression test for the ORIGINAL F7 complaint, fixed the way the +// coordinator ruled it must be (adversarial-review round 2): health, not +// an all-candidates cache pre-pass, is what keeps a genuinely dead +// primary from being hit again. alpha fails enough consecutive REAL +// requests to trip its own request-health breaker (requestHealthTracker, +// failover.go) — each one also falling over to beta, whose successful +// response gets cached after the very first request — and only THEN, +// once alpha is excluded from the candidate list entirely, does a +// further request reach beta's cache without alpha being dialed again. +func TestHandleChat_Failover_F7_CachedFailoverResponse_NeverHitsDeadPrimary(t *testing.T) { + var alphaCalls, betaCalls int64 + alphaSrv := countingServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`, &alphaCalls) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + redisLn := newBehavioralRedisServer(t) + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + cfg.Redis = &RedisConfig{Address: redisLn.Addr().String()} + cfg.Cache = CacheConfig{Enabled: true, TTL: "1m", MaxBodyBytes: 1 << 20} + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + reqBody := map[string]any{"model": "shared", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + + // Drive enough requests to trip alpha's OWN request-health breaker. + // beta's response is cached after the first of these, so alphaCalls + // keeps climbing (alpha itself is still healthy enough to be tried + // every time) while betaCalls stops climbing once its cache entry + // exists. + for i := 0; i < requestBreakerFailureThreshold; i++ { + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", reqBody) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + } + require.Equal(t, int64(requestBreakerFailureThreshold), atomic.LoadInt64(&alphaCalls)) + require.Equal(t, int64(1), atomic.LoadInt64(&betaCalls), "beta's own cache entry must absorb every request after the first") + + // One more request: alpha is now request-unhealthy and must be + // excluded from the candidate list entirely — it must not be dialed + // a further time just to discover beta's cache entry. + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", reqBody) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "hit", rec.Header().Get("X-Llmgw-Cache")) + assert.Equal(t, int64(requestBreakerFailureThreshold), atomic.LoadInt64(&alphaCalls), "alpha must not be dialed again once it is request-unhealthy") + assert.Equal(t, int64(1), atomic.LoadInt64(&betaCalls)) +} + +// TestHandleChat_Failover_HealthyPrimaryNeverServedFromAnotherCandidatesCache +// is the direct regression test for adversarial-review round 2, finding +// 2: the round-1 F7 pre-pass checked every candidate's cache before +// attempting any of them, which silently substituted a DIFFERENT +// candidate's cached response while the actual primary was perfectly +// healthy — proven: alpha healthy and serving a distinct body, never +// called, the client instead got beta's stale cached body with +// X-Llmgw-Cache: hit and no "failing over" log line at all. beta's cache +// entry here is planted directly (simulating a real entry left over +// from an EARLIER, unrelated failover event, now stale) while alpha has +// never failed — a fresh request must reach alpha for real. +func TestHandleChat_Failover_HealthyPrimaryNeverServedFromAnotherCandidatesCache(t *testing.T) { + const alphaRespBody = `{"id":"chatcmpl-alpha","object":"chat.completion","model":"shared","choices":[{"index":0,"message":{"role":"assistant","content":"from alpha"},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1}}` + + var alphaCalls, betaCalls int64 + alphaSrv := countingServer(http.StatusOK, alphaRespBody, &alphaCalls) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + redisLn := newBehavioralRedisServer(t) + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + cfg.Redis = &RedisConfig{Address: redisLn.Addr().String()} + cfg.Cache = CacheConfig{Enabled: true, TTL: "1m", MaxBodyBytes: 1 << 20} + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + reqBody := map[string]any{"model": "shared", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + hashed := map[string]any{"model": "shared", "messages": reqBody["messages"]} + betaKey := cacheKey("beta", "shared", "shared", cacheEndpointChat, hashed) + gw.cache.store(betaKey, http.StatusOK, "application/json", []byte(successRespBody), time.Minute) + + origStderr := captureStderrStart() + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", reqBody) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + logOutput := captureStderrStop(origStderr) + + require.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, alphaRespBody, rec.Body.String(), "a healthy primary's OWN real response must be served, never another candidate's unrelated cache entry") + assert.Equal(t, int64(1), atomic.LoadInt64(&alphaCalls), "alpha must actually be dialed") + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls)) + assert.NotContains(t, logOutput, "failing over", "no failover happened, so no failover log line may appear") +} + +// TestHandleChat_Failover_SingleProvider_UpstreamError_BehavesExactlyAsBefore +// pins the "single-provider deployment behaves exactly as before" +// requirement explicitly for this branch, with failover left at its +// (now disabled) default: with no second candidate to try, an upstream +// error must be returned to the client unchanged, identical to v0.1 +// behavior, regardless of the enabled switch either way. +func TestHandleChat_Failover_SingleProvider_UpstreamError_BehavesExactlyAsBefore(t *testing.T) { + srv := jsonServer(http.StatusTooManyRequests, `{"error":{"message":"rate limited upstream","type":"rate_limit_error"}}`) + defer srv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "openai": {Type: "openai", BaseURL: srv.URL, APIKey: "k", Models: []string{"gpt-test"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice"}}} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "gpt-test", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusTooManyRequests, rec.Code, "a single-provider deployment must still surface the upstream error unchanged — there is nothing to fail over to") + var out struct { + Error struct { + Type string `json:"type"` + } `json:"error"` + } + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &out)) + assert.Equal(t, "upstream_error", out.Error.Type) +} + +// TestHandleChat_Failover_DefaultConfig_ByteIdenticalToPreFailover is the +// regression test for the coordinator's default-off ruling: a Config that +// never mentions "failover" at all — the shape of every config written +// before this feature existed, and the shape most operators keep using +// after upgrading without opting in — must behave byte-identically to +// having no failover code at all. Would FAIL if validateFailoverConfig's +// default reverted to enabled. +func TestHandleChat_Failover_DefaultConfig_ByteIdenticalToPreFailover(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "alpha": {Type: "openai", BaseURL: alphaSrv.URL, APIKey: "k", Models: []string{"shared"}}, + "beta": {Type: "openai", BaseURL: betaSrv.URL, APIKey: "k", Models: []string{"shared"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + // cfg.Failover is deliberately left untouched — the zero value, what + // every config predating this feature has. + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusInternalServerError, rec.Code, "an unconfigured Failover block must leave alpha's own 500 unchanged") + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls), "beta must never be called when failover was never opted into") +} + +// TestHandleChat_Failover_DisabledPath_NeverTouchesRequestHealthTracker is +// the direct regression test for adversarial-review round 2, finding 3: +// on the disabled path (single candidate, the common case for most +// deployments) g.failoverHealth.record was still called unconditionally, +// taking requestHealthTracker's tracker-wide mutex (stateFor's t.mu) on +// EVERY metered request even though that state is never READ on this +// path at all (orderedFailoverCandidates' own early return never calls +// healthy() when disabled) — dead work plus a new global contention +// point. Proven the only way observable from outside the package: the +// tracker's internal per-provider state must never even be CREATED for +// a provider whose every request goes through the disabled path, +// including one that fails. +func TestHandleChat_Failover_DisabledPath_NeverTouchesRequestHealthTracker(t *testing.T) { + srv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer srv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "openai": {Type: "openai", BaseURL: srv.URL, APIKey: "k", Models: []string{"gpt-test"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + // cfg.Failover left at its zero value: disabled, the common case. + + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + body := map[string]any{"model": "gpt-test", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + require.Equal(t, http.StatusInternalServerError, rec.Code) + + gw.failoverHealth.mu.Lock() + stateCount := len(gw.failoverHealth.states) + gw.failoverHealth.mu.Unlock() + assert.Equal(t, 0, stateCount, "the disabled path must never record into requestHealthTracker at all, even for a failing provider") +} + +// TestHandleChat_Failover_ExplicitlyDisabled_5xxNeverFallsThrough proves +// the on/off switch actually gates the behavior end to end. +func TestHandleChat_Failover_ExplicitlyDisabled_5xxNeverFallsThrough(t *testing.T) { + var betaCalls int64 + alphaSrv := jsonServer(http.StatusInternalServerError, `{"error":{"message":"down","type":"server_error"}}`) + defer alphaSrv.Close() + betaSrv := countingServer(http.StatusOK, successRespBody, &betaCalls) + defer betaSrv.Close() + + cfg := twoProviderFailoverConfig(alphaSrv, betaSrv) + no := false + cfg.Failover = FailoverConfig{Enabled: &no} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + + body := map[string]any{"model": "shared", "messages": []any{}} + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusInternalServerError, rec.Code, "with failover disabled, alpha's own 500 must be returned as-is, wrapped in the upstream-error envelope") + assert.Equal(t, int64(0), atomic.LoadInt64(&betaCalls)) +} + +// TestHandleChat_Failover_ContextDeadlineExceeded_ClassifiedEligible_ +// AndRecordsHealthFailure proves timeout classification end to end +// through the SAME mechanism recordProviderAttempt already uses +// (isDeadlineExceeded) — see this test file's package-level note on why a +// full "alpha times out, beta still serves" HTTP scenario cannot be +// demonstrated: every candidate in one logical request shares ONE +// request context, so once that context's own deadline is what makes +// alpha's failure a genuine timeout, zero budget remains for any +// subsequent candidate. This proves the half that IS achievable and +// correct: alpha's attempt is recorded as a request-health failure +// exactly like a plain connection error would be. +func TestHandleChat_Failover_ContextDeadlineExceeded_RecordsRequestHealthFailure(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(300 * time.Millisecond) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(successRespBody)) + })) + defer srv.Close() + + cfg := CreateConfig() + cfg.Providers = map[string]*ProviderConfig{ + "alpha": {Type: "openai", BaseURL: srv.URL, APIKey: "k", Models: []string{"shared"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}}} + // Explicitly opt in: request-health is only ever recorded while + // failover is enabled (adversarial-review round 2, finding 3 — it + // is never READ on the disabled path, so recording it there was + // dead work and a needless global-mutex hit on every request). + yes := true + cfg.Failover = FailoverConfig{Enabled: &yes} + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + h, err := New(context.Background(), next, cfg, "llmgw") + require.NoError(t, err) + gw, ok := h.(*Gateway) + require.True(t, ok) + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", + strings.NewReader(`{"model":"shared","messages":[{"role":"user","content":"hi"}]}`)).WithContext(ctx) + req.Header.Set("Authorization", "Bearer sk-alice") + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + assert.True(t, gw.failoverHealth.healthy("alpha"), "one deadline-exceeded attempt alone must not yet open the breaker (below threshold)") + + st := gw.failoverHealth.stateFor("alpha") + st.mu.Lock() + failures := st.consecutiveFailures + st.mu.Unlock() + assert.Equal(t, 1, failures, "the deadline-exceeded attempt must be classified as a request-health failure, not silently ignored") +} + +// capturedStderr holds the state captureStderrStart needs to hand back to +// captureStderrStop — the same os.Pipe swap +// TestHandleChat_MidStreamUpstreamDrop_NoTrailingEnvelope already uses in +// routes_unified_test.go, factored out here so the table-driven fallthrough +// test above can capture the 404 loud-log assertion per sub-test. +type capturedStderr struct { + orig *os.File + pr *os.File + pw *os.File +} + +// captureStderrStart redirects os.Stderr to an in-memory pipe. Callers +// must not run this concurrently with another goroutine writing to +// os.Stderr in the SAME process (the same constraint the existing +// mid-stream-drop test already accepts) — every caller in this file runs +// its captured request synchronously, sequentially, never inside +// t.Parallel(). +func captureStderrStart() capturedStderr { + orig := os.Stderr + pr, pw, err := os.Pipe() + if err != nil { + panic(err) // os.Pipe failing is not a condition any test here can meaningfully recover from + } + os.Stderr = pw + return capturedStderr{orig: orig, pr: pr, pw: pw} +} + +// captureStderrStop restores os.Stderr and returns everything written to +// it since the matching captureStderrStart call. +func captureStderrStop(c capturedStderr) string { + os.Stderr = c.orig + _ = c.pw.Close() + out, _ := io.ReadAll(c.pr) + return string(out) +} diff --git a/llmgateway.go b/llmgateway.go index ed15ff0..08d06d8 100644 --- a/llmgateway.go +++ b/llmgateway.go @@ -70,7 +70,13 @@ type Config struct { // at all) resolves to every documented default and behaves exactly // like a deployment with no circuit breaker at all for a provider // that never fails — see validateBreakerConfig's own doc comment. - Breaker BreakerConfig `json:"breaker"` + // Failover configures feat/failover — cross-provider failover for a + // bare model id more than one configured provider serves. See + // FailoverConfig's own doc comment (failover.go) for the full + // contract; a struct value, not a pointer, for the same + // zero-means-default reason as Breaker/Retry/Cache below. + Failover FailoverConfig `json:"failover"` + Breaker BreakerConfig `json:"breaker"` // Retry is a struct value, not a pointer, because its own Enabled // field is the on/off signal (unlike Redis/Users, where the block's // mere presence is the signal) — so its tag omits "omitempty": @@ -493,18 +499,24 @@ func defaultBodyAdmissionCap() int { const maxExplicitBodyAdmissionCap = 10_000 // Gateway is the Traefik middleware handler. +// +// Field order below is fieldalignment-verified (golangci-lint's govet +// enable-all, run with -fix against a scratch copy to derive the exact +// zero-waste sequence, then hand-applied here so every field keeps its +// original doc comment) — see providerState's own doc comment +// (registry.go) for the general convention. type Gateway struct { - next http.Handler - cfg *Config - auth *authStore - limiter *limiter - registry *modelRegistry - adapters map[string]providerAdapter + next http.Handler // targetClient is the shared, connection-pooled *http.Client the // MCP/A2A target proxy (mcp_a2a.go) issues every upstream request // through — built once via newAdapterHTTPClient, the same constructor // each provider adapter uses for its own client. targetClient *http.Client + auth *authStore + limiter *limiter + registry *modelRegistry + adapters map[string]providerAdapter + cfg *Config // cache is nil whenever response caching is not configured or not // usable (cfg.Cache.Enabled is false, or true with no config.Redis — // see buildResponseCache, cache.go). Every call site checks for nil @@ -525,7 +537,25 @@ type Gateway struct { // resized afterward, matching a Go channel's own fixed-capacity // contract. bodyAdmission chan struct{} - name string + // failoverHealth is feat/failover's own per-pod, in-memory + // request-path health signal (failover.go), built once, here, by + // newGateway. Every method on it is nil-receiver-safe, so a Gateway + // assembled directly (bypassing newGateway, as a few older tests do) + // degrades to "failover never skips a candidate for request health" + // rather than a nil-pointer panic. + failoverHealth *requestHealthTracker + name string + // failoverLogGate rate-limits runMeteredCall's generic "failing over" + // log line (routes_unified.go) to once per storeErrorLogEvery + // (adversarial-review fix, F10) — reuses auth.go's own logGate type, + // the identical technique shouldLogAuthFailure/logStoreError already + // apply elsewhere in this package. Zero-value-usable: the first call + // on a fresh Gateway always logs. Deliberately does NOT gate the 404 + // loud-log line (operator ruling: that one must always log). + failoverLogGate logGate + // failover is Config.Failover, validated and resolved once by + // newGateway (validateFailoverConfig, failover.go). + failover failoverConfig } // telemetryStartupOnce keeps the anonymous "plugin loaded" ping to one per @@ -634,6 +664,17 @@ func newGateway(ctx context.Context, next http.Handler, config *Config, name str } g := &Gateway{next: next, name: name, cfg: config, auth: auth} + // feat/failover: validated once, here — see FailoverConfig's own doc + // comment (failover.go) for the default (disabled, coordinator + // ruling) and what "single-provider deployment behaves exactly as + // before" means in practice. + failoverCfg, err := validateFailoverConfig(config.Failover) + if err != nil { + return nil, err + } + g.failover = failoverCfg + g.failoverHealth = newRequestHealthTracker() + // bodyAdmissionCap: an explicit MaxInFlightBodyRequests always wins // over the self-tuned default (house rule: explicit override always // wins) — see Config.MaxInFlightBodyRequests' own doc comment. 0 (the diff --git a/routes_unified.go b/routes_unified.go index 143404e..9ef5a58 100644 --- a/routes_unified.go +++ b/routes_unified.go @@ -5,10 +5,12 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "math" "net/http" "strconv" + "time" ) // maxRequestBytes caps a client request body decoded into a @@ -256,123 +258,348 @@ func (g *Gateway) runUnified(w http.ResponseWriter, r *http.Request, u *user, gr // envelope shape they answer in. logPrefix names the route in every log // line this function writes, so a log reader can always tell which route // produced a given line without the classification logic existing twice. +// +// feat/failover: this function now tries a LIST of candidate providers +// (g.registry.resolveWithCandidates/g.orderedFailoverCandidates, +// failover.go), not just the one resolve alone would pick, and is the +// ONE hook point both /v1/chat/completions (runUnified, above) and +// /v1/messages (handleMessages, routes_messages.go) share — per the +// brief's own instruction, failover is wired here once rather than +// forked into a second implementation. THE HARD CONSTRAINT this loop is +// built around: call(...) writes directly to respWriter, which tees +// straight through to sw; the very first byte written commits the +// response, so a candidate is only ever retried while sw.wroteHeader is +// still false. Every step that was a one-shot computation before this +// feature (the cache key, the attempt recorder's bound provider name, +// the usage/canonical id billed) now happens fresh INSIDE the loop, once +// per candidate — this is what "rebind the attempt recorder per +// attempt" and "cache under the winning provider's key" actually mean +// in code, not just in the brief's prose. func (g *Gateway) runMeteredCall(sw *statusTrackingWriter, r *http.Request, scopes []limitScope, body []byte, req map[string]any, requestedModel string, grp *group, endpoint, logPrefix string, envelope envelopeWriter, writeUpstream providerUpstreamErrorWriter, call adapterCall) { - adapter, upstreamModel, canonical, err := g.registry.resolve(requestedModel, grp) + // F9 (adversarial-review, optional perf fix): a disabled deployment + // never needs the candidate pool at all — resolvePrimaryOnly skips + // failoverCandidates' own provider scan entirely, so a config that + // never opted into failover pays nothing extra for a pool it will + // never use. + var primary resolveCandidate + var extra []resolveCandidate + var err error + if g.failover.enabled { + primary, extra, err = g.registry.resolveWithCandidates(requestedModel, grp) + } else { + primary, err = g.registry.resolvePrimaryOnly(requestedModel, grp) + } if err != nil { writeModelResolveErrorEnvelope(sw, err, envelope) return } + // Sequence matters (each step's own doc comment explains why): + // health/discovery ordering first, so candidates[0] below is the + // ACTUAL first-attempted candidate the cost guard compares everyone + // else against; cost filtering second; the maxAttempts cap last, so + // it never re-admits a candidate the cost guard already removed. + candidates := g.orderedFailoverCandidates(primary, extra) + candidates = g.filterCandidatesByCost(candidates, requestedModel) + candidates = capFailoverCandidates(candidates, g.failover.maxAttempts) - req["model"] = upstreamModel - - // cacheable gates every cache step below on spec §2's scope: a - // non-streaming request, with a working cache (g.cache is nil - // whenever caching is off or Redis was absent — buildResponseCache, - // cache.go), for a group that has not opted out (groupCacheEnabled). - // cacheKey is computed here — after "model" is rewritten to - // upstreamModel but deliberately BEFORE gatewayAliasKey is injected - // below — so the hashed request body reflects exactly what goes - // upstream. requestedModel (the client's own, un-rewritten alias - // string) and endpoint are passed as separate cacheKey arguments, - // not left in req: a translating adapter bakes the echoed alias - // straight into the cached response body's own "model" field, so two - // clients requesting the same upstream model under different alias - // forms (e.g. "claude-x" vs "anthropic/claude-x") must never collide - // into one cache entry (cacheKey's own doc comment, cache.go). + // cacheable/streaming gate every cache step below on spec §2's scope, + // exactly as before this feature: a non-streaming request, with a + // working cache (g.cache is nil whenever caching is off or Redis was + // absent — buildResponseCache, cache.go), for a group that has not + // opted out (groupCacheEnabled). Computed once, outside the loop: it + // depends only on req["stream"]/grp, neither of which a failover + // attempt changes. streaming, _ := req["stream"].(bool) cacheable := !streaming && g.cache != nil && groupCacheEnabled(grp) - var cacheKeyStr string - if cacheable { - cacheKeyStr = cacheKey(adapter.name(), upstreamModel, requestedModel, endpoint, req) - if cached, hit := g.cache.lookup(cacheKeyStr); hit { - // A cache hit accounts the request only — checkAndCount - // above already counted it — never token/cost, and never - // runs the zero-usage estimate branch below: the client - // never reached the upstream provider its body size would - // be estimating against. - sw.Header().Set("X-Llmgw-Cache", "hit") - if cached.ContentType != "" { - sw.Header().Set("Content-Type", cached.ContentType) + + // pristineReq is the client's own decoded request, snapshotted ONCE, + // BEFORE any candidate's adapter ever touches it, with "model" + // already rewritten to the upstream id every candidate in this + // failover chain shares by construction (failoverCandidates, + // failover.go, only ever adds providers serving the IDENTICAL bare + // id resolve's primary already resolved to). Every attempt below + // builds its OWN fresh top-level copy from this snapshot rather than + // reusing or mutating one shared map (adversarial-review fix, F1): + // openai-type adapters mutate req in place — chatCompletion forces + // stream_options.include_usage=true onto it (provider_openai.go) — + // and reusing the same map across attempts let a LATER candidate + // read back an EARLIER candidate's own forced injection and + // misreport clientAskedUsage=true to a client that never asked for + // it, leaking the gateway's own suppressed usage-only SSE chunk. + // Proven against a single-provider control; this is what fixes it + // for openai, openai-audio, and copilot alike. + pristineReq := make(map[string]any, len(req)+1) + for k, v := range req { + pristineReq[k] = v + } + if len(candidates) > 0 { + pristineReq["model"] = candidates[0].upstreamModel + } + + var lastErr error + var lastProviderName string + for i, cand := range candidates { + if i > 0 { + // F4 fix (adversarial-review): a shared request context an + // EARLIER candidate already exhausted must never be + // misattributed as THIS candidate's own failure. There is no + // per-candidate timeout budget in this architecture (every + // candidate shares r's SAME context — see README's own + // documented limitation), so once an earlier candidate burns + // the deadline, attempting the next one would fail near- + // instantly for a reason that has nothing to do with its own + // health. Stop here with the LAST REAL error instead of + // chasing further doomed candidates and wrongly opening a + // perfectly healthy provider's request-health breaker. + if r.Context().Err() != nil { + g.handleAdapterErrorEnvelope(sw, lastErr, lastProviderName, logPrefix, envelope, writeUpstream) + return } - sw.WriteHeader(cached.Status) - _, _ = sw.Write(cached.Body) + + // checkAndCount above (admitRequest, this file) already + // counted this request exactly once — never double-count: a + // failover attempt is a NEW upstream attempt, not a new + // logical request, so nothing here calls checkAndCount + // again. Per-provider ATTEMPT counters, below, still record + // every attempt independently via the freshly-bound + // recorder. + // + // Rate-limited (F10 fix, adversarial-review): during a + // sustained outage this line would otherwise be one + // blocking synchronous stderr write per request on shared + // Traefik ingress — the exact failure shouldLogAuthFailure/ + // logStoreError (auth.go/limits.go) already exist to + // prevent elsewhere in this package. The 404 line below is + // deliberately NOT gated by this (operator ruling: it must + // always log). + if log, suppressed := g.failoverLogGate.shouldLog(time.Now()); log { + suffix := "" + if suppressed > 0 { + suffix = fmt.Sprintf(" (%d more failovers suppressed since last log)", suppressed) + } + g.warnf("%s: failing over from provider %q to %q for model %q%s", logPrefix, candidates[i-1].providerName, cand.providerName, requestedModel, suffix) + } + + // F2 fix, correctly scoped (adversarial-review round 2 — the + // round-1 version cleared unconditionally, including i == 0, + // which deleted response headers a DIFFERENT, earlier Traefik + // middleware had already staged before this plugin ever ran + // (CORS, HSTS, request-id, ...) on the default, non-failover + // path — proven by differential harness, and it contradicted + // this package's own "byte-identical to a gateway built + // before this field existed" claim. Only i > 0 needs this at + // all: newSSEWriter (sse.go) sets Content-Type: text/event- + // stream, Cache-Control: no-cache, and X-Accel-Buffering: no + // the moment a streaming attempt starts, before any byte is + // written, and if THAT PRIOR candidate then failed before + // writing (still safe to fail over, THE HARD CONSTRAINT + // below), those staged headers must not survive into a + // plain-JSON response from THIS candidate. Safe to clear + // unconditionally once i > 0: this loop only ever reaches a + // new iteration while sw.wroteHeader is false, so nothing + // committed is ever at risk of being wiped. + hdr := sw.Header() + for k := range hdr { + hdr.Del(k) + } + } + + // attemptReq is a FRESH top-level copy of pristineReq for every + // candidate (F1 fix, this function's own doc comment above). + // gatewayAliasKey is (re-)injected here: every providerAdapter + // implementation unconditionally deletes it before it ever + // touches the network (its own doc comment, providers.go), so + // each fresh copy needs it set again to echo the client's own + // requested alias correctly (anthropic/gemini-type adapters read + // it to build their own response envelope). + attemptReq := make(map[string]any, len(pristineReq)+1) + for k, v := range pristineReq { + attemptReq[k] = v + } + attemptReq[gatewayAliasKey] = requestedModel + + // Cache lookup for THIS candidate, immediately before its own + // attempt (adversarial-review round 2 ruling — supersedes the + // round-1 "check every candidate's cache up front" pre-pass, + // which silently substituted ANOTHER candidate's cached response + // while the actual primary was perfectly healthy: proven — + // alpha healthy and serving a distinct body, never called, the + // client instead got beta's cached body with no failure and no + // "failing over" log line, i.e. failover with no failure. Health + // (F3/F6, this file/failover.go) is the correct mechanism for + // "do not call a dead primary": once a genuinely broken + // candidate is marked request-unhealthy it is excluded from the + // candidate list entirely (orderedFailoverCandidates, + // failover.go), so the NEXT candidate's cache is still reached + // on a later request without ever dialing the dead one — the + // pre-pass solved that problem by accident, with the side + // effect of mis-serving healthy traffic from an unrelated + // candidate's cache. cacheKeyStr is computed from pristineReq, + // BEFORE gatewayAliasKey was injected above would have mattered + // — gatewayAliasKey is a purely internal echo-back mechanism + // cacheKey does not itself strip (its own doc comment, + // cache.go), so hashing attemptReq instead would add noise no + // two requests would otherwise disagree on. + var cacheKeyStr string + if cacheable { + cacheKeyStr = cacheKey(cand.providerName, cand.upstreamModel, requestedModel, endpoint, pristineReq) + if cached, hit := g.cache.lookup(cacheKeyStr); hit { + sw.Header().Set("X-Llmgw-Cache", "hit") + if cached.ContentType != "" { + sw.Header().Set("Content-Type", cached.ContentType) + } + sw.WriteHeader(cached.Status) + _, _ = sw.Write(cached.Body) + return + } + // Set before call() below writes anything — headers must + // precede the body a miss is about to produce (spec §2). A + // later iteration's own miss overwrites this harmlessly: it + // is never observable until SOME candidate actually writes, + // by which point the loop has already committed to that + // candidate's outcome (THE HARD CONSTRAINT, this function's + // own doc comment). + sw.Header().Set("X-Llmgw-Cache", "miss") + } + + // respWriter is sw, wrapped in a capture tee only when this + // request is cacheable — cacheCaptureWriter buffers everything + // written so a 200 non-stream response can be stored after + // call() returns, without any adapter knowing caching exists + // (adapterCall's contract is unchanged either way). A fresh + // capture per candidate: reusing one across attempts would let a + // candidate that never wrote anything (the common failover case) + // leave a stale buffer behind — capture.status stays 0 either + // way, so this is defensive clarity, not a proven bug fix. + var respWriter http.ResponseWriter = sw + var capture *cacheCaptureWriter + if cacheable { + capture = newCacheCaptureWriter(sw, g.cache.maxBodyBytes) + respWriter = capture + } + + // Feature A (v0.22) attempt recorder, REBOUND every iteration to + // THIS candidate's own provider name (feat/failover's own "never + // misattribute a retried attempt to the first provider" + // requirement) — every upstream attempt call() makes, via + // upstreamJSON's retryPolicy.do (providers.go), reports through + // here. limiter.recordProviderAttempt keeps its own existing + // per-raw-attempt classification unchanged (limits.go); feat/ + // failover's OWN request-path health gate is recorded separately + // below, once per candidate rather than once per raw attempt — + // see requestHealthTracker.record's own doc comment (failover.go) + // for why (F3/F6 fix, adversarial-review). + providerName := cand.providerName + upstreamModel := cand.upstreamModel + ctx := withAttemptRecorder(r.Context(), func(resp *http.Response, attemptErr error) { + g.limiter.recordProviderAttempt(providerName, upstreamModel, resp, attemptErr) + }) + result, callErr := call(cand.adapter, ctx, respWriter, attemptReq) + + // Request-path health, recorded ONCE per candidate per logical + // request (F3/F6 fix, adversarial-review) — see + // requestHealthTracker.record's own doc comment (failover.go) + // for the full ruling: failoverEligible's classification + // (everything except HTTP 400 is a failure) is reused directly, + // not limiter's narrower isTransient/isDeadlineExceeded one, so + // a provider answering 401/403/404 to every request is + // eventually routed around instead of retried forever. Guarded + // by g.failover.enabled (adversarial-review round 2, finding 3): + // this state is never READ on the disabled path + // (orderedFailoverCandidates' own early return never calls + // healthy()), so recording it there was dead work plus a new + // global-mutex contention point (requestHealthTracker.stateFor's + // own t.mu) on EVERY metered request, disabled or not. + if g.failover.enabled { + g.failoverHealth.record(providerName, !failoverEligible(callErr)) + } + + // Usage is accounted before the error branch below runs, not + // after: every adapter that can fail mid-stream (forwardStream + // in each of the three provider files) still returns whatever + // usage it had already captured alongside the error — a usage + // chunk that arrived just before a dropped connection must + // still be billed, or a client that aborts right after that + // frame arrives could repeat the trick to dodge every + // token/cost budget. A non-streaming failure (providerHTTPError, + // translateError, or a connection failure before any write) + // always carries zero usage by contract, so accounting it here + // is a no-op — account skips every write once both total tokens + // and cost are zero. routes_messages.go's + // callTranslatedMessages/callAnthropicMessagesPassthrough follow + // the identical contract for their own error paths (item 1 fix, + // 2026-08-22 review). This still runs exactly once per + // CANDIDATE, including a candidate that goes on to fail over — + // its own (zero, by contract) usage is billed as a no-op, never + // skipped, so a later successful candidate's real usage is never + // silently doubled with a phantom first entry either. + if callErr == nil && result.total() == 0 { + if streaming { + g.logf("%s: zero usage reported for a streaming response from model %q; accounting the request only", logPrefix, cand.canonical) + } else { + result.prompt = int64(math.Ceil(float64(len(body)) / 4)) + result.estimated = true + } + } + g.limiter.account(scopes, result, unifiedCostMicros(cand.canonical, cand.upstreamModel, result, g.cfg.Pricing)) + if result.estimated { + g.logf("%s: usage for model %q logged as estimated (%d prompt tokens derived from request body size, not the provider's reported usage)", logPrefix, cand.canonical, result.prompt) + } + + // A miss stores the response after everything above has already + // run — accounting must never be skipped or delayed waiting on a + // cache write. Only a genuine upstream 200 is stored (spec §2's + // "on 200 non-stream, tee and SET"): a non-2xx status never + // reaches here as capture.status (forwardJSON/translate error + // paths return *providerHTTPError/*translateError instead of + // writing through respWriter, so callErr is non-nil and + // capture.status stays 0), and callErr == nil is checked + // directly regardless. capture.oversize excludes a response + // cacheCaptureWriter stopped buffering past maxBodyBytes: + // store's own maxBodyBytes check would reject it too, but only + // after being handed a silently truncated body — skip the call + // outright instead of ever constructing a corrupt cache entry. + if cacheable && callErr == nil && capture.status == http.StatusOK && !capture.oversize { + g.cache.store(cacheKeyStr, capture.status, capture.contentType, capture.buf.Bytes(), effectiveTTL(g.cache, grp)) + } + + if callErr == nil { return } - // Set before call() below writes anything — headers must precede - // the body a miss is about to produce (spec §2). - sw.Header().Set("X-Llmgw-Cache", "miss") - } - req[gatewayAliasKey] = requestedModel - - // respWriter is sw, wrapped in a capture tee only when this request - // is cacheable — cacheCaptureWriter buffers everything written so a - // 200 non-stream response can be stored after call() returns, without - // any adapter knowing caching exists (adapterCall's contract is - // unchanged either way). - var respWriter http.ResponseWriter = sw - var capture *cacheCaptureWriter - if cacheable { - capture = newCacheCaptureWriter(sw, g.cache.maxBodyBytes) - respWriter = capture - } - - // Feature A (v0.22): every upstream attempt call() makes — via - // upstreamJSON's retryPolicy.do, providers.go — reports through to - // this recorder, closing over the adapter/model this request already - // resolved to. See attemptRecorder's own doc comment (providers.go). - ctx := withAttemptRecorder(r.Context(), func(resp *http.Response, attemptErr error) { - g.limiter.recordProviderAttempt(adapter.name(), upstreamModel, resp, attemptErr) - }) - result, callErr := call(adapter, ctx, respWriter, req) - - // Usage is accounted before the error branch below runs, not after: - // every adapter that can fail mid-stream (forwardStream in each of the - // three provider files) still returns whatever usage it had already - // captured alongside the error — a usage chunk that arrived just before - // a dropped connection must still be billed, or a client that aborts - // right after that frame arrives could repeat the trick to dodge every - // token/cost budget. A non-streaming failure (providerHTTPError, - // translateError, or a connection failure before any write) always - // carries zero usage by contract, so accounting it here is a no-op — - // account skips every write once both total tokens and cost are zero. - // routes_messages.go's callTranslatedMessages/ - // callAnthropicMessagesPassthrough follow the identical contract for - // their own error paths (item 1 fix, 2026-08-22 review: a translation - // failure AFTER a successful, billed upstream call must still return - // that call's real usage, not usage{}). - if callErr == nil && result.total() == 0 { - if streaming { - g.logf("%s: zero usage reported for a streaming response from model %q; accounting the request only", logPrefix, canonical) - } else { - result.prompt = int64(math.Ceil(float64(len(body)) / 4)) - result.estimated = true + // THE HARD CONSTRAINT: once sw has written anything, status and + // body are committed and can never be retried elsewhere — a + // mid-stream failure (headers, or a partial SSE body, already + // reached the client) always falls straight through to the + // existing, unchanged error classification, exactly as it did + // before this feature existed, regardless of how many candidates + // remain. + if sw.wroteHeader { + g.handleAdapterErrorEnvelope(sw, callErr, providerName, logPrefix, envelope, writeUpstream) + return } - } - g.limiter.account(scopes, result, unifiedCostMicros(canonical, upstreamModel, result, g.cfg.Pricing)) - if result.estimated { - g.logf("%s: usage for model %q logged as estimated (%d prompt tokens derived from request body size, not the provider's reported usage)", logPrefix, canonical, result.prompt) - } - // A miss stores the response after everything above has already run — - // accounting must never be skipped or delayed waiting on a cache - // write. Only a genuine upstream 200 is stored (spec §2's "on 200 - // non-stream, tee and SET"): a non-2xx status never reaches here as - // capture.status (forwardJSON/translate error paths return - // *providerHTTPError/*translateError instead of writing through - // respWriter, so callErr is non-nil and capture.status stays 0), and - // callErr == nil is checked directly regardless. capture.oversize - // excludes a response cacheCaptureWriter stopped buffering past - // maxBodyBytes: store's own maxBodyBytes check would reject it too, - // but only after being handed a silently truncated body — skip the - // call outright instead of ever constructing a corrupt cache entry. - if cacheable && callErr == nil && capture.status == http.StatusOK && !capture.oversize { - g.cache.store(cacheKeyStr, capture.status, capture.contentType, capture.buf.Bytes(), effectiveTTL(g.cache, grp)) - } + lastErr = callErr + lastProviderName = providerName - if callErr != nil { - g.handleAdapterErrorEnvelope(sw, callErr, adapter.name(), logPrefix, envelope, writeUpstream) - return + hasNext := i < len(candidates)-1 && failoverEligible(callErr) + if hasNext && isProviderNotFoundError(callErr) { + // Loud, non-suppressed log line (brief: "Failover after a 404 + // must log loudly ... a 404 usually means a real + // configuration mistake, and silently succeeding elsewhere + // would hide it") — deliberately g.errorf, not g.logf, and + // deliberately its own line rather than folded into the + // generic "failing over" line above the next iteration + // already prints, and deliberately NOT gated by + // failoverLogGate (operator ruling: this one must always + // log). + g.errorf("%s: provider %q returned 404 for model %q; failing over to %q — this usually means a real provider/model configuration mistake, not a transient outage", logPrefix, providerName, requestedModel, candidates[i+1].providerName) + } + if !hasNext { + g.handleAdapterErrorEnvelope(sw, callErr, providerName, logPrefix, envelope, writeUpstream) + return + } + // Falls through to the next candidate. } } diff --git a/tools/yaegi-check/main.go b/tools/yaegi-check/main.go index af663a2..935046e 100644 --- a/tools/yaegi-check/main.go +++ b/tools/yaegi-check/main.go @@ -346,7 +346,37 @@ func run() error { })) defer brkUpstream.Close() - attemptAccountingOverride := `{"breaker":{"failureThreshold":2,"openDuration":"3s","maxOpenDuration":"6s"},` + + // failoverAUpstream/failoverBUpstream back exerciseFailover (feat/ + // failover, below): two providers configured with the IDENTICAL bare + // model id (failoverModelID) — the exact shape registry.go's bareWinner + // collision handles, and the shape production logs show for whisper-1/ + // tts-1/tts-1-hd/the embeddings models across openai/openai-audio/ + // copilot. A always answers 500; B always answers 200. This proves the + // failover candidate loop (runMeteredCall, routes_unified.go) falls + // through and serves correctly under the REAL interpreter, not merely + // compiled — this package's own doc comment is exactly why that + // distinction matters on this codebase. + failoverAUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":{"message":"failover-a is down","type":"server_error"}}`)) + })) + defer failoverAUpstream.Close() + failoverBHits := new(int64) + failoverBUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(failoverBHits, 1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"id":"c-failover","object":"chat.completion","model":"` + failoverModelID + `","choices":[{"index":0,"message":{"role":"assistant","content":"served by failover-b"},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1}}`)) + })) + defer failoverBUpstream.Close() + + // failover.enabled must be set explicitly here: FailoverConfig.Enabled + // defaults to false (coordinator ruling — a version upgrade with no + // config change must preserve prior behavior), so without this the + // exerciseFailover probe below would find failover-a's 500 returned + // to the client unchanged, never reaching failover-b at all. + attemptAccountingOverride := `{"failover":{"enabled":true},"breaker":{"failureThreshold":2,"openDuration":"3s","maxOpenDuration":"6s"},` + `"providers":{"openai":{"type":"openai","baseUrl":"` + upstream.URL + `","apiKey":"sk-up","models":["` + testDataWantModel + `","` + builtinLookupModelID + `"]},` + `"brk":{"type":"openai","baseUrl":"` + brkUpstream.URL + `","apiKey":"sk-up","discovery":true,"discoveryInterval":"1ms"},"` + slowProviderName + `":{"type":"openai","baseUrl":"` + slowUpstream.URL + `","apiKey":"sk-up","models":["` + slowProviderModel + `"]},` + @@ -354,7 +384,9 @@ func run() error { // (exerciseMessagesRoute, below): a real anthropic-type provider, // interpreted end to end through // callAnthropicMessagesPassthrough. - `"anthropic":{"type":"anthropic","baseUrl":"` + anthropicProbeUpstream.URL + `","apiKey":"sk-anth","models":["claude-test"]}},` + // #nosec G101 -- test fixture literal, not a real credential + `"anthropic":{"type":"anthropic","baseUrl":"` + anthropicProbeUpstream.URL + `","apiKey":"sk-anth","models":["claude-test"]},` + // #nosec G101 -- test fixture literal, not a real credential + `"failover-a":{"type":"openai","baseUrl":"` + failoverAUpstream.URL + `","apiKey":"sk-up","models":["` + failoverModelID + `"]},` + + `"failover-b":{"type":"openai","baseUrl":"` + failoverBUpstream.URL + `","apiKey":"sk-up","models":["` + failoverModelID + `"]}},` + // modelMeta (feature v0.23): a config-override entry for // testDataWantModel, so exerciseHandler's GET /v1/models // assertion below proves resolveModelMeta's config-override @@ -405,7 +437,7 @@ func run() error { return fmt.Errorf("%s.New's returned value does not implement http.Handler", pkgName) } - if err := exerciseHandler(handler, builtinLookupContextTokens); err != nil { + if err := exerciseHandler(handler, builtinLookupContextTokens, failoverBHits); err != nil { return err } // exerciseBreaker (feat/provider-health, adversarial-review round 2): @@ -429,6 +461,11 @@ func run() error { // the config-override path testDataWantModel already exercises. const builtinLookupModelID = "gpt-4o" +// failoverModelID is the bare model id both "failover-a" and "failover-b" +// (run(), above) are configured with — the run()-owned upstream servers +// exerciseFailover (below) drives. +const failoverModelID = "failover-shared" + // mcpProbeServerName is the federated MCP server run() configures against // mcpProbeUpstream, and mcpProbeToolName is a tool id carrying its // "_" prefix so resolveFederatedTool routes POST /mcp's @@ -634,8 +671,8 @@ func exerciseBreaker(handler http.Handler, hits, healthy *int64) error { // for real under Yaegi, not merely that the package's imports resolve. // builtinContextTokens is builtinLookupModelID's own real, extracted // builtinModelMetaTable value (SHOULD-6) — see that constant's own doc -// comment. -func exerciseHandler(handler http.Handler, builtinContextTokens int) error { +// comment. failoverBHits backs exerciseFailover's own call, below. +func exerciseHandler(handler http.Handler, builtinContextTokens int, failoverBHits *int64) error { authedReq := httptest.NewRequest(http.MethodGet, "/v1/models", nil) authedReq.Header.Set("Authorization", "Bearer "+testDataUserAPIKey) authedRec := httptest.NewRecorder() @@ -702,6 +739,10 @@ func exerciseHandler(handler http.Handler, builtinContextTokens int) error { return err } + if err := exerciseFailover(handler, failoverBHits); err != nil { + return err + } + if err := exerciseFederatedTooLarge(handler); err != nil { return err } @@ -1013,6 +1054,38 @@ func exerciseFederatedTooLarge(handler http.Handler) error { return nil } +// exerciseFailover drives one real POST /v1/chat/completions against the +// interpreted handler for failoverModelID — a bare model id both +// "failover-a" (always 500s) and "failover-b" (always 200s) are +// configured with — proving the failover candidate loop (runMeteredCall, +// routes_unified.go/failover.go) falls through a genuine provider +// failure and serves the second provider's response under the REAL +// interpreter. Compiled tests already prove this (failover_test.go); this +// harness exists because compiled tests have repeatedly passed on this +// codebase while the interpreted shape failed (this package's own doc +// comment). +func exerciseFailover(handler http.Handler, bHits *int64) error { + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader( + `{"model":"`+failoverModelID+`","messages":[{"role":"user","content":"hi"}]}`, + )) + req.Header.Set("Authorization", "Bearer "+testDataUserAPIKey) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + return fmt.Errorf("POST /v1/chat/completions (feat/failover harness): status = %d, want 200 (failover-a's 500 must fall through to failover-b), body=%s", rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "served by failover-b") { + return fmt.Errorf("POST /v1/chat/completions (feat/failover harness): body does not contain failover-b's own content, want the SECOND provider's response: %s", rec.Body.String()) + } + if atomic.LoadInt64(bHits) != 1 { + return fmt.Errorf("feat/failover harness: failover-b upstream hit count = %d, want exactly 1", atomic.LoadInt64(bHits)) + } + fmt.Println("yaegi-check: failover fell through from failover-a to failover-b and served its response") + return nil +} + func exerciseAttemptAccounting(handler http.Handler) error { chatReq := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader( `{"model":"`+testDataWantModel+`","messages":[{"role":"user","content":"hi"}]}`,