fix(error): map customer-fixable upstream config to 400, not 500 (#367)

[jarvis9443]

Closes #367.

Problem

BridgeError::Config mapped every config error to HTTP 500. But many are customer-fixable upstream-config issues — empty provider_key.secret, missing api_base (the #365 guard), missing model_name, malformed request shape. A 500 tells the SDK to retry and monitoring to alert on a server fault, when the root cause is on the operator's side.

Fix

A distinct BridgeError::InvalidUpstreamConfig variant → 400 / error.type: "invalid_request_error", and non-retryable (a 4xx; retry/failover won't help). The customer-fixable construction sites across all five provider bridges (anthropic, openai, azure-openai, bedrock, vertex) are converted:

• empty provider_key.secret
• missing model_name
• the api_base family-bridge guard
• invalid api-key header chars
• split_system request-shape validation

Errors we cause stay BridgeError::Config → 500 (correct): body serialization, our generated request_id header encoding, internal request construction, and runtime credential/token minting (which can fail transiently). Azure URL-shape validation also stays 500 for now — a follow-up can move it to the new variant if desired.

Behavior change

Affected upstream-config errors now return 400 instead of 500. The error envelope carries type: "invalid_request_error". No change to successful requests or to genuine upstream/transport failures.

Tests

• Gateway: InvalidUpstreamConfig → 400 / invalid_request_error; Config stays 500.
• is_retryable(InvalidUpstreamConfig) is false.
• Each provider's converted-error tests now assert InvalidUpstreamConfig (these fail on the old Config mapping → fail-before/pass-after).
• E2E invalid-upstream-config-400: the issue's own scenario — an openrouter PK admitted without api_base returns 400, not 500. Verified against the live compose stack.

Summary by CodeRabbit

• Bug Fixes

  • Configuration validation errors now return HTTP 400 status instead of 500, with improved error messaging indicating which upstream settings are misconfigured.
  • Invalid configuration errors are no longer retried, preventing unnecessary upstream failures.

• Tests

  • Added validation test confirming upstream configuration errors return correct HTTP 400 status code.

[coderabbitai]

Warning

Review limit reached

@jarvis9443, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 4 minutes and 11 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74f898cc-7316-45c7-9fd9-f3a2304e5ab5

📥 Commits

Reviewing files that changed from the base of the PR and between da46c95 and 58fed0c.

📒 Files selected for processing (7)

• crates/aisix-provider-anthropic/src/bridge.rs
• crates/aisix-provider-azure-openai/src/bridge.rs
• crates/aisix-provider-bedrock/src/bridge.rs
• crates/aisix-provider-openai/src/bridge.rs
• crates/aisix-provider-vertex/src/bridge.rs
• crates/aisix-provider-vertex/src/token_mint.rs
• tests/e2e/src/cases/invalid-upstream-config-400-e2e.test.ts

📝 Walkthrough

Walkthrough

This PR introduces a new BridgeError::InvalidUpstreamConfig variant to distinguish customer-fixable upstream configuration failures from generic config errors. The variant maps to HTTP 400 and invalid_request_error type, is applied across all five provider bridges (OpenAI, Anthropic, Azure, Bedrock, Vertex) for validation failures, and is integrated into proxy error classification and retry logic with E2E validation.

Changes

Configuration error classification refactoring

Layer / File(s)	Summary
Core error variant definition and HTTP mapping crates/aisix-gateway/src/bridge.rs	BridgeError::InvalidUpstreamConfig(String) variant added with error message template. http_status() and error_type() methods extended to map the new variant to HTTP 400 and invalid_request_error. Unit tests verify the mapping.
Provider bridge configuration error handling crates/aisix-provider-openai/src/bridge.rs, crates/aisix-provider-anthropic/src/bridge.rs, crates/aisix-provider-azure-openai/src/bridge.rs, crates/aisix-provider-bedrock/src/bridge.rs, crates/aisix-provider-vertex/src/bridge.rs	All five provider bridges update configuration validation to emit InvalidUpstreamConfig instead of Config for missing/invalid api_base, api_key, model_name, and split_system failures. Unit tests updated to assert the new error variant across family-bridge safety checks, empty secret checks, missing model name, and invalid header character scenarios.
Proxy error classification and retry logic crates/aisix-proxy/src/chat.rs, crates/aisix-proxy/src/routing.rs	routing_error_class classifies InvalidUpstreamConfig as "invalid_config" for telemetry/logging. is_retryable marks the error as non-retryable (returns false). Unit test verifies non-retryability.
End-to-end HTTP 400 validation tests/e2e/src/cases/invalid-upstream-config-400-e2e.test.ts	E2E test provisions an intentionally misconfigured openrouter provider key with missing api_base, performs a chat completion request through the proxy, and asserts the resulting APIError maps to HTTP 400 status with invalid_request_error type instead of 500.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: introducing a new error variant that maps customer-fixable upstream config issues to HTTP 400 instead of 500.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/aisix-provider-anthropic/src/bridge.rs`:
- Around line 151-153: The api_key(ctx) helper currently only checks
k.is_empty() and returns potentially-invalid non-empty secrets to
chat()/chat_stream() which set the "x-api-key" header; update api_key(ctx) to
validate provider_key.secret contents (trim whitespace, reject whitespace-only,
and reject control/invalid bytes or non-printable characters) and return
Err(BridgeError::InvalidUpstreamConfig(...)) for invalid values referencing
provider_key.secret, so chat() and chat_stream() always receive a validated &str
for the header; keep the BridgeError::InvalidUpstreamConfig variant for these
cases and ensure error messages identify the invalid provider_key.secret.

In `@crates/aisix-provider-bedrock/src/bridge.rs`:
- Around line 474-475: The Bedrock credential validation currently returns
BridgeError::Config for customer-fixable issues; update BedrockSecret::parse to
return BridgeError::InvalidUpstreamConfig (with descriptive messages) instead of
BridgeError::Config for empty/invalid provider_key.secret and similar validation
failures, and change the empty-region check in build_client to return
BridgeError::InvalidUpstreamConfig when secret.region is empty; ensure the error
strings clearly reference the invalid field (e.g. "provider_key.secret
missing/invalid" and "secret.region missing") so they align with the new 400
contract and match the existing use of BridgeError::InvalidUpstreamConfig (as
used for missing model_name).

In `@crates/aisix-provider-openai/src/bridge.rs`:
- Around line 337-339: The malformed Bearer/token validation currently done in
build_request_headers (using HeaderValue::from_str ->
BridgeError::InvalidUpstreamConfig) is only applied to chat and chat_stream;
update embed, complete, and generate_image to perform the same validation so
malformed provider_key.secret surfaces as InvalidUpstreamConfig instead of
Transport errors—either call build_request_headers from those functions (embed,
complete, generate_image) or replicate the
HeaderValue::from_str(...).map_err(|e| BridgeError::InvalidUpstreamConfig(...))?
logic before calling .header(header::AUTHORIZATION, ...) so the Authorization
header is validated consistently.

In `@crates/aisix-provider-vertex/src/bridge.rs`:
- Around line 2627-2630: The test asserts InvalidUpstreamConfig for missing
model_name but resolve_api_base() still returns BridgeError::Config for invalid
provider_key.api_base inputs, causing those errors to be treated as 500; update
resolve_api_base() to return BridgeError::InvalidUpstreamConfig (with a
descriptive message) for each branch that currently returns BridgeError::Config
on invalid api_base (the same branches referenced in Lines 187-232), so
upstream-config validation failures consistently use InvalidUpstreamConfig;
search for resolve_api_base, replace the BridgeError::Config returns for
api_base validation with BridgeError::InvalidUpstreamConfig while preserving the
original error text.

In `@tests/e2e/src/cases/invalid-upstream-config-400-e2e.test.ts`:
- Line 68: Rename the test title string used in the test invocation that
currently reads "empty provider_key secret surfaces as a 400, not a 500" to
accurately describe the scenario being validated (missing api_base with a
non-empty secret); update the test(...) call so its first argument reflects
something like "missing api_base with non-empty provider secret surfaces as a
400, not a 500" to avoid confusion during triage (locate the test by the
test(...) invocation in invalid-upstream-config-400-e2e.test.ts).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d194f81-da6b-43a2-9854-6861f0d7a203

📥 Commits

Reviewing files that changed from the base of the PR and between 961ea3b and da46c95.

📒 Files selected for processing (9)

• crates/aisix-gateway/src/bridge.rs
• crates/aisix-provider-anthropic/src/bridge.rs
• crates/aisix-provider-azure-openai/src/bridge.rs
• crates/aisix-provider-bedrock/src/bridge.rs
• crates/aisix-provider-openai/src/bridge.rs
• crates/aisix-provider-vertex/src/bridge.rs
• crates/aisix-proxy/src/chat.rs
• crates/aisix-proxy/src/routing.rs
• tests/e2e/src/cases/invalid-upstream-config-400-e2e.test.ts