feat(ai): make stream connect timeout configurable - #2696
Open
1688mengdie wants to merge 4 commits into
Open
Conversation
added 4 commits
August 30, 2026 22:00
The streaming HTTP client previously used a hardcoded TCP connect timeout (STREAM_CONNECT_TIMEOUT_SECS = 10). This rigid configuration lacks flexibility for diverse network environments and does not follow the project's existing timeout configuration pattern (e.g., stream_idle_timeout_secs, stream_ttft_timeout_secs), which all use Option<u64> with serde defaults. This change introduces stream_connect_timeout_secs: Option<u64> to AIConfig with default value Some(10) (preserving original behavior). The propagation path is: AIConfig.stream_connect_timeout_secs → StreamOptions.connect_timeout → http::create_http_client() → reqwest::ClientBuilder::connect_timeout(if-let). Cleanup: Removed dead code STREAM_CONNECT_TIMEOUT_SECS constant. Frontend integration: Added config component (AIModelConfig.tsx) with load/save/render cycle, plus TypeScript type definition (index.ts). i18n coverage: English (en-US), Simplified Chinese (zh-CN), Traditional Chinese (zh-TW) labels/hints/placeholders. Test: Light test (compile pass / type-check / i18n:audit 0 warning) AI: Semantic quality optimization rewrite (mirroring ttft chain pattern, aligned with upstream Duration style)
Root cause: G2 introduced conditional connect_timeout setting (if let Some(timeout) = connect_timeout) which caused http client to have NO connect timeout when stream_options.connect_timeout=None. The test lightweight_client_negotiates_with_the_production_server uses StreamOptions::default() where connect_timeout=None, leading to indefinite connection wait and session/reloadContext method not being advertised in capabilities negotiation. Fix: Restore unconditional 10s default timeout using unwrap_or(10), mirroring pre-G2 behavior and following ttft chain pattern.
The test explicit_none_stream_timeouts_mean_wait_indefinitely constructs an AIConfig with idle/ttft None and ..Default::default(), but stream_connect_timeout defaults to Some(10) via the G2 field. Set connect None explicitly so the 'wait indefinitely' assertion holds.
Revert the if-let change from ca77749 back to connect_timeout .unwrap_or(Duration::from_secs(10)). The lightweight client stream needs the http layer connect_timeout to default to 10s so it does not hang; keeping the mod.rs explicit-none test fix untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The streaming HTTP client previously used a hardcoded TCP connect timeout (STREAM_CONNECT_TIMEOUT_SECS = 10). This rigid configuration lacks flexibility for diverse network environments and does not follow the project's existing timeout configuration pattern (e.g., stream_idle_timeout_secs, stream_ttft_timeout_secs), which all use Option with serde defaults.
Root Cause
The timeout was defined as a module-level constant (pub(crate) const STREAM_CONNECT_TIMEOUT_SECS: u64 = 10) and directly injected into the Rust TLS builder via .connect_timeout(Duration::from_secs(...)). This approach:
Fix
This change introduces stream_connect_timeout_secs: Option to AIConfig with:
eqwest::ClientBuilder::connect_timeout(if-let Some(timeout))
Verification
Test: Light test (compile pass / type-check / i18n:audit 0 warning)
AI: Semantic quality optimization rewrite (mirroring ttft chain pattern, aligned with upstream Duration style)
Closes #2694
Commit List