verifier_tools/verify: Increase httpClient timeout for large binary i… - #31
Merged
Merged
Conversation
…nfo downloads Previously, `httpClient` in `internal/tiles/reader.go` had a flat 30-second `Timeout`, which in `net/http` bounds the entire request lifecycle including reading the full response body (`io.ReadAll(resp.Body)`). Fetching the ~210 MB `package_info.txt` for `google_1p_apk (2026/01)` required ~56 Mbps of sustained throughput and failed with `context deadline exceeded` on slower or shared connections. Separate connection/header timeouts from the total body-read timeout: 1. Configure `http.Transport` with `DialContext` (`10s` timeout), `ForceAttemptHTTP2: true` (to preserve HTTP/2 multiplexing with a custom dialer), `TLSHandshakeTimeout` (`10s`), and `ResponseHeaderTimeout` (`30s`) so unreachable or unresponsive servers still time out within 30 seconds. 2. Increase `httpClient.Timeout` to 5 minutes (`defaultHTTPTimeout`) to allow large legacy binary info files to finish streaming. 3. Wrap `http.NewRequestWithContext` and `httpClient.Do` errors with `%w` in `readFromURLContext` to preserve timeout/cancellation error chains. Test: - `cd verifier_tools/verify && go test -race -v ./...` - Added `TestHTTPClientTimeoutConfiguration` and `TestReadFromURLContextStreamingAndCancellation` in `internal/tiles/reader_test.go`. Change-Id: Ia17354f130b28880ef75117b98c42581fa8a89f8
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.
…nfo downloads
Previously,
httpClientininternal/tiles/reader.gohad a flat 30-secondTimeout, which innet/httpbounds the entire request lifecycle including reading the full response body (io.ReadAll(resp.Body)). Fetching the ~210 MBpackage_info.txtforgoogle_1p_apk (2026/01)required ~56 Mbps of sustained throughput and failed withcontext deadline exceededon slower or shared connections.Separate connection/header timeouts from the total body-read timeout:
http.TransportwithDialContext(10stimeout),ForceAttemptHTTP2: true(to preserve HTTP/2 multiplexing with a custom dialer),TLSHandshakeTimeout(10s), andResponseHeaderTimeout(30s) so unreachable or unresponsive servers still time out within 30 seconds.httpClient.Timeoutto 5 minutes (defaultHTTPTimeout) to allow large legacy binary info files to finish streaming.http.NewRequestWithContextandhttpClient.Doerrors with%winreadFromURLContextto preserve timeout/cancellation error chains.Test:
cd verifier_tools/verify && go test -race -v ./...TestHTTPClientTimeoutConfigurationandTestReadFromURLContextStreamingAndCancellationininternal/tiles/reader_test.go.Change-Id: Ia17354f130b28880ef75117b98c42581fa8a89f8