Skip to content

fix: honor NetworkHttpClient RequestConfig in makeRequest - #972

Open
arimu1 wants to merge 1 commit into
twilio:mainfrom
arimu1:fix/913-requestconfig-override
Open

fix: honor NetworkHttpClient RequestConfig in makeRequest#972
arimu1 wants to merge 1 commit into
twilio:mainfrom
arimu1:fix/913-requestconfig-override

Conversation

@arimu1

@arimu1 arimu1 commented Aug 28, 2026

Copy link
Copy Markdown

Fixes #913

NetworkHttpClient(RequestConfig) stores the caller's config on the Apache HttpClientBuilder via setDefaultRequestConfig, but makeRequest always called httpUriRequestBase.setConfig(DEFAULT_REQUEST_CONFIG). Per-request config wins in HttpClient 5, so custom timeouts (e.g. response timeout) were silently ignored.

Changes

  • Store the constructor RequestConfig in a field (defaults to DEFAULT_REQUEST_CONFIG for the no-arg path).
  • In makeRequest, apply this.requestConfig on each request instead of hardcoding DEFAULT_REQUEST_CONFIG.
  • For NetworkHttpClient(HttpClientBuilder) (isCustomClient=true), skip per-request setConfig so builder-level defaults remain in effect.

Test plan

  • mvn test -Dtest=NetworkHttpClientTest — 21 tests pass (JDK 17 / JBR 17.0.14, macOS darwin)
  • New testMakeRequestUsesConstructorRequestConfig — verifies a custom RequestConfig (12345 ms response timeout) is applied on the executed HttpUriRequestBase
  • New testCustomHttpClientBuilderDoesNotSetPerRequestConfig — verifies the HttpClientBuilder constructor path does not override per-request config
  • Confirmed both new tests fail on unfixed main (constructor config replaced by DEFAULT_REQUEST_CONFIG; custom builder path incorrectly received default config)

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] NetworkHttpClient RequestConfig overridden in makeRequest method

1 participant