Skip to content

Commit edbe0e4

Browse files
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7wpfleger96
andcommitted
fix(agent): restore 10s connect timeout and retry mid-body read errors
The streaming-timeout change rebound connect_timeout to cfg.llm_timeout (120s), so a dead endpoint blocked the TCP/TLS handshake for two minutes before failing — and connect is retried, compounding it. Connect time and inter-chunk stall time are different concerns; restore connect_timeout to a fixed 10s independent of llm_timeout. With the total request timeout removed, a connection reset mid-body became the main body-read failure and was hard-failing on the first occurrence. reqwest's chunk() surfaces a reset/truncated body as a decode error, so is_retryable_transport_error now treats is_decode() as retryable; the read-error arm mirrors the stall arm (backoff + resend, hard-fail on the final attempt). JSON-parse failures take the separate serde path and are unaffected. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
1 parent 4ab0701 commit edbe0e4

1 file changed

Lines changed: 107 additions & 3 deletions

File tree

crates/sprout-agent/src/llm.rs

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ pub struct Llm {
4646
impl Llm {
4747
pub fn new(cfg: &Config) -> Result<Self, AgentError> {
4848
let http = Client::builder()
49-
.connect_timeout(cfg.llm_timeout)
49+
// Fixed short connect timeout: a dead/unroutable endpoint must
50+
// fail fast at the TCP/TLS handshake, independent of llm_timeout
51+
// (which governs in-flight inter-chunk stalls, a different concern).
52+
.connect_timeout(std::time::Duration::from_secs(10))
5053
.build()
5154
.map_err(|e| AgentError::Llm(format!("http: {e}")))?;
5255
let auth = build_token_source(cfg)?;
@@ -777,7 +780,11 @@ async fn backoff_with_jitter(attempt: u32) {
777780
/// Body-serialization happens before the retry loop, so `is_request()` here
778781
/// is always a network failure, never a malformed request we'd just resend.
779782
fn is_retryable_transport_error(e: &reqwest::Error) -> bool {
780-
e.is_timeout() || e.is_connect() || e.is_request()
783+
// `is_decode` covers mid-body read failures: reqwest's `chunk()` wraps a
784+
// reset/truncated response body as a decode error, which is a transient
785+
// transport fault and safe to resend. JSON-parse failures take a separate
786+
// path (`serde_json` on the fully-buffered body), so they never reach here.
787+
e.is_timeout() || e.is_connect() || e.is_request() || e.is_decode()
781788
}
782789

783790
async fn post<F>(
@@ -884,7 +891,24 @@ where
884891
return serde_json::from_slice(&buf)
885892
.map_err(|e| AgentError::Llm(format!("json: {e}")));
886893
}
887-
Err(e) => return Err(AgentError::Llm(format!("read: {e}"))),
894+
Err(e) => {
895+
// A read error mid-body (connection reset, broken pipe,
896+
// TLS failure) is transport-class, same as a stall. With
897+
// the total request timeout gone this is the main body-read
898+
// failure, so retry the whole request when attempts remain
899+
// and the error is transient, mirroring the stall arm.
900+
if attempt + 1 < MAX_RETRIES && is_retryable_transport_error(&e) {
901+
tracing::warn!(
902+
attempt = attempt + 1,
903+
max_attempts = MAX_RETRIES,
904+
error = %e,
905+
"llm: response body read error, retrying"
906+
);
907+
backoff_with_jitter(attempt).await;
908+
break;
909+
}
910+
return Err(AgentError::Llm(format!("read: {e}")));
911+
}
888912
}
889913
}
890914
}
@@ -1518,6 +1542,86 @@ mod tests {
15181542
assert_eq!(out, serde_json::json!({ "ok": true }));
15191543
}
15201544

1545+
/// A connection that delivers partial body bytes then is reset mid-body
1546+
/// (before the terminating chunk) surfaces as a transport read error on
1547+
/// `chunk().await`. That error is transient and must retry like a stall:
1548+
/// the next attempt serves a complete body and the call succeeds.
1549+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
1550+
async fn post_retries_on_read_error_mid_body() {
1551+
use std::sync::atomic::{AtomicU32, Ordering};
1552+
use std::sync::Arc;
1553+
use tokio::io::{AsyncReadExt, AsyncWriteExt};
1554+
use tokio::net::TcpListener;
1555+
1556+
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
1557+
let url = format!("http://{}/v1/x", listener.local_addr().unwrap());
1558+
let accepts = Arc::new(AtomicU32::new(0));
1559+
let accepts_srv = accepts.clone();
1560+
1561+
tokio::spawn(async move {
1562+
loop {
1563+
let (mut sock, _) = match listener.accept().await {
1564+
Ok(p) => p,
1565+
Err(_) => return,
1566+
};
1567+
let n = accepts_srv.fetch_add(1, Ordering::SeqCst);
1568+
tokio::spawn(async move {
1569+
let mut buf = Vec::new();
1570+
let mut tmp = [0u8; 4096];
1571+
while !buf.windows(4).any(|w| w == b"\r\n\r\n") {
1572+
match sock.read(&mut tmp).await {
1573+
Ok(0) | Err(_) => return,
1574+
Ok(k) => buf.extend_from_slice(&tmp[..k]),
1575+
}
1576+
}
1577+
if n == 0 {
1578+
// First attempt: send chunked headers and a partial
1579+
// chunk, then drop the socket without the terminating
1580+
// chunk. The truncated chunked body makes reqwest's
1581+
// `chunk()` return a transport read error.
1582+
let headers = "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\
1583+
Transfer-Encoding: chunked\r\nConnection: close\r\n\r\n";
1584+
let _ = sock.write_all(headers.as_bytes()).await;
1585+
let _ = sock.write_all(b"4\r\n{\"ok\r\n").await;
1586+
let _ = sock.flush().await;
1587+
drop(sock);
1588+
return;
1589+
}
1590+
// Subsequent attempts: serve a complete body.
1591+
let body = "{\"ok\":true}";
1592+
let resp = format!(
1593+
"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\
1594+
Content-Length: {}\r\nConnection: close\r\n\r\n{}",
1595+
body.len(),
1596+
body,
1597+
);
1598+
let _ = sock.write_all(resp.as_bytes()).await;
1599+
let _ = sock.shutdown().await;
1600+
});
1601+
}
1602+
});
1603+
1604+
// Fresh socket per retry so the reset connection isn't reused.
1605+
let client = Client::builder().pool_max_idle_per_host(0).build().unwrap();
1606+
// Generous chunk timeout: this test exercises the read-error arm, not
1607+
// the stall arm — the failure must come from the reset, not a timeout.
1608+
let out = post(
1609+
&client,
1610+
&url,
1611+
&serde_json::json!({}),
1612+
Duration::from_secs(5),
1613+
|b| b,
1614+
)
1615+
.await
1616+
.expect("post should succeed after retrying the read error");
1617+
assert_eq!(out, serde_json::json!({ "ok": true }));
1618+
assert!(
1619+
accepts.load(Ordering::SeqCst) >= 2,
1620+
"server should have seen at least 2 connection attempts, saw {}",
1621+
accepts.load(Ordering::SeqCst)
1622+
);
1623+
}
1624+
15211625
// ---- usage / input-token extraction -------------------------------------
15221626

15231627
#[test]

0 commit comments

Comments
 (0)