Skip to content

Move connection timeout configuration from RequestConfig to ConnectionConfig in Apache HttpClient 5 #6293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

joviegas
Copy link
Contributor

@joviegas joviegas commented Jul 23, 2025

Motivation and Context

This change addresses the use of deprecated APIs in Apache HttpClient 5. The setConnectTimeout() method in RequestConfig.Builder is deprecated and should be replaced with proper connection-level timeout configuration via ConnectionConfig.

In Apache HttpClient 5, the architecture separates:

  • Connection-level timeouts (connect timeout, socket timeout, TTL) → configured via ConnectionConfig
  • Request-level timeouts (connection request timeout, response timeout) → configured via RequestConfig

This change aligns our implementation with Apache HttpClient 5's recommended practices and eliminates deprecated API usage warnings.

Modifications

  • Removed connectionTimeout from Apache5HttpRequestConfig - This timeout is now managed at the connection manager level
  • Added getConnectionConfig() method in ApacheConnectionManagerFactory to centralize connection-level timeout configuration
  • Moved connection timeout, socket timeout, and TTL configuration to ConnectionConfig instead of RequestConfig
  • Updated addRequestConfig() method in Apache5HttpRequestFactory to remove deprecated setConnectTimeout() call
  • Updated all test files to remove references to connection timeout in request configuration
  • Added timeout validation tests to ensure both connection and socket timeouts work correctly

Testing

  • Added connectionTimeout_exceedsLimit_throwsException() test - Validates connection timeout using non-routable IP address (192.0.2.1) with 100ms timeout
  • Added socketTimeout_exceedsLimit_throwsException() test - Validates socket timeout using WireMock with 2-second delay and 500ms timeout
  • Updated existing tests - Removed connection timeout configuration from MetricReportingTest and ApacheHttpRequestFactoryTest
  • Verified backward compatibility - All existing functionality continues to work, timeouts are just configured at the appropriate architectural level
  • Cross-platform compatibility - Tests account for different exception messages across JVM implementations using multiple timeout-related keywords

The tests confirm that:

  1. Connection timeouts are properly enforced when connecting to unreachable endpoints
  2. Socket timeouts are properly enforced when waiting for slow server responses
  3. The timeout configuration at the ConnectionConfig level works as expected
  4. No regression in existing timeout functionality

Screenshots (if appropriate)

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas joviegas requested a review from a team as a code owner July 23, 2025 16:27
@joviegas joviegas changed the title Replacing deprecated API like connectionTimeout on RequestConfig and … Move connection timeout configuration from RequestConfig to ConnectionConfig in Apache HttpClient 5 Jul 23, 2025
Copy link

ConnectionConfig.Builder connectionConfigBuilder =
ConnectionConfig.custom()
.setConnectTimeout(
Timeout.of(standardOptions.get(SdkHttpConfigurationOption.CONNECTION_TIMEOUT).toMillis(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: looks like we can just use Timeout.ofMilliseconds so this is less verbose

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@joviegas joviegas merged commit 2590e6e into feature/master/apache5x Jul 23, 2025
23 of 26 checks passed
Copy link

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants