From 0d80812b7beb604594bb01fe6a383d49bbc44732 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Fri, 10 Jul 2026 19:47:44 +0800 Subject: [PATCH 1/2] =?UTF-8?q?feat(routing):=20fallback=5Fon=5Fstatuses?= =?UTF-8?q?=20=E2=80=94=20opt=20selected=20upstream=20statuses=20into=20re?= =?UTF-8?q?try/failover?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some providers use non-429 4xx codes for transient conditions (model overload, queue full, quota exhaustion). The gateway's default treats any non-429 4xx as the caller's error and relays it, so a model group fails a request its other targets could have served (AISIX-Cloud#1012). Routing models gain an explicit opt-in list: routing: strategy: failover targets: [...] fallback_on_statuses: [408, 409, 422] A listed status becomes retryable in is_retryable() — the single predicate every dispatch loop consults — so it participates in both same-target retries and failover, exactly like retry_on_429 does for 429. Unset means unchanged behavior. 5xx codes are already retryable; listing them is a no-op. The list never affects non-status failures (customer-fixable config/credentials stay terminal). Schema constrains entries to 400-599. Threaded through all six dispatch call sites (chat streaming + non-streaming, messages, responses, count_tokens). Per-attempt telemetry already records each attempt's upstream status and kind (initial/retry/fallback), so a rule-triggered failover is attributable from existing fields. Control-plane exposure (cp-admin schema, validation, projection, dashboard) ships as the paired AISIX-Cloud PR; this changes the DP first so the CP projection lands against a DP that accepts the field (deny_unknown_fields). Ref api7/AISIX-Cloud#1012 --- crates/aisix-core/src/models/routing.rs | 12 + crates/aisix-proxy/src/chat.rs | 18 +- crates/aisix-proxy/src/count_tokens.rs | 8 +- crates/aisix-proxy/src/messages.rs | 8 +- crates/aisix-proxy/src/responses.rs | 8 +- crates/aisix-proxy/src/routing.rs | 98 +++++++- schemas/resources/model.schema.json | 10 + schemas/resources/routing.schema.json | 13 + .../cases/fallback-on-statuses-e2e.test.ts | 235 ++++++++++++++++++ 9 files changed, 393 insertions(+), 17 deletions(-) create mode 100644 tests/e2e/src/cases/fallback-on-statuses-e2e.test.ts diff --git a/crates/aisix-core/src/models/routing.rs b/crates/aisix-core/src/models/routing.rs index 0122e9e1..ebb4a6d0 100644 --- a/crates/aisix-core/src/models/routing.rs +++ b/crates/aisix-core/src/models/routing.rs @@ -170,6 +170,10 @@ pub struct Routing { /// Whether upstream 429 participates in retries and failover. #[serde(default, skip_serializing_if = "Option::is_none")] pub retry_on_429: Option, + /// Additional upstream HTTP status codes that participate in retries and failover. By default a non-429 4xx response is treated as a caller error and returned as-is; providers that use 4xx codes for transient conditions (model overload, queue full, quota exhaustion) can be listed here, for example `[408, 409]`. 5xx codes are already retryable, so listing them changes nothing. Authentication (`401`/`403`) and validation (`400`) codes should only be listed when the provider is known to use them for transient failures. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[schemars(inner(range(min = 400, max = 599)))] + pub fallback_on_statuses: Option>, /// Policy to apply when every target is unavailable because of runtime health or cooldown state. #[serde(default, skip_serializing_if = "Option::is_none")] pub when_all_unavailable: Option, @@ -205,6 +209,12 @@ impl Routing { self.retry_on_429.unwrap_or(false) } + /// Configured status codes that opt into retry/failover; empty when + /// unset (the default behavior). + pub fn fallback_on_statuses_or_default(&self) -> &[u16] { + self.fallback_on_statuses.as_deref().unwrap_or(&[]) + } + pub fn when_all_unavailable_or_default(&self) -> WhenAllUnavailablePolicy { self.when_all_unavailable.unwrap_or_default() } @@ -258,6 +268,7 @@ mod tests { retries: Some(0), max_fallbacks: Some(0), retry_on_429: None, + fallback_on_statuses: None, when_all_unavailable: None, sticky: None, }; @@ -272,6 +283,7 @@ mod tests { retries: None, max_fallbacks: Some(99), retry_on_429: None, + fallback_on_statuses: None, when_all_unavailable: None, sticky: None, }; diff --git a/crates/aisix-proxy/src/chat.rs b/crates/aisix-proxy/src/chat.rs index ee7195f5..3e6bf11a 100644 --- a/crates/aisix-proxy/src/chat.rs +++ b/crates/aisix-proxy/src/chat.rs @@ -1104,6 +1104,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); + let fallback_statuses: Vec = virtual_entry + .value + .routing + .as_ref() + .map(|r| r.fallback_on_statuses_or_default().to_vec()) + .unwrap_or_default(); let is_routing_request = virtual_entry.value.routing.is_some() || virtual_entry.value.is_semantic(); let mut stream_routing = RoutingTelemetry::default(); @@ -1241,7 +1247,7 @@ async fn dispatch( error_message: attempt_error_message(&err), latency_ms, }); - let retryable = is_retryable(&err, retry_on_429); + let retryable = is_retryable(&err, retry_on_429, &fallback_statuses); tracing::warn!( target_model = %model.display_name, error = %err, @@ -1819,6 +1825,12 @@ async fn dispatch( .as_ref() .map(|routing| routing.retry_on_429_or_default()) .unwrap_or(false); + let fallback_statuses: Vec = virtual_entry + .value + .routing + .as_ref() + .map(|routing| routing.fallback_on_statuses_or_default().to_vec()) + .unwrap_or_default(); let is_routing_request = virtual_entry.value.routing.is_some() || virtual_entry.value.is_semantic(); let mut routing = RoutingTelemetry::default(); @@ -1924,7 +1936,7 @@ async fn dispatch( error_message: attempt_error_message(&err), latency_ms: attempt_latency_ms, }); - let retryable = is_retryable(&err, retry_on_429); + let retryable = is_retryable(&err, retry_on_429, &fallback_statuses); tracing::warn!( target_model = %model.display_name, target_attempt = attempt_idx + 1, @@ -1971,7 +1983,7 @@ async fn dispatch( break; } if let Some(err) = last_err.as_ref() { - if !is_retryable(err, retry_on_429) { + if !is_retryable(err, retry_on_429, &fallback_statuses) { break; } } diff --git a/crates/aisix-proxy/src/count_tokens.rs b/crates/aisix-proxy/src/count_tokens.rs index 07a74b49..b3bfd178 100644 --- a/crates/aisix-proxy/src/count_tokens.rs +++ b/crates/aisix-proxy/src/count_tokens.rs @@ -193,6 +193,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); + let fallback_statuses: Vec = model_entry + .value + .routing + .as_ref() + .map(|r| r.fallback_on_statuses_or_default().to_vec()) + .unwrap_or_default(); let mut last_err: Option = None; let mut any_anthropic = false; @@ -218,7 +224,7 @@ async fn dispatch( Err(e) => { let retryable = matches!( &e, - ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429) + ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, &fallback_statuses) ); last_err = Some(e); if !retryable { diff --git a/crates/aisix-proxy/src/messages.rs b/crates/aisix-proxy/src/messages.rs index afd9d776..3f47753b 100644 --- a/crates/aisix-proxy/src/messages.rs +++ b/crates/aisix-proxy/src/messages.rs @@ -623,6 +623,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); + let fallback_statuses: Vec = model_entry + .value + .routing + .as_ref() + .map(|r| r.fallback_on_statuses_or_default().to_vec()) + .unwrap_or_default(); // Routing target names only matter on the telemetry for a real Model // Group; a direct model leaves `attempt_model` empty (its `model_id` // already identifies it), matching chat.rs. @@ -734,7 +740,7 @@ async fn dispatch( Err(e) => { let retryable = matches!( &e, - ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429) + ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, &fallback_statuses) ); let (error_class, error_message) = attempt_error_from_proxy(&e); routing.attempts.push(AttemptRecord { diff --git a/crates/aisix-proxy/src/responses.rs b/crates/aisix-proxy/src/responses.rs index d0d020d0..df403a71 100644 --- a/crates/aisix-proxy/src/responses.rs +++ b/crates/aisix-proxy/src/responses.rs @@ -494,6 +494,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); + let fallback_statuses: Vec = model_entry + .value + .routing + .as_ref() + .map(|r| r.fallback_on_statuses_or_default().to_vec()) + .unwrap_or_default(); let is_routing_request = model_entry.value.routing.is_some(); let mut routing = RoutingTelemetry::default(); // `routing.retries` — same-target retries (with backoff) before failing @@ -626,7 +632,7 @@ async fn dispatch( Err(e) => { let retryable = matches!( &e, - ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429) + ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, &fallback_statuses) ); let (error_class, error_message) = attempt_error_from_proxy(&e); routing.attempts.push(AttemptRecord { diff --git a/crates/aisix-proxy/src/routing.rs b/crates/aisix-proxy/src/routing.rs index 3140d579..9e84cd28 100644 --- a/crates/aisix-proxy/src/routing.rs +++ b/crates/aisix-proxy/src/routing.rs @@ -47,9 +47,18 @@ const FALLBACK_ALL_UNHEALTHY_RETRY_AFTER: Duration = Duration::from_secs(30); /// as retryable. Non-429 4xx is the caller's mistake — retrying won't /// help and may amplify damage. Everything else (5xx, timeout, /// transport, decode, config, stream abort) gets the retry/failover path. -pub fn is_retryable(err: &BridgeError, retry_on_429: bool) -> bool { +/// +/// `fallback_on_statuses` (AISIX-Cloud#1012) is the routing model's +/// explicit opt-in list for providers that use 4xx codes for transient +/// conditions (overload, queue full, quota): a status in the list is +/// retryable regardless of the default classification. Empty by default, +/// which preserves the historical behavior exactly. +pub fn is_retryable(err: &BridgeError, retry_on_429: bool, fallback_on_statuses: &[u16]) -> bool { match err { BridgeError::UpstreamStatus { status, .. } => { + if fallback_on_statuses.contains(status) { + return true; + } if *status == 429 { return retry_on_429; } @@ -524,6 +533,7 @@ mod tests { retries: None, max_fallbacks, retry_on_429: None, + fallback_on_statuses: None, when_all_unavailable: None, sticky: None, } @@ -917,32 +927,98 @@ mod tests { fn is_retryable_distinguishes_4xx_from_other_failures() { assert!(!is_retryable( &BridgeError::upstream_status(400, "bad request"), - false + false, + &[] )); assert!(!is_retryable( &BridgeError::upstream_status(429, "rate limited"), - false + false, + &[] )); assert!(is_retryable( &BridgeError::upstream_status(429, "rate limited"), - true + true, + &[] )); assert!(is_retryable( &BridgeError::upstream_status(502, "bad gateway"), - false + false, + &[] + )); + assert!(is_retryable( + &BridgeError::Timeout { elapsed_ms: 1 }, + false, + &[] + )); + assert!(is_retryable( + &BridgeError::Transport("conn".into()), + false, + &[] )); - assert!(is_retryable(&BridgeError::Timeout { elapsed_ms: 1 }, false)); - assert!(is_retryable(&BridgeError::Transport("conn".into()), false)); assert!(is_retryable( &BridgeError::UpstreamDecode("x".into()), - false + false, + &[] + )); + assert!(is_retryable( + &BridgeError::Config("bad key".into()), + false, + &[] )); - assert!(is_retryable(&BridgeError::Config("bad key".into()), false)); - assert!(is_retryable(&BridgeError::StreamAborted, false)); + assert!(is_retryable(&BridgeError::StreamAborted, false, &[])); // #367: customer-fixable config is a 4xx — not retryable. assert!(!is_retryable( &BridgeError::InvalidUpstreamConfig("no api_base".into()), - false + false, + &[] + )); + } + + /// AISIX-Cloud#1012: `fallback_on_statuses` opts specific upstream + /// status codes into retry/failover. The list is additive — codes not + /// listed keep the default classification — and it never resurrects + /// non-status failures (customer-fixable config stays terminal). + #[test] + fn fallback_on_statuses_opts_specific_codes_into_retry() { + // A listed 4xx becomes retryable. + assert!(is_retryable( + &BridgeError::upstream_status(408, "request timeout"), + false, + &[408, 409] + )); + assert!(is_retryable( + &BridgeError::upstream_status(409, "conflict"), + false, + &[408, 409] + )); + // Codes NOT in the list keep the default: terminal. + assert!(!is_retryable( + &BridgeError::upstream_status(422, "unprocessable"), + false, + &[408, 409] + )); + assert!(!is_retryable( + &BridgeError::upstream_status(400, "bad request"), + false, + &[408, 409] + )); + // 429 in the list works without retry_on_429. + assert!(is_retryable( + &BridgeError::upstream_status(429, "rate limited"), + false, + &[429] + )); + // 5xx stays retryable whether or not listed. + assert!(is_retryable( + &BridgeError::upstream_status(503, "unavailable"), + false, + &[408] + )); + // The list is status-scoped: it never affects non-status errors. + assert!(!is_retryable( + &BridgeError::InvalidUpstreamConfig("no api_base".into()), + false, + &[400, 401, 403] )); } diff --git a/schemas/resources/model.schema.json b/schemas/resources/model.schema.json index 79b24954..6e1258c6 100644 --- a/schemas/resources/model.schema.json +++ b/schemas/resources/model.schema.json @@ -360,6 +360,16 @@ "Routing": { "additionalProperties": false, "properties": { + "fallback_on_statuses": { + "description": "Additional upstream HTTP status codes that participate in retries and failover. By default a non-429 4xx response is treated as a caller error and returned as-is; providers that use 4xx codes for transient conditions (model overload, queue full, quota exhaustion) can be listed here, for example `[408, 409]`. 5xx codes are already retryable, so listing them changes nothing. Authentication (`401`/`403`) and validation (`400`) codes should only be listed when the provider is known to use them for transient failures.", + "items": { + "format": "uint16", + "maximum": 599.0, + "minimum": 400.0, + "type": "integer" + }, + "type": "array" + }, "max_fallbacks": { "description": "Max number of later targets to attempt after the initial target fails permanently. When omitted, all later targets may be attempted.", "format": "uint32", diff --git a/schemas/resources/routing.schema.json b/schemas/resources/routing.schema.json index 44139231..663d5b6f 100644 --- a/schemas/resources/routing.schema.json +++ b/schemas/resources/routing.schema.json @@ -6,6 +6,19 @@ "targets" ], "properties": { + "fallback_on_statuses": { + "description": "Additional upstream HTTP status codes that participate in retries and failover. By default a non-429 4xx response is treated as a caller error and returned as-is; providers that use 4xx codes for transient conditions (model overload, queue full, quota exhaustion) can be listed here, for example `[408, 409]`. 5xx codes are already retryable, so listing them changes nothing. Authentication (`401`/`403`) and validation (`400`) codes should only be listed when the provider is known to use them for transient failures.", + "type": [ + "array", + "null" + ], + "items": { + "type": "integer", + "format": "uint16", + "maximum": 599.0, + "minimum": 400.0 + } + }, "max_fallbacks": { "description": "Max number of later targets to attempt after the initial target fails permanently. When omitted, all later targets may be attempted.", "type": [ diff --git a/tests/e2e/src/cases/fallback-on-statuses-e2e.test.ts b/tests/e2e/src/cases/fallback-on-statuses-e2e.test.ts new file mode 100644 index 00000000..00ebdd03 --- /dev/null +++ b/tests/e2e/src/cases/fallback-on-statuses-e2e.test.ts @@ -0,0 +1,235 @@ +import { createHash } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + AdminClient, + EtcdClient, + spawnApp, + startOpenAiUpstream, + waitConfigPropagation, + type OpenAiUpstream, + type SpawnedApp, +} from "../harness/index.js"; + +// AISIX-Cloud#1012: some providers use non-429 4xx codes for transient +// conditions (model overload, queue full, quota). By default the gateway +// treats a non-429 4xx as the caller's error and returns it as-is; a +// routing model can now opt SPECIFIC codes into retry/failover via +// `fallback_on_statuses`. This drives two identical two-target groups — +// first target always answers 422, second target healthy — and pins: +// +// 1. default group: the 422 is relayed to the caller (behavior +// unchanged; the second target is never consulted) +// 2. `fallback_on_statuses: [422]` group: the request fails over and +// succeeds on the second target +// 3. codes NOT in the list keep the default (a 400 from the same +// configured group still relays) + +const CALLER_PLAINTEXT = "sk-fos-caller"; +const CALLER_KEY_HASH = createHash("sha256").update(CALLER_PLAINTEXT).digest("hex"); + +describe("fallback_on_statuses e2e: opt-in 4xx failover (#1012)", () => { + let app: SpawnedApp | undefined; + let overloaded: OpenAiUpstream | undefined; + let badRequest: OpenAiUpstream | undefined; + let healthy: OpenAiUpstream | undefined; + let etcdReachable = false; + + beforeAll(async () => { + etcdReachable = await new EtcdClient().ping(); + if (!etcdReachable) return; + + // "Overloaded" provider: answers every call with 422 (the shape some + // model services use for queue-full/overload conditions). + overloaded = await startOpenAiUpstream({ + status: 422, + errorBody: { error: { message: "model overloaded, try later", type: "overloaded" } }, + }); + // Provider that rejects with a plain 400 (a genuine caller error). + badRequest = await startOpenAiUpstream({ + status: 400, + errorBody: { error: { message: "bad request", type: "invalid_request_error" } }, + }); + healthy = await startOpenAiUpstream({ + nonStreamBody: { + id: "cmpl-fos", + object: "chat.completion", + created: Math.floor(Date.now() / 1000), + model: "gpt-4o-mini", + choices: [ + { + index: 0, + message: { role: "assistant", content: "served by the backup" }, + finish_reason: "stop", + }, + ], + usage: { prompt_tokens: 4, completion_tokens: 4, total_tokens: 8 }, + }, + }); + + app = await spawnApp(); + const admin = new AdminClient(app.adminUrl, app.adminKey); + + const overloadedPk = await admin.createProviderKey({ + display_name: "fos-overloaded-pk", + secret: "sk-mock", + api_base: `${overloaded.baseUrl}/v1`, + }); + await admin.createModel({ + display_name: "fos-overloaded", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: overloadedPk.id, + cooldown: { enabled: false }, + }); + const badPk = await admin.createProviderKey({ + display_name: "fos-bad-pk", + secret: "sk-mock", + api_base: `${badRequest.baseUrl}/v1`, + }); + await admin.createModel({ + display_name: "fos-bad", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: badPk.id, + cooldown: { enabled: false }, + }); + const healthyPk = await admin.createProviderKey({ + display_name: "fos-healthy-pk", + secret: "sk-mock", + api_base: `${healthy.baseUrl}/v1`, + }); + await admin.createModel({ + display_name: "fos-healthy", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: healthyPk.id, + }); + + // Same two targets, with and without the opt-in. + await admin.createModel({ + display_name: "fos-default-group", + routing: { + strategy: "failover", + targets: [{ model: "fos-overloaded" }, { model: "fos-healthy" }], + }, + }); + await admin.createModel({ + display_name: "fos-configured-group", + routing: { + strategy: "failover", + targets: [{ model: "fos-overloaded" }, { model: "fos-healthy" }], + fallback_on_statuses: [422], + }, + }); + // Configured group whose FIRST target 400s — 400 is not in the list, + // so it must still relay. + await admin.createModel({ + display_name: "fos-unlisted-group", + routing: { + strategy: "failover", + targets: [{ model: "fos-bad" }, { model: "fos-healthy" }], + fallback_on_statuses: [422], + }, + }); + + await admin.createApiKey({ + key_hash: CALLER_KEY_HASH, + allowed_models: [ + "fos-default-group", + "fos-configured-group", + "fos-unlisted-group", + "fos-healthy", + ], + }); + + await waitConfigPropagation(async () => { + const r = await chat("fos-healthy"); + return r.status === 200; + }); + }); + + afterAll(async () => { + await app?.exit(); + await overloaded?.close(); + await badRequest?.close(); + await healthy?.close(); + }); + + async function chat(model: string): Promise { + const res = await fetch(`${app!.proxyUrl}/v1/chat/completions`, { + method: "POST", + headers: { + authorization: `Bearer ${CALLER_PLAINTEXT}`, + "content-type": "application/json", + }, + body: JSON.stringify({ + model, + messages: [{ role: "user", content: "route me" }], + }), + }); + return res; + } + + test("default: a non-429 4xx from the first target is relayed, no failover", async (ctx) => { + if (!etcdReachable || !app || !overloaded || !healthy) { + ctx.skip(); + return; + } + // Gate on the group being resolvable (not 404) before asserting. + await waitConfigPropagation(async () => { + const probe = await chat("fos-default-group"); + await probe.text(); + return probe.status !== 404; + }); + + const healthyBefore = healthy.receivedRequests.length; + const res = await chat("fos-default-group"); + const body = await res.text(); + expect(res.status).toBe(422); + expect(body).toContain("model overloaded"); + // The healthy backup was never consulted — 4xx stays terminal. + expect(healthy.receivedRequests.length).toBe(healthyBefore); + }); + + test("fallback_on_statuses [422]: the same 422 fails over and succeeds", async (ctx) => { + if (!etcdReachable || !app || !overloaded || !healthy) { + ctx.skip(); + return; + } + await waitConfigPropagation(async () => { + const probe = await chat("fos-configured-group"); + await probe.text(); + return probe.status !== 404; + }); + + const overloadedBefore = overloaded.receivedRequests.length; + const healthyBefore = healthy.receivedRequests.length; + const res = await chat("fos-configured-group"); + const body = JSON.parse(await res.text()) as { + choices: Array<{ message: { content: string } }>; + }; + expect(res.status).toBe(200); + expect(body.choices[0]!.message.content).toContain("served by the backup"); + // First target was tried (and answered 422), then the backup won. + expect(overloaded.receivedRequests.length).toBeGreaterThan(overloadedBefore); + expect(healthy.receivedRequests.length).toBe(healthyBefore + 1); + }); + + test("codes not in the list keep the default: a 400 still relays", async (ctx) => { + if (!etcdReachable || !app || !badRequest || !healthy) { + ctx.skip(); + return; + } + await waitConfigPropagation(async () => { + const probe = await chat("fos-unlisted-group"); + await probe.text(); + return probe.status !== 404; + }); + + const healthyBefore = healthy.receivedRequests.length; + const res = await chat("fos-unlisted-group"); + await res.text(); + expect(res.status).toBe(400); + expect(healthy.receivedRequests.length).toBe(healthyBefore); + }); +}); From 5c0dfcf64ebd415af4473530e3ddebd3e8b4aed0 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Fri, 10 Jul 2026 20:46:33 +0800 Subject: [PATCH 2/2] refactor(routing): borrow the fallback-status list; pin the 400-599 range at the validator Audit follow-ups: the six dispatch sites now borrow the configured slice instead of cloning per request, and a schema-validation test pins that out-of-range entries are rejected while in-range lists pass. --- crates/aisix-core/src/models/schema.rs | 23 +++++++++++++++++++++++ crates/aisix-proxy/src/chat.rs | 18 +++++++++--------- crates/aisix-proxy/src/count_tokens.rs | 8 ++++---- crates/aisix-proxy/src/messages.rs | 8 ++++---- crates/aisix-proxy/src/responses.rs | 8 ++++---- 5 files changed, 44 insertions(+), 21 deletions(-) diff --git a/crates/aisix-core/src/models/schema.rs b/crates/aisix-core/src/models/schema.rs index c5ab6078..3b59c5ab 100644 --- a/crates/aisix-core/src/models/schema.rs +++ b/crates/aisix-core/src/models/schema.rs @@ -1433,6 +1433,29 @@ mod tests { assert!(validate_model(&v).is_err()); } + #[test] + fn routing_fallback_on_statuses_range_is_enforced() { + // AISIX-Cloud#1012: entries outside 400-599 are rejected by the + // same committed-schema validation the admin API and etcd watch + // paths share; an in-range list passes. + let bad = json!({ + "display_name": "router-fos", + "routing": { + "targets": [{"model": "x"}], + "fallback_on_statuses": [300] + } + }); + assert!(validate_model(&bad).is_err()); + let good = json!({ + "display_name": "router-fos", + "routing": { + "targets": [{"model": "x"}], + "fallback_on_statuses": [408, 422] + } + }); + validate_model(&good).unwrap(); + } + #[test] fn cooldown_rejects_invalid_status_code() { let v = json!({ diff --git a/crates/aisix-proxy/src/chat.rs b/crates/aisix-proxy/src/chat.rs index 3e6bf11a..7deb7de0 100644 --- a/crates/aisix-proxy/src/chat.rs +++ b/crates/aisix-proxy/src/chat.rs @@ -1104,12 +1104,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); - let fallback_statuses: Vec = virtual_entry + let fallback_statuses: &[u16] = virtual_entry .value .routing .as_ref() - .map(|r| r.fallback_on_statuses_or_default().to_vec()) - .unwrap_or_default(); + .map(|r| r.fallback_on_statuses_or_default()) + .unwrap_or(&[]); let is_routing_request = virtual_entry.value.routing.is_some() || virtual_entry.value.is_semantic(); let mut stream_routing = RoutingTelemetry::default(); @@ -1247,7 +1247,7 @@ async fn dispatch( error_message: attempt_error_message(&err), latency_ms, }); - let retryable = is_retryable(&err, retry_on_429, &fallback_statuses); + let retryable = is_retryable(&err, retry_on_429, fallback_statuses); tracing::warn!( target_model = %model.display_name, error = %err, @@ -1825,12 +1825,12 @@ async fn dispatch( .as_ref() .map(|routing| routing.retry_on_429_or_default()) .unwrap_or(false); - let fallback_statuses: Vec = virtual_entry + let fallback_statuses: &[u16] = virtual_entry .value .routing .as_ref() - .map(|routing| routing.fallback_on_statuses_or_default().to_vec()) - .unwrap_or_default(); + .map(|routing| routing.fallback_on_statuses_or_default()) + .unwrap_or(&[]); let is_routing_request = virtual_entry.value.routing.is_some() || virtual_entry.value.is_semantic(); let mut routing = RoutingTelemetry::default(); @@ -1936,7 +1936,7 @@ async fn dispatch( error_message: attempt_error_message(&err), latency_ms: attempt_latency_ms, }); - let retryable = is_retryable(&err, retry_on_429, &fallback_statuses); + let retryable = is_retryable(&err, retry_on_429, fallback_statuses); tracing::warn!( target_model = %model.display_name, target_attempt = attempt_idx + 1, @@ -1983,7 +1983,7 @@ async fn dispatch( break; } if let Some(err) = last_err.as_ref() { - if !is_retryable(err, retry_on_429, &fallback_statuses) { + if !is_retryable(err, retry_on_429, fallback_statuses) { break; } } diff --git a/crates/aisix-proxy/src/count_tokens.rs b/crates/aisix-proxy/src/count_tokens.rs index b3bfd178..ffeb82a5 100644 --- a/crates/aisix-proxy/src/count_tokens.rs +++ b/crates/aisix-proxy/src/count_tokens.rs @@ -193,12 +193,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); - let fallback_statuses: Vec = model_entry + let fallback_statuses: &[u16] = model_entry .value .routing .as_ref() - .map(|r| r.fallback_on_statuses_or_default().to_vec()) - .unwrap_or_default(); + .map(|r| r.fallback_on_statuses_or_default()) + .unwrap_or(&[]); let mut last_err: Option = None; let mut any_anthropic = false; @@ -224,7 +224,7 @@ async fn dispatch( Err(e) => { let retryable = matches!( &e, - ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, &fallback_statuses) + ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, fallback_statuses) ); last_err = Some(e); if !retryable { diff --git a/crates/aisix-proxy/src/messages.rs b/crates/aisix-proxy/src/messages.rs index 3f47753b..6affb026 100644 --- a/crates/aisix-proxy/src/messages.rs +++ b/crates/aisix-proxy/src/messages.rs @@ -623,12 +623,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); - let fallback_statuses: Vec = model_entry + let fallback_statuses: &[u16] = model_entry .value .routing .as_ref() - .map(|r| r.fallback_on_statuses_or_default().to_vec()) - .unwrap_or_default(); + .map(|r| r.fallback_on_statuses_or_default()) + .unwrap_or(&[]); // Routing target names only matter on the telemetry for a real Model // Group; a direct model leaves `attempt_model` empty (its `model_id` // already identifies it), matching chat.rs. @@ -740,7 +740,7 @@ async fn dispatch( Err(e) => { let retryable = matches!( &e, - ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, &fallback_statuses) + ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, fallback_statuses) ); let (error_class, error_message) = attempt_error_from_proxy(&e); routing.attempts.push(AttemptRecord { diff --git a/crates/aisix-proxy/src/responses.rs b/crates/aisix-proxy/src/responses.rs index df403a71..6e40d899 100644 --- a/crates/aisix-proxy/src/responses.rs +++ b/crates/aisix-proxy/src/responses.rs @@ -494,12 +494,12 @@ async fn dispatch( .as_ref() .map(|r| r.retry_on_429_or_default()) .unwrap_or(false); - let fallback_statuses: Vec = model_entry + let fallback_statuses: &[u16] = model_entry .value .routing .as_ref() - .map(|r| r.fallback_on_statuses_or_default().to_vec()) - .unwrap_or_default(); + .map(|r| r.fallback_on_statuses_or_default()) + .unwrap_or(&[]); let is_routing_request = model_entry.value.routing.is_some(); let mut routing = RoutingTelemetry::default(); // `routing.retries` — same-target retries (with backoff) before failing @@ -632,7 +632,7 @@ async fn dispatch( Err(e) => { let retryable = matches!( &e, - ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, &fallback_statuses) + ProxyError::Bridge(be) if crate::routing::is_retryable(be, retry_on_429, fallback_statuses) ); let (error_class, error_message) = attempt_error_from_proxy(&e); routing.attempts.push(AttemptRecord {