fix(pi): align credential retry scope

This commit is contained in:
SaladDay
2026-08-03 08:32:41 +00:00
parent a2fa86bea1
commit fff5d7ede9
4 changed files with 104 additions and 43 deletions
+84 -39
View File
@@ -204,9 +204,10 @@ pub(crate) async fn handle_pi_native(
};
let status = response.status();
if retryable_status(status) && network_budget.has_remaining() {
let status_disposition = upstream_status_disposition(status);
if status_disposition.is_retryable() && network_budget.has_remaining() {
let error = format!("Pi upstream returned retryable status {status}");
let status_health = provider_health_disposition(status);
let status_health = status_disposition.provider_health();
if status_health == ProviderHealthDisposition::Unhealthy {
provider_health_failure = Some(error.clone());
}
@@ -223,7 +224,17 @@ pub(crate) async fn handle_pi_native(
provider_health_failure.as_deref(),
),
});
continue;
match status_disposition {
UpstreamStatusDisposition::RetryEndpoint => continue,
// Every endpoint in one provider group is cloned from the
// same credential plan. Preserve this response as a
// fallback, but reserve the remaining network budget for
// a provider that can own a different credential.
UpstreamStatusDisposition::RetryProvider => break,
UpstreamStatusDisposition::ReturnResponse => {
unreachable!("a non-retryable status cannot enter the retry branch")
}
}
}
let selected_is_failover = materialized.is_failover;
let provider_health =
@@ -507,6 +518,54 @@ enum ProviderHealthDisposition {
Neutral,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum UpstreamStatusDisposition {
ReturnResponse,
RetryEndpoint,
RetryProvider,
}
impl UpstreamStatusDisposition {
const fn is_retryable(self) -> bool {
!matches!(self, Self::ReturnResponse)
}
const fn provider_health(self) -> ProviderHealthDisposition {
match self {
Self::ReturnResponse => ProviderHealthDisposition::Healthy,
Self::RetryEndpoint => ProviderHealthDisposition::Unhealthy,
Self::RetryProvider => ProviderHealthDisposition::Neutral,
}
}
}
fn upstream_status_disposition(status: StatusCode) -> UpstreamStatusDisposition {
if matches!(status, StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN) {
// Pi's provider owns authentication (API key or OAuth), while its
// custom endpoints only replace the URL. Authentication rejection is
// therefore neutral for endpoint health and can only benefit from a
// distinct provider credential.
return UpstreamStatusDisposition::RetryProvider;
}
if (!status.is_client_error() && !status.is_server_error())
|| matches!(
status,
StatusCode::BAD_REQUEST
| StatusCode::METHOD_NOT_ALLOWED
| StatusCode::NOT_ACCEPTABLE
| StatusCode::PAYLOAD_TOO_LARGE
| StatusCode::URI_TOO_LONG
| StatusCode::UNSUPPORTED_MEDIA_TYPE
| StatusCode::UNPROCESSABLE_ENTITY
| StatusCode::NOT_IMPLEMENTED
)
{
UpstreamStatusDisposition::ReturnResponse
} else {
UpstreamStatusDisposition::RetryEndpoint
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct ProviderHealthOutcome {
disposition: ProviderHealthDisposition,
@@ -515,7 +574,7 @@ struct ProviderHealthOutcome {
impl ProviderHealthOutcome {
fn from_status(status: StatusCode, prior_failure: Option<&str>) -> Self {
match provider_health_disposition(status) {
match upstream_status_disposition(status).provider_health() {
ProviderHealthDisposition::Healthy => Self {
disposition: ProviderHealthDisposition::Healthy,
error: None,
@@ -541,19 +600,6 @@ impl ProviderHealthOutcome {
}
}
fn provider_health_disposition(status: StatusCode) -> ProviderHealthDisposition {
if matches!(status, StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN) {
// Authentication/authorization belongs to the candidate credential,
// not endpoint availability. Failover may still try another
// credential, but this provider's health and circuit stay unchanged.
ProviderHealthDisposition::Neutral
} else if retryable_status(status) {
ProviderHealthDisposition::Unhealthy
} else {
ProviderHealthDisposition::Healthy
}
}
async fn settle_provider_health(
state: &ProxyState,
catalog_epoch: u64,
@@ -773,23 +819,6 @@ fn merge_candidate_headers(incoming: &HeaderMap, candidate: &HeaderMap) -> Heade
merged
}
fn retryable_status(status: StatusCode) -> bool {
if !status.is_client_error() && !status.is_server_error() {
return false;
}
!matches!(
status,
StatusCode::BAD_REQUEST
| StatusCode::METHOD_NOT_ALLOWED
| StatusCode::NOT_ACCEPTABLE
| StatusCode::PAYLOAD_TOO_LARGE
| StatusCode::URI_TOO_LONG
| StatusCode::UNSUPPORTED_MEDIA_TYPE
| StatusCode::UNPROCESSABLE_ENTITY
| StatusCode::NOT_IMPLEMENTED
)
}
struct PreparedPiResponse {
response: Response,
finalization_deferred: bool,
@@ -1418,9 +1447,14 @@ mod tests {
#[test]
fn retry_policy_matches_pi_contract_matrix() {
for status in [StatusCode::UNAUTHORIZED, StatusCode::FORBIDDEN] {
assert_eq!(
upstream_status_disposition(status),
UpstreamStatusDisposition::RetryProvider,
"{status}"
);
}
for status in [
StatusCode::UNAUTHORIZED,
StatusCode::FORBIDDEN,
StatusCode::NOT_FOUND,
StatusCode::REQUEST_TIMEOUT,
StatusCode::CONFLICT,
@@ -1428,7 +1462,11 @@ mod tests {
StatusCode::IM_A_TEAPOT,
StatusCode::BAD_GATEWAY,
] {
assert!(retryable_status(status), "{status}");
assert_eq!(
upstream_status_disposition(status),
UpstreamStatusDisposition::RetryEndpoint,
"{status}"
);
}
for status in [
StatusCode::BAD_REQUEST,
@@ -1440,7 +1478,11 @@ mod tests {
StatusCode::UNPROCESSABLE_ENTITY,
StatusCode::NOT_IMPLEMENTED,
] {
assert!(!retryable_status(status), "{status}");
assert_eq!(
upstream_status_disposition(status),
UpstreamStatusDisposition::ReturnResponse,
"{status}"
);
}
}
@@ -1473,7 +1515,10 @@ mod tests {
.await
.expect("request local Pi capture endpoint");
assert_eq!(response.status(), expected);
assert!(retryable_status(response.status()));
assert_eq!(
upstream_status_disposition(response.status()),
UpstreamStatusDisposition::RetryProvider
);
assert_eq!(
ProviderHealthOutcome::from_status(response.status(), None),
ProviderHealthOutcome {