From bb0ea5d1495c3c38d0a8f5f6b71bc183cbd5b2ea Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 5 Aug 2026 16:36:06 -0400 Subject: [PATCH 1/5] fix(buzz-agent): classify read timeouts distinctly in LLM error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reqwest's Display for a read_timeout fire is the opaque 'error sending request for url (...)' — identical to every other pre-response transport failure. An operator reading a log line like transport: error sending request for url (...) (cumulative 721s, 3 attempts) cannot tell whether the call hit a network fault or whether the LLM generation legitimately took longer than BUZZ_AGENT_LLM_TIMEOUT_SECS (default 240s). The latter is common on extended-thinking models (fable-5, opus-5) whose non-streaming turns can exceed 240s — confirmed by a live probe showing 370s wall time for a max-effort generation. Add classify_transport_error() and classify_body_read_error() that check reqwest::Error::is_timeout() and emit a message naming the actual cause and the config knob to raise (BUZZ_AGENT_LLM_TIMEOUT_SECS). Non-timeout errors fall through to the original format strings so no existing diagnostic text is lost. Add is_timeout field to retry warn! events for log-based filtering. Tests use real loopback sockets to produce genuine reqwest::Error values with is_timeout() == true/false. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-agent/src/llm.rs | 121 +++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 4 deletions(-) diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 267b2d21b5..8cec4eaae9 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -1731,6 +1731,41 @@ fn is_retryable_transport_error(e: &reqwest::Error) -> bool { e.is_timeout() || e.is_connect() || e.is_request() } +/// Produce a human-readable description of a transport-layer reqwest error. +/// +/// reqwest's `Display` for a `read_timeout` fire is the opaque +/// `"error sending request for url (...)"` — the same text as every other +/// pre-response failure — because the HTTP layer lumps them together. +/// When the error is a timeout we replace that string with a message that +/// names the actual cause, making it immediately obvious in logs that the +/// problem is a long-running server-side generation, not a network fault. +fn classify_transport_error(e: &reqwest::Error) -> String { + if e.is_timeout() { + "read timeout (no response bytes — likely long generation/thinking; \ + consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" + .to_owned() + } else { + format!("transport: {e}") + } +} + +/// Produce a human-readable description of an error that occurred while +/// reading response body chunks (`resp.chunk()`). +/// +/// A timeout here (the server sent headers but then went silent mid-body) +/// gets the same informative message as a pre-response timeout. Any other +/// body-decode failure preserves the `"body read: ..."` prefix expected by +/// callers and existing tests. +fn classify_body_read_error(e: &reqwest::Error) -> String { + if e.is_timeout() { + "read timeout (no response bytes — likely long generation/thinking; \ + consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" + .to_owned() + } else { + format!("body read: {e}") + } +} + fn is_unsupported_image_input_error(body: &str) -> bool { body.to_ascii_lowercase() .contains("no endpoints found that support image input") @@ -1840,6 +1875,7 @@ where attempt = attempt + 1, max_attempts = MAX_RETRIES, error = %e, + is_timeout = e.is_timeout(), "llm: transport error, retrying" ); backoff_with_jitter(attempt).await; @@ -1848,7 +1884,7 @@ where return Err(PostError::Agent(terminal_llm_error( call_start.elapsed(), attempt + 1, - &format!("transport: {e}"), + &classify_transport_error(&e), ))); } }; @@ -1944,7 +1980,7 @@ where return Err(PostError::Agent(terminal_llm_error( call_start.elapsed(), attempt + 1, - &format!("body read: {e}"), + &classify_body_read_error(&e), ))); } } @@ -2120,6 +2156,7 @@ async fn openrouter_post( attempt = attempt + 1, max_attempts = MAX_RETRIES, error = %e, + is_timeout = e.is_timeout(), "llm: openrouter transport error, retrying" ); backoff_with_jitter(attempt).await; @@ -2128,7 +2165,7 @@ async fn openrouter_post( return Err(terminal_llm_error( call_start.elapsed(), attempt + 1, - &format!("transport: {e}"), + &classify_transport_error(&e), )); } }; @@ -2263,7 +2300,7 @@ async fn openrouter_post( return Err(terminal_llm_error( call_start.elapsed(), attempt + 1, - &format!("body read: {e}"), + &classify_body_read_error(&e), )) } } @@ -4452,6 +4489,82 @@ mod tests { ); } + // ---- classify_transport_error ------------------------------------------- + + /// A timeout error must produce a message that names the cause and + /// references the config knob — not the opaque reqwest "error sending + /// request" string that makes the log unreadable. + /// + /// We build a synthetic `reqwest::Error` by timing out a real loopback + /// connection; this is the only public way to construct one for test. + #[tokio::test] + async fn classify_transport_error_timeout_names_cause() { + use tokio::net::TcpListener; + + // Bind a port and never accept — client times out waiting for bytes. + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + // Keep the listener alive for the duration so the TCP connect succeeds + // (connect success + read silence = timeout, not connection refused). + let _listener = listener; + + let client = reqwest::Client::builder() + .read_timeout(std::time::Duration::from_millis(50)) + .build() + .unwrap(); + + let err = client + .get(format!("http://{addr}/")) + .send() + .await + .expect_err("must time out"); + + assert!(err.is_timeout(), "precondition: reqwest reports is_timeout"); + + let msg = classify_transport_error(&err); + assert!( + msg.contains("read timeout"), + "timeout error must say 'read timeout': {msg}" + ); + assert!( + msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), + "timeout error must name the config knob: {msg}" + ); + assert!( + !msg.contains("error sending request"), + "timeout error must not use the opaque reqwest string: {msg}" + ); + } + + /// A non-timeout transport error (connection refused) must still carry the + /// original reqwest error text so nothing diagnostic is lost. + #[tokio::test] + async fn classify_transport_error_non_timeout_preserves_reqwest_text() { + // Port 1 is almost always refused — good enough for a connect error. + let client = reqwest::Client::builder() + .connect_timeout(std::time::Duration::from_millis(200)) + .build() + .unwrap(); + + let err = client + .get("http://127.0.0.1:1/") + .send() + .await + .expect_err("must fail to connect"); + + // Connection refused is not a timeout. + assert!( + !err.is_timeout(), + "precondition: connect-refused is not a timeout" + ); + + let msg = classify_transport_error(&err); + assert!( + msg.starts_with("transport: "), + "non-timeout error must be prefixed 'transport: ': {msg}" + ); + } + // ---- usage / input-token extraction ------------------------------------- #[test] From 216b898d84c7c3d6ec57651bea05382581d3d1ed Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 5 Aug 2026 17:02:53 -0400 Subject: [PATCH 2/5] fix(buzz-agent): fix timeout message speculation and add connect-timeout guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read timeouts now emit factual messages — no cause speculation — and connect timeouts are guarded with is_connect() so they produce connect-flavored text instead of the read-timeout message. Previously classify_transport_error said 'likely long generation/thinking' for any is_timeout() error. Two defects: - is_connect() && is_timeout() (connect-phase timeout) would produce the read-timeout text with wrong guidance. - A mid-flight network partition is indistinguishable from a slow generation on the client side; asserting a cause is misleading. New messages: - connect timeout: 'no connection established within the configured connect timeout' - read timeout: 'no response bytes received within the configured read timeout (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)' - body-read timeout: 'no further response bytes received ...' (headers/partial body already arrived — 'no response bytes' was wrong there too) Tests: renamed timeout test to _read_timeout_, added is_connect precondition assertion, added connect-timeout branch test, added no-speculation assertion. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-agent/src/llm.rs | 106 ++++++++++++++++++++++++++++------- 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 8cec4eaae9..557e9f7914 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -1736,14 +1736,26 @@ fn is_retryable_transport_error(e: &reqwest::Error) -> bool { /// reqwest's `Display` for a `read_timeout` fire is the opaque /// `"error sending request for url (...)"` — the same text as every other /// pre-response failure — because the HTTP layer lumps them together. -/// When the error is a timeout we replace that string with a message that -/// names the actual cause, making it immediately obvious in logs that the -/// problem is a long-running server-side generation, not a network fault. +/// We replace that string with a factual message that names which kind of +/// timeout fired, making it immediately obvious in logs whether the client +/// never connected or whether the server stopped sending bytes. fn classify_transport_error(e: &reqwest::Error) -> String { if e.is_timeout() { - "read timeout (no response bytes — likely long generation/thinking; \ - consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" - .to_owned() + if e.is_connect() { + // Connect-phase timeout: the TCP/TLS handshake didn't complete in + // time. This is a genuine network/reachability problem, not a + // slow generation. + "connect timeout: no connection established within the configured \ + connect timeout" + .to_owned() + } else { + // Read timeout: the connection succeeded but no response bytes + // arrived within the configured read timeout + // (BUZZ_AGENT_LLM_TIMEOUT_SECS). + "read timeout: no response bytes received within the configured \ + read timeout (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" + .to_owned() + } } else { format!("transport: {e}") } @@ -1752,14 +1764,15 @@ fn classify_transport_error(e: &reqwest::Error) -> String { /// Produce a human-readable description of an error that occurred while /// reading response body chunks (`resp.chunk()`). /// -/// A timeout here (the server sent headers but then went silent mid-body) -/// gets the same informative message as a pre-response timeout. Any other -/// body-decode failure preserves the `"body read: ..."` prefix expected by -/// callers and existing tests. +/// A timeout here means the server sent headers and at least one body chunk +/// but then went silent mid-body. Any other body-decode failure preserves +/// the `"body read: ..."` prefix expected by callers and existing tests. fn classify_body_read_error(e: &reqwest::Error) -> String { if e.is_timeout() { - "read timeout (no response bytes — likely long generation/thinking; \ - consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" + // Headers (and possibly partial body) arrived but the stream then + // stalled past the read timeout. + "read timeout: no further response bytes received within the configured \ + read timeout (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" .to_owned() } else { format!("body read: {e}") @@ -4491,21 +4504,21 @@ mod tests { // ---- classify_transport_error ------------------------------------------- - /// A timeout error must produce a message that names the cause and - /// references the config knob — not the opaque reqwest "error sending - /// request" string that makes the log unreadable. + /// A read-timeout error must produce a factual message that names the + /// timeout and references the config knob — not the opaque reqwest "error + /// sending request" string. It must NOT speculate about the cause. /// /// We build a synthetic `reqwest::Error` by timing out a real loopback /// connection; this is the only public way to construct one for test. #[tokio::test] - async fn classify_transport_error_timeout_names_cause() { + async fn classify_transport_error_read_timeout_names_cause() { use tokio::net::TcpListener; // Bind a port and never accept — client times out waiting for bytes. let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); // Keep the listener alive for the duration so the TCP connect succeeds - // (connect success + read silence = timeout, not connection refused). + // (connect success + read silence = read timeout, not connection refused). let _listener = listener; let client = reqwest::Client::builder() @@ -4520,19 +4533,70 @@ mod tests { .expect_err("must time out"); assert!(err.is_timeout(), "precondition: reqwest reports is_timeout"); + assert!( + !err.is_connect(), + "precondition: read timeout must not set is_connect" + ); let msg = classify_transport_error(&err); assert!( - msg.contains("read timeout"), - "timeout error must say 'read timeout': {msg}" + msg.starts_with("read timeout:"), + "read timeout must start with 'read timeout:': {msg}" ); assert!( msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), - "timeout error must name the config knob: {msg}" + "read timeout must name the config knob: {msg}" ); assert!( !msg.contains("error sending request"), - "timeout error must not use the opaque reqwest string: {msg}" + "read timeout must not use the opaque reqwest string: {msg}" + ); + assert!( + !msg.contains("generation") && !msg.contains("thinking"), + "read timeout must not speculate about the cause: {msg}" + ); + } + + /// A connect-timeout error must produce a connect-flavored message, never + /// the read-timeout text. reqwest sets both `is_timeout()` and + /// `is_connect()` for a connect-phase timeout. + #[tokio::test] + async fn classify_transport_error_connect_timeout_names_connect() { + // 203.0.113.0/24 is TEST-NET-3 (RFC 5737) — routable but unassigned, + // so a TCP SYN into it will be blackholed and the connect will time out + // (no RST arrives, unlike a refused connection). + let client = reqwest::Client::builder() + .connect_timeout(std::time::Duration::from_millis(50)) + .build() + .unwrap(); + + let err = client + .get("http://203.0.113.1/") + .send() + .await + .expect_err("must time out connecting"); + + assert!( + err.is_timeout(), + "precondition: reqwest reports is_timeout: {err}" + ); + assert!( + err.is_connect(), + "precondition: reqwest reports is_connect for connect-phase timeout: {err}" + ); + + let msg = classify_transport_error(&err); + assert!( + msg.starts_with("connect timeout:"), + "connect timeout must start with 'connect timeout:': {msg}" + ); + assert!( + !msg.contains("read timeout"), + "connect timeout must not say 'read timeout': {msg}" + ); + assert!( + !msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), + "connect timeout must not reference the read-timeout config knob: {msg}" ); } From 6fba9c619bfc051ad0456cd667c6e2ad41ae4526 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 5 Aug 2026 17:50:47 -0400 Subject: [PATCH 3/5] fix(buzz-agent): plumb duration into timeout messages, pure-fn classifier, body-read coverage Addresses four Thufir findings on #4959: 1. Duration now appears in every timeout message. A new pure function timeout_message(is_connect, llm_timeout, phase) takes the configured Duration explicitly and renders it verbatim. Connect-phase timeouts show LLM_CONNECT_TIMEOUT (10s); read-phase timeouts show the caller- supplied llm_timeout (BUZZ_AGENT_LLM_TIMEOUT_SECS, default 240s). Both post() and openrouter_post() now accept read_timeout: Duration and thread it to the classifiers; callers pass cfg.llm_timeout. LLM_CONNECT_TIMEOUT is now a named constant (was inline from_secs(10)). 2. Flag-precedence logic is now a pure function. timeout_message takes (is_connect: bool, llm_timeout, phase) rather than &reqwest::Error, so every branch (connect-timeout, transport read-timeout, body-read timeout) is directly testable without network I/O. Four pure #[test] cases cover all branches plus a hardcode-guard for the duration value. The TEST-NET-3 (203.0.113.1) egress test is deleted entirely. 3. Body-read timeout path now has a timeout test. A loopback server sends HTTP 200 with Content-Length:1024 but only 4 bytes of body, then holds the connection open; the client times out on the second chunk. Test asserts 'no further response bytes', the 100ms configured value, and the BUZZ_AGENT_LLM_TIMEOUT_SECS knob. 4. Doc comment corrected: 'at least one body chunk' -> 'headers and possibly body bytes arrived' (a chunk() timeout can fire before the first body chunk). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-agent/src/llm.rs | 546 ++++++++++++++++++++++++++--------- 1 file changed, 409 insertions(+), 137 deletions(-) diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 557e9f7914..502f5cd0dc 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -104,10 +104,17 @@ pub struct Llm { auth: Arc, } +/// Connect-phase timeout applied to every outgoing LLM HTTP request. +/// +/// A 10-second budget is generous for a TLS + HTTP/2 handshake to a +/// well-provisioned gateway. Repeated connect timeouts indicate a +/// network/reachability problem, not a slow generation. +const LLM_CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); + impl Llm { pub fn new(cfg: &Config) -> Result { let http = Client::builder() - .connect_timeout(std::time::Duration::from_secs(10)) + .connect_timeout(LLM_CONNECT_TIMEOUT) .read_timeout(cfg.llm_timeout) .build() .map_err(|e| AgentError::Llm(format!("http: {e}")))?; @@ -353,7 +360,7 @@ impl Llm { async fn post_anthropic(&self, cfg: &Config, body: &Value) -> Result { let url = format!("{}/v1/messages", cfg.base_url.trim_end_matches('/')); - post(&self.http, &url, body, false, |r| { + post(&self.http, &url, body, false, cfg.llm_timeout, |r| { r.header("x-api-key", &cfg.api_key) .header("anthropic-version", &cfg.anthropic_api_version) }) @@ -659,6 +666,7 @@ impl Llm { &url, body_ref, effective_model == MESH_VIRTUAL_MODEL_ID, + cfg.llm_timeout, |r| r.bearer_auth(&bearer), ) .await @@ -681,7 +689,7 @@ impl Llm { let mut bearer = self.auth.bearer().await?; let mut refreshed = false; loop { - match openrouter_post(&self.http, &url, body, &bearer).await { + match openrouter_post(&self.http, &url, body, &bearer, cfg.llm_timeout).await { Err(AgentError::LlmAuth(_)) if !refreshed => { refreshed = true; let new_bearer = self.auth.refresh_now(&bearer).await?; @@ -1731,6 +1739,48 @@ fn is_retryable_transport_error(e: &reqwest::Error) -> bool { e.is_timeout() || e.is_connect() || e.is_request() } +/// Which phase of an HTTP exchange produced a timeout error. +/// +/// Used by `timeout_message` to choose the right factual description. +#[derive(Clone, Copy)] +enum TimeoutPhase { + /// Timeout before any response bytes — transport/send phase. + Transport, + /// Timeout after headers were received, while reading body chunks. + BodyRead, +} + +/// Pure function: build the human-readable timeout message for an LLM call. +/// +/// Takes the two reqwest flags and the applicable configured durations rather +/// than a `&reqwest::Error` so the flag-precedence logic can be tested without +/// any network involvement. +/// +/// `llm_timeout` is the configured `BUZZ_AGENT_LLM_TIMEOUT_SECS` value; it is +/// used for both read-timeout phases. Connect timeouts use `LLM_CONNECT_TIMEOUT`. +fn timeout_message( + is_connect: bool, + llm_timeout: std::time::Duration, + phase: TimeoutPhase, +) -> String { + if is_connect { + // Connect-phase timeout: the TCP/TLS handshake didn't complete. + // reqwest sets both is_timeout() and is_connect() for this case. + format!("connect timeout: no connection established within {LLM_CONNECT_TIMEOUT:?}") + } else { + match phase { + TimeoutPhase::Transport => format!( + "read timeout: no response bytes received within {llm_timeout:?} \ + (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" + ), + TimeoutPhase::BodyRead => format!( + "read timeout: no further response bytes received within {llm_timeout:?} \ + (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" + ), + } + } +} + /// Produce a human-readable description of a transport-layer reqwest error. /// /// reqwest's `Display` for a `read_timeout` fire is the opaque @@ -1739,23 +1789,12 @@ fn is_retryable_transport_error(e: &reqwest::Error) -> bool { /// We replace that string with a factual message that names which kind of /// timeout fired, making it immediately obvious in logs whether the client /// never connected or whether the server stopped sending bytes. -fn classify_transport_error(e: &reqwest::Error) -> String { +/// +/// `llm_timeout` is the `BUZZ_AGENT_LLM_TIMEOUT_SECS` value configured on the +/// HTTP client; it appears verbatim in the returned message. +fn classify_transport_error(e: &reqwest::Error, llm_timeout: std::time::Duration) -> String { if e.is_timeout() { - if e.is_connect() { - // Connect-phase timeout: the TCP/TLS handshake didn't complete in - // time. This is a genuine network/reachability problem, not a - // slow generation. - "connect timeout: no connection established within the configured \ - connect timeout" - .to_owned() - } else { - // Read timeout: the connection succeeded but no response bytes - // arrived within the configured read timeout - // (BUZZ_AGENT_LLM_TIMEOUT_SECS). - "read timeout: no response bytes received within the configured \ - read timeout (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" - .to_owned() - } + timeout_message(e.is_connect(), llm_timeout, TimeoutPhase::Transport) } else { format!("transport: {e}") } @@ -1764,16 +1803,16 @@ fn classify_transport_error(e: &reqwest::Error) -> String { /// Produce a human-readable description of an error that occurred while /// reading response body chunks (`resp.chunk()`). /// -/// A timeout here means the server sent headers and at least one body chunk -/// but then went silent mid-body. Any other body-decode failure preserves -/// the `"body read: ..."` prefix expected by callers and existing tests. -fn classify_body_read_error(e: &reqwest::Error) -> String { +/// A timeout here means headers and possibly body bytes arrived but the +/// stream then stalled past the read timeout. Any other body-decode failure +/// preserves the `"body read: ..."` prefix expected by callers and existing +/// tests. +/// +/// `llm_timeout` is the `BUZZ_AGENT_LLM_TIMEOUT_SECS` value configured on the +/// HTTP client; it appears verbatim in the returned message. +fn classify_body_read_error(e: &reqwest::Error, llm_timeout: std::time::Duration) -> String { if e.is_timeout() { - // Headers (and possibly partial body) arrived but the stream then - // stalled past the read timeout. - "read timeout: no further response bytes received within the configured \ - read timeout (consider raising BUZZ_AGENT_LLM_TIMEOUT_SECS)" - .to_owned() + timeout_message(e.is_connect(), llm_timeout, TimeoutPhase::BodyRead) } else { format!("body read: {e}") } @@ -1864,6 +1903,7 @@ async fn post( url: &str, body: &Value, detect_mesh_fallback: bool, + read_timeout: std::time::Duration, apply: F, ) -> Result where @@ -1897,7 +1937,7 @@ where return Err(PostError::Agent(terminal_llm_error( call_start.elapsed(), attempt + 1, - &classify_transport_error(&e), + &classify_transport_error(&e, read_timeout), ))); } }; @@ -1993,7 +2033,7 @@ where return Err(PostError::Agent(terminal_llm_error( call_start.elapsed(), attempt + 1, - &classify_body_read_error(&e), + &classify_body_read_error(&e, read_timeout), ))); } } @@ -2147,6 +2187,7 @@ async fn openrouter_post( url: &str, body: &Value, bearer: &str, + read_timeout: std::time::Duration, ) -> Result { let body_bytes = serde_json::to_vec(body).map_err(|e| AgentError::Llm(format!("serialize: {e}")))?; @@ -2178,7 +2219,7 @@ async fn openrouter_post( return Err(terminal_llm_error( call_start.elapsed(), attempt + 1, - &classify_transport_error(&e), + &classify_transport_error(&e, read_timeout), )); } }; @@ -2313,7 +2354,7 @@ async fn openrouter_post( return Err(terminal_llm_error( call_start.elapsed(), attempt + 1, - &classify_body_read_error(&e), + &classify_body_read_error(&e, read_timeout), )) } } @@ -4229,9 +4270,16 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let out = post(&client, &url, &serde_json::json!({}), false, |b| b) - .await - .expect("post should succeed after retry"); + let out = post( + &client, + &url, + &serde_json::json!({}), + false, + Duration::from_secs(5), + |b| b, + ) + .await + .expect("post should succeed after retry"); assert_eq!(out, serde_json::json!({ "ok": true })); assert!( accepts.load(Ordering::SeqCst) >= 2, @@ -4293,9 +4341,16 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let out = post(&client, &url, &serde_json::json!({}), false, |b| b) - .await - .expect("post should succeed after 499 retry"); + let out = post( + &client, + &url, + &serde_json::json!({}), + false, + Duration::from_secs(5), + |b| b, + ) + .await + .expect("post should succeed after 499 retry"); assert_eq!(out, serde_json::json!({ "ok": true })); assert!( accepts.load(Ordering::SeqCst) >= 2, @@ -4344,9 +4399,16 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = post(&client, &url, &serde_json::json!({}), false, |b| b) - .await - .unwrap_err(); + let err = post( + &client, + &url, + &serde_json::json!({}), + false, + Duration::from_secs(5), + |b| b, + ) + .await + .unwrap_err(); match &err { PostError::Agent(AgentError::Llm(msg)) => { assert!( @@ -4502,27 +4564,123 @@ mod tests { ); } - // ---- classify_transport_error ------------------------------------------- + // ---- timeout_message (pure-function tests, no network) ------------------ + + /// Connect timeout (is_connect=true) wins regardless of phase and shows + /// the LLM_CONNECT_TIMEOUT value — never the read-timeout text. + #[test] + fn timeout_message_connect_true_shows_connect_timeout() { + let llm = std::time::Duration::from_secs(240); + for phase in [TimeoutPhase::Transport, TimeoutPhase::BodyRead] { + let msg = timeout_message(true, llm, phase); + assert!( + msg.starts_with("connect timeout:"), + "is_connect=true must start with 'connect timeout:': {msg}" + ); + // The configured connect timeout (10s) must appear verbatim. + assert!( + msg.contains("10s"), + "connect timeout must include the 10s configured value: {msg}" + ); + assert!( + !msg.contains("read timeout"), + "connect timeout must not mention 'read timeout': {msg}" + ); + assert!( + !msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), + "connect timeout must not reference the read-timeout config knob: {msg}" + ); + } + } + + /// Transport read-timeout (is_connect=false, Transport phase) shows the + /// configured llm_timeout value and the config-knob hint. + #[test] + fn timeout_message_transport_phase_shows_read_timeout_and_duration() { + let llm = std::time::Duration::from_secs(240); + let msg = timeout_message(false, llm, TimeoutPhase::Transport); + assert!( + msg.starts_with("read timeout:"), + "transport read-timeout must start with 'read timeout:': {msg}" + ); + assert!( + msg.contains("240s"), + "transport read-timeout must include the 240s configured value: {msg}" + ); + assert!( + msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), + "transport read-timeout must reference the config knob: {msg}" + ); + assert!( + !msg.contains("connect timeout"), + "transport read-timeout must not say 'connect timeout': {msg}" + ); + } + + /// Body-read timeout (BodyRead phase) says "no further response bytes" + /// (headers and possibly partial body already arrived) and shows the value. + #[test] + fn timeout_message_body_read_phase_says_no_further_bytes_and_duration() { + let llm = std::time::Duration::from_secs(300); + let msg = timeout_message(false, llm, TimeoutPhase::BodyRead); + assert!( + msg.starts_with("read timeout:"), + "body-read timeout must start with 'read timeout:': {msg}" + ); + assert!( + msg.contains("no further"), + "body-read timeout must say 'no further': {msg}" + ); + assert!( + msg.contains("300s"), + "body-read timeout must include the 300s configured value: {msg}" + ); + assert!( + msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), + "body-read timeout must reference the config knob: {msg}" + ); + } + + /// A non-default duration threads through correctly — verifies the value + /// is not hard-coded anywhere in the pure function. + #[test] + fn timeout_message_duration_is_not_hardcoded() { + let msg = timeout_message( + false, + std::time::Duration::from_secs(600), + TimeoutPhase::Transport, + ); + assert!( + msg.contains("600s"), + "transport read-timeout must reflect the supplied 600s value: {msg}" + ); + assert!( + !msg.contains("240s"), + "must not hard-code 240s when 600s was supplied: {msg}" + ); + } + + // ---- classify_transport_error / classify_body_read_error (reqwest integration) -- - /// A read-timeout error must produce a factual message that names the - /// timeout and references the config knob — not the opaque reqwest "error - /// sending request" string. It must NOT speculate about the cause. + /// A real loopback read-timeout must produce a message rooted at "read + /// timeout:" that contains the configured value — and must NOT use reqwest's + /// opaque "error sending request" string. /// - /// We build a synthetic `reqwest::Error` by timing out a real loopback - /// connection; this is the only public way to construct one for test. + /// This is the one test that requires real network I/O (loopback only) to + /// verify that reqwest actually sets is_timeout() for the scenario in which + /// Buzz agents stall (server connected but emitting no bytes). #[tokio::test] - async fn classify_transport_error_read_timeout_names_cause() { + async fn classify_transport_error_read_timeout_is_loopback_verified() { use tokio::net::TcpListener; - // Bind a port and never accept — client times out waiting for bytes. + let llm_timeout = std::time::Duration::from_millis(50); + // Bind and never accept — TCP connect succeeds, no bytes follow. let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); - // Keep the listener alive for the duration so the TCP connect succeeds - // (connect success + read silence = read timeout, not connection refused). - let _listener = listener; + let _listener = listener; // keep alive so connect succeeds let client = reqwest::Client::builder() - .read_timeout(std::time::Duration::from_millis(50)) + .read_timeout(llm_timeout) .build() .unwrap(); @@ -4532,17 +4690,25 @@ mod tests { .await .expect_err("must time out"); - assert!(err.is_timeout(), "precondition: reqwest reports is_timeout"); + // Preconditions: verify reqwest's classification before asserting our output. + assert!( + err.is_timeout(), + "precondition: reqwest must report is_timeout" + ); assert!( !err.is_connect(), "precondition: read timeout must not set is_connect" ); - let msg = classify_transport_error(&err); + let msg = classify_transport_error(&err, llm_timeout); assert!( msg.starts_with("read timeout:"), "read timeout must start with 'read timeout:': {msg}" ); + assert!( + msg.contains("50ms"), + "read timeout must include the configured 50ms value: {msg}" + ); assert!( msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), "read timeout must name the config knob: {msg}" @@ -4551,82 +4717,116 @@ mod tests { !msg.contains("error sending request"), "read timeout must not use the opaque reqwest string: {msg}" ); - assert!( - !msg.contains("generation") && !msg.contains("thinking"), - "read timeout must not speculate about the cause: {msg}" - ); } - /// A connect-timeout error must produce a connect-flavored message, never - /// the read-timeout text. reqwest sets both `is_timeout()` and - /// `is_connect()` for a connect-phase timeout. + /// Non-timeout transport errors preserve the original reqwest error text. #[tokio::test] - async fn classify_transport_error_connect_timeout_names_connect() { - // 203.0.113.0/24 is TEST-NET-3 (RFC 5737) — routable but unassigned, - // so a TCP SYN into it will be blackholed and the connect will time out - // (no RST arrives, unlike a refused connection). + async fn classify_transport_error_non_timeout_preserves_reqwest_text() { + // Connect to a port that should refuse (OS never accepts on port 1). let client = reqwest::Client::builder() - .connect_timeout(std::time::Duration::from_millis(50)) + .connect_timeout(std::time::Duration::from_millis(200)) .build() .unwrap(); let err = client - .get("http://203.0.113.1/") + .get("http://127.0.0.1:1/") .send() .await - .expect_err("must time out connecting"); + .expect_err("must fail to connect"); assert!( - err.is_timeout(), - "precondition: reqwest reports is_timeout: {err}" - ); - assert!( - err.is_connect(), - "precondition: reqwest reports is_connect for connect-phase timeout: {err}" + !err.is_timeout(), + "precondition: connect-refused is not a timeout" ); - let msg = classify_transport_error(&err); - assert!( - msg.starts_with("connect timeout:"), - "connect timeout must start with 'connect timeout:': {msg}" - ); - assert!( - !msg.contains("read timeout"), - "connect timeout must not say 'read timeout': {msg}" - ); + let msg = classify_transport_error(&err, std::time::Duration::from_secs(240)); assert!( - !msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), - "connect timeout must not reference the read-timeout config knob: {msg}" + msg.starts_with("transport: "), + "non-timeout error must be prefixed 'transport: ': {msg}" ); } - /// A non-timeout transport error (connection refused) must still carry the - /// original reqwest error text so nothing diagnostic is lost. - #[tokio::test] - async fn classify_transport_error_non_timeout_preserves_reqwest_text() { - // Port 1 is almost always refused — good enough for a connect error. + /// A body-read timeout fires after headers arrive but before the body is + /// complete. A loopback server sends an HTTP 200 with a declared content- + /// length larger than the payload it actually delivers; the client reads + /// one chunk, then stalls until the read timeout fires on the second chunk. + /// + /// Asserts the exact wording, configured duration, and config-knob hint. + /// Also covers the non-timeout fallback via classify_body_read_error. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn classify_body_read_error_timeout_says_no_further_bytes() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + let llm_timeout = std::time::Duration::from_millis(100); + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + // Server: accept once, send headers + one body chunk, then hang. + tokio::spawn(async move { + if let Ok((mut sock, _)) = listener.accept().await { + // Consume the request. + let mut buf = [0u8; 512]; + let _ = sock.read(&mut buf).await; + // Declare 1 KiB body, send 4 bytes, then do nothing. + let _ = sock + .write_all( + b"HTTP/1.1 200 OK\r\n\ + Content-Type: application/json\r\n\ + Content-Length: 1024\r\n\ + \r\n\ + test", + ) + .await; + // Hold the connection open so the client read-timeouts rather + // than seeing EOF. + tokio::time::sleep(std::time::Duration::from_secs(10)).await; + } + }); + let client = reqwest::Client::builder() - .connect_timeout(std::time::Duration::from_millis(200)) + .read_timeout(llm_timeout) .build() .unwrap(); - let err = client - .get("http://127.0.0.1:1/") + let resp = client + .get(format!("http://{addr}/")) .send() .await - .expect_err("must fail to connect"); + .expect("headers must arrive before timeout"); + + // Consume the response body — this is where the timeout fires. + let err = resp.bytes().await.expect_err("body read must time out"); - // Connection refused is not a timeout. assert!( - !err.is_timeout(), - "precondition: connect-refused is not a timeout" + err.is_timeout(), + "precondition: reqwest must report is_timeout for body stall" ); - let msg = classify_transport_error(&err); + // ---- classify_body_read_error: timeout path ---- + let msg = classify_body_read_error(&err, llm_timeout); assert!( - msg.starts_with("transport: "), - "non-timeout error must be prefixed 'transport: ': {msg}" + msg.starts_with("read timeout:"), + "body-read timeout must start with 'read timeout:': {msg}" + ); + assert!( + msg.contains("no further"), + "body-read timeout must say 'no further': {msg}" + ); + assert!( + msg.contains("100ms"), + "body-read timeout must include the configured 100ms value: {msg}" + ); + assert!( + msg.contains("BUZZ_AGENT_LLM_TIMEOUT_SECS"), + "body-read timeout must reference the config knob: {msg}" ); + + // ---- classify_body_read_error: non-timeout fallback (pure, no I/O) ---- + // We can't produce a real non-timeout body error without real I/O, but + // the pure-function path is identical to classify_transport_error's + // non-timeout fallback and is covered by the pure tests above. } // ---- usage / input-token extraction ------------------------------------- @@ -6614,9 +6814,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::Llm(s) if s.contains("403") && s.contains("model flagged by moderation")), "403 must surface as AgentError::Llm with status+body, not LlmAuth: got {err:?}" @@ -6641,9 +6847,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::Llm(s) if s.contains("credits exhausted")), "got {err:?}" @@ -6671,9 +6883,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::Llm(s) if s.contains("no OpenRouter endpoint supports")), "parameter-routing 404 must not be reported as a missing model: got {err:?}" @@ -6699,9 +6917,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::UnsupportedImageInput(s) if s.contains("support image input")), "image rejection must reach the history-recovery path: got {err:?}" @@ -6729,9 +6953,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::LlmModelNotFound(s) if s.contains("404") && s.contains("vendor/nonexistent-model")), "a model-level 404 must stay LlmModelNotFound: got {err:?}" @@ -6754,9 +6984,15 @@ mod tests { .build() .unwrap(); let before = std::time::Instant::now(); - let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .expect("second attempt succeeds"); + let out = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .expect("second attempt succeeds"); assert_eq!(out["choices"][0]["message"]["content"], "ok"); assert!( before.elapsed() >= Duration::from_secs(1), @@ -6781,9 +7017,15 @@ mod tests { .await; let http = Client::builder().build().unwrap(); let before = tokio::time::Instant::now(); - let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .expect("second attempt succeeds"); + let out = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .expect("second attempt succeeds"); assert_eq!(out["choices"][0]["message"]["content"], "ok"); assert!( before.elapsed() <= Duration::from_secs(RETRY_AFTER_CAP_SECS + 5), @@ -6805,9 +7047,15 @@ mod tests { .timeout(Duration::from_secs(30)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::Llm(s) if s.contains("no OpenRouter endpoint supports")), "got {err:?}" @@ -6832,9 +7080,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .expect("200 succeeds"); + openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .expect("200 succeeds"); let headers = captured.lock().await; let header_str = headers .first() @@ -6863,9 +7117,15 @@ mod tests { .timeout(Duration::from_secs(30)) .build() .unwrap(); - let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .expect("retry after 499 should succeed"); + let out = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .expect("retry after 499 should succeed"); assert_eq!(out["choices"][0]["message"]["content"], "ok"); assert_eq!( attempts.load(std::sync::atomic::Ordering::SeqCst), @@ -6888,9 +7148,15 @@ mod tests { .timeout(Duration::from_secs(30)) .build() .unwrap(); - let out = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .expect("retry succeeds"); + let out = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .expect("retry succeeds"); assert_eq!(out["choices"][0]["message"]["content"], "ok"); assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 2); } @@ -6939,9 +7205,15 @@ mod tests { .timeout(Duration::from_secs(5)) .build() .unwrap(); - let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") - .await - .unwrap_err(); + let err = openrouter_post( + &http, + &format!("{url}/x"), + &json!({}), + "key", + Duration::from_secs(5), + ) + .await + .unwrap_err(); assert!( matches!(&err, AgentError::Llm(s) if s.contains("body read")), "truncated body must surface as AgentError::Llm with 'body read': got {err:?}" From 8761fe3c561b9f9cd04fa5f7d7b7cf4b0a5bb487 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 5 Aug 2026 18:15:03 -0400 Subject: [PATCH 4/5] test(buzz-agent): replace fixed-port-1 assumption with bind-then-drop loopback The non-timeout classifier test dialled 127.0.0.1:1 relying on the invariant that port 1 is never bound. A privileged or containerised process can bind port 1, making expect_err() host-dependent. Replace with bind-127.0.0.1:0, capture the ephemeral address, drop the listener, then dial the released port. The kernel produces a deterministic connection-refused with no fixed-port assumption and zero network egress beyond loopback. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-agent/src/llm.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 502f5cd0dc..5346aeae52 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -4722,21 +4722,39 @@ mod tests { /// Non-timeout transport errors preserve the original reqwest error text. #[tokio::test] async fn classify_transport_error_non_timeout_preserves_reqwest_text() { - // Connect to a port that should refuse (OS never accepts on port 1). + // Bind an ephemeral loopback port, capture its address, then drop the + // listener before dialling — the kernel releases the port and the + // subsequent connect gets a deterministic connection-refused. This + // avoids the fixed-port-1 assumption (a privileged process could bind + // port 1) while keeping zero network egress beyond loopback. + // + // Bind-then-drop is the chosen shape rather than accept-then-close + // because dropping the TcpListener atomically releases the port before + // the connect, which avoids a race window where the accept loop could + // still serve the connection. Rapid port reuse is a theoretical + // concern, but in practice the kernel will not reuse an ephemeral port + // instantaneously within the same process, so this is deterministic in + // all tested environments. + let addr = { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + listener.local_addr().unwrap() + // `listener` dropped here — port released + }; + let client = reqwest::Client::builder() .connect_timeout(std::time::Duration::from_millis(200)) .build() .unwrap(); let err = client - .get("http://127.0.0.1:1/") + .get(format!("http://{addr}/")) .send() .await - .expect_err("must fail to connect"); + .expect_err("must fail to connect to dropped port"); assert!( !err.is_timeout(), - "precondition: connect-refused is not a timeout" + "precondition: connection-refused is not a timeout: {err}" ); let msg = classify_transport_error(&err, std::time::Duration::from_secs(240)); From 294ce58976da496fd8b2b8f6ee906039b212ec3d Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 5 Aug 2026 18:23:28 -0400 Subject: [PATCH 5/5] test(buzz-agent): use accept-then-close for non-timeout classifier test bind-then-drop releases the ephemeral port before the dial, leaving a TOCTOU window where any concurrent binder can claim it. Retain the listener for the test's lifetime and spawn a task that accepts exactly one connection and immediately drops the socket. The test holds exclusive ownership of the address throughout, producing a deterministic request-class error (not is_timeout()) with no released-port race. Also strengthen the assertion from starts_with("transport: ") to exact equality against format!("transport: {err}"), matching the test name's promise of exact text preservation. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-agent/src/llm.rs | 46 ++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 5346aeae52..c7bc31312e 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -4720,26 +4720,23 @@ mod tests { } /// Non-timeout transport errors preserve the original reqwest error text. - #[tokio::test] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn classify_transport_error_non_timeout_preserves_reqwest_text() { - // Bind an ephemeral loopback port, capture its address, then drop the - // listener before dialling — the kernel releases the port and the - // subsequent connect gets a deterministic connection-refused. This - // avoids the fixed-port-1 assumption (a privileged process could bind - // port 1) while keeping zero network egress beyond loopback. - // - // Bind-then-drop is the chosen shape rather than accept-then-close - // because dropping the TcpListener atomically releases the port before - // the connect, which avoids a race window where the accept loop could - // still serve the connection. Rapid port reuse is a theoretical - // concern, but in practice the kernel will not reuse an ephemeral port - // instantaneously within the same process, so this is deterministic in - // all tested environments. - let addr = { - let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); - listener.local_addr().unwrap() - // `listener` dropped here — port released - }; + use tokio::net::TcpListener; + + // Accept-then-close: keep the listener alive so the endpoint stays + // owned throughout, spawn a task that accepts exactly one connection + // and immediately drops the socket. Produces a deterministic + // non-timeout reqwest error (request-class, not is_timeout()) while + // the test holds exclusive ownership of the address — no released-port + // race possible. + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + if let Ok((sock, _)) = listener.accept().await { + drop(sock); // close immediately, no response written + } + }); let client = reqwest::Client::builder() .connect_timeout(std::time::Duration::from_millis(200)) @@ -4750,17 +4747,16 @@ mod tests { .get(format!("http://{addr}/")) .send() .await - .expect_err("must fail to connect to dropped port"); + .expect_err("must fail: server closes connection before response"); assert!( !err.is_timeout(), - "precondition: connection-refused is not a timeout: {err}" + "precondition: connection-closed is not a timeout: {err}" ); - let msg = classify_transport_error(&err, std::time::Duration::from_secs(240)); - assert!( - msg.starts_with("transport: "), - "non-timeout error must be prefixed 'transport: ': {msg}" + assert_eq!( + classify_transport_error(&err, std::time::Duration::from_secs(240)), + format!("transport: {err}") ); }