Skip to content
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

Add remark about timeout scope on HttpCompletionOptions enum #10750

Merged
merged 8 commits into from
Jan 29, 2025

Conversation

foviedoITBA
Copy link
Contributor

Summary

Update the HttpCompletionOption enum docs to clarify its effect on the HttpClient timeout.

Issue

HttpCompletionOptions effectively affect the scope of the timeout on HttpClient operations. The timeout always applies up until the HTTP client operations complete. Because HttpCompletionOptions control when the operations complete, they effectively change when the timeout stops applying. This might be implicit and obvious in hindsight, but it's routinely overlooked by developers. This MVP article is frequently cited by developers using ResponseHeadersRead, yet the article mentions nothing about the timeout scope: https://www.stevejgordon.co.uk/using-httpcompletionoption-responseheadersread-to-improve-httpclient-performance-dotnet

Solution

Update the HttpCompletionOption enum docs to explicitly call this behavior out.

@foviedoITBA foviedoITBA requested a review from a team as a code owner December 10, 2024 20:33
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Dec 10, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl

Copy link

Learn Build status updates of commit 175f557:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Http/HttpCompletionOption.xml ⚠️Warning View Details

xml/System.Net.Http/HttpCompletionOption.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'HttpCompletionOption'.
  • Line 0, Column 0: [Warning: disallowed-html-tag - See documentation] HTML tag 'see' isn't allowed. Replace it with approved Markdown or escape the brackets if the content is a placeholder.
  • Line 0, Column 0: [Warning: disallowed-html-tag - See documentation] HTML tag 'see' isn't allowed. Replace it with approved Markdown or escape the brackets if the content is a placeholder.
  • Line 0, Column 0: [Warning: disallowed-html-tag - See documentation] HTML tag 'see' isn't allowed. Replace it with approved Markdown or escape the brackets if the content is a placeholder.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Copy link

Learn Build status updates of commit 87b4042:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Http/HttpCompletionOption.xml ⚠️Warning View Details

xml/System.Net.Http/HttpCompletionOption.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'HttpCompletionOption'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

@foviedoITBA foviedoITBA requested a review from a team as a code owner December 20, 2024 19:39
Copy link

Learn Build status updates of commit 1a44c31:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Http/HttpCompletionOption.xml ⚠️Warning View Details
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.cs ✅Succeeded View
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.csproj ✅Succeeded
snippets/csharp/System.Net.Http/HttpCompletionOption/Program.cs ✅Succeeded

xml/System.Net.Http/HttpCompletionOption.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'HttpCompletionOption'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Copy link

Learn Build status updates of commit 1c8336f:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.cs ✅Succeeded View
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.csproj ✅Succeeded
snippets/csharp/System.Net.Http/HttpCompletionOption/Program.cs ✅Succeeded
xml/System.Net.Http/HttpCompletionOption.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@foviedoITBA
Copy link
Contributor Author

@dotnet-policy-service agree company="Microsoft"

Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

Thank you for the help here and sorry about the delayed review

… and rewording

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
Copy link

Learn Build status updates of commit 04c37e4:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.cs ✅Succeeded View
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.csproj ✅Succeeded
snippets/csharp/System.Net.Http/HttpCompletionOption/Program.cs ✅Succeeded
xml/System.Net.Http/HttpCompletionOption.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link

Learn Build status updates of commit 95bbed4:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.cs ✅Succeeded View
snippets/csharp/System.Net.Http/HttpCompletionOption/HttpCompletionOptionSnippets.csproj ✅Succeeded
snippets/csharp/System.Net.Http/HttpCompletionOption/Program.cs ✅Succeeded
xml/System.Net.Http/HttpCompletionOption.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@MihaZupan MihaZupan merged commit f113ee0 into dotnet:main Jan 29, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Http community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants