-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Configure CONNECT timeout on Netty proxying #47554
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
base: main
Are you sure you want to change the base?
Configure CONNECT timeout on Netty proxying #47554
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances proxy connection handling by applying the configured CONNECT timeout to proxy connection attempts in the Netty HTTP client. Previously, the connect timeout was only applied to the main HTTP client connection but not to the proxy CONNECT operation itself.
Key Changes:
- Extracted
connectTimeoutMilliscalculation to a reusable variable to avoid code duplication - Applied the connect timeout to authenticated proxy handlers (custom
HttpProxyHandler) - Applied the connect timeout to non-authenticated proxies via Reactor Netty's
connectTimeoutMillis()method
...ure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java
Outdated
Show resolved
Hide resolved
...ure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java
Outdated
Show resolved
Hide resolved
samvaity
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm overall
| nettyHttpClient | ||
| = nettyHttpClient.proxy(proxy -> proxy.type(toReactorNettyProxyType(buildProxyOptions.getType())) | ||
| .socketAddress(buildProxyOptions.getAddress()) | ||
| .connectTimeoutMillis(connectTimeoutMillis) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there any testing changes needed to validate?
Description
When proxying adds the configured CONNECT timeout to the proxy connection attempt.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines