diff --git a/crates/aisix-gateway/src/bridge.rs b/crates/aisix-gateway/src/bridge.rs index db5996c7..13725d67 100644 --- a/crates/aisix-gateway/src/bridge.rs +++ b/crates/aisix-gateway/src/bridge.rs @@ -23,6 +23,63 @@ use std::time::Duration; use crate::chat::{ChatChunk, ChatFormat, ChatResponse, EmbeddingRequest, EmbeddingResponse}; +/// Maximum number of bytes read from an upstream error response body +/// before attempting JSON envelope parse. Bounds memory and parser cost +/// when an upstream returns something pathological (an HTML error page +/// from a fronting WAF, or an unexpectedly large debug dump). +pub const MAX_UPSTREAM_ERROR_BODY_BYTES: usize = 64 * 1024; + +/// Maximum length of the human-readable `message` string carried inside +/// [`BridgeError::UpstreamStatus`]. The full body is parsed into +/// [`UpstreamErrorView`] when JSON-shaped; the truncated string is the +/// fallback shown to clients when parsing fails. +pub const MAX_UPSTREAM_ERROR_MESSAGE_BYTES: usize = 1024; + +/// Which wire format the upstream that produced this error speaks. The +/// envelope-rendering layer uses this together with [`UpstreamErrorView`] +/// to decide whether the upstream `kind` / `code` can be forwarded +/// verbatim or needs translation to the client's wire shape. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UpstreamWire { + /// OpenAI-compatible envelope: `{error:{message,type,code,param}}`. + OpenAI, + /// Anthropic envelope: `{type:"error",error:{type,message}}`. + Anthropic, + /// Azure OpenAI envelope: OpenAI-like with `error.inner_error.code` + /// quirks for content policy violations. + AzureOpenAI, + /// AWS Bedrock structured error from the strongly-typed SDK; `kind` + /// carries the AWS exception code (e.g. `"ThrottlingException"`). + Bedrock, + /// Vertex AI envelope: `{error:{code:int,message,status}}` where + /// `status` is the canonical gRPC code string. + Vertex, + /// Wire format unknown / not applicable (tests, synthesised errors, + /// the legacy convenience constructors). Renders as the generic + /// `upstream_error` envelope with no translation attempt. + Unknown, +} + +/// Structured view of an upstream error envelope, populated by each +/// bridge after best-effort parsing of its provider's known shape. +/// `None` everywhere means parsing failed (non-JSON body, malformed +/// JSON, or unfamiliar envelope shape); callers fall back to the +/// truncated raw message on [`BridgeError::UpstreamStatus::message`]. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct UpstreamErrorView { + /// Provider-native error-type token, unchanged from the upstream + /// envelope (e.g. Anthropic `"rate_limit_error"`, OpenAI + /// `"rate_limit_exceeded"`, Bedrock `"ThrottlingException"`). + pub kind: Option, + /// Human-readable upstream message, post-parse. + pub message: Option, + /// OpenAI envelope only. Other providers populate via the + /// translation table at render time. + pub code: Option, + /// OpenAI envelope only. + pub param: Option, +} + /// Context carried through the whole request lifecycle. /// /// The proxy layer fills this in after it has authenticated the request @@ -77,10 +134,21 @@ pub enum BridgeError { /// backoff hints. Bridges that cannot parse the header (or where /// the header is absent) leave this `None`; the cooldown layer /// falls back to its configured default in that case. + /// `message` is a best-effort human-readable string for logs and + /// the fallback envelope when [`parsed`] is `None`. When [`parsed`] + /// is `Some`, the envelope-rendering layer (`error_translate`) uses + /// the structured fields and [`wire`] to produce a client-shape + /// envelope; `message` is kept around for logs and as a + /// last-resort fallback if a parsed field is missing. #[error("upstream returned HTTP {status}: {message}")] UpstreamStatus { status: u16, message: String, + /// Boxed to keep [`BridgeError`] small enough that + /// `Result<_, ProxyError>` doesn't trip `clippy::result_large_err` + /// once the four optional envelope fields are added. + parsed: Option>, + wire: UpstreamWire, retry_after: Option, }, #[error("upstream returned an unparseable body: {0}")] @@ -94,18 +162,21 @@ pub enum BridgeError { } impl BridgeError { - /// Convenience constructor for upstream status errors when no - /// `Retry-After` is available. Keeps existing call sites readable. + /// Convenience constructor for synthesised upstream errors (tests, + /// cooldown fixtures) where no real upstream envelope is involved. + /// Sets [`UpstreamWire::Unknown`] and `parsed: None`. pub fn upstream_status(status: u16, message: impl Into) -> Self { Self::UpstreamStatus { status, message: message.into(), + parsed: None, + wire: UpstreamWire::Unknown, retry_after: None, } } - /// Convenience constructor for upstream status errors that carry - /// a parsed `Retry-After` hint. + /// Convenience constructor for synthesised upstream errors that + /// carry a parsed `Retry-After` hint. See [`upstream_status`]. pub fn upstream_status_with_retry_after( status: u16, message: impl Into, @@ -114,6 +185,8 @@ impl BridgeError { Self::UpstreamStatus { status, message: message.into(), + parsed: None, + wire: UpstreamWire::Unknown, retry_after, } } @@ -139,6 +212,138 @@ pub fn parse_retry_after(headers: &http::HeaderMap) -> Option { Some(Duration::from_secs(seconds)) } +/// Drain an upstream error response (capped at +/// [`MAX_UPSTREAM_ERROR_BODY_BYTES`]) and produce a +/// [`BridgeError::UpstreamStatus`] with a best-effort parsed view of +/// the envelope. +/// +/// The `parse` closure runs only when the response declares an +/// `application/json` content-type — this guards against fronting WAFs +/// or load balancers returning HTML error pages that would otherwise be +/// fed to a JSON parser and either fail expensively or surface +/// nonsensical fragments. +/// +/// `parse` returning `None` is treated as "envelope shape unknown"; the +/// fallback in that case is the truncated raw body string in +/// [`BridgeError::UpstreamStatus::message`], same as for non-JSON +/// bodies. +pub async fn capture_upstream_error_http( + status: http::StatusCode, + resp: reqwest::Response, + wire: UpstreamWire, + parse: impl FnOnce(&[u8]) -> Option, +) -> BridgeError { + let retry_after = parse_retry_after(resp.headers()); + let content_type = resp + .headers() + .get(http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .map(str::to_ascii_lowercase); + let body = read_body_capped(resp, MAX_UPSTREAM_ERROR_BODY_BYTES).await; + let parsed = content_type + .as_deref() + .map(content_type_is_json) + .unwrap_or(false) + .then(|| parse(&body)) + .flatten() + // Truncate every parsed string at the same cap as the outer + // `message`. Otherwise a hostile or buggy upstream emitting a + // 60 KB `error.message` / `error.code` / `error.type` / + // `error.param` would reach the customer envelope verbatim — + // the cap exists exactly to prevent that. AWS exception codes + // / Anthropic types / OpenAI codes are bounded vocabulary in + // practice but the cap applies defensively. + .map(|mut v| { + let cap = MAX_UPSTREAM_ERROR_MESSAGE_BYTES; + v.message = v.message.map(|m| truncate_lossy(&m, cap)); + v.kind = v.kind.map(|k| truncate_lossy(&k, cap)); + v.code = v.code.map(|c| truncate_lossy(&c, cap)); + v.param = v.param.map(|p| truncate_lossy(&p, cap)); + v + }); + let message = parsed + .as_ref() + .and_then(|v| v.message.clone()) + .unwrap_or_else(|| String::from_utf8_lossy(&body).into_owned()); + BridgeError::UpstreamStatus { + status: status.as_u16(), + message: truncate_lossy(&message, MAX_UPSTREAM_ERROR_MESSAGE_BYTES), + parsed: parsed.map(Box::new), + wire, + retry_after, + } +} + +/// Read the response body, stopping after `limit` bytes. Used to bound +/// upstream-error parsing cost regardless of `Content-Length`. Errors +/// during read surface as an empty buffer — the caller falls through +/// to a parse-failure path and emits the generic `upstream_error` +/// envelope, which matches the pre-fix behaviour for that edge. +/// +/// Public so non-OpenAI / non-Anthropic bridges (Vertex, Azure) can +/// enforce the same cap when they need a custom parse path (e.g. +/// extracting only `kind` from the upstream envelope while suppressing +/// the `message` for operator-taxonomy redaction). +pub async fn read_body_capped(resp: reqwest::Response, limit: usize) -> bytes::Bytes { + use futures::StreamExt; + let mut buf = bytes::BytesMut::with_capacity(limit.min(16 * 1024)); + let mut stream = resp.bytes_stream(); + // Continue draining the stream past `limit` so the underlying + // hyper connection can be returned to reqwest's keep-alive pool. + // Stopping the iteration mid-stream taints the connection and + // forces a new TCP handshake on the next upstream call — a real + // cost when an upstream is flapping and producing a burst of + // error responses. The extra reads only discard bytes; memory + // stays bounded by `limit`. + while let Some(chunk) = stream.next().await { + let Ok(chunk) = chunk else { break }; + if buf.len() >= limit { + continue; + } + let remaining = limit - buf.len(); + let take = chunk.len().min(remaining); + buf.extend_from_slice(&chunk[..take]); + } + buf.freeze() +} + +/// Content-Type token starts with `application/json` (RFC 7231 §3.1.1.1 +/// allows a trailing `; charset=…` parameter, so a prefix match is the +/// right shape here — exact equality misses `application/json; charset=utf-8`). +/// +/// Public so non-OpenAI / non-Anthropic bridges (Vertex, Azure) can +/// apply the same JSON-only guard when they need a custom parse path +/// that doesn't route through [`capture_upstream_error_http`]. +pub fn content_type_is_json(ct: &str) -> bool { + let ct = ct.trim_start(); + ct.starts_with("application/json") +} + +/// Convenience: read the `Content-Type` header from a [`reqwest::Response`] +/// and decide whether it's `application/json` per [`content_type_is_json`]. +/// Returns `false` when the header is missing or non-ASCII. +pub fn response_is_json(resp: &reqwest::Response) -> bool { + resp.headers() + .get(http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .map(|ct| content_type_is_json(&ct.to_ascii_lowercase())) + .unwrap_or(false) +} + +/// Truncate a string to at most `max` bytes, splitting only on a UTF-8 +/// boundary. Appends an ellipsis when truncation occurred so log +/// readers can tell the message was cut. +fn truncate_lossy(s: &str, max: usize) -> String { + if s.len() <= max { + return s.to_string(); + } + let mut end = max; + while end > 0 && !s.is_char_boundary(end) { + end -= 1; + } + format!("{}…", &s[..end]) +} + impl BridgeError { /// Stable HTTP status mapping. The proxy layer uses this to build /// its OpenAI-compatible `{error:{message,type,...}}` envelope. diff --git a/crates/aisix-gateway/src/lib.rs b/crates/aisix-gateway/src/lib.rs index 7e19232e..73288aff 100644 --- a/crates/aisix-gateway/src/lib.rs +++ b/crates/aisix-gateway/src/lib.rs @@ -25,7 +25,11 @@ pub mod chat; pub mod hub; pub mod sse; -pub use bridge::{parse_retry_after, Bridge, BridgeContext, BridgeError, ChatChunkStream}; +pub use bridge::{ + capture_upstream_error_http, content_type_is_json, parse_retry_after, read_body_capped, + response_is_json, Bridge, BridgeContext, BridgeError, ChatChunkStream, UpstreamErrorView, + UpstreamWire, MAX_UPSTREAM_ERROR_BODY_BYTES, MAX_UPSTREAM_ERROR_MESSAGE_BYTES, +}; pub use chat::{ ChatChunk, ChatDelta, ChatFormat, ChatMessage, ChatResponse, EmbeddingObject, EmbeddingRequest, EmbeddingResponse, EmbeddingUsage, FinishReason, Role, UsageStats, diff --git a/crates/aisix-provider-anthropic/src/bridge.rs b/crates/aisix-provider-anthropic/src/bridge.rs index 5f434c49..b9b8d050 100644 --- a/crates/aisix-provider-anthropic/src/bridge.rs +++ b/crates/aisix-provider-anthropic/src/bridge.rs @@ -131,21 +131,44 @@ fn upstream_model(ctx: &BridgeContext) -> Result<&str, BridgeError> { } async fn map_http_error(status: StatusCode, resp: reqwest::Response) -> BridgeError { - let retry_after = aisix_gateway::parse_retry_after(resp.headers()); - let message = resp.text().await.unwrap_or_default(); - BridgeError::upstream_status_with_retry_after( - status.as_u16(), - truncate(&message, 1024), - retry_after, + aisix_gateway::capture_upstream_error_http( + status, + resp, + aisix_gateway::UpstreamWire::Anthropic, + parse_anthropic_error_envelope, ) + .await } -fn truncate(s: &str, n: usize) -> String { - if s.len() <= n { - s.to_string() - } else { - format!("{}…", &s[..n]) +/// Parse the Anthropic error envelope: +/// +/// ```json +/// {"type": "error", "error": {"type": "...", "message": "..."}} +/// ``` +/// +/// Anthropic does not carry `code` or `param` fields — those stay +/// `None`. The translation table at render time derives an OpenAI +/// `code` from `kind` when crossing wire formats. +/// +/// Reference: +fn parse_anthropic_error_envelope(body: &[u8]) -> Option { + #[derive(serde::Deserialize)] + struct Outer { + error: Inner, } + #[derive(serde::Deserialize)] + struct Inner { + #[serde(rename = "type")] + kind: Option, + message: Option, + } + let outer: Outer = serde_json::from_slice(body).ok()?; + Some(aisix_gateway::UpstreamErrorView { + kind: outer.error.kind, + message: outer.error.message, + code: None, + param: None, + }) } async fn with_deadline( @@ -371,10 +394,10 @@ mod tests { let server = MockServer::start().await; Mock::given(method("POST")) .and(path("/v1/messages")) - .respond_with( - ResponseTemplate::new(400) - .set_body_string(r#"{"error":{"type":"invalid_request","message":"bad"}}"#), - ) + .respond_with(ResponseTemplate::new(400).set_body_raw( + r#"{"error":{"type":"invalid_request","message":"bad"}}"#.as_bytes(), + "application/json", + )) .mount(&server) .await; @@ -383,10 +406,19 @@ mod tests { let err = bridge.chat(&req(), &ctx).await.unwrap_err(); match err { BridgeError::UpstreamStatus { - status, message, .. + status, + message, + parsed, + .. } => { assert_eq!(status, 400); - assert!(message.contains("invalid_request")); + // After #322: bridge parses Anthropic envelope into a + // structured view; `message` is now the upstream's + // `error.message`, not the raw JSON body. + assert_eq!(message, "bad"); + let parsed = parsed.expect("envelope parsed"); + assert_eq!(parsed.kind.as_deref(), Some("invalid_request")); + assert_eq!(parsed.message.as_deref(), Some("bad")); } other => panic!("unexpected: {other:?}"), } diff --git a/crates/aisix-provider-azure-openai/src/bridge.rs b/crates/aisix-provider-azure-openai/src/bridge.rs index 0a2b0f9a..36caff52 100644 --- a/crates/aisix-provider-azure-openai/src/bridge.rs +++ b/crates/aisix-provider-azure-openai/src/bridge.rs @@ -270,16 +270,29 @@ fn upstream_model(ctx: &BridgeContext) -> Result<&str, BridgeError> { /// **Audit M1 — sensitive-info redaction:** Azure error envelopes /// (`{"error": {"code": "...", "message": "..."}}`) often include the /// operator-defined deployment id (e.g. "The API deployment for this -/// resource does not exist.") or the resource hostname. Surfacing -/// these verbatim to a downstream API caller leaks operator-internal -/// taxonomy, so we map the status to a canned phrase here. The full -/// upstream body still lives in the DP-side request log via -/// `request_id` (tracing in callers), accessible to operators but not -/// to customers. +/// resource does not exist.") or the resource hostname. The +/// customer-visible `message` stays a canned phrase. We do read +/// `error.code` into [`UpstreamErrorView::kind`] — these are a small +/// closed set of Azure-defined tokens (`DeploymentNotFound`, +/// `content_filter`, …), stable taxonomy rather than operator data — +/// so the envelope-translation layer can derive an OpenAI `code`. +/// [`UpstreamErrorView::message`] stays `None`. +/// +/// Azure-specific quirk: content-policy violations nest under +/// `error.inner_error.code` *or* `error.innererror.code` (Azure emits +/// both casings depending on endpoint). +/// `"ResponsibleAIPolicyViolation"` on the inner code overrides the +/// outer `code` so the gateway can recognise the policy hit even when +/// the outer code is the generic `invalid_request_error`. async fn map_http_error(status: StatusCode, resp: reqwest::Response) -> BridgeError { let retry_after = aisix_gateway::parse_retry_after(resp.headers()); - // Drain the body to free the connection, but ignore the content. - let _ = resp.text().await; + let is_json = aisix_gateway::response_is_json(&resp); + let body = + aisix_gateway::read_body_capped(resp, aisix_gateway::MAX_UPSTREAM_ERROR_BODY_BYTES).await; + // Skip the serde parse on non-JSON bodies (HTML error page from a + // load-balancer / front door). Same guard as + // `capture_upstream_error_http`. + let kind = is_json.then(|| parse_azure_error_code(&body)).flatten(); let message = match status.as_u16() { 401 | 403 => "upstream authentication failed".to_string(), 404 => "upstream deployment or model not found".to_string(), @@ -287,10 +300,63 @@ async fn map_http_error(status: StatusCode, resp: reqwest::Response) -> BridgeEr 409 => "upstream conflict".to_string(), 413 => "upstream request entity too large".to_string(), 429 => "upstream rate limited".to_string(), - 500..=599 => format!("upstream returned {}", status.as_u16()), _ => format!("upstream returned {}", status.as_u16()), }; - BridgeError::upstream_status_with_retry_after(status.as_u16(), message, retry_after) + // Azure's envelope only has a single `error.code` field (no + // separate `type`). Downstream OpenAI clients expect both + // `error.type` AND `error.code` — populate both `kind` and `code` + // from the upstream token so the translation layer can either + // pass it through (for OpenAI-compat tokens like + // `rate_limit_exceeded`) or override with a derived code (for + // Azure-specific tokens like `DeploymentNotFound`). + let parsed = kind.as_ref().map(|k| { + Box::new(aisix_gateway::UpstreamErrorView { + kind: Some(k.clone()), + message: None, + code: Some(k.clone()), + param: None, + }) + }); + BridgeError::UpstreamStatus { + status: status.as_u16(), + message, + parsed, + wire: aisix_gateway::UpstreamWire::AzureOpenAI, + retry_after, + } +} + +/// Extract the Azure error code from the envelope, applying the +/// `inner_error` / `innererror` content-policy quirk. +fn parse_azure_error_code(body: &[u8]) -> Option { + #[derive(serde::Deserialize)] + struct Outer { + error: Inner, + } + #[derive(serde::Deserialize)] + struct Inner { + code: Option, + #[serde(rename = "inner_error")] + inner_error: Option, + innererror: Option, + } + #[derive(serde::Deserialize)] + struct InnerInner { + code: Option, + } + let outer: Outer = serde_json::from_slice(body).ok()?; + let inner_code = outer + .error + .inner_error + .as_ref() + .or(outer.error.innererror.as_ref()) + .and_then(|i| i.code.clone()); + // Content-policy violation on `inner_error` overrides the outer + // generic code. + if inner_code.as_deref() == Some("ResponsibleAIPolicyViolation") { + return inner_code; + } + outer.error.code } /// Wrap a future in the optional deadline. `None` → no timeout. @@ -1283,6 +1349,7 @@ mod tests { status, retry_after, message, + .. } => { assert_eq!(status, 429); assert_eq!(retry_after, Some(std::time::Duration::from_secs(30))); @@ -1295,6 +1362,143 @@ mod tests { } } + /// Copilot review (PR #323): Azure's envelope only has `error.code` + /// (no separate `type`). For OpenAI-compatible tokens that Azure + /// inherits unchanged (e.g. `rate_limit_exceeded`), the bridge + /// must populate BOTH `parsed.kind` AND `parsed.code` from the + /// upstream — otherwise the downstream OpenAI client receives + /// `error.type=rate_limit_exceeded` but `error.code=null`, which + /// is exactly the SDK-retry-logic break that issue #322 calls out. + #[tokio::test] + async fn chat_429_preserves_openai_compatible_code_for_sdk_retry() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/openai/deployments/gpt4o-prod/chat/completions")) + .respond_with(ResponseTemplate::new(429).set_body_raw( + br#"{"error":{"code":"rate_limit_exceeded","message":"slow down"}}"#.as_slice(), + "application/json", + )) + .mount(&server) + .await; + let bridge = + AzureOpenAiBridge::new().with_url_override(mock_chat_url(&server.uri(), "gpt4o-prod")); + let ctx = canonical_test_ctx(); + let req = ChatFormat::new("my-azure-gpt4", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { parsed, .. } => { + let parsed = parsed.expect("envelope parsed"); + assert_eq!(parsed.kind.as_deref(), Some("rate_limit_exceeded")); + assert_eq!( + parsed.code.as_deref(), + Some("rate_limit_exceeded"), + "OpenAI-compat code must flow through view.code for SDK retry" + ); + } + other => panic!("expected UpstreamStatus, got {other:?}"), + } + } + + /// Copilot review (PR #323): when upstream returns a non-JSON + /// body (HTML error page from Azure Front Door, etc.), the parser + /// must skip the serde attempt rather than try to deserialize + /// `...` as JSON. Same content-type guard as + /// `capture_upstream_error_http`. + #[tokio::test] + async fn chat_400_non_json_body_skips_envelope_parse() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/openai/deployments/gpt4o-prod/chat/completions")) + .respond_with(ResponseTemplate::new(400).set_body_raw( + b"403 Forbidden by Front Door".as_slice(), + "text/html", + )) + .mount(&server) + .await; + let bridge = + AzureOpenAiBridge::new().with_url_override(mock_chat_url(&server.uri(), "gpt4o-prod")); + let ctx = canonical_test_ctx(); + let req = ChatFormat::new("my-azure-gpt4", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { parsed, .. } => { + assert!( + parsed.is_none(), + "non-JSON body must not produce a parsed view; got {parsed:?}" + ); + } + other => panic!("expected UpstreamStatus, got {other:?}"), + } + } + + /// Audit fix (PR #323 follow-up): the `inner_error` casing + /// variant is what most Azure docs show, but Azure ALSO emits + /// `innererror` (smushed) on some endpoints. Both must be + /// recognised by the parser. + #[tokio::test] + async fn chat_400_with_innererror_smushed_casing_also_lifts_kind() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/openai/deployments/gpt4o-prod/chat/completions")) + .respond_with(ResponseTemplate::new(400).set_body_raw( + br#"{"error":{"code":"invalid_request_error","message":"blocked","innererror":{"code":"ResponsibleAIPolicyViolation"}}}"#.as_slice(), + "application/json", + )) + .mount(&server) + .await; + let bridge = + AzureOpenAiBridge::new().with_url_override(mock_chat_url(&server.uri(), "gpt4o-prod")); + let ctx = canonical_test_ctx(); + let req = ChatFormat::new("my-azure-gpt4", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { parsed, .. } => { + let parsed = parsed.expect("innererror parsed"); + assert_eq!(parsed.kind.as_deref(), Some("ResponsibleAIPolicyViolation")); + } + other => panic!("expected UpstreamStatus, got {other:?}"), + } + } + + /// Audit fix (PR #323 MEDIUM-2): structured-parse path — + /// Azure-specific `inner_error.code = ResponsibleAIPolicyViolation` + /// must surface as `parsed.kind`, not be flattened under the outer + /// `error.code`. `wire: AzureOpenAI` must be set so the + /// translation layer picks the Azure-aware code map. + /// `parsed.message` stays `None` (deployment id leak). + #[tokio::test] + async fn chat_400_with_inner_error_responsible_ai_lifts_kind() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/openai/deployments/gpt4o-prod/chat/completions")) + .respond_with(ResponseTemplate::new(400).set_body_raw( + br#"{"error":{"code":"invalid_request_error","message":"blocked","inner_error":{"code":"ResponsibleAIPolicyViolation"}}}"#.as_slice(), + "application/json", + )) + .mount(&server) + .await; + let bridge = + AzureOpenAiBridge::new().with_url_override(mock_chat_url(&server.uri(), "gpt4o-prod")); + let ctx = canonical_test_ctx(); + let req = ChatFormat::new("my-azure-gpt4", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { + status, + wire, + parsed, + .. + } => { + assert_eq!(status, 400); + assert_eq!(wire, aisix_gateway::UpstreamWire::AzureOpenAI); + let parsed = parsed.expect("inner_error parsed"); + assert_eq!(parsed.kind.as_deref(), Some("ResponsibleAIPolicyViolation")); + assert!(parsed.message.is_none()); + } + other => panic!("expected UpstreamStatus, got {other:?}"), + } + } + /// Audit L3: `chat_against_full_bridge_dispatch` calls the real /// Azure DNS (`acme-west.openai.azure.com`). Marked `#[ignore]` /// because (a) CI runners with corporate proxies may resolve it diff --git a/crates/aisix-provider-bedrock/src/bridge.rs b/crates/aisix-provider-bedrock/src/bridge.rs index 8a9492ec..eb13fdbe 100644 --- a/crates/aisix-provider-bedrock/src/bridge.rs +++ b/crates/aisix-provider-bedrock/src/bridge.rs @@ -384,7 +384,14 @@ fn map_sdk_error( fn map_service_error( svc: ServiceError, ) -> BridgeError { - let raw = svc.into_raw(); + // SECURITY: AWS error messages embed operator-internal taxonomy + // (ARNs, region, account id, IAM role names). The canned status- + // keyed phrase reaches the customer; the parsed view surfaces only + // the AWS error CODE (e.g. "ThrottlingException") for the + // error_translate layer to translate to OpenAI / Anthropic shape. + // `parsed.message` is intentionally left `None`. + let kind = svc.err().meta().code().map(str::to_string); + let raw = svc.raw(); let status = raw.status().as_u16(); // Convert smithy HeaderMap → http::HeaderMap so we can reuse the // gateway-level `parse_retry_after` helper. Headers with invalid @@ -404,12 +411,21 @@ fn map_service_error( 404 => "upstream model not found".to_string(), 408 => "upstream request timeout".to_string(), 429 => "upstream rate limited".to_string(), - 500..=599 => format!("upstream returned {status}"), _ => format!("upstream returned {status}"), }; + let parsed = kind.as_ref().map(|k| { + Box::new(aisix_gateway::UpstreamErrorView { + kind: Some(k.clone()), + message: None, + code: None, + param: None, + }) + }); BridgeError::UpstreamStatus { status, message, + parsed, + wire: aisix_gateway::UpstreamWire::Bedrock, retry_after, } } @@ -1323,6 +1339,9 @@ mod tests { status, message, retry_after, + wire, + parsed, + .. } => { assert_eq!(status, 429); assert_eq!(message, "upstream rate limited"); @@ -1339,6 +1358,20 @@ mod tests { Some(std::time::Duration::from_secs(42)), "Retry-After must reach BridgeError::UpstreamStatus" ); + // Audit fix (PR #323 MEDIUM-2): pin `wire` so a + // refactor that breaks cross-wire translation fails + // here. `parsed.kind` should carry the AWS exception + // name (the SDK derives this from `__type` / + // X-Amzn-ErrorType). `parsed.message` stays None for + // operator-taxonomy redaction (ARNs, account ids). + assert_eq!(wire, aisix_gateway::UpstreamWire::Bedrock); + if let Some(view) = parsed { + assert!( + view.message.is_none(), + "bedrock must NOT surface upstream message; got {:?}", + view.message + ); + } } other => panic!("expected UpstreamStatus with retry_after, got {other:?}"), } diff --git a/crates/aisix-provider-openai/src/bridge.rs b/crates/aisix-provider-openai/src/bridge.rs index ed7b4e8b..0086c31c 100644 --- a/crates/aisix-provider-openai/src/bridge.rs +++ b/crates/aisix-provider-openai/src/bridge.rs @@ -237,21 +237,46 @@ fn upstream_model(ctx: &BridgeContext) -> Result<&str, BridgeError> { } async fn map_http_error(status: StatusCode, resp: reqwest::Response) -> BridgeError { - let retry_after = aisix_gateway::parse_retry_after(resp.headers()); - let message = resp.text().await.unwrap_or_default(); - BridgeError::upstream_status_with_retry_after( - status.as_u16(), - truncate(&message, 1024), - retry_after, + aisix_gateway::capture_upstream_error_http( + status, + resp, + aisix_gateway::UpstreamWire::OpenAI, + parse_openai_error_envelope, ) + .await } -fn truncate(s: &str, n: usize) -> String { - if s.len() <= n { - s.to_string() - } else { - format!("{}…", &s[..n]) +/// Parse the canonical OpenAI error envelope: +/// +/// ```json +/// {"error": {"message": "...", "type": "...", "code": "...", "param": "..."}} +/// ``` +/// +/// Returns `None` when the body is not JSON of that shape; the caller +/// falls back to the truncated raw body string for the `message` field +/// and emits a generic `upstream_error` envelope. +/// +/// Reference: +fn parse_openai_error_envelope(body: &[u8]) -> Option { + #[derive(serde::Deserialize)] + struct Outer { + error: Inner, + } + #[derive(serde::Deserialize)] + struct Inner { + message: Option, + #[serde(rename = "type")] + kind: Option, + code: Option, + param: Option, } + let outer: Outer = serde_json::from_slice(body).ok()?; + Some(aisix_gateway::UpstreamErrorView { + kind: outer.error.kind, + message: outer.error.message, + code: outer.error.code, + param: outer.error.param, + }) } /// Wrap a future in the optional deadline. `None` → no timeout. @@ -785,6 +810,85 @@ mod tests { } } + /// Audit fix (PR #323): the [`aisix_gateway::MAX_UPSTREAM_ERROR_BODY_BYTES`] + /// (64 KB) cap must actually fire on an oversized upstream error + /// body, otherwise a misbehaved upstream could pin a worker's + /// memory. Pins the cap as a regression test — exercises + /// `read_body_capped` end-to-end through the OpenAI bridge. + #[tokio::test] + async fn non_streaming_oversize_error_body_truncated_to_max_message_bytes() { + let server = MockServer::start().await; + // 200 KB body — well above the 64 KB read cap and the 1024-byte + // message cap. + let huge_body = "x".repeat(200 * 1024); + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(429).set_body_string(huge_body)) + .mount(&server) + .await; + + let bridge = OpenAiBridge::new(); + let ctx = sample_ctx(&server.uri()); + let err = bridge.chat(&req(), &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { message, .. } => { + // Outer message must respect the 1024-byte cap + the + // ellipsis marker. 8 bytes of slack for the ellipsis + // (3-byte UTF-8 char) plus any partial-codepoint + // alignment back-off. + assert!( + message.len() <= aisix_gateway::MAX_UPSTREAM_ERROR_MESSAGE_BYTES + 8, + "outer message must be truncated; got {} bytes", + message.len() + ); + } + other => panic!("unexpected: {other:?}"), + } + } + + /// Audit fix (PR #323): JSON-shaped envelopes with a huge inner + /// `error.message` must ALSO be truncated. Without this, an + /// upstream OpenAI-shape envelope embedding a 60 KB message would + /// bypass the cap by going through the parsed-view path. Pins + /// HIGH-2 from the audit. + #[tokio::test] + async fn non_streaming_oversize_parsed_message_truncated() { + let server = MockServer::start().await; + let huge = "y".repeat(60 * 1024); + let body = format!(r#"{{"error":{{"message":"{huge}","type":"big","code":"big_code"}}}}"#); + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with( + ResponseTemplate::new(400).set_body_raw(body.as_bytes(), "application/json"), + ) + .mount(&server) + .await; + + let bridge = OpenAiBridge::new(); + let ctx = sample_ctx(&server.uri()); + let err = bridge.chat(&req(), &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { + message, parsed, .. + } => { + assert!( + message.len() <= aisix_gateway::MAX_UPSTREAM_ERROR_MESSAGE_BYTES + 8, + "outer message exceeded cap: {} bytes", + message.len() + ); + let parsed = parsed.expect("envelope parsed"); + let pm = parsed.message.as_ref().expect("parsed message present"); + assert!( + pm.len() <= aisix_gateway::MAX_UPSTREAM_ERROR_MESSAGE_BYTES + 8, + "parsed.message exceeded cap: {} bytes", + pm.len() + ); + assert_eq!(parsed.code.as_deref(), Some("big_code")); + } + other => panic!("unexpected: {other:?}"), + } + } + #[tokio::test] async fn non_streaming_429_surfaces_retry_after_header() { let server = MockServer::start().await; diff --git a/crates/aisix-provider-vertex/src/bridge.rs b/crates/aisix-provider-vertex/src/bridge.rs index 236b830f..1f0414e4 100644 --- a/crates/aisix-provider-vertex/src/bridge.rs +++ b/crates/aisix-provider-vertex/src/bridge.rs @@ -262,10 +262,22 @@ where /// /// **Audit-aware:** Vertex error envelopes (`{"error": {"code": /// 403, "message": "Permission denied for project foo-bar-prod"}}`) -/// leak operator project ids. We map to canned status-keyed phrases. +/// leak operator project ids. The customer-visible `message` is a +/// canned status-keyed phrase. We *do* read the upstream's +/// `error.status` (a gRPC canonical code such as `"RESOURCE_EXHAUSTED"`) +/// into [`UpstreamErrorView::kind`] so the envelope-translation layer +/// can derive an OpenAI `code` — that token is a stable taxonomy +/// label, not operator-internal data. [`UpstreamErrorView::message`] +/// is intentionally left `None`. async fn map_http_error(status: StatusCode, resp: reqwest::Response) -> BridgeError { let retry_after = aisix_gateway::parse_retry_after(resp.headers()); - let _ = resp.text().await; // drain body, discard content + let is_json = aisix_gateway::response_is_json(&resp); + let body = + aisix_gateway::read_body_capped(resp, aisix_gateway::MAX_UPSTREAM_ERROR_BODY_BYTES).await; + // Skip the serde parse on non-JSON bodies (HTML / text error pages + // from a fronting WAF or load balancer). Same guard as + // `capture_upstream_error_http`. + let kind = is_json.then(|| parse_vertex_error_status(&body)).flatten(); let message = match status.as_u16() { 401 | 403 => "upstream authentication failed".to_string(), 404 => "upstream model or endpoint not found".to_string(), @@ -273,7 +285,40 @@ async fn map_http_error(status: StatusCode, resp: reqwest::Response) -> BridgeEr 429 => "upstream rate limited".to_string(), _ => format!("upstream returned {}", status.as_u16()), }; - BridgeError::upstream_status_with_retry_after(status.as_u16(), message, retry_after) + let parsed = kind.as_ref().map(|_| { + Box::new(aisix_gateway::UpstreamErrorView { + kind: kind.clone(), + message: None, + code: None, + param: None, + }) + }); + BridgeError::UpstreamStatus { + status: status.as_u16(), + message, + parsed, + wire: aisix_gateway::UpstreamWire::Vertex, + retry_after, + } +} + +/// Extract just the `error.status` field (the gRPC canonical code) from +/// a Vertex error body. The body shape is +/// `{"error": {"code": int, "message": "...", "status": "...", "details": [...]}}` +/// per . Returns `None` +/// when the body is not JSON of that shape — we intentionally do not +/// surface `error.message` (it embeds operator project ids). +fn parse_vertex_error_status(body: &[u8]) -> Option { + #[derive(serde::Deserialize)] + struct Outer { + error: Inner, + } + #[derive(serde::Deserialize)] + struct Inner { + status: Option, + } + let outer: Outer = serde_json::from_slice(body).ok()?; + outer.error.status } #[async_trait] @@ -1385,7 +1430,11 @@ mod tests { let err = bridge.chat(&req, &ctx).await.unwrap_err(); match err { BridgeError::UpstreamStatus { - status, message, .. + status, + message, + wire, + parsed, + .. } => { assert_eq!(status, 403); assert!( @@ -1393,6 +1442,102 @@ mod tests { && !message.contains("Permission denied on project"), "upstream body must not leak project id; got message={message:?}" ); + // Audit fix (PR #323 MEDIUM-2): pin the wire tag so a + // refactor that breaks the cross-wire translation + // pipeline fails this test loudly. parsed.message + // must stay None for operator-taxonomy redaction. + assert_eq!(wire, aisix_gateway::UpstreamWire::Vertex); + if let Some(view) = parsed { + assert!( + view.message.is_none(), + "vertex must NOT surface upstream message (project ids leak); \ + got {:?}", + view.message + ); + } + } + other => panic!("expected UpstreamStatus, got {other:?}"), + } + } + + /// Copilot review (PR #323): non-JSON body (HTML error page from a + /// fronting load balancer) must NOT trigger serde parsing — the + /// bridge applies the same content-type guard as + /// `capture_upstream_error_http`. + #[tokio::test] + async fn chat_gemini_non_json_body_skips_envelope_parse() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(403).set_body_raw( + b"403 Forbidden".as_slice(), + "text/html", + )) + .mount(&server) + .await; + + let bridge = VertexBridge::new().with_api_base_override(server.uri()); + let ctx = BridgeContext::new( + "req-1", + sample_model_with("gemini-1.5-pro"), + sample_pk_with_secret(valid_secret_json()), + ); + let req = ChatFormat::new("my-gemini", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { parsed, .. } => { + assert!( + parsed.is_none(), + "non-JSON body must skip parser; got {parsed:?}" + ); + } + other => panic!("expected UpstreamStatus, got {other:?}"), + } + } + + /// Audit fix (PR #323 MEDIUM-2): structured-parse path — upstream + /// returns a Vertex envelope with `error.status` set; the bridge + /// must extract it as `parsed.kind` for the cross-wire translation + /// layer to derive an OpenAI `code`. `parsed.message` stays `None` + /// (operator project ids leak otherwise). + #[tokio::test] + async fn chat_gemini_429_populates_parsed_kind_from_grpc_status() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(429).set_body_json(serde_json::json!({ + "error": { + "code": 429, + "message": "Quota exceeded for project my-secret-proj", + "status": "RESOURCE_EXHAUSTED" + } + }))) + .mount(&server) + .await; + + let bridge = VertexBridge::new().with_api_base_override(server.uri()); + let ctx = BridgeContext::new( + "req-1", + sample_model_with("gemini-1.5-pro"), + sample_pk_with_secret(valid_secret_json()), + ); + let req = ChatFormat::new("my-gemini", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::UpstreamStatus { + status, + message, + wire, + parsed, + .. + } => { + assert_eq!(status, 429); + assert_eq!(wire, aisix_gateway::UpstreamWire::Vertex); + assert!( + !message.contains("my-secret-proj"), + "must not leak project id; got {message:?}" + ); + let parsed = parsed.expect("status field parsed into view"); + assert_eq!(parsed.kind.as_deref(), Some("RESOURCE_EXHAUSTED")); + assert!(parsed.message.is_none()); } other => panic!("expected UpstreamStatus, got {other:?}"), } diff --git a/crates/aisix-proxy/src/error.rs b/crates/aisix-proxy/src/error.rs index 4e30fd34..b323b00a 100644 --- a/crates/aisix-proxy/src/error.rs +++ b/crates/aisix-proxy/src/error.rs @@ -32,8 +32,13 @@ pub struct ErrorEnvelope { #[derive(Debug, Serialize, Clone)] pub struct ErrorBody { pub message: String, + /// `error.type` token. Was `&'static str` before #322 — widened to + /// owned `String` because the type can now reflect an upstream- + /// derived OpenAI taxonomy token (`rate_limit_exceeded`, + /// `insufficient_quota`, …) when the error_translate layer maps a + /// non-OpenAI upstream to OpenAI shape. #[serde(rename = "type")] - pub kind: &'static str, + pub kind: String, #[serde(skip_serializing_if = "Option::is_none")] pub param: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -41,11 +46,11 @@ pub struct ErrorBody { } impl ErrorEnvelope { - pub fn new(message: impl Into, kind: &'static str) -> Self { + pub fn new(message: impl Into, kind: impl Into) -> Self { Self { error: ErrorBody { message: message.into(), - kind, + kind: kind.into(), param: None, code: None, }, @@ -155,6 +160,28 @@ impl ProxyError { } pub fn envelope(&self) -> ErrorEnvelope { + // Bridge-surface upstream errors get special handling: the + // bridge has best-effort-parsed the upstream envelope into a + // structured [`UpstreamErrorView`], and for same-wire 4xx + // (OpenAI upstream + OpenAI client) we forward the parsed + // fields directly instead of wrapping them inside the + // gateway's generic `upstream_error` envelope. + // + // 5xx and non-JSON bodies fall back to the generic envelope — + // upstream internal-server-error detail (engine names, queue + // depth, etc.) is operator-internal and must not bleed through. + // Cross-wire translation (Anthropic / Bedrock / Vertex / Azure + // → OpenAI shape) ships in a follow-up via `error_translate`. + if let ProxyError::Bridge(aisix_gateway::BridgeError::UpstreamStatus { + status, + message, + parsed, + wire, + .. + }) = self + { + return render_bridge_upstream_envelope(*status, message, parsed.as_deref(), *wire); + } let env = ErrorEnvelope::new(self.to_string(), self.kind()); match self { ProxyError::BudgetExceeded(_) => env.with_code("budget_exceeded"), @@ -163,6 +190,46 @@ impl ProxyError { } } +/// Build the customer-visible envelope for an upstream HTTP error. +/// +/// **4xx**: delegate to [`crate::error_translate::render_openai_envelope`], +/// which (a) passes OpenAI-wire fields verbatim, (b) translates +/// Anthropic / Bedrock / Vertex / AzureOpenAI taxonomy via per-wire +/// tables so the OpenAI-shape `error.type` and `error.code` carry the +/// retry semantics SDKs depend on. +/// +/// **5xx**: emit a canned `upstream returned {status}` message under +/// `type: upstream_error`. Upstream 5xx bodies routinely embed +/// operator-internal detail (engine names, shard ids, queue depth, +/// ARNs in raw AWS messages) — surfacing them to the customer leaks +/// internal taxonomy. The full upstream body remains in operator +/// logs via tracing. +/// +/// **`UpstreamWire::Unknown`** (cooldown fixtures / synthesised +/// errors): legacy generic envelope. +fn render_bridge_upstream_envelope( + status: u16, + message: &str, + parsed: Option<&aisix_gateway::UpstreamErrorView>, + wire: aisix_gateway::UpstreamWire, +) -> ErrorEnvelope { + let is_4xx = (400..500).contains(&status); + if is_4xx && !matches!(wire, aisix_gateway::UpstreamWire::Unknown) { + return ErrorEnvelope { + error: crate::error_translate::render_openai_envelope(parsed, wire, message), + }; + } + let safe_message = if (500..600).contains(&status) { + // Suppress upstream `error.message` on 5xx — engine names / + // shard ids / ARNs commonly appear here and are not customer + // information. + format!("upstream returned {status}") + } else { + message.to_string() + }; + ErrorEnvelope::new(safe_message, "upstream_error") +} + impl IntoResponse for ProxyError { fn into_response(self) -> Response { let status = self.status(); diff --git a/crates/aisix-proxy/src/error_translate.rs b/crates/aisix-proxy/src/error_translate.rs new file mode 100644 index 00000000..4146b582 --- /dev/null +++ b/crates/aisix-proxy/src/error_translate.rs @@ -0,0 +1,446 @@ +//! Cross-wire error-envelope translation. +//! +//! Each upstream provider speaks a different error-envelope taxonomy: +//! +//! | Wire | `error.type` examples | Has `code`/`param`? | +//! |-------------|----------------------------------------|---------------------| +//! | OpenAI | `rate_limit_exceeded`, `invalid_api_key` | yes | +//! | Anthropic | `rate_limit_error`, `overloaded_error` | no | +//! | Bedrock | `ThrottlingException`, `ValidationException` | no | +//! | Vertex | `RESOURCE_EXHAUSTED`, `PERMISSION_DENIED` (gRPC) | no | +//! | AzureOpenAI | mostly OpenAI-shape; quirks for content-policy | partial | +//! +//! OpenAI SDKs that drive the customer's retry strategy switch on +//! `error.code` (e.g. `rate_limit_exceeded` vs `insufficient_quota`) +//! and `error.type`. If we forward an Anthropic `rate_limit_error` +//! verbatim to a downstream OpenAI SDK, that retry logic doesn't fire. +//! This module maps each non-OpenAI upstream taxonomy to the OpenAI +//! taxonomy so the client-side SDK keeps working regardless of which +//! upstream the gateway routed to. +//! +//! Authoritative sources for the taxonomies: +//! - OpenAI: +//! - Anthropic: +//! - Bedrock: +//! and per-operation error variants on `InvokeModelError`. +//! - Vertex / Google: +//! (canonical gRPC `Status.code` enum). + +use aisix_gateway::{UpstreamErrorView, UpstreamWire}; + +use crate::error::ErrorBody; + +/// Customer-visible OpenAI-shape envelope body for an upstream error. +/// +/// Caller is responsible for gating on a 4xx status — 5xx and non-4xx +/// upstream errors stay in the generic `upstream_error` envelope. +pub(crate) fn render_openai_envelope( + view: Option<&UpstreamErrorView>, + wire: UpstreamWire, + fallback_message: &str, +) -> ErrorBody { + let Some(view) = view else { + return generic(fallback_message); + }; + let message = view + .message + .clone() + .unwrap_or_else(|| fallback_message.to_string()); + let upstream_kind = view.kind.as_deref(); + let (kind, derived_code) = match wire { + UpstreamWire::OpenAI => ( + upstream_kind + .map(str::to_string) + .unwrap_or_else(|| "upstream_error".to_string()), + view.code.clone(), + ), + UpstreamWire::AzureOpenAI => translate_azure(upstream_kind), + UpstreamWire::Anthropic => translate_anthropic(upstream_kind), + UpstreamWire::Bedrock => translate_bedrock(upstream_kind), + UpstreamWire::Vertex => translate_vertex(upstream_kind), + UpstreamWire::Unknown => ( + upstream_kind + .map(str::to_string) + .unwrap_or_else(|| "upstream_error".to_string()), + view.code.clone(), + ), + }; + ErrorBody { + message, + kind, + param: view.param.clone(), + // - OpenAI same-wire: pass through the upstream's `code` verbatim. + // - AzureOpenAI: prefer the derived code when the translation + // table has an explicit mapping (e.g. `DeploymentNotFound` + // → `model_not_found`), otherwise pass through the upstream + // `code` (Azure shares OpenAI's taxonomy for the bulk of + // codes, so `rate_limit_exceeded` etc. should flow through). + // - Anthropic / Bedrock / Vertex: the upstream `code` field is + // either absent or operator-leaky (Vertex numeric codes + // embed internal taxonomy) — only the derived code reaches + // the customer. + code: match wire { + UpstreamWire::OpenAI => view.code.clone(), + UpstreamWire::AzureOpenAI => derived_code.or_else(|| view.code.clone()), + _ => derived_code, + }, + } +} + +fn generic(message: &str) -> ErrorBody { + ErrorBody { + message: message.to_string(), + kind: "upstream_error".to_string(), + param: None, + code: None, + } +} + +/// Anthropic `error.type` → OpenAI `(type, code)`. Reference: +/// . `permission_error` and +/// `request_too_large` are deliberately exhaustive — the upstream +/// reference impl this gateway is benchmarked against falls through to +/// a generic error on those two cases. +fn translate_anthropic(kind: Option<&str>) -> (String, Option) { + match kind { + Some("invalid_request_error") => ("invalid_request_error".into(), None), + Some("authentication_error") => ( + "invalid_request_error".into(), + Some("invalid_api_key".into()), + ), + Some("permission_error") => ( + "invalid_request_error".into(), + Some("permission_denied".into()), + ), + Some("not_found_error") => ( + "invalid_request_error".into(), + Some("model_not_found".into()), + ), + Some("request_too_large") => ( + "invalid_request_error".into(), + Some("request_too_large".into()), + ), + Some("rate_limit_error") => ( + "rate_limit_exceeded".into(), + Some("rate_limit_exceeded".into()), + ), + Some("overloaded_error") => ("api_error".into(), Some("overloaded".into())), + Some("api_error") => ("api_error".into(), None), + _ => ("upstream_error".into(), None), + } +} + +/// AWS Bedrock `InvokeModelError` variant name → OpenAI `(type, code)`. +/// Reference: AWS SDK for Rust, `aws-sdk-bedrockruntime`'s generated +/// `InvokeModelError` enum, and +/// . +fn translate_bedrock(kind: Option<&str>) -> (String, Option) { + match kind { + Some("ThrottlingException") => ( + "rate_limit_exceeded".into(), + Some("rate_limit_exceeded".into()), + ), + Some("ServiceQuotaExceededException") => ( + "rate_limit_exceeded".into(), + Some("insufficient_quota".into()), + ), + Some("ValidationException") => ("invalid_request_error".into(), None), + Some("AccessDeniedException") => ( + "invalid_request_error".into(), + Some("permission_denied".into()), + ), + Some("ResourceNotFoundException") => ( + "invalid_request_error".into(), + Some("model_not_found".into()), + ), + Some("ModelNotReadyException") => ("api_error".into(), Some("model_not_ready".into())), + Some("ModelTimeoutException") => ("api_error".into(), Some("timeout".into())), + Some("ModelStreamErrorException") => ("api_error".into(), Some("stream_error".into())), + Some("ModelErrorException") => ("api_error".into(), Some("model_error".into())), + Some("InternalServerException") => ("api_error".into(), None), + Some("ServiceUnavailableException") => ("api_error".into(), Some("overloaded".into())), + _ => ("api_error".into(), None), + } +} + +/// Google canonical gRPC status code → OpenAI `(type, code)`. The +/// upstream `error.status` field carries the gRPC code as a string +/// (e.g. `"RESOURCE_EXHAUSTED"`). Reference: +/// and the protobuf +/// `google.rpc.Code` enum. +fn translate_vertex(kind: Option<&str>) -> (String, Option) { + match kind { + Some("RESOURCE_EXHAUSTED") => ( + "rate_limit_exceeded".into(), + Some("rate_limit_exceeded".into()), + ), + Some("PERMISSION_DENIED") => ( + "invalid_request_error".into(), + Some("permission_denied".into()), + ), + Some("UNAUTHENTICATED") => ( + "invalid_request_error".into(), + Some("invalid_api_key".into()), + ), + Some("INVALID_ARGUMENT") => ("invalid_request_error".into(), None), + Some("NOT_FOUND") => ( + "invalid_request_error".into(), + Some("model_not_found".into()), + ), + Some("FAILED_PRECONDITION") | Some("OUT_OF_RANGE") | Some("ALREADY_EXISTS") => { + ("invalid_request_error".into(), None) + } + Some("UNAVAILABLE") => ("api_error".into(), Some("overloaded".into())), + Some("DEADLINE_EXCEEDED") => ("api_error".into(), Some("timeout".into())), + Some("INTERNAL") | Some("ABORTED") | Some("CANCELLED") | Some("UNKNOWN") => { + ("api_error".into(), None) + } + _ => ("api_error".into(), None), + } +} + +/// Azure OpenAI `error.code` → OpenAI `(type, code)`. Azure error +/// codes are mostly identical to OpenAI's, with a handful of +/// Azure-specific tokens. Reference: Azure OpenAI REST docs, error +/// codes section. +fn translate_azure(kind: Option<&str>) -> (String, Option) { + match kind { + // Azure-specific deployment / content-policy codes. + Some("DeploymentNotFound") => ( + "invalid_request_error".into(), + Some("model_not_found".into()), + ), + Some("ResponsibleAIPolicyViolation") => ( + "invalid_request_error".into(), + Some("content_policy_violation".into()), + ), + Some("content_filter") => ( + "invalid_request_error".into(), + Some("content_policy_violation".into()), + ), + Some("invalid_encrypted_content") => ( + "invalid_request_error".into(), + Some("invalid_encrypted_content".into()), + ), + // Everything else: Azure aligns with OpenAI taxonomy — pass + // through the upstream kind as the OpenAI type, no derived + // code (the caller will prefer any upstream-supplied `code`). + Some(k) => (k.to_string(), None), + None => ("upstream_error".into(), None), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn view(kind: &str) -> UpstreamErrorView { + UpstreamErrorView { + kind: Some(kind.into()), + message: Some("upstream said hi".into()), + code: None, + param: None, + } + } + + #[test] + fn anthropic_rate_limit_translates_to_openai_rate_limit_exceeded() { + let v = view("rate_limit_error"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "fallback"); + assert_eq!(body.kind, "rate_limit_exceeded"); + assert_eq!(body.code.as_deref(), Some("rate_limit_exceeded")); + assert_eq!(body.message, "upstream said hi"); + } + + #[test] + fn anthropic_overloaded_maps_to_api_error_with_overloaded_code() { + let v = view("overloaded_error"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "fb"); + assert_eq!(body.kind, "api_error"); + assert_eq!(body.code.as_deref(), Some("overloaded")); + } + + #[test] + fn anthropic_authentication_carries_invalid_api_key_code() { + let v = view("authentication_error"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("invalid_api_key")); + } + + #[test] + fn anthropic_permission_error_maps_to_permission_denied_code() { + let v = view("permission_error"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("permission_denied")); + } + + #[test] + fn anthropic_not_found_maps_to_model_not_found() { + let v = view("not_found_error"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("model_not_found")); + } + + #[test] + fn anthropic_unknown_falls_back_to_upstream_error() { + let v = view("brand_new_anthropic_error_type"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "fb"); + assert_eq!(body.kind, "upstream_error"); + assert!(body.code.is_none()); + } + + #[test] + fn bedrock_throttling_translates_to_openai_rate_limit_exceeded() { + let v = view("ThrottlingException"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Bedrock, "fb"); + assert_eq!(body.kind, "rate_limit_exceeded"); + assert_eq!(body.code.as_deref(), Some("rate_limit_exceeded")); + } + + #[test] + fn bedrock_service_quota_exceeded_distinguishes_insufficient_quota() { + // SDK retry logic should pick `insufficient_quota` over generic + // `rate_limit_exceeded` because the recovery path differs + // (quota lift vs backoff). + let v = view("ServiceQuotaExceededException"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Bedrock, "fb"); + assert_eq!(body.kind, "rate_limit_exceeded"); + assert_eq!(body.code.as_deref(), Some("insufficient_quota")); + } + + #[test] + fn bedrock_validation_maps_to_invalid_request_with_no_code() { + let v = view("ValidationException"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Bedrock, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert!(body.code.is_none()); + } + + #[test] + fn bedrock_access_denied_carries_permission_denied_code() { + let v = view("AccessDeniedException"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Bedrock, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("permission_denied")); + } + + #[test] + fn bedrock_unhandled_falls_back_to_api_error() { + let v = view("BrandNewBedrockException"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Bedrock, "fb"); + assert_eq!(body.kind, "api_error"); + assert!(body.code.is_none()); + } + + #[test] + fn vertex_resource_exhausted_translates_to_openai_rate_limit_exceeded() { + let v = view("RESOURCE_EXHAUSTED"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Vertex, "fb"); + assert_eq!(body.kind, "rate_limit_exceeded"); + assert_eq!(body.code.as_deref(), Some("rate_limit_exceeded")); + } + + #[test] + fn vertex_permission_denied_maps_to_permission_denied_code() { + let v = view("PERMISSION_DENIED"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Vertex, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("permission_denied")); + } + + #[test] + fn vertex_unauthenticated_maps_to_invalid_api_key_code() { + let v = view("UNAUTHENTICATED"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Vertex, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("invalid_api_key")); + } + + #[test] + fn vertex_unavailable_maps_to_api_error_with_overloaded_code() { + let v = view("UNAVAILABLE"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Vertex, "fb"); + assert_eq!(body.kind, "api_error"); + assert_eq!(body.code.as_deref(), Some("overloaded")); + } + + #[test] + fn vertex_deadline_exceeded_maps_to_timeout_code() { + let v = view("DEADLINE_EXCEEDED"); + let body = render_openai_envelope(Some(&v), UpstreamWire::Vertex, "fb"); + assert_eq!(body.kind, "api_error"); + assert_eq!(body.code.as_deref(), Some("timeout")); + } + + #[test] + fn azure_deployment_not_found_maps_to_model_not_found() { + let v = view("DeploymentNotFound"); + let body = render_openai_envelope(Some(&v), UpstreamWire::AzureOpenAI, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("model_not_found")); + } + + #[test] + fn azure_content_policy_violation_translates_through_inner_error() { + // Azure surfaces ResponsibleAIPolicyViolation under + // inner_error.code; the bridge parser lifts it to the top-level + // kind, and this translation rewrites it to the OpenAI string + // code that SDKs recognise. + let v = view("ResponsibleAIPolicyViolation"); + let body = render_openai_envelope(Some(&v), UpstreamWire::AzureOpenAI, "fb"); + assert_eq!(body.kind, "invalid_request_error"); + assert_eq!(body.code.as_deref(), Some("content_policy_violation")); + } + + #[test] + fn azure_unknown_kind_passes_through_when_openai_compatible() { + // Azure shares OpenAI's taxonomy for the vast majority of error + // codes; new ones should pass through rather than collapse to + // a generic `upstream_error`. + let v = view("some_future_openai_compat_code"); + let body = render_openai_envelope(Some(&v), UpstreamWire::AzureOpenAI, "fb"); + assert_eq!(body.kind, "some_future_openai_compat_code"); + } + + #[test] + fn openai_same_wire_preserves_upstream_code_and_param() { + // The same-wire path treats the upstream as authoritative — + // `code` and `param` flow through unchanged, including codes + // that aren't in any table (forward-compat for OpenAI taxonomy + // additions). + let v = UpstreamErrorView { + kind: Some("rate_limit_exceeded".into()), + message: Some("hi".into()), + code: Some("custom_code_added_yesterday".into()), + param: Some("model".into()), + }; + let body = render_openai_envelope(Some(&v), UpstreamWire::OpenAI, "fb"); + assert_eq!(body.kind, "rate_limit_exceeded"); + assert_eq!(body.code.as_deref(), Some("custom_code_added_yesterday")); + assert_eq!(body.param.as_deref(), Some("model")); + } + + #[test] + fn missing_view_uses_fallback_message_and_generic_kind() { + let body = render_openai_envelope(None, UpstreamWire::Anthropic, "raw upstream text"); + assert_eq!(body.kind, "upstream_error"); + assert_eq!(body.message, "raw upstream text"); + assert!(body.code.is_none()); + } + + #[test] + fn missing_parsed_message_falls_back_to_raw_message() { + let v = UpstreamErrorView { + kind: Some("rate_limit_error".into()), + message: None, + code: None, + param: None, + }; + let body = render_openai_envelope(Some(&v), UpstreamWire::Anthropic, "raw fallback"); + assert_eq!(body.kind, "rate_limit_exceeded"); + assert_eq!(body.message, "raw fallback"); + } +} diff --git a/crates/aisix-proxy/src/lib.rs b/crates/aisix-proxy/src/lib.rs index 765c4472..0191ae7a 100644 --- a/crates/aisix-proxy/src/lib.rs +++ b/crates/aisix-proxy/src/lib.rs @@ -34,6 +34,7 @@ pub(crate) mod cooldown; mod dispatch; mod embeddings; mod error; +mod error_translate; pub mod health; mod http_client; mod images; @@ -723,6 +724,140 @@ mod tests { assert_eq!(v["error"]["type"], "upstream_error"); } + /// Issue #322: when an OpenAI upstream returns a coded 4xx with the + /// standard `{error:{message,type,code,param}}` envelope, every + /// field reaches the customer verbatim. SDKs that switch on + /// `error.code` to decide retry strategy depend on this — flattening + /// to a generic `upstream_error` envelope silently downgrades their + /// retry intelligence. + #[tokio::test] + async fn upstream_openai_4xx_forwards_full_envelope_per_issue_322() { + let upstream = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(429).set_body_raw( + br#"{"error":{"message":"upstream forced 429","type":"upstream_test_fixture","code":"forced_429","param":"model"}}"#.as_slice(), + "application/json", + )) + .mount(&upstream) + .await; + + let hub = Arc::new(Hub::new()); + hub.register(Provider::Openai, Arc::new(openai_test_bridge())); + let snap = seed_snapshot("my-gpt4", &["my-gpt4"], &upstream.uri()); + let app = build_router(build_state(snap, hub)); + + let req = Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("authorization", "Bearer sk-caller") + .header("content-type", "application/json") + .body(Body::from( + r#"{"model":"my-gpt4","messages":[{"role":"user","content":"hi"}]}"#, + )) + .unwrap(); + let resp = run(app, req).await; + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + let bytes = to_bytes(resp.into_body(), 2048).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(v["error"]["message"], "upstream forced 429"); + assert_eq!(v["error"]["type"], "upstream_test_fixture"); + assert_eq!(v["error"]["code"], "forced_429"); + assert_eq!(v["error"]["param"], "model"); + } + + /// Issue #322 fallback contract: when the upstream body is not a + /// recognisable JSON envelope (HTML error page, garbled text), the + /// gateway must NOT crash or surface raw bytes; it falls back to + /// the generic `upstream_error` envelope with the truncated body + /// as `message`. This pins the content-type guard. + #[tokio::test] + async fn upstream_4xx_non_json_body_falls_back_to_generic_envelope() { + let upstream = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(400).set_body_raw( + b"403 Forbidden by WAF".as_slice(), + "text/html", + )) + .mount(&upstream) + .await; + + let hub = Arc::new(Hub::new()); + hub.register(Provider::Openai, Arc::new(openai_test_bridge())); + let snap = seed_snapshot("my-gpt4", &["my-gpt4"], &upstream.uri()); + let app = build_router(build_state(snap, hub)); + + let req = Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("authorization", "Bearer sk-caller") + .header("content-type", "application/json") + .body(Body::from( + r#"{"model":"my-gpt4","messages":[{"role":"user","content":"hi"}]}"#, + )) + .unwrap(); + let resp = run(app, req).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let bytes = to_bytes(resp.into_body(), 2048).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + // Generic envelope: type=upstream_error, message contains the + // truncated raw body (no JSON parse attempted on text/html). + assert_eq!(v["error"]["type"], "upstream_error"); + assert!(v["error"].get("code").is_none() || v["error"]["code"].is_null()); + } + + /// Issue #322 sanity check on the 5xx branch: upstream 5xx still + /// collapses to 502 with the generic envelope AND the upstream + /// `error.message` is suppressed. Engine names / shard ids / queue + /// depth routinely appear in upstream 5xx bodies (in this fixture: + /// "engine offline shard 47") — those are operator-internal and + /// must not bleed through to the customer envelope. + #[tokio::test] + async fn upstream_openai_5xx_with_json_envelope_collapses_and_redacts_message() { + let upstream = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(503).set_body_raw( + br#"{"error":{"message":"engine offline shard 47","type":"server_error","code":"engine_overloaded"}}"#.as_slice(), + "application/json", + )) + .mount(&upstream) + .await; + + let hub = Arc::new(Hub::new()); + hub.register(Provider::Openai, Arc::new(openai_test_bridge())); + let snap = seed_snapshot("my-gpt4", &["my-gpt4"], &upstream.uri()); + let app = build_router(build_state(snap, hub)); + + let req = Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("authorization", "Bearer sk-caller") + .header("content-type", "application/json") + .body(Body::from( + r#"{"model":"my-gpt4","messages":[{"role":"user","content":"hi"}]}"#, + )) + .unwrap(); + let resp = run(app, req).await; + assert_eq!(resp.status(), StatusCode::BAD_GATEWAY); + let bytes = to_bytes(resp.into_body(), 2048).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(v["error"]["type"], "upstream_error"); + let msg = v["error"]["message"].as_str().unwrap(); + assert!( + !msg.contains("engine offline") && !msg.contains("shard 47"), + "upstream 5xx `error.message` must NOT leak to customer; got: {msg:?}" + ); + // Upstream `code` (engine_overloaded) must also not pass + // through on 5xx. + assert!( + v["error"].get("code").is_none() || v["error"]["code"].is_null(), + "upstream 5xx `error.code` must not pass through; got code={:?}", + v["error"]["code"] + ); + } + /// Cross-provider contract: Anthropic upstream 5xx → client sees an /// OpenAI-shape envelope `{error:{type:"upstream_error",...}}` with /// status 502 (collapsed per `BridgeError::http_status`, see @@ -770,9 +905,13 @@ mod tests { assert!(v["error"]["message"].is_string()); } - /// Cross-provider 4xx pass-through: Anthropic upstream 400 reaches - /// the client as 400 + OpenAI-shape envelope (status flows from - /// `BridgeError::UpstreamStatus.http_status()` 4xx branch). + /// Cross-provider 4xx translation: Anthropic upstream 400 reaches + /// the OpenAI-client side with the OpenAI-shape `error.type` / + /// `error.code` derived from Anthropic's `error.type` via the + /// translation table in [`crate::error_translate`]. Anthropic + /// `invalid_request_error` maps to OpenAI `invalid_request_error` + /// (taxonomy overlap — same token); other Anthropic types like + /// `rate_limit_error` map to distinct OpenAI tokens. #[tokio::test] async fn upstream_anthropic_400_passes_through_with_openai_envelope() { use aisix_provider_anthropic::AnthropicBridge; @@ -780,8 +919,9 @@ mod tests { let upstream = MockServer::start().await; Mock::given(method("POST")) .and(path("/v1/messages")) - .respond_with(ResponseTemplate::new(400).set_body_string( - r#"{"type":"error","error":{"type":"invalid_request_error","message":"bad input"}}"#, + .respond_with(ResponseTemplate::new(400).set_body_raw( + br#"{"type":"error","error":{"type":"invalid_request_error","message":"bad input"}}"#.as_slice(), + "application/json", )) .mount(&upstream) .await; @@ -811,7 +951,60 @@ mod tests { assert_eq!(resp.status(), StatusCode::BAD_REQUEST); let v: serde_json::Value = serde_json::from_slice(&to_bytes(resp.into_body(), 1024).await.unwrap()).unwrap(); - assert_eq!(v["error"]["type"], "upstream_error"); + assert_eq!(v["error"]["type"], "invalid_request_error"); + assert_eq!(v["error"]["message"], "bad input"); + // Anthropic `invalid_request_error` doesn't derive an OpenAI + // string code — translation table emits `code: null`. + assert!(v["error"].get("code").is_none() || v["error"]["code"].is_null()); + } + + /// Issue #322 cross-wire contract: Anthropic upstream `rate_limit_error` + /// must translate to OpenAI `rate_limit_exceeded` (both as + /// `error.type` and `error.code`) so OpenAI SDK retry logic that + /// switches on `error.code` recognises the rate-limit failure + /// regardless of which upstream the gateway routed to. + #[tokio::test] + async fn upstream_anthropic_rate_limit_translates_to_openai_rate_limit_exceeded() { + use aisix_provider_anthropic::AnthropicBridge; + + let upstream = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/messages")) + .respond_with(ResponseTemplate::new(429).set_body_raw( + br#"{"type":"error","error":{"type":"rate_limit_error","message":"slow down"}}"#.as_slice(), + "application/json", + )) + .mount(&upstream) + .await; + + let snap = AisixSnapshot::new(); + snap.provider_keys + .insert(matrix_anthropic_pk(&upstream.uri())); + snap.models.insert(anthropic_model_entry("my-claude")); + snap.apikeys + .insert(apikey_entry("sk-caller", &["my-claude"])); + let hub = Arc::new(Hub::new()); + hub.register(Provider::Anthropic, Arc::new(AnthropicBridge::new())); + let app = build_router(build_state(snap, hub)); + + let body = serde_json::json!({ + "model": "my-claude", + "messages": [{"role": "user", "content": "hi"}] + }); + let req = Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("authorization", "Bearer sk-caller") + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(); + let resp = run(app, req).await; + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + let v: serde_json::Value = + serde_json::from_slice(&to_bytes(resp.into_body(), 1024).await.unwrap()).unwrap(); + assert_eq!(v["error"]["type"], "rate_limit_exceeded"); + assert_eq!(v["error"]["code"], "rate_limit_exceeded"); + assert_eq!(v["error"]["message"], "slow down"); } /// Garbage upstream body (200 + non-JSON) must surface as 502 with