mirror of
https://github.com/instructkr/claw-code.git
synced 2026-10-14 00:00:19 +08:00
fix(clippy): box ApiError::Api payload to clear result_large_err
`ApiError` is the `Err` half of nearly every `Result` in the api crate, and
its `Api` variant carried ~139 bytes inline (8 fields, four of them
`Option<String>`). That tripped clippy::result_large_err on 35 signatures
and, with -D warnings, made the whole crate unbuildable under the repo's
own documented verification command.
Move the payload into `ApiErrorDetails` behind a `Box`:
- `ApiError::Api(Box<ApiErrorDetails>)`, with `ApiError::api(..)` to
construct and `api_details()` to borrow
- `ApiErrorDetails` re-exported from the crate root
- adding a field here can no longer re-inflate every signature
This also simplifies `enrich_bearer_auth_error`, which previously
destructured and rebuilt all eight fields at five separate return points
just to append a hint; it now mutates the boxed payload in place.
Also clears the remaining pre-existing lints that blocked -D warnings:
unused test bindings, a redundant `use serde_json`, and an explicit
`.into_iter()` in claw-rag-service.
Verified: cargo test --workspace passes, 0 failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q9McEEH32ZtyM62GSXVSyJ
(cherry picked from commit e9210bd75eb76e5d88b2ae797f537102bcabd41d)
This commit is contained in:
@@ -52,19 +52,13 @@ pub enum ApiError {
|
||||
body_snippet: String,
|
||||
source: serde_json::Error,
|
||||
},
|
||||
Api {
|
||||
status: reqwest::StatusCode,
|
||||
error_type: Option<String>,
|
||||
message: Option<String>,
|
||||
request_id: Option<String>,
|
||||
body: String,
|
||||
retryable: bool,
|
||||
/// Suggested user action based on error type (e.g., "Reduce prompt size" for 413)
|
||||
suggested_action: Option<String>,
|
||||
/// Parsed Retry-After header value (seconds) for 429 responses.
|
||||
/// When present, overrides the exponential backoff delay.
|
||||
retry_after: Option<Duration>,
|
||||
},
|
||||
/// A provider returned an error response.
|
||||
///
|
||||
/// Boxed: the payload is ~139 bytes, and `ApiError` is the `Err` half of nearly every
|
||||
/// `Result` in this crate, so inlining it inflates the size of every one of those returns
|
||||
/// (clippy::result_large_err). Boxing keeps `ApiError` small and means adding a field here
|
||||
/// cannot silently re-inflate every signature in the crate.
|
||||
Api(Box<ApiErrorDetails>),
|
||||
RetriesExhausted {
|
||||
attempts: u32,
|
||||
last_error: Box<ApiError>,
|
||||
@@ -81,7 +75,38 @@ pub enum ApiError {
|
||||
},
|
||||
}
|
||||
|
||||
/// Payload of [`ApiError::Api`], held behind a `Box` so the enum stays small.
|
||||
#[derive(Debug)]
|
||||
pub struct ApiErrorDetails {
|
||||
pub status: reqwest::StatusCode,
|
||||
pub error_type: Option<String>,
|
||||
pub message: Option<String>,
|
||||
pub request_id: Option<String>,
|
||||
pub body: String,
|
||||
pub retryable: bool,
|
||||
/// Suggested user action based on error type (e.g., "Reduce prompt size" for 413)
|
||||
pub suggested_action: Option<String>,
|
||||
/// Parsed Retry-After header value (seconds) for 429 responses.
|
||||
/// When present, overrides the exponential backoff delay.
|
||||
pub retry_after: Option<Duration>,
|
||||
}
|
||||
|
||||
impl ApiError {
|
||||
/// Convenience constructor so callers do not have to spell out the `Box` at every site.
|
||||
#[must_use]
|
||||
pub fn api(details: ApiErrorDetails) -> Self {
|
||||
Self::Api(Box::new(details))
|
||||
}
|
||||
|
||||
/// Borrow the provider-error payload, if this is one.
|
||||
#[must_use]
|
||||
pub fn api_details(&self) -> Option<&ApiErrorDetails> {
|
||||
match self {
|
||||
Self::Api(details) => Some(details),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub const fn missing_credentials(
|
||||
provider: &'static str,
|
||||
@@ -137,7 +162,7 @@ impl ApiError {
|
||||
/// over the computed backoff delay when it exists.
|
||||
pub fn retry_after(&self) -> Option<Duration> {
|
||||
match self {
|
||||
Self::Api { retry_after, .. } => *retry_after,
|
||||
Self::Api(details) => details.retry_after,
|
||||
Self::RetriesExhausted { last_error, .. } => last_error.retry_after(),
|
||||
_ => None,
|
||||
}
|
||||
@@ -146,7 +171,7 @@ impl ApiError {
|
||||
pub fn is_retryable(&self) -> bool {
|
||||
match self {
|
||||
Self::Http(error) => error.is_connect() || error.is_timeout() || error.is_request(),
|
||||
Self::Api { retryable, .. } => *retryable,
|
||||
Self::Api(details) => details.retryable,
|
||||
Self::RetriesExhausted { last_error, .. } => last_error.is_retryable(),
|
||||
Self::MissingCredentials { .. }
|
||||
| Self::ContextWindowExceeded { .. }
|
||||
@@ -164,7 +189,7 @@ impl ApiError {
|
||||
#[must_use]
|
||||
pub fn request_id(&self) -> Option<&str> {
|
||||
match self {
|
||||
Self::Api { request_id, .. } => request_id.as_deref(),
|
||||
Self::Api(details) => details.request_id.as_deref(),
|
||||
Self::RetriesExhausted { last_error, .. } => last_error.request_id(),
|
||||
Self::MissingCredentials { .. }
|
||||
| Self::ContextWindowExceeded { .. }
|
||||
@@ -191,12 +216,12 @@ impl ApiError {
|
||||
Self::MissingCredentials { .. } | Self::ExpiredOAuthToken | Self::Auth(_) => {
|
||||
"provider_auth"
|
||||
}
|
||||
Self::Api { status, .. } if matches!(status.as_u16(), 401 | 403) => "provider_auth",
|
||||
Self::Api(details) if matches!(details.status.as_u16(), 401 | 403) => "provider_auth",
|
||||
Self::ContextWindowExceeded { .. } => "context_window",
|
||||
Self::Api { .. } if self.is_context_window_failure() => "context_window",
|
||||
Self::Api { status, .. } if status.as_u16() == 429 => "provider_rate_limit",
|
||||
Self::Api { .. } if self.is_generic_fatal_wrapper() => "provider_internal",
|
||||
Self::Api { .. } => "provider_error",
|
||||
Self::Api(_) if self.is_context_window_failure() => "context_window",
|
||||
Self::Api(details) if details.status.as_u16() == 429 => "provider_rate_limit",
|
||||
Self::Api(_) if self.is_generic_fatal_wrapper() => "provider_internal",
|
||||
Self::Api(_) => "provider_error",
|
||||
Self::Http(_) | Self::InvalidSseFrame(_) | Self::BackoffOverflow { .. } => {
|
||||
"provider_transport"
|
||||
}
|
||||
@@ -208,11 +233,12 @@ impl ApiError {
|
||||
#[must_use]
|
||||
pub fn is_generic_fatal_wrapper(&self) -> bool {
|
||||
match self {
|
||||
Self::Api { message, body, .. } => {
|
||||
message
|
||||
Self::Api(details) => {
|
||||
details
|
||||
.message
|
||||
.as_deref()
|
||||
.is_some_and(looks_like_generic_fatal_wrapper)
|
||||
|| looks_like_generic_fatal_wrapper(body)
|
||||
|| looks_like_generic_fatal_wrapper(&details.body)
|
||||
}
|
||||
Self::RetriesExhausted { last_error, .. } => last_error.is_generic_fatal_wrapper(),
|
||||
Self::MissingCredentials { .. }
|
||||
@@ -233,17 +259,13 @@ impl ApiError {
|
||||
pub fn is_context_window_failure(&self) -> bool {
|
||||
match self {
|
||||
Self::ContextWindowExceeded { .. } => true,
|
||||
Self::Api {
|
||||
status,
|
||||
message,
|
||||
body,
|
||||
..
|
||||
} => {
|
||||
matches!(status.as_u16(), 400 | 413 | 422)
|
||||
&& (message
|
||||
Self::Api(details) => {
|
||||
matches!(details.status.as_u16(), 400 | 413 | 422)
|
||||
&& (details
|
||||
.message
|
||||
.as_deref()
|
||||
.is_some_and(looks_like_context_window_error)
|
||||
|| looks_like_context_window_error(body))
|
||||
|| looks_like_context_window_error(&details.body))
|
||||
}
|
||||
Self::RetriesExhausted { last_error, .. } => last_error.is_context_window_failure(),
|
||||
Self::MissingCredentials { .. }
|
||||
@@ -327,14 +349,14 @@ impl Display for ApiError {
|
||||
"failed to parse {provider} response for model {model}: {source}; first 200 chars of body: {body_snippet}"
|
||||
),
|
||||
// #28: enhance 401/403 errors with actionable auth guidance
|
||||
Self::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
..
|
||||
} if matches!(status.as_u16(), 401 | 403) => {
|
||||
Self::Api(details) if matches!(details.status.as_u16(), 401 | 403) => {
|
||||
let (status, error_type, message, request_id, body) = (
|
||||
&details.status,
|
||||
&details.error_type,
|
||||
&details.message,
|
||||
&details.request_id,
|
||||
&details.body,
|
||||
);
|
||||
if let (Some(error_type), Some(message)) = (error_type, message) {
|
||||
write!(f, "api returned {status} ({error_type})")?;
|
||||
if let Some(request_id) = request_id {
|
||||
@@ -356,14 +378,14 @@ impl Display for ApiError {
|
||||
Run `claw doctor` to verify your credential configuration."
|
||||
)
|
||||
}
|
||||
Self::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
..
|
||||
} => {
|
||||
Self::Api(details) => {
|
||||
let (status, error_type, message, request_id, body) = (
|
||||
&details.status,
|
||||
&details.error_type,
|
||||
&details.message,
|
||||
&details.request_id,
|
||||
&details.body,
|
||||
);
|
||||
if let (Some(error_type), Some(message)) = (error_type, message) {
|
||||
write!(f, "api returned {status} ({error_type})")?;
|
||||
if let Some(request_id) = request_id {
|
||||
@@ -469,6 +491,7 @@ fn truncate_body_snippet(body: &str, max_chars: usize) -> String {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::ApiErrorDetails;
|
||||
use super::{truncate_body_snippet, ApiError};
|
||||
|
||||
#[test]
|
||||
@@ -533,7 +556,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn detects_generic_fatal_wrapper_and_classifies_it_as_provider_internal() {
|
||||
let error = ApiError::Api {
|
||||
let error = ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::INTERNAL_SERVER_ERROR,
|
||||
error_type: Some("api_error".to_string()),
|
||||
message: Some(
|
||||
@@ -545,7 +568,7 @@ mod tests {
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
assert!(error.is_generic_fatal_wrapper());
|
||||
assert_eq!(error.safe_failure_class(), "provider_internal");
|
||||
@@ -557,7 +580,7 @@ mod tests {
|
||||
fn retries_exhausted_preserves_nested_request_id_and_failure_class() {
|
||||
let error = ApiError::RetriesExhausted {
|
||||
attempts: 3,
|
||||
last_error: Box::new(ApiError::Api {
|
||||
last_error: Box::new(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::BAD_GATEWAY,
|
||||
error_type: Some("api_error".to_string()),
|
||||
message: Some(
|
||||
@@ -569,7 +592,7 @@ mod tests {
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
}),
|
||||
}))),
|
||||
};
|
||||
|
||||
assert!(error.is_generic_fatal_wrapper());
|
||||
@@ -579,7 +602,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn classifies_provider_context_window_errors() {
|
||||
let error = ApiError::Api {
|
||||
let error = ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::BAD_REQUEST,
|
||||
error_type: Some("invalid_request_error".to_string()),
|
||||
message: Some(
|
||||
@@ -591,7 +614,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
assert!(error.is_context_window_failure());
|
||||
assert_eq!(error.safe_failure_class(), "context_window");
|
||||
@@ -600,7 +623,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn classifies_openai_configured_limit_errors_as_context_window_failures() {
|
||||
let error = ApiError::Api {
|
||||
let error = ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::BAD_REQUEST,
|
||||
error_type: Some("invalid_request_error".to_string()),
|
||||
message: Some(
|
||||
@@ -612,7 +635,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
assert!(error.is_context_window_failure());
|
||||
assert_eq!(error.safe_failure_class(), "context_window");
|
||||
|
||||
@@ -10,7 +10,7 @@ pub use client::{
|
||||
oauth_token_is_expired, read_base_url, read_xai_base_url, resolve_saved_oauth_token,
|
||||
resolve_startup_auth_source, MessageStream, OAuthTokenSet, ProviderClient,
|
||||
};
|
||||
pub use error::ApiError;
|
||||
pub use error::{ApiError, ApiErrorDetails};
|
||||
pub use http_client::{
|
||||
build_http_client, build_http_client_or_default, build_http_client_with,
|
||||
build_http_client_with_opts, ProxyConfig, TimeoutConfig,
|
||||
|
||||
@@ -12,7 +12,7 @@ use serde::Deserialize;
|
||||
use serde_json::{Map, Value};
|
||||
use telemetry::{AnalyticsEvent, AnthropicRequestProfile, ClientIdentity, SessionTracer};
|
||||
|
||||
use crate::error::ApiError;
|
||||
use crate::error::{ApiError, ApiErrorDetails};
|
||||
use crate::http_client::build_http_client_or_default;
|
||||
use crate::prompt_cache::{PromptCache, PromptCacheRecord, PromptCacheStats};
|
||||
|
||||
@@ -892,7 +892,7 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
let retryable = is_retryable_status(status);
|
||||
let retry_after = parse_retry_after(&headers, status);
|
||||
|
||||
Err(ApiError::Api {
|
||||
Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status,
|
||||
error_type: parsed_error
|
||||
.as_ref()
|
||||
@@ -905,7 +905,7 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
retryable,
|
||||
suggested_action: None,
|
||||
retry_after,
|
||||
})
|
||||
})))
|
||||
}
|
||||
|
||||
fn parse_retry_after(
|
||||
@@ -952,86 +952,30 @@ fn is_retryable_400(status: reqwest::StatusCode, body: &str) -> bool {
|
||||
const SK_ANT_BEARER_HINT: &str = "sk-ant-* keys go in ANTHROPIC_API_KEY (x-api-key header), not ANTHROPIC_AUTH_TOKEN (Bearer header). Move your key to ANTHROPIC_API_KEY.";
|
||||
|
||||
fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError {
|
||||
let ApiError::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
..
|
||||
} = error
|
||||
else {
|
||||
let ApiError::Api(mut details) = error else {
|
||||
return error;
|
||||
};
|
||||
if status.as_u16() != 401 {
|
||||
return ApiError::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
if details.status.as_u16() != 401 {
|
||||
return ApiError::Api(details);
|
||||
}
|
||||
let Some(bearer_token) = auth.bearer_token() else {
|
||||
return ApiError::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
return ApiError::Api(details);
|
||||
};
|
||||
if !bearer_token.starts_with("sk-ant-") {
|
||||
return ApiError::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
return ApiError::Api(details);
|
||||
}
|
||||
// Only append the hint when the AuthSource is pure BearerToken. If both
|
||||
// api_key and bearer_token are present (`ApiKeyAndBearer`), the x-api-key
|
||||
// header is already being sent alongside the Bearer header and the 401
|
||||
// is coming from a different cause — adding the hint would be misleading.
|
||||
if auth.api_key().is_some() {
|
||||
return ApiError::Api {
|
||||
status,
|
||||
error_type,
|
||||
message,
|
||||
request_id,
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
};
|
||||
}
|
||||
let enriched_message = match message {
|
||||
Some(existing) => Some(format!("{existing} — hint: {SK_ANT_BEARER_HINT}")),
|
||||
None => Some(format!("hint: {SK_ANT_BEARER_HINT}")),
|
||||
};
|
||||
ApiError::Api {
|
||||
status,
|
||||
error_type,
|
||||
message: enriched_message,
|
||||
request_id,
|
||||
body,
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
return ApiError::Api(details);
|
||||
}
|
||||
details.message = Some(match details.message.take() {
|
||||
Some(existing) => format!("{existing} — hint: {SK_ANT_BEARER_HINT}"),
|
||||
None => format!("hint: {SK_ANT_BEARER_HINT}"),
|
||||
});
|
||||
ApiError::Api(details)
|
||||
}
|
||||
|
||||
fn anthropic_wire_model(model: &str) -> &str {
|
||||
@@ -1083,6 +1027,7 @@ struct AnthropicErrorBody {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{ALT_REQUEST_ID_HEADER, REQUEST_ID_HEADER};
|
||||
use crate::error::ApiErrorDetails;
|
||||
use std::io::{Read, Write};
|
||||
use std::net::TcpListener;
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
@@ -1647,7 +1592,7 @@ mod tests {
|
||||
fn enrich_bearer_auth_error_appends_sk_ant_hint_on_401_with_pure_bearer_token() {
|
||||
// given
|
||||
let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string());
|
||||
let error = crate::error::ApiError::Api {
|
||||
let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::UNAUTHORIZED,
|
||||
error_type: Some("authentication_error".to_string()),
|
||||
message: Some("Invalid bearer token".to_string()),
|
||||
@@ -1656,7 +1601,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
// when
|
||||
let enriched = super::enrich_bearer_auth_error(error, &auth);
|
||||
@@ -1678,8 +1623,8 @@ mod tests {
|
||||
"request id should still flow through the enriched error: {rendered}"
|
||||
);
|
||||
match enriched {
|
||||
crate::error::ApiError::Api { status, .. } => {
|
||||
assert_eq!(status, reqwest::StatusCode::UNAUTHORIZED);
|
||||
crate::error::ApiError::Api(details) => {
|
||||
assert_eq!(details.status, reqwest::StatusCode::UNAUTHORIZED);
|
||||
}
|
||||
other => panic!("expected Api variant, got {other:?}"),
|
||||
}
|
||||
@@ -1689,7 +1634,7 @@ mod tests {
|
||||
fn enrich_bearer_auth_error_leaves_non_401_errors_unchanged() {
|
||||
// given
|
||||
let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string());
|
||||
let error = crate::error::ApiError::Api {
|
||||
let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::INTERNAL_SERVER_ERROR,
|
||||
error_type: Some("api_error".to_string()),
|
||||
message: Some("internal server error".to_string()),
|
||||
@@ -1698,7 +1643,7 @@ mod tests {
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
// when
|
||||
let enriched = super::enrich_bearer_auth_error(error, &auth);
|
||||
@@ -1719,7 +1664,7 @@ mod tests {
|
||||
fn enrich_bearer_auth_error_ignores_401_when_bearer_token_is_not_sk_ant() {
|
||||
// given
|
||||
let auth = AuthSource::BearerToken("oauth-access-token-opaque".to_string());
|
||||
let error = crate::error::ApiError::Api {
|
||||
let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::UNAUTHORIZED,
|
||||
error_type: Some("authentication_error".to_string()),
|
||||
message: Some("Invalid bearer token".to_string()),
|
||||
@@ -1728,7 +1673,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
// when
|
||||
let enriched = super::enrich_bearer_auth_error(error, &auth);
|
||||
@@ -1748,7 +1693,7 @@ mod tests {
|
||||
api_key: "sk-ant-api03-legitimate".to_string(),
|
||||
bearer_token: "sk-ant-api03-deadbeef".to_string(),
|
||||
};
|
||||
let error = crate::error::ApiError::Api {
|
||||
let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::UNAUTHORIZED,
|
||||
error_type: Some("authentication_error".to_string()),
|
||||
message: Some("Invalid bearer token".to_string()),
|
||||
@@ -1757,7 +1702,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
// when
|
||||
let enriched = super::enrich_bearer_auth_error(error, &auth);
|
||||
@@ -1774,7 +1719,7 @@ mod tests {
|
||||
fn enrich_bearer_auth_error_ignores_401_when_auth_source_has_no_bearer() {
|
||||
// given
|
||||
let auth = AuthSource::ApiKey("sk-ant-api03-legitimate".to_string());
|
||||
let error = crate::error::ApiError::Api {
|
||||
let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::UNAUTHORIZED,
|
||||
error_type: Some("authentication_error".to_string()),
|
||||
message: Some("Invalid x-api-key".to_string()),
|
||||
@@ -1783,7 +1728,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
// when
|
||||
let enriched = super::enrich_bearer_auth_error(error, &auth);
|
||||
|
||||
@@ -7,7 +7,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
|
||||
use serde::Deserialize;
|
||||
use serde_json::{json, Value};
|
||||
|
||||
use crate::error::ApiError;
|
||||
use crate::error::{ApiError, ApiErrorDetails};
|
||||
use crate::http_client::build_http_client_or_default;
|
||||
use crate::types::{
|
||||
ContentBlockDelta, ContentBlockDeltaEvent, ContentBlockStartEvent, ContentBlockStopEvent,
|
||||
@@ -239,7 +239,7 @@ impl OpenAiCompatClient {
|
||||
.get("code")
|
||||
.and_then(serde_json::Value::as_u64)
|
||||
.map(|c| c as u16);
|
||||
return Err(ApiError::Api {
|
||||
return Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::from_u16(code.unwrap_or(400))
|
||||
.unwrap_or(reqwest::StatusCode::BAD_REQUEST),
|
||||
error_type: err_obj
|
||||
@@ -255,7 +255,7 @@ impl OpenAiCompatClient {
|
||||
.unwrap_or(reqwest::StatusCode::BAD_REQUEST),
|
||||
),
|
||||
retry_after: None,
|
||||
});
|
||||
})));
|
||||
}
|
||||
}
|
||||
let payload = serde_json::from_str::<ChatCompletionResponse>(&body).map_err(|error| {
|
||||
@@ -1613,7 +1613,7 @@ fn parse_sse_frame(
|
||||
.map(|c| c as u16);
|
||||
let status = reqwest::StatusCode::from_u16(code.unwrap_or(500))
|
||||
.unwrap_or(reqwest::StatusCode::INTERNAL_SERVER_ERROR);
|
||||
return Err(ApiError::Api {
|
||||
return Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status,
|
||||
error_type: err_obj
|
||||
.get("type")
|
||||
@@ -1625,12 +1625,12 @@ fn parse_sse_frame(
|
||||
retryable: false,
|
||||
suggested_action: suggested_action_for_status(status),
|
||||
retry_after: None,
|
||||
});
|
||||
})));
|
||||
}
|
||||
}
|
||||
// Detect HTML responses
|
||||
if trimmed.starts_with('<') || trimmed.starts_with("<!") {
|
||||
return Err(ApiError::Api {
|
||||
return Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::BAD_REQUEST,
|
||||
error_type: Some("invalid_response".to_string()),
|
||||
message: Some(
|
||||
@@ -1641,7 +1641,7 @@ fn parse_sse_frame(
|
||||
retryable: false,
|
||||
suggested_action: Some("verify the API endpoint URL is correct".to_string()),
|
||||
retry_after: None,
|
||||
});
|
||||
})));
|
||||
}
|
||||
return Ok(None);
|
||||
}
|
||||
@@ -1665,7 +1665,7 @@ fn parse_sse_frame(
|
||||
.map(|c| c as u16);
|
||||
let status = reqwest::StatusCode::from_u16(code.unwrap_or(400))
|
||||
.unwrap_or(reqwest::StatusCode::BAD_REQUEST);
|
||||
return Err(ApiError::Api {
|
||||
return Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status,
|
||||
error_type: err_obj
|
||||
.get("type")
|
||||
@@ -1677,13 +1677,13 @@ fn parse_sse_frame(
|
||||
retryable: false,
|
||||
suggested_action: suggested_action_for_status(status),
|
||||
retry_after: None,
|
||||
});
|
||||
})));
|
||||
}
|
||||
}
|
||||
// Detect HTML or other non-JSON responses early for better error messages
|
||||
let trimmed_payload = payload.trim();
|
||||
if trimmed_payload.starts_with('<') || trimmed_payload.starts_with("<!") {
|
||||
return Err(ApiError::Api {
|
||||
return Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: reqwest::StatusCode::BAD_REQUEST,
|
||||
error_type: Some("invalid_response".to_string()),
|
||||
message: Some(
|
||||
@@ -1694,7 +1694,7 @@ fn parse_sse_frame(
|
||||
retryable: false,
|
||||
suggested_action: Some("verify the API endpoint URL is correct".to_string()),
|
||||
retry_after: None,
|
||||
});
|
||||
})));
|
||||
}
|
||||
serde_json::from_str::<ChatCompletionChunk>(&payload)
|
||||
.map(Some)
|
||||
@@ -1754,7 +1754,7 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
|
||||
let suggested_action = suggested_action_for_status(status);
|
||||
|
||||
Err(ApiError::Api {
|
||||
Err(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status,
|
||||
error_type: parsed_error
|
||||
.as_ref()
|
||||
@@ -1767,7 +1767,7 @@ async fn expect_success(response: reqwest::Response) -> Result<reqwest::Response
|
||||
retryable,
|
||||
suggested_action,
|
||||
retry_after,
|
||||
})
|
||||
})))
|
||||
}
|
||||
|
||||
fn parse_retry_after(
|
||||
|
||||
@@ -585,14 +585,11 @@ async fn surfaces_retry_exhaustion_for_persistent_retryable_errors() {
|
||||
last_error,
|
||||
} => {
|
||||
assert_eq!(attempts, 2);
|
||||
assert!(matches!(
|
||||
*last_error,
|
||||
ApiError::Api {
|
||||
status: reqwest::StatusCode::SERVICE_UNAVAILABLE,
|
||||
retryable: true,
|
||||
..
|
||||
}
|
||||
));
|
||||
let details = last_error
|
||||
.api_details()
|
||||
.expect("last error should be a provider Api error");
|
||||
assert_eq!(details.status, reqwest::StatusCode::SERVICE_UNAVAILABLE);
|
||||
assert!(details.retryable);
|
||||
}
|
||||
other => panic!("expected retries exhausted, got {other:?}"),
|
||||
}
|
||||
|
||||
@@ -66,7 +66,7 @@ async fn flush_path_batch(
|
||||
#[cfg(feature = "qdrant-index")]
|
||||
let mut qdrant_points: Vec<ChunkPoint> = Vec::with_capacity(batch.len());
|
||||
|
||||
for ((ord, t), vec) in batch.drain(..).zip(vecs.into_iter()) {
|
||||
for ((ord, t), vec) in batch.drain(..).zip(vecs) {
|
||||
let dim = vec.len();
|
||||
let cid = insert_chunk(conn, path, ord, &t)?;
|
||||
insert_embedding(conn, cid, dim, &vec)?;
|
||||
|
||||
@@ -13242,8 +13242,8 @@ fn format_context_window_blocked_error(session_id: &str, error: &api::ApiError)
|
||||
));
|
||||
lines.push(format!(" Context window {context_window_tokens} tokens"));
|
||||
}
|
||||
api::ApiError::Api { message, body, .. } => {
|
||||
let detail = message.as_deref().unwrap_or(body).trim();
|
||||
api::ApiError::Api(details) => {
|
||||
let detail = details.message.as_deref().unwrap_or(&details.body).trim();
|
||||
if !detail.is_empty() {
|
||||
lines.push(format!(
|
||||
" Detail {}",
|
||||
@@ -13253,7 +13253,9 @@ fn format_context_window_blocked_error(session_id: &str, error: &api::ApiError)
|
||||
}
|
||||
api::ApiError::RetriesExhausted { last_error, .. } => {
|
||||
let detail = match last_error.as_ref() {
|
||||
api::ApiError::Api { message, body, .. } => message.as_deref().unwrap_or(body),
|
||||
api::ApiError::Api(details) => {
|
||||
details.message.as_deref().unwrap_or(&details.body)
|
||||
}
|
||||
other => return format_context_window_blocked_error(session_id, other),
|
||||
}
|
||||
.trim();
|
||||
@@ -14556,7 +14558,7 @@ mod tests {
|
||||
SessionLifecycleSummary, SlashCommand, StatusUsage, TerminalTail, TmuxPaneSnapshot,
|
||||
DEFAULT_MODEL, LATEST_SESSION_REFERENCE, STUB_COMMANDS,
|
||||
};
|
||||
use api::{ApiError, MessageResponse, OutputContentBlock, Usage};
|
||||
use api::{ApiError, ApiErrorDetails, MessageResponse, OutputContentBlock, Usage};
|
||||
use plugins::{
|
||||
PluginManager, PluginManagerConfig, PluginTool, PluginToolDefinition, PluginToolPermission,
|
||||
};
|
||||
@@ -14601,7 +14603,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn opaque_provider_wrapper_surfaces_failure_class_session_and_trace() {
|
||||
let error = ApiError::Api {
|
||||
let error = ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: "500".parse().expect("status"),
|
||||
error_type: Some("api_error".to_string()),
|
||||
message: Some(
|
||||
@@ -14613,7 +14615,7 @@ mod tests {
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
let rendered = format_user_visible_api_error("session-issue-22", &error);
|
||||
assert!(rendered.contains("provider_internal"));
|
||||
@@ -14625,7 +14627,7 @@ mod tests {
|
||||
fn retry_exhaustion_uses_retry_failure_class_for_generic_provider_wrapper() {
|
||||
let error = ApiError::RetriesExhausted {
|
||||
attempts: 3,
|
||||
last_error: Box::new(ApiError::Api {
|
||||
last_error: Box::new(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: "502".parse().expect("status"),
|
||||
error_type: Some("api_error".to_string()),
|
||||
message: Some(
|
||||
@@ -14637,7 +14639,7 @@ mod tests {
|
||||
retryable: true,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
}),
|
||||
}))),
|
||||
};
|
||||
|
||||
let rendered = format_user_visible_api_error("session-issue-22", &error);
|
||||
@@ -14690,7 +14692,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn provider_context_window_errors_are_reframed_with_same_guidance() {
|
||||
let error = ApiError::Api {
|
||||
let error = ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: "400".parse().expect("status"),
|
||||
error_type: Some("invalid_request_error".to_string()),
|
||||
message: Some(
|
||||
@@ -14702,7 +14704,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
let rendered = format_user_visible_api_error("session-issue-32", &error);
|
||||
assert!(rendered.contains("context_window_blocked"), "{rendered}");
|
||||
@@ -14724,7 +14726,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn openai_configured_limit_errors_are_rendered_as_context_window_guidance() {
|
||||
let error = ApiError::Api {
|
||||
let error = ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: "400".parse().expect("status"),
|
||||
error_type: Some("invalid_request_error".to_string()),
|
||||
message: Some(
|
||||
@@ -14736,7 +14738,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
};
|
||||
}));
|
||||
|
||||
let rendered = format_user_visible_api_error("session-issue-32", &error);
|
||||
assert!(rendered.contains("Context window blocked"), "{rendered}");
|
||||
@@ -14762,7 +14764,7 @@ mod tests {
|
||||
fn retry_wrapped_context_window_errors_keep_recovery_guidance() {
|
||||
let error = ApiError::RetriesExhausted {
|
||||
attempts: 2,
|
||||
last_error: Box::new(ApiError::Api {
|
||||
last_error: Box::new(ApiError::Api(Box::new(ApiErrorDetails {
|
||||
status: "413".parse().expect("status"),
|
||||
error_type: Some("invalid_request_error".to_string()),
|
||||
message: Some("Request is too large for this model's context window.".to_string()),
|
||||
@@ -14771,7 +14773,7 @@ mod tests {
|
||||
retryable: false,
|
||||
suggested_action: None,
|
||||
retry_after: None,
|
||||
}),
|
||||
}))),
|
||||
};
|
||||
|
||||
let rendered = format_user_visible_api_error("session-issue-32", &error);
|
||||
|
||||
@@ -2,7 +2,6 @@ use std::io::{self, IsTerminal, Write};
|
||||
|
||||
use runtime::{save_user_provider_settings, ConfigLoader, RuntimeProviderConfig};
|
||||
|
||||
use serde_json;
|
||||
|
||||
const PROVIDERS: &[(&str, &str, &str)] = &[
|
||||
("1", "Anthropic", "anthropic"),
|
||||
|
||||
@@ -3455,7 +3455,7 @@ fn export_json_has_kind_702() {
|
||||
// On success stdout has kind:export; on failure stderr has type:error.
|
||||
// Either way, both envelopes must be valid JSON.
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr)
|
||||
let _stderr = String::from_utf8_lossy(&output.stderr)
|
||||
.lines()
|
||||
.filter(|l| l.starts_with('{'))
|
||||
.collect::<Vec<_>>()
|
||||
@@ -3547,8 +3547,8 @@ fn config_parse_error_has_typed_error_kind_and_hint_764() {
|
||||
!output.status.success(),
|
||||
"malformed settings.json should cause non-zero exit"
|
||||
);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let _stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let _stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let json_line = stdout
|
||||
.lines()
|
||||
@@ -3582,8 +3582,8 @@ fn login_logout_removed_subcommands_have_error_kind_and_hint_765() {
|
||||
!output.status.success(),
|
||||
"claw {subcmd} should exit non-zero"
|
||||
);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let _stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let _stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let json_line = stdout
|
||||
.lines()
|
||||
|
||||
Reference in New Issue
Block a user