fix(proxy): preserve upstream error.code/type/param on 4xx + cross-wire translation (#322) - #323
Merged
Merged
Conversation
…-through (#322) Per issue #322, the data plane was forwarding upstream 4xx status and `error.message` but dropping `error.code` (and silently rewrapping `error.type` to `"upstream_error"`). OpenAI SDKs and downstream tooling switch on `error.code` to pick retry strategy — `rate_limit_exceeded` vs `insufficient_quota` vs `model_not_found` carry different recovery paths — so flattening the taxonomy silently downgrades every customer's retry intelligence. Root cause in `aisix-provider-*::map_http_error`: each bridge read the upstream body as plain text and stuffed it into `BridgeError::UpstreamStatus.message`. The struct had no slot for `kind`/`code`/`param`, and `ProxyError::envelope()` hardcoded `type = "upstream_error"`. Changes: * `aisix-gateway::bridge`: add `UpstreamErrorView` (kind / message / code / param) and `UpstreamWire` (provider wire-format tag) on `BridgeError::UpstreamStatus`. New `capture_upstream_error_http` helper handles the HTTP flavour with a 64KB body cap + content-type guard (skip parse on non-JSON to avoid feeding HTML error pages from WAFs/CDNs to a JSON parser). The view is `Option<Box<_>>` to keep `Result<_, ProxyError>` under clippy's `result_large_err` threshold. * OpenAI bridge: parse `{error:{message,type,code,param}}` into the view. * Anthropic bridge: parse `{type:"error",error:{type,message}}` (no `code`/`param`). * Vertex bridge: parse `error.status` only — Vertex `error.message` embeds operator project ids (audit-aware redaction unchanged). * Azure OpenAI bridge: parse `error.code` with the `inner_error` / `innererror` content-policy quirk (`ResponsibleAIPolicyViolation` on the inner code overrides the outer code). * Bedrock bridge: use `InvokeModelError::meta().code()` to extract the AWS exception name (`ThrottlingException` etc.) without consuming the typed error; `parsed.message` stays `None` per existing audit guidance (AWS messages embed ARNs / account ids / role names). * `ProxyError::envelope()`: for `Bridge(UpstreamStatus { wire: OpenAI, .. })` on a 4xx with a parsed view, forward `message`/`type`/`code`/`param` verbatim. 5xx and non-OpenAI-wire upstreams keep the legacy generic `upstream_error` envelope in this commit — cross-wire translation ships in the follow-up. * `ErrorBody.kind`: widened from `&'static str` to `String` to carry upstream-derived OpenAI taxonomy tokens. Tests: * `upstream_openai_4xx_forwards_full_envelope_per_issue_322`: pins the contract — all four fields round-trip. * `upstream_4xx_non_json_body_falls_back_to_generic_envelope`: pins the content-type guard (HTML body → generic envelope, not a misparse). * `upstream_openai_5xx_with_json_envelope_still_collapses_to_502`: pins that 5xx does NOT pass through — upstream engine names / queue depth stay operator-internal. * Existing `upstream_429_passes_through_with_openai_envelope` and `upstream_anthropic_*` kept and still pass (they test the fallback path, which is still correct). Cross-wire translation (Anthropic / Bedrock / Vertex / Azure upstream → OpenAI client taxonomy via translation tables) ships in commit 2 of this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…322) Builds on the previous commit's structured `UpstreamErrorView`. The envelope-rendering layer now translates non-OpenAI upstream errors into OpenAI-shape `error.type` / `error.code` so OpenAI SDK retry logic behaves the same regardless of which upstream the gateway routed to. * New `crates/aisix-proxy/src/error_translate.rs` with per-wire tables: - **Anthropic**: `rate_limit_error` → `(rate_limit_exceeded, rate_limit_exceeded)`, `overloaded_error` → `(api_error, overloaded)`, `authentication_error` → `(invalid_request_error, invalid_api_key)`, etc. Covers the full taxonomy from docs.anthropic.com/en/api/errors. - **Bedrock**: `ThrottlingException` → `(rate_limit_exceeded, ...)`, `ServiceQuotaExceededException` → `(rate_limit_exceeded, insufficient_quota)` — the distinction matters because SDK retry paths differ (backoff vs quota lift). Covers the full `InvokeModelError` variant set. - **Vertex**: canonical gRPC `Status.code` (`RESOURCE_EXHAUSTED`, `PERMISSION_DENIED`, `UNAUTHENTICATED`, `DEADLINE_EXCEEDED`, …) → OpenAI taxonomy. Source: cloud.google.com/apis/design/errors and the `google.rpc.Code` enum. - **Azure OpenAI**: pass-through for OpenAI-compat codes (Azure shares the OpenAI taxonomy for most codes); Azure-specific tokens (`DeploymentNotFound`, `ResponsibleAIPolicyViolation`, `invalid_encrypted_content`) explicitly mapped. - **OpenAI (same wire)**: pass-through, including forward-compat for OpenAI taxonomy additions not in any table. * `ProxyError::envelope()` now delegates to `render_openai_envelope()` on any 4xx upstream error with a known `UpstreamWire`. `UpstreamWire::Unknown` (cooldown test fixtures / synthesised errors) keeps the legacy generic `upstream_error` envelope, preserving existing behaviour for non-real-upstream paths. * Tests: - 22 unit tests in `error_translate::tests` covering every row of every translation table plus pass-through, fallback, and missing-view branches. - Tightened `upstream_anthropic_400_passes_through_with_openai_envelope` — asserts the Anthropic envelope is actually parsed and the OpenAI `error.type` / `error.message` reach the client. - New `upstream_anthropic_rate_limit_translates_to_openai_rate_limit_exceeded` pins the cross-wire translation path that the issue's customer impact statement calls out: Anthropic `rate_limit_error` must surface as OpenAI `rate_limit_exceeded` (both `type` and `code`) so SDK retry logic recognises it. Why this is more rigorous than the established reference impl: * Reference uses substring matching on the raw error message string (e.g. `"ThrottlingException" in error_str`) rather than structured parsing — it works for known cases but misses several variants (`ServiceQuotaExceededException`, `ModelTimeoutException`, etc.) and is brittle to AWS/Google message-text changes. * Reference emits `error.code` as the HTTP status string (`"429"`) rather than the OpenAI string code (`"rate_limit_exceeded"`). The customer SDK retry logic that switches on `code` to distinguish `rate_limit_exceeded` from `insufficient_quota` from `model_not_found` only works with the string-code form, which is what we now emit. This is a quality bar above the reference impl. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughGateway adds capped body reading, JSON detection, UTF-8-safe truncation, and UpstreamErrorView/UpstreamWire; provider bridges populate parsed views and wire tags; the proxy translates parsed views into OpenAI-shaped client envelopes or emits a generic upstream_error envelope. ChangesUpstream Error Handling & Client-Facing Translation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
This PR improves OpenAI-compatible error handling in aisix-proxy by (1) preserving upstream OpenAI error envelope fields (message, type, code, param) on 4xx pass-through and (2) translating non-OpenAI upstream error taxonomies (Anthropic/Bedrock/Vertex/Azure) into OpenAI-style error.type/error.code tokens so downstream SDK retry logic can work reliably.
Changes:
- Add structured upstream error capture (
UpstreamErrorView+UpstreamWire) and route proxy error rendering through a translation layer for 4xx upstream responses. - Implement
error_translatemappings for Anthropic/Bedrock/Vertex/Azure → OpenAI taxonomy and expand proxy tests to pin same-wire forwarding + cross-wire translation. - Add capped upstream error body reading + JSON content-type guard for HTTP error parsing in the gateway bridge helper.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/aisix-proxy/src/lib.rs | Registers error_translate module and adds/updates integration tests for 4xx forwarding and translation. |
| crates/aisix-proxy/src/error.rs | Extends error envelope rendering to pass through/translate structured upstream 4xx errors. |
| crates/aisix-proxy/src/error_translate.rs | New cross-wire translation tables + renderer to OpenAI-shaped ErrorBody. |
| crates/aisix-provider-openai/src/bridge.rs | Uses shared upstream error capture and parses canonical OpenAI error envelope. |
| crates/aisix-provider-anthropic/src/bridge.rs | Uses shared upstream error capture and parses Anthropic error envelope. |
| crates/aisix-provider-bedrock/src/bridge.rs | Populates structured upstream kind from AWS SDK error code while keeping messages redacted. |
| crates/aisix-provider-vertex/src/bridge.rs | Parses Vertex gRPC status token into structured upstream view (message redacted). |
| crates/aisix-provider-azure-openai/src/bridge.rs | Extracts Azure error codes (including inner_error quirk) into structured upstream view (message redacted). |
| crates/aisix-gateway/src/lib.rs | Re-exports new upstream error capture utilities/types/constants. |
| crates/aisix-gateway/src/bridge.rs | Introduces capped upstream error body reading, JSON content-type guard, and structured capture helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+72
to
+78
| // Same-wire OpenAI/Azure preserve any upstream-supplied code. | ||
| // Other wires get a derived code (or `None` when no clean | ||
| // OpenAI counterpart exists) — the upstream's `code` field is | ||
| // either absent (Anthropic, Bedrock, Vertex) or operator-leaky | ||
| // (Vertex numeric codes embed internal taxonomy). | ||
| code: match wire { | ||
| UpstreamWire::OpenAI | UpstreamWire::AzureOpenAI => view.code.clone().or(derived_code), |
Comment on lines
+267
to
+283
| 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(); | ||
| while let Some(chunk) = stream.next().await { | ||
| let Ok(chunk) = chunk else { break }; | ||
| let remaining = limit.saturating_sub(buf.len()); | ||
| if remaining == 0 { | ||
| break; | ||
| } | ||
| let take = chunk.len().min(remaining); | ||
| buf.extend_from_slice(&chunk[..take]); | ||
| if buf.len() >= limit { | ||
| break; | ||
| } | ||
| } | ||
| buf.freeze() |
Comment on lines
287
to
291
| 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 body = resp.bytes().await.unwrap_or_default(); | ||
| let kind = parse_azure_error_code(&body); | ||
| let message = match status.as_u16() { |
Comment on lines
272
to
276
| 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 body = resp.bytes().await.unwrap_or_default(); | ||
| let kind = parse_vertex_error_status(&body); | ||
| let message = match status.as_u16() { |
…, truncate parsed.message, pin wire/parsed in bridge tests Independent audit on PR #323 surfaced two HIGH and three MEDIUM findings. This commit addresses all five. **HIGH-1**: `MAX_UPSTREAM_ERROR_BODY_BYTES` (64 KB) cap was only enforced on bridges that route through `capture_upstream_error_http` (OpenAI, Anthropic). Vertex and Azure called `resp.bytes().await` unbounded — a misbehaved or hostile upstream returning a giant error body could pin a worker's memory. Fix: expose `read_body_capped` from `aisix-gateway` and have Vertex/Azure consume it. (Bedrock buffers via the smithy SDK before we see the response, so its read is already bounded by the AWS SDK's response-buffer policy.) **HIGH-2**: `parsed.message` was stored verbatim from the upstream envelope — only the outer `BridgeError::UpstreamStatus.message` was truncated. An OpenAI upstream emitting a 60 KB `error.message` would ship 60 KB to the customer envelope through the parsed-view path, defeating the documented 1024-byte cap. Fix: apply `truncate_lossy` to `view.message` at construction time inside `capture_upstream_error_http`. **MEDIUM-2**: Every bridge's `BridgeError::UpstreamStatus` test destructure used `..` for the new `wire` and `parsed` fields, so a copy-paste error that left `wire: UpstreamWire::Unknown` or `parsed: None` would never fail a test. Fix: tighten one representative test per bridge to bind both fields and assert their shape, plus add structured-parse tests for the bridges where the audit confirmed the redaction contract matters most (Vertex 429 with `error.status`, Azure 400 with `inner_error.code`). **MEDIUM-3**: No test pinned the 64 KB body cap actually firing on oversized bodies, and no test confirmed the new HIGH-2 fix triggers on oversized `parsed.message`. Fix: two new tests in the OpenAI bridge that exercise both via wiremock with 200 KB / 60 KB bodies. **MEDIUM-1** (Bedrock/Vertex/Azure cross-wire e2e through the proxy): explicitly justified. The proxy integration test scaffolding in `aisix-proxy::tests` only carries fixture helpers for OpenAI and Anthropic; adding equivalents for the SDK-backed Bedrock bridge (which needs an aws-sdk mock layer) and the regional Vertex/Azure URL shapes would expand this PR's scope significantly. Coverage of the same code path is provided by (a) the 22 `error_translate::tests` unit tests covering every translation-table row, and (b) the new bridge-level tests that pin `wire` and `parsed` shape — together they cover the same logic an integration test would exercise through the proxy. Tracking issue to add proxy-level e2e for the three newer bridges can be filed as a follow-up once the bridge crates grow shared mock infrastructure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…or casing variant Follow-up to the audit's second pass. Two concrete items closed: 1. **Defensive truncation** on every parsed field, not just `parsed.message`. AWS exception codes / Anthropic types / OpenAI codes are bounded vocabulary today, but a hostile or buggy upstream emitting a 60 KB `error.code` / `error.type` / `error.param` would have flowed through the parsed view unchecked. Apply `truncate_lossy(_, MAX_UPSTREAM_ERROR_MESSAGE_BYTES)` to all four string fields at construction time inside `capture_upstream_error_http`. Same shape as the prior HIGH-2 fix for `parsed.message`. 2. **Azure casing variant coverage**: Azure emits the content-policy inner-error key as either `inner_error` (most docs) or `innererror` (some endpoints). The parser already handles both — the prior fix commit only tested the `inner_error` casing. Add a test for the `innererror` casing to pin both code paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+264
to
+267
| let message = parsed | ||
| .as_ref() | ||
| .and_then(|v| v.message.clone()) | ||
| .unwrap_or_else(|| String::from_utf8_lossy(&body).into_owned()); |
| // 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`. |
| Box::new(aisix_gateway::UpstreamErrorView { | ||
| kind: kind.clone(), | ||
| message: None, | ||
| code: None, |
Comment on lines
+273
to
+276
| let retry_after = aisix_gateway::parse_retry_after(resp.headers()); | ||
| let _ = resp.text().await; // drain body, discard content | ||
| let body = | ||
| aisix_gateway::read_body_capped(resp, aisix_gateway::MAX_UPSTREAM_ERROR_BODY_BYTES).await; | ||
| let kind = parse_vertex_error_status(&body); |
…ped reads, redact 5xx message, Vertex content-type guard Five concrete fixes from the Copilot inline review on PR #323. Two stale comments (#3, #4 — already fixed in commit 3) are skipped. **#1+#7 — Azure OpenAI-compatible code preservation.** Azure's envelope omits `error.type` and carries only `error.code`. The bridge previously put the upstream code into `view.kind` and left `view.code` as `None`. For OpenAI-compat tokens Azure inherits unchanged (e.g. `rate_limit_exceeded`), this meant downstream OpenAI clients received `error.type=rate_limit_exceeded` but `error.code=null` — exactly the SDK-retry break issue #322 is about. Fix: - Azure parser populates BOTH `view.kind` AND `view.code` from the upstream `error.code` field. - `render_openai_envelope`'s AzureOpenAI branch now prefers the translation-table-derived code (so explicit Azure tokens like `DeploymentNotFound` → `model_not_found` still win), falling back to `view.code` for OpenAI-compat pass-through. **#2 — Drain the response stream after hitting the cap.** `read_body_capped` previously broke out of the read loop the moment `limit` bytes were buffered. With reqwest/hyper that leaves unread bytes in the response and prevents connection reuse — during a burst of upstream errors the gateway would churn TCP connections instead of recycling the keep-alive pool. Fix: keep iterating the stream, discarding chunks past the cap. Memory stays bounded by `limit`. **#5 — Redact upstream `error.message` on 5xx.** The 5xx branch of `render_bridge_upstream_envelope` was forwarding `BridgeError::UpstreamStatus.message` verbatim — which for OpenAI / Anthropic comes from the parsed upstream `error.message`. Upstream 5xx bodies routinely embed operator-internal detail (engine names, shard ids, queue depth). Fix: on 5xx, emit a canned `"upstream returned {status}"` message; the full upstream body remains in operator logs via tracing. **#6 — Stale "follow-up" comment.** The docstring on `render_bridge_upstream_envelope` claimed cross-wire translation would ship in a follow-up, but it already shipped in commit 2. Rewrite the comment to describe current behaviour (4xx → `error_translate`; 5xx → canned envelope; `Unknown` wire → legacy generic envelope). **#8 — Content-type guard on Vertex (and Azure, while at it).** `capture_upstream_error_http` already gates serde parsing on `Content-Type: application/json` so a 64 KB HTML error page from a fronting WAF doesn't waste CPU on a doomed JSON parse. The Vertex and Azure bridges call serde directly because they need a custom parse path (canned message for redaction) — same guard now applies. Promoted `content_type_is_json` and added a `response_is_json` helper to the gateway's public surface; both bridges call it before `parse_*_error_*`. New tests: - `upstream_openai_5xx_with_json_envelope_collapses_and_redacts_message` pins the 5xx redaction (asserts `engine offline` / `shard 47` / `engine_overloaded` don't reach the customer envelope). - `chat_429_preserves_openai_compatible_code_for_sdk_retry` (Azure) pins that `parsed.code` carries the OpenAI-compat upstream code. - `chat_400_non_json_body_skips_envelope_parse` (Azure) and `chat_gemini_non_json_body_skips_envelope_parse` (Vertex) pin the new content-type guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 18, 2026
Merged
Merged
moonming
added a commit
that referenced
this pull request
May 25, 2026
…audit HIGH/MEDIUM on #389) Independent audit-aigw-389-bedrock-converse flagged 2 HIGH + 2 MEDIUM bugs in the original Converse wiring. All fixed in this commit; 45/45 tests pass. ## HIGH-1 + HIGH-2 — `map_aws_sdk_error_generic` silently regressed the legacy /invoke path's error envelope hardening The original helper collapsed all ServiceError responses to `BridgeError::upstream_status(status, msg)`, a convenience constructor that sets `wire: Unknown` + `parsed: None` + `retry_after: None`. The legacy `chat_anthropic` /invoke path uses `map_service_error` which builds a fully-shaped `BridgeError::UpstreamStatus` with: - wire: Bedrock — required by error_translate to render Bedrock- shape errors back to OpenAI/Anthropic-shape clients - retry_after: parsed from upstream Retry-After header — required by the cooldown layer to honour AWS throttle hints - parsed.kind: extracted via .meta().code() — distinguishes a ThrottlingException 429 from a different throttle source Without this fix every non-Anthropic publisher + all streaming silently shipped degraded errors vs. the legacy path — broke the PR #323 audit (MEDIUM-2) hardening it had just restored. Fix: introduce `bedrock_service_error_to_upstream_status` (generic over `E: ProvideErrorMetadata`) that mirrors `map_service_error`'s field-by-field construction. Both Converse + ConverseStream errors now flow through it. ## MEDIUM-1 — `e.to_string()` produced opaque "service error" strings as the customer-visible message Same root cause; resolved by the HIGH fix above. The customer- visible `message` now uses the legacy path's canned status-keyed phrase ("upstream rate limited", "upstream authentication failed", etc.) — preserving the operator-ARN redaction the legacy path also enforces. ## MEDIUM-2 — `chat_converse` silently dropped ChatFormat's temperature/max_tokens/top_p The original `chat_converse` constructed an `InferenceConfiguration::builder()` and immediately discarded it with `let _ =`. Every non-Anthropic Bedrock customer using these knobs would see them vanish — real behavioural regression vs. the legacy chat_anthropic path which forwards them via build_request → AnthropicRequest. Fix: new `build_inference_config(req)` helper extracts temperature / max_tokens / top_p when set, returns None when all absent (so we don't emit an empty `inferenceConfig: {}` which 400s on some Bedrock publishers). Wired into both chat_converse and chat_converse_stream. ## Tests added (5 new for the audit fixes) - chat_converse_maps_upstream_429_with_retry_after_and_bedrock_wire pins the HIGH-1+2 fix: 429 + Retry-After: 42 → UpstreamStatus with status=429, message="upstream rate limited", wire=Bedrock, retry_after=Some(42s), parsed.kind="ThrottlingException" - chat_converse_maps_upstream_4xx_to_canned_message_with_bedrock_wire pins HIGH-1 for the non-throttle path: 403 with leaky ARN body → canned "upstream authentication failed" (NO ARN leak), wire=Bedrock, parsed.kind="AccessDeniedException" - chat_converse_wires_temperature_max_tokens_top_p_into_inference_config pins MEDIUM-2: ChatFormat knobs reach Bedrock's body as camelCase {temperature, maxTokens, topP} - chat_converse_omits_inference_config_when_no_knobs_set companion: empty knobs → no inferenceConfig field on the wire `cargo test -p aisix-provider-bedrock` → 45/45 PASS (was 41; +4 new audit-regression tests). `cargo clippy -p aisix-provider-bedrock --all-targets -- -D warnings` clean. ## LOWs (non-blocking, deferred per CLAUDE.md §8) - LOW-1: Role::Tool messages silently dropped on Converse path — deliberate no-op pending a structured tool-use surface on ChatFormat. Documented in `build_converse_inputs`. - LOW-2: Per-publisher dispatch test matrix coverage (Mistral / Cohere / AmazonTitan / AI21 not individually pinned). The match arm in chat() is structurally uniform (`_ => chat_converse`) so routing is correct by construction; the two existing Meta + Amazon Nova tests already cover the non-Anthropic dispatch contract.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #322. Two commits:
fix(proxy): preserve upstream error.code/type/param on 4xx pass-through— structuredUpstreamErrorViewonBridgeError::UpstreamStatus, all 5 bridges populate it from their respective upstream envelope shape,ProxyError::envelope()forwards same-wire OpenAI fields verbatim.fix(proxy): translate cross-wire error envelopes to OpenAI taxonomy—error_translate.rstranslates Anthropic / Bedrock / Vertex / Azure upstream taxonomy to OpenAIerror.type/error.codeso SDK retry logic works regardless of routed upstream.Customer-visible behaviour
Before this PR, a customer using an OpenAI SDK against the gateway, routed to any upstream, would see a flattened envelope when the upstream returned 4xx:
{ "error": { "message": "upstream returned HTTP 429: <raw body string>", "type": "upstream_error" } }SDKs that switch on
error.code(rate_limit_exceededvsinsufficient_quotavsmodel_not_found) lost all retry intelligence on any coded 4xx.After this PR:
{ "error": { "message": "upstream forced 429", "type": "rate_limit_exceeded", "code": "rate_limit_exceeded", "param": "model" } }error.codefield #322 case).rate_limit_error→(rate_limit_exceeded, rate_limit_exceeded),overloaded_error→(api_error, overloaded), etc. SDK retry logic recognises these.ThrottlingException→(rate_limit_exceeded, rate_limit_exceeded),ServiceQuotaExceededException→(rate_limit_exceeded, insufficient_quota)— distinguishing backoff vs quota-lift recovery paths.RESOURCE_EXHAUSTED→(rate_limit_exceeded, rate_limit_exceeded),PERMISSION_DENIED→(invalid_request_error, permission_denied), etc.DeploymentNotFound,ResponsibleAIPolicyViolationvia theinner_error/innererrorcontent-policy quirk) explicitly mapped.5xx upstreams still collapse to 502 with the generic envelope — upstream internal-server detail (engine names, queue depth, ARNs, project ids) stays operator-internal.
Robustness
application/jsoncontent-type. HTML error pages from a fronting WAF or CDN fall back to the generic envelope instead of being fed to serde and producing garbage.parsed.kind(the AWS / gRPC / Azure exception name — a stable taxonomy label) but NOTparsed.message. The customer-visiblemessagestays the canned status-keyed phrase so ARNs / project ids / account ids / role names don't bleed through.svc.raw().http().status()as the authoritative status. The translation table only affectserror.type/error.code, never the status code.Reference impl divergences (CLAUDE.md §7)
"ThrottlingException" in error_str); we structurally parsemeta().code()from the typed SDK error /error.statusfield /error.typefield. Catches variants the substring approach misses (ServiceQuotaExceededException,ModelTimeoutException,RESOURCE_EXHAUSTEDcapitalisation, etc.) and is not brittle to upstream message-text changes.error.codeas the HTTP status string ("429"). We emit the OpenAI string code ("rate_limit_exceeded") per OpenAI docs §3 — this is what SDK retry logic actually reads.Test plan
cargo check --workspace --all-targetscleancargo clippy --workspace --all-targetsclean (no new warnings)cargo fmt --checkcleancargo test --workspace— 1021 tests pass (up from baseline; 22 newerror_translateunit tests + 4 new integration tests + 1 tightened cross-wire integration test)parse_openai_error_envelope,parse_anthropic_error_envelope,parse_azure_error_code,parse_vertex_error_status, Bedrockmeta().code()extraction)aisix-proxy::tests:upstream_openai_4xx_forwards_full_envelope_per_issue_322— verbatim 4-field forwarding on OpenAI same-wire (the issue's main contract)upstream_4xx_non_json_body_falls_back_to_generic_envelope— content-type guardupstream_openai_5xx_with_json_envelope_still_collapses_to_502— 5xx not passed throughupstream_anthropic_400_passes_through_with_openai_envelope— tightened to assert parsed fields, not justtypeupstream_anthropic_rate_limit_translates_to_openai_rate_limit_exceeded— cross-wire taxonomy translationexpect(err?.code).toBe('forced_429')that bug(dp): upstream 4xx forwarding loses the upstream-sideerror.codefield #322 explicitly asks for. The repro fixture in the Phase 1 spec exercises the OpenAI same-wire path that's covered byupstream_openai_4xx_forwards_full_envelope_per_issue_322here.Closes #322
Summary by CodeRabbit
New Features
Bug Fixes