Skip to content

Commit c7fe0be

Browse files
author
leynos
committed
Preserve provider transport failure diagnostics
Classify reqwest failures before converting them to LLMError, retain their HTTP status and bounded redacted source chain in metadata, and log those fields in ACP generation telemetry. Cover both the shared OpenAI-compatible path and the custom-provider OpenAIProvider route used by Arli.
1 parent b3515e8 commit c7fe0be

6 files changed

Lines changed: 311 additions & 8 deletions

File tree

crates/codegen/vtcode-acp/src/zed/agent/handlers.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,24 @@ struct GenerationTelemetry {
289289
estimated_output_tokens: u64,
290290
}
291291

292+
#[derive(Debug, Default, PartialEq, Eq)]
293+
struct ProviderErrorTelemetry<'a> {
294+
code: Option<&'a str>,
295+
status: Option<u16>,
296+
detail: Option<&'a str>,
297+
}
298+
299+
fn provider_error_telemetry(error: &LLMError) -> ProviderErrorTelemetry<'_> {
300+
let LLMError::Network { metadata: Some(metadata), .. } = error else {
301+
return ProviderErrorTelemetry::default();
302+
};
303+
ProviderErrorTelemetry {
304+
code: metadata.code.as_deref(),
305+
status: metadata.status,
306+
detail: metadata.message.as_deref(),
307+
}
308+
}
309+
292310
impl GenerationTelemetry {
293311
fn start() -> Self {
294312
Self {
@@ -359,6 +377,7 @@ impl GenerationTelemetry {
359377
error: &LLMError,
360378
) {
361379
let snapshot = runtime.telemetry_snapshot();
380+
let error_telemetry = provider_error_telemetry(error);
362381
warn!(
363382
provider = runtime.provider_name(),
364383
generation_elapsed_ms = duration_millis(self.started_at.elapsed()),
@@ -369,6 +388,9 @@ impl GenerationTelemetry {
369388
active_provider_permits = snapshot.active_permits,
370389
permit_limit = ?snapshot.permit_limit,
371390
circuit_breaker_state = snapshot.circuit_breaker_state,
391+
provider_error_code = ?error_telemetry.code,
392+
provider_error_status = ?error_telemetry.status,
393+
provider_error_detail = ?error_telemetry.detail,
372394
provider_error = %error,
373395
"ACP provider generation attempt failed"
374396
);
@@ -1671,6 +1693,31 @@ mod tests {
16711693

16721694
use super::*;
16731695

1696+
#[test]
1697+
fn provider_error_telemetry_exposes_structured_network_diagnostics() {
1698+
let error = LLMError::Network {
1699+
message: "request failed".to_string(),
1700+
metadata: Some(LLMErrorMetadata::new(
1701+
"Arli AI",
1702+
Some(504),
1703+
Some("reqwest_timeout_error".to_string()),
1704+
None,
1705+
None,
1706+
None,
1707+
Some("operation timed out".to_string()),
1708+
)),
1709+
};
1710+
1711+
assert_eq!(
1712+
provider_error_telemetry(&error),
1713+
ProviderErrorTelemetry {
1714+
code: Some("reqwest_timeout_error"),
1715+
status: Some(504),
1716+
detail: Some("operation timed out"),
1717+
}
1718+
);
1719+
}
1720+
16741721
proptest! {
16751722
#[test]
16761723
fn streaming_eligibility_depends_only_on_provider_support_and_stop_hooks(

crates/codegen/vtcode-llm/src/providers/common.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ pub(crate) async fn send_chat_completions(
465465
.json(payload)
466466
.send()
467467
.await
468-
.map_err(|error| super::error_handling::format_network_error(provider_name, &error))
468+
.map_err(|error| super::error_handling::format_reqwest_network_error(provider_name, &error))
469469
}
470470

471471
/// Aborts the spawned streaming task when the consumer stream is dropped.

crates/codegen/vtcode-llm/src/providers/error_handling.rs

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,88 @@ use reqwest::Response;
88
use serde_json::Value;
99
use vtcode_commons::sanitizer::sanitize_provider_diagnostic;
1010

11+
/// Stable classification for failures reported by reqwest.
12+
///
13+
/// The value is persisted in [`LLMErrorMetadata::code`] so callers can inspect
14+
/// the transport failure without parsing reqwest's display text.
15+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
16+
pub(crate) enum ReqwestErrorKind {
17+
Connect,
18+
Timeout,
19+
Request,
20+
Body,
21+
Decode,
22+
Redirect,
23+
Status,
24+
Unknown,
25+
}
26+
27+
impl ReqwestErrorKind {
28+
const fn metadata_code(self) -> &'static str {
29+
match self {
30+
Self::Connect => "reqwest_connect_error",
31+
Self::Timeout => "reqwest_timeout_error",
32+
Self::Request => "reqwest_request_error",
33+
Self::Body => "reqwest_body_error",
34+
Self::Decode => "reqwest_decode_error",
35+
Self::Redirect => "reqwest_redirect_error",
36+
Self::Status => "reqwest_status_error",
37+
Self::Unknown => "reqwest_unknown_error",
38+
}
39+
}
40+
}
41+
42+
#[derive(Debug, Clone, Copy, Default)]
43+
struct ReqwestErrorFlags {
44+
is_connect: bool,
45+
is_timeout: bool,
46+
is_request: bool,
47+
is_body: bool,
48+
is_decode: bool,
49+
is_redirect: bool,
50+
is_status: bool,
51+
}
52+
53+
impl From<&reqwest::Error> for ReqwestErrorFlags {
54+
fn from(error: &reqwest::Error) -> Self {
55+
Self {
56+
is_connect: error.is_connect(),
57+
is_timeout: error.is_timeout(),
58+
is_request: error.is_request(),
59+
is_body: error.is_body(),
60+
is_decode: error.is_decode(),
61+
is_redirect: error.is_redirect(),
62+
is_status: error.is_status(),
63+
}
64+
}
65+
}
66+
67+
const fn classify_reqwest_flags(flags: ReqwestErrorFlags) -> ReqwestErrorKind {
68+
// Some reqwest errors occupy more than one category. Prefer the most
69+
// actionable classification over the generic request/body buckets.
70+
if flags.is_timeout {
71+
ReqwestErrorKind::Timeout
72+
} else if flags.is_connect {
73+
ReqwestErrorKind::Connect
74+
} else if flags.is_decode {
75+
ReqwestErrorKind::Decode
76+
} else if flags.is_redirect {
77+
ReqwestErrorKind::Redirect
78+
} else if flags.is_status {
79+
ReqwestErrorKind::Status
80+
} else if flags.is_body {
81+
ReqwestErrorKind::Body
82+
} else if flags.is_request {
83+
ReqwestErrorKind::Request
84+
} else {
85+
ReqwestErrorKind::Unknown
86+
}
87+
}
88+
89+
pub(crate) fn classify_reqwest_error(error: &reqwest::Error) -> ReqwestErrorKind {
90+
classify_reqwest_flags(error.into())
91+
}
92+
1193
#[derive(Debug, Clone, Default)]
1294
struct ApiResponseMetadata {
1395
request_id: Option<String>,
@@ -111,6 +193,54 @@ pub(crate) fn format_network_error(provider: &str, error: &impl std::fmt::Displa
111193
LLMError::Network { message: formatted_error, metadata: None }
112194
}
113195

196+
/// Formats a reqwest transport failure while preserving its structured
197+
/// classification and source chain in canonical LLM error metadata.
198+
#[cold]
199+
pub(crate) fn format_reqwest_network_error(provider: &str, error: &reqwest::Error) -> LLMError {
200+
let kind = classify_reqwest_error(error);
201+
let formatted_error = error_display::format_llm_error(provider, &format!("network error: {error}"));
202+
let source_chain = reqwest_source_chain(error);
203+
LLMError::Network {
204+
message: formatted_error,
205+
metadata: Some(LLMErrorMetadata::new(
206+
provider,
207+
error.status().map(|status| status.as_u16()),
208+
Some(kind.metadata_code().to_owned()),
209+
None,
210+
None,
211+
None,
212+
source_chain,
213+
)),
214+
}
215+
}
216+
217+
fn reqwest_source_chain(error: &reqwest::Error) -> Option<String> {
218+
use std::error::Error as _;
219+
220+
const MAX_SOURCE_DEPTH: usize = 8;
221+
222+
let mut current = error.source();
223+
let mut depth = 0;
224+
let mut diagnostic = String::new();
225+
while let Some(source) = current {
226+
if !diagnostic.is_empty() {
227+
diagnostic.push_str(": ");
228+
}
229+
diagnostic.push_str(&source.to_string());
230+
current = source.source();
231+
depth += 1;
232+
if depth == MAX_SOURCE_DEPTH {
233+
break;
234+
}
235+
}
236+
237+
if diagnostic.is_empty() {
238+
None
239+
} else {
240+
Some(sanitize_provider_diagnostic(diagnostic.as_bytes()))
241+
}
242+
}
243+
114244
/// Handle JSON parsing errors with consistent formatting
115245
#[cold]
116246
pub(crate) fn format_parse_error(provider: &str, error: &impl std::fmt::Display) -> LLMError {
@@ -333,6 +463,87 @@ fn extract_response_metadata(response: &Response) -> ApiResponseMetadata {
333463
mod tests {
334464
use super::*;
335465

466+
#[test]
467+
fn reqwest_error_classification_covers_transport_categories() {
468+
let cases = [
469+
(ReqwestErrorFlags { is_connect: true, ..Default::default() }, ReqwestErrorKind::Connect),
470+
(ReqwestErrorFlags { is_timeout: true, ..Default::default() }, ReqwestErrorKind::Timeout),
471+
(ReqwestErrorFlags { is_request: true, ..Default::default() }, ReqwestErrorKind::Request),
472+
(ReqwestErrorFlags { is_body: true, ..Default::default() }, ReqwestErrorKind::Body),
473+
(ReqwestErrorFlags { is_decode: true, ..Default::default() }, ReqwestErrorKind::Decode),
474+
(ReqwestErrorFlags { is_redirect: true, ..Default::default() }, ReqwestErrorKind::Redirect),
475+
(ReqwestErrorFlags { is_status: true, ..Default::default() }, ReqwestErrorKind::Status),
476+
(ReqwestErrorFlags::default(), ReqwestErrorKind::Unknown),
477+
];
478+
479+
for (flags, expected) in cases {
480+
assert_eq!(classify_reqwest_flags(flags), expected);
481+
}
482+
}
483+
484+
#[test]
485+
fn reqwest_error_classification_prefers_specific_categories() {
486+
let request_timeout = ReqwestErrorFlags {
487+
is_timeout: true,
488+
is_request: true,
489+
..Default::default()
490+
};
491+
let request_connect = ReqwestErrorFlags {
492+
is_connect: true,
493+
is_request: true,
494+
..Default::default()
495+
};
496+
let body_decode = ReqwestErrorFlags {
497+
is_body: true,
498+
is_decode: true,
499+
..Default::default()
500+
};
501+
502+
assert_eq!(classify_reqwest_flags(request_timeout), ReqwestErrorKind::Timeout);
503+
assert_eq!(classify_reqwest_flags(request_connect), ReqwestErrorKind::Connect);
504+
assert_eq!(classify_reqwest_flags(body_decode), ReqwestErrorKind::Decode);
505+
}
506+
507+
#[test]
508+
fn reqwest_error_metadata_codes_are_stable() {
509+
assert_eq!(ReqwestErrorKind::Connect.metadata_code(), "reqwest_connect_error");
510+
assert_eq!(ReqwestErrorKind::Timeout.metadata_code(), "reqwest_timeout_error");
511+
assert_eq!(ReqwestErrorKind::Request.metadata_code(), "reqwest_request_error");
512+
assert_eq!(ReqwestErrorKind::Body.metadata_code(), "reqwest_body_error");
513+
assert_eq!(ReqwestErrorKind::Decode.metadata_code(), "reqwest_decode_error");
514+
assert_eq!(ReqwestErrorKind::Redirect.metadata_code(), "reqwest_redirect_error");
515+
assert_eq!(ReqwestErrorKind::Status.metadata_code(), "reqwest_status_error");
516+
assert_eq!(ReqwestErrorKind::Unknown.metadata_code(), "reqwest_unknown_error");
517+
}
518+
519+
#[tokio::test]
520+
async fn reqwest_redirect_error_is_preserved_in_llm_metadata() {
521+
use wiremock::{Mock, MockServer, ResponseTemplate, matchers::path};
522+
523+
let server = MockServer::start().await;
524+
Mock::given(path("/loop"))
525+
.respond_with(ResponseTemplate::new(302).insert_header("location", format!("{}/loop", server.uri())))
526+
.mount(&server)
527+
.await;
528+
let client = reqwest::Client::builder()
529+
.redirect(reqwest::redirect::Policy::limited(1))
530+
.build()
531+
.expect("redirect-limited test client should build");
532+
let error = client
533+
.post(format!("{}/loop", server.uri()))
534+
.send()
535+
.await
536+
.expect_err("redirect loop should exceed the configured limit");
537+
538+
let llm_error = format_reqwest_network_error("Test provider", &error);
539+
match llm_error {
540+
LLMError::Network { metadata, .. } => {
541+
assert_eq!(metadata.as_ref().and_then(|value| value.code.as_deref()), Some("reqwest_redirect_error"));
542+
}
543+
other => panic!("expected a network error, got {other:?}"),
544+
}
545+
}
546+
336547
#[test]
337548
fn test_rate_limit_detection() {
338549
assert!(is_rate_limit_error(429, ""));

crates/codegen/vtcode-llm/src/providers/openai/provider.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -489,12 +489,9 @@ impl OpenAIProvider {
489489
format!("vtcode-{}", Uuid::new_v4())
490490
}
491491

492-
fn format_network_error(&self, error: impl std::fmt::Display) -> provider::LLMError {
492+
fn format_network_error(&self, error: &reqwest::Error) -> provider::LLMError {
493493
let label = self.provider_display_override.as_deref().unwrap_or("OpenAI");
494-
provider::LLMError::Network {
495-
message: error_display::format_llm_error(label, &format!("Network error: {error}")),
496-
metadata: None,
497-
}
494+
super::super::error_handling::format_reqwest_network_error(label, error)
498495
}
499496

500497
fn format_auth_error(&self, error: impl std::fmt::Display) -> provider::LLMError {
@@ -572,14 +569,14 @@ impl OpenAIProvider {
572569
F: Fn(&OpenAIRequestAuth) -> reqwest::RequestBuilder,
573570
{
574571
let auth = self.current_request_auth().await?;
575-
let response = build_request(&auth).send().await.map_err(|e| self.format_network_error(e))?;
572+
let response = build_request(&auth).send().await.map_err(|e| self.format_network_error(&e))?;
576573

577574
if self.uses_refreshable_auth() && Self::auth_retryable_status(response.status()) {
578575
let retry_auth = self.refresh_request_auth_for_retry().await?;
579576
return build_request(&retry_auth)
580577
.send()
581578
.await
582-
.map_err(|e| self.format_network_error(e));
579+
.map_err(|e| self.format_network_error(&e));
583580
}
584581

585582
Ok(response)

crates/codegen/vtcode-llm/src/providers/openai/provider/tests.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,47 @@ async fn custom_provider_auth_retries_with_refreshed_tokens_after_401() {
10771077
);
10781078
}
10791079

1080+
#[tokio::test]
1081+
async fn custom_provider_network_error_preserves_display_name_and_reqwest_classification() {
1082+
let Some(server) = start_mock_server_or_skip().await else {
1083+
return;
1084+
};
1085+
Mock::given(path("/redirect-loop"))
1086+
.respond_with(ResponseTemplate::new(302).insert_header("location", format!("{}/redirect-loop", server.uri())))
1087+
.mount(&server)
1088+
.await;
1089+
let provider = OpenAIProvider::from_custom_config(
1090+
"arli".to_string(),
1091+
"Arli AI".to_string(),
1092+
Some("test-key".to_string()),
1093+
Some("test-model".to_string()),
1094+
None,
1095+
None,
1096+
None,
1097+
None,
1098+
None,
1099+
None,
1100+
None,
1101+
);
1102+
let client = reqwest::Client::builder()
1103+
.redirect(reqwest::redirect::Policy::limited(1))
1104+
.build()
1105+
.expect("redirect-limited test client should build");
1106+
1107+
let error = provider
1108+
.send_authorized(|_| client.get(format!("{}/redirect-loop", server.uri())))
1109+
.await
1110+
.expect_err("redirect loop should exceed the configured limit");
1111+
1112+
match error {
1113+
provider::LLMError::Network { message, metadata } => {
1114+
assert!(message.contains("Arli AI"), "custom provider display name missing: {message}");
1115+
assert_eq!(metadata.as_ref().and_then(|value| value.code.as_deref()), Some("reqwest_redirect_error"));
1116+
}
1117+
other => panic!("expected a network error, got {other:?}"),
1118+
}
1119+
}
1120+
10801121
fn chat_completion_response_body(text: &str) -> Value {
10811122
json!({
10821123
"id": "chatcmpl-test",

0 commit comments

Comments
 (0)