From 55c0c64671279044aca366e34bb42abff7c6755d Mon Sep 17 00:00:00 2001 From: baiqing Date: Fri, 15 May 2026 11:43:39 +0800 Subject: [PATCH] fix(llm): don't retry on timeout to avoid duplicate billing (pr_agent #443 round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pr_agent #443 review 指出 Duplicate Request 风险： > Retrying after send() fails can submit the same LLM request twice if the > first attempt already reached the provider but the connection dropped > before a response was returned. For non-idempotent completion calls, that > can mean duplicate billing and two completions for one user action. 事实分析 reqwest::Error 各 variant： - is_connect() → TCP 握手没建立，server 不可能收到 → 安全 retry - is_request() → HTTP 请求层错误（构造问题），server 没收到完整请求 → 安全 retry - is_timeout() → client 设置的 timeout 到了，server 可能已经收到并在处理 （非幂等 completion 调用）→ 不安全 retry，会重复 billing 修法：从 retry 触发条件移除 `|| e.is_timeout()`。timeout 直接返回 LLMError::Timeout，不再重试。 user 实际看到的失败模式是 "error sending request"（reqwest::is_connect），不在 被移除范围内，覆盖率不变。 注：pr_agent 同轮提的 "Ticket compliance ❌ Not compliant 442" 是 false positive —— pr_agent 把 PR description 里提到的 #442 当成本 PR 的 ticket id；#442 是另一 个独立 PR（A-D 默认值 / 提示词重构），已 merge。 cargo test 263 全过。 --- openless-all/app/src-tauri/src/polish.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/openless-all/app/src-tauri/src/polish.rs b/openless-all/app/src-tauri/src/polish.rs index 3ec14f8a..38a8ca7c 100644 --- a/openless-all/app/src-tauri/src/polish.rs +++ b/openless-all/app/src-tauri/src/polish.rs @@ -1236,15 +1236,17 @@ pub(crate) fn http_client_builder(base_url: &str, timeout_secs: u64) -> reqwest: } } -/// 发请求 + 网络抖动 retry：connect / request / timeout 三类 transient 错误首次失败时 -/// 等 500ms 重试一次，再失败按原样返回。HTTP 4xx/5xx 不在这里触发——那些走 response -/// 的 status 分支单独处理。 +/// 发请求 + 网络抖动 retry：**只**对 `is_connect()` / `is_request()` 这两类「服务端 +/// 必然没收到」的失败重试一次。`is_timeout()` 故意**不**重试——超时时服务端可能已经 +/// 在处理请求并扣计费（LLM completion 是非幂等动作），重试会导致重复 billing + 重复 +/// completion。HTTP 4xx/5xx 不在这里触发——那些走 response.status() 分支单独处理。 /// /// 调用前提：传入的 RequestBuilder body 必须是内存型（json / form），不能是 stream /// reader——retry 用 `try_clone()` 复制 RequestBuilder，stream body 不支持。 /// -/// 对流式 SSE 路径 retry 是安全的：失败发生在 `send().await` 阶段，response 还没回 -/// 来 → on_delta 必然未被调用 → 不会有「已流式输出的字被重复」的问题。 +/// 对流式 SSE 路径 retry 是安全的：connect / request 类失败发生在 TCP 握手 / HTTP +/// 请求写出阶段，response 还没回 → on_delta 必然未被调用 → 不会有「已流式输出的字 +/// 被重复」的问题。 async fn send_with_transient_retry( request: reqwest::RequestBuilder, ) -> Result { @@ -1254,7 +1256,7 @@ async fn send_with_transient_retry( .expect("memory-backed body (json/form) must be clonable for retry"); match initial.send().await { Ok(r) => Ok(r), - Err(e) if e.is_connect() || e.is_request() || e.is_timeout() => { + Err(e) if e.is_connect() || e.is_request() => { log::warn!( "[llm] send transient failure, retry in {}ms: {}", RETRY_DELAY_MS,