fix(bottlecap): thread flush_timeout into HTTP fallback client#1215
Draft
jchrostek-dd wants to merge 1 commit intomainfrom
Draft
fix(bottlecap): thread flush_timeout into HTTP fallback client#1215jchrostek-dd wants to merge 1 commit intomainfrom
jchrostek-dd wants to merge 1 commit intomainfrom
Conversation
The fallback in get_client used reqwest::Client::new() which carries no timeout, allowing it to outlast the configured flush_timeout. Replace it with create_reqwest_client_builder() + .timeout() so the fallback client honours the same limit as the happy path. Inner unwrap_or_else(Client::new) preserves the pre-existing behaviour only when the FIPS builder itself fails. Adds a unit test that forces the fallback via an unparseable proxy_https value and asserts no panic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
get_client(bottlecap/src/http.rs) calledreqwest::Client::new()when the configured proxy URI was unparseable, producing a client with no timeout — requests could block indefinitely regardless offlush_timeout.Client::new()withcreate_reqwest_client_builder().ok().and_then(|b| b.timeout(Duration::from_secs(config.flush_timeout)).build().ok()).unwrap_or_else(reqwest::Client::new)so the fallback client honours the configured timeout.//TODOcomment and added a unit test that forces the fallback via an unparseableproxy_httpsvalue.Reviewer notes
unwrap_or_else(reqwest::Client::new)is a last-resort safety net for the case wherecreate_reqwest_client_builder()itself fails on a FIPS build; the pre-existing infinite-timeout behaviour is preserved only for that edge case.🤖 Generated with Claude Code