From 34b779881b7ad88ed43bf87a5cccce318a088d43 Mon Sep 17 00:00:00 2001 From: "Jakub A. W" Date: Tue, 28 Jul 2026 11:49:11 +0200 Subject: [PATCH] fix(failover): fail over on relayed upstream failures wearing 4xx statuses Aggregator-style providers can relay a transient failure of their own upstream as a client error: OpenCode Zen returns 400 "Upstream request failed" when its upstream breaks, so requests with configured failover targets still died with the raw 400. ShouldAttemptFailover now treats a message that blames an upstream ("upstream" plus failure phrasing) as failover-eligible regardless of the status code, mirroring the existing model-availability heuristic. The three inline fragment-scan loops are extracted into named fragment lists checked via one containsAny helper. Closes #605 Co-Authored-By: Claude Fable 5 --- docs/features/failover.mdx | 3 ++ internal/gateway/failover.go | 85 ++++++++++++++++++++----------- internal/gateway/failover_test.go | 10 ++++ 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/docs/features/failover.mdx b/docs/features/failover.mdx index 0fe0af3e6..1f9342db3 100644 --- a/docs/features/failover.mdx +++ b/docs/features/failover.mdx @@ -80,6 +80,9 @@ Failover is attempted only after the primary request returns: - `5xx` - `429` - model unavailable, unsupported, or not found style errors +- upstream failure messages relayed with a `4xx` status (aggregator providers + such as OpenCode Zen can report a transient failure of their own upstream as + `400 "Upstream request failed"`) It currently applies to translated `/v1/chat/completions`, `/v1/responses`, and `/v1/messages` requests, not `/v1/embeddings`. diff --git a/internal/gateway/failover.go b/internal/gateway/failover.go index c25e5311e..913c42227 100644 --- a/internal/gateway/failover.go +++ b/internal/gateway/failover.go @@ -230,6 +230,55 @@ func tryFailoverStream( return nil, "", "", "", "", lastErr } +// Message fragments that mark an error as failover-eligible when they appear +// alongside the anchor word checked in ShouldAttemptFailover. +var ( + // modelUnavailableFragments signal the model itself is gone or refused, + // regardless of the status code the provider chose. + modelUnavailableFragments = []string{ + "not found", + "does not exist", + "unsupported", + "unavailable", + "not available", + "deprecated", + "retired", + "disabled", + } + // upstreamFailureFragments signal an aggregator-style provider (OpenCode + // Zen, OpenRouter) relaying a transient failure of *its* upstream, e.g. + // OpenCode's 400 "Upstream request failed". These are server-side + // failures wearing a client-error status, so a failover target may still + // succeed. + upstreamFailureFragments = []string{ + "failed", + "error", + "unavailable", + "timed out", + "timeout", + } + // retiredModel404Fragments cover 404s with availability phrasing but no + // literal "model": providers report retired models this way. Plain + // endpoint 404s must not match — those are genuine routing misses. + retiredModel404Fragments = []string{ + "unsupported", + "unavailable", + "not available", + "deprecated", + "retired", + "disabled", + } +) + +func containsAny(message string, fragments []string) bool { + for _, fragment := range fragments { + if strings.Contains(message, fragment) { + return true + } + } + return false +} + // ShouldAttemptFailover reports whether err should trigger translated failover. func ShouldAttemptFailover(err error) bool { var gatewayErr *core.GatewayError @@ -252,36 +301,14 @@ func ShouldAttemptFailover(err error) bool { } message := strings.ToLower(strings.TrimSpace(gatewayErr.Message)) - if strings.Contains(message, "model") { - for _, fragment := range []string{ - "not found", - "does not exist", - "unsupported", - "unavailable", - "not available", - "deprecated", - "retired", - "disabled", - } { - if strings.Contains(message, fragment) { - return true - } - } + if strings.Contains(message, "model") && containsAny(message, modelUnavailableFragments) { + return true } - - if status == http.StatusNotFound { - for _, fragment := range []string{ - "unsupported", - "unavailable", - "not available", - "deprecated", - "retired", - "disabled", - } { - if strings.Contains(message, fragment) { - return true - } - } + if strings.Contains(message, "upstream") && containsAny(message, upstreamFailureFragments) { + return true + } + if status == http.StatusNotFound && containsAny(message, retiredModel404Fragments) { + return true } return false diff --git a/internal/gateway/failover_test.go b/internal/gateway/failover_test.go index e17e53952..53259daa8 100644 --- a/internal/gateway/failover_test.go +++ b/internal/gateway/failover_test.go @@ -196,6 +196,16 @@ func TestShouldAttemptFailover(t *testing.T) { {"route 404", http.StatusNotFound, "404 page not found", false}, {"unknown path 404", http.StatusNotFound, "no route for /v1/foo", false}, + // Aggregator providers relay transient failures of their own upstream + // as 4xx client errors; those must fall back (issue #605). + {"opencode upstream 400", http.StatusBadRequest, "Error from provider (Console Go): Upstream request failed", true}, + {"upstream timeout 400", http.StatusBadRequest, "upstream timed out", true}, + {"upstream unavailable 400", http.StatusBadRequest, "upstream provider is currently unavailable", true}, + + // Mentioning an upstream without failure phrasing is a genuine + // validation error and must NOT fall back. + {"upstream capability 400", http.StatusBadRequest, "parameter tools is not supported by the upstream provider", false}, + // A plain client error without availability phrasing is not retried. {"plain 400", http.StatusBadRequest, "invalid request", false}, }