-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Updated HttpClient main topic to address user feedback from issues. #3986
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
Conversation
@davidsh @scalablecory @karelz @stephentoub Please review |
@dotnet/ncl can someone please review this PR? |
@samsp-msft Can you incorporate/respond to the feedback when you get a chance? |
Co-authored-by: Jan Jahoda <[email protected]>
Co-authored-by: Jan Jahoda <[email protected]>
Co-authored-by: Jan Jahoda <[email protected]>
Co-authored-by: Cory Nelson <[email protected]>
Co-authored-by: Cory Nelson <[email protected]>
@scalablecory @aik-jahoda can you please take a look again and approve if it looks good? |
xml/System.Net.Http/HttpClient.xml
Outdated
9. <xref:System.Net.Http.HttpClient.SendAsync%2A> | ||
|
||
<xref:System.Net.Http.HttpClient> is intended to be instantiated once and re-used throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors. Below is an example using HttpClient correctly. | ||
<xref:System.Net.Http.HttpClient> is intended to be instantiated once and re-used throughout the life of an application. HttpClient is designed to pool connections, and re-use a connection across multiple requests. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors. Below is an example using HttpClient correctly. |
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.
This will result in SocketException errors.
Though they'll be wrapped when actually emerging into user code, right? e.g. in an HttpRequestException?
@samsp-msft will you have a chance to address the latest suggestions? There's also a merge conflict to fix. |
@samsp-msft just in case you missed the ping, it looks like it would be valuable to get merged 😀 |
Merge conflict resolved, added in @stephentoub's changes
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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.
@samsp-msft I left some suggestions mostly around active voice. LMK if you want me to commit them for you. It would be great to get this merged today.
Docs Build status updates of commit 19922bc: ✅ Validation status: passed
For more details, please refer to the build report. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
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
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Docs Build status updates of commit 89367bb: ✅ Validation status: passed
For more details, please refer to the build report. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
Summary
Added more detail about the behavior and usage of HttpClient trying to address user feedback and confusion.
Fixes dotnet/docs#11901
Fixes /issues/3523
Fixes /issues/1518
Fixes /issues/3276
Fixes /issues/1326