fix: force HTTP/1.1 for LLM API requests#22
Conversation
reqwest with rustls-tls negotiates HTTP/2 via ALPN, but some API servers (notably DeepSeek behind CloudFront) have body-framing issues with HTTP/2 on long-running responses. This causes reqwest to fail with "error decoding response body" when the response takes >30s (common for reasoning models like deepseek-v4-pro). Forcing HTTP/1.1 avoids this class of errors entirely without any performance penalty for single-request LLM calls.
There was a problem hiding this comment.
Code Review
This pull request modifies the global reqwest client builder in src/utils.rs to force HTTP/1.1. The review feedback identifies that this change impacts the Anthropic provider, contrary to the PR's initial assumptions, and suggests adding a code comment to document the rationale for this protocol restriction to avoid future regressions.
| pub fn build_reqwest_client() -> reqwest::Client { | ||
| reqwest::Client::builder() | ||
| .no_proxy() | ||
| .http1_only() |
There was a problem hiding this comment.
The PR description's assumption that the Anthropic provider uses a separate client path and is unaffected is incorrect; AnthropicProvider in src/provider.rs (line 92) calls this build_reqwest_client() function. Consequently, this change forces HTTP/1.1 for Anthropic as well, which contradicts the stated test plan and indicates that this provider may not have been verified with the protocol change.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
REJECTED — Over-engineeringThis PR forces HTTP/1.1 for all providers to fix a problem that only affects DeepSeek behind CloudFront. Problems:
Recommendation:Close this PR. The correct approach is provider-specific — add |
The original fix forced HTTP/1.1 for ALL providers via .http1_only() on the shared build_reqwest_client(), affecting OpenAI, Gemini, Anthropic, and OpenRouter when only DeepSeek behind CloudFront has the HTTP/2 body-framing issue. Changes: - Revert .http1_only() from build_reqwest_client() (no longer global) - Add build_reqwest_client_http1() for providers that need HTTP/1.1 - LLMClient::new_openai_compatible() stays HTTP/2-capable (default) - LLMClient::new_openai_compatible_http1() forces HTTP/1.1 - create_provider() checks provider_name for 'deepseek' and routes to the HTTP/1.1 client only for DeepSeek providers
Reopened with provider-specific fixThe original concern (close comment below) was that Changes
What changed from the original PR
Files changed
Verification
|
Summary
.http1_only()to the shared reqwest client builderProblem
reqwest with
rustls-tlsnegotiates HTTP/2 via ALPN. DeepSeek's API (behind CloudFront) has HTTP/2 body-framing issues on long-running responses — when a reasoning model takes >30s to respond, the HTTP/2 stream frame boundaries can corrupt or truncate the response body, causing reqwest's.text().awaitto fail with "error decoding response body."This error persisted even after PR #20 increased the timeout to 600s, confirming it's not a timeout issue but an HTTP/2 protocol-level problem.
Why HTTP/1.1
Test plan
deepseek-v4-procompletes requests without "error decoding response body"