Repository navigation
Keep the default timeouts on API requests - #318
Merged
Merged
Conversation
|
coverage: 96.472% (+0.09%) from 96.386% — ccr-f803d45a-request-timeouts into master |
The config set on each API request to turn off URI normalization replaces the client's default config as a whole, and was built from scratch: it dropped the five-minute connect, read and connection request timeouts, so a request to a server which never answers waited for good. It now starts from the default config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DeS4NogitfTa3EDcB8k6a3
uwolfer
force-pushed
the
ccr-f803d45a-request-timeouts
branch
from
October 7, 2026 21:02
eb1006d to
3ee69e5
Compare
|
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
3 participants
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
GerritHttpClientFactorygives each client a defaultRequestConfigwith 5-minute connect, socket (read) and connection-request timeouts.GerritRestClientthen sets a config of its own on every API request to turn off URI normalization (b8bdea0, "Disable implicit URL normalization"). A config set on the request replaces the client's default config as a whole. Because that one was built withRequestConfig.custom(), every API request ran with all three timeouts unset. A request to a server that accepts the connection and never answers waits forever.Requests sent without a config of their own, i.e. the session check and the login ahead of an API request, kept the default timeouts.
It also shows up through
HttpClientBuilderExtensions. gerrit-intellij-plugin sets only a connect timeout through an extension. On HTTPS, the socket factory makes the connect timeout the socket's read timeout for the handshake, and with the socket timeout left unset nothing replaces it afterwards. So every HTTPS read was cut off after the connect timeout (uwolfer/gerrit-intellij-plugin#718 works around this on the plugin side).Change
The request's config now starts from the client's default config (
RequestConfig.copy(...)) and only turns off URI normalization, so API requests keep the 5-minute timeouts.Testing
GerritRestClientTest#testRequestKeepsDefaultTimeouts: an extension records the config each request goes out with.mvn test: 547 tests, 0 failures.🤖 Generated with Claude Code
https://claude.ai/code/session_01DeS4NogitfTa3EDcB8k6a3
Generated by Claude Code