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

Fix (Http)WebRequest (Begin|End)GetResponse samples #11030

Merged
merged 8 commits into from
Mar 4, 2025

Conversation

antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Feb 28, 2025

These samples were not handling the asyncResult.CompletedSynchronously == true case correctly: changing the sample code to use smaller buffer leads to stack overflow because of recursion on synchronous completions.

Besides the fix there the PR adds some consolidation:

  • Cleanup and (somewhat) modernize the sample code
  • Remove the Timer-based custom timeout implementation, use WebRequest.Timeout instead
  • Collapse various samples into one sample
    • Unifiy the HttpWebRequest and WebRequest samples since they are practically identical
    • Delete CPP and VB samples given we don't have the capacity and the intention to maintain them.

Fixes #8662

Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl

Copy link

Learn Build status updates of commit efe60c7:

✅ Validation status: passed

File Status Preview URL Details
snippets/cpp/VS_Snippets_Remoting/System.Net.HttpWebRequest.BeginGetResponse/CPP/begingetresponse.cpp ✅Succeeded n/a (file deleted or renamed)
snippets/cpp/VS_Snippets_Remoting/WebRequest_BeginGetResponse/CPP/webrequest_begingetresponse.cpp ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/HttpWebRequest/Abort/begingetresponse.cs ✅Succeeded View
snippets/csharp/System.Net/HttpWebRequest/Abort/Project.csproj ✅Succeeded
snippets/csharp/System.Net/HttpWebRequest/EndGetResponse/httpwebrequest_begingetresponse.cs ✅Succeeded
snippets/csharp/System.Net/HttpWebRequest/EndGetResponse/Project.csproj ✅Succeeded
snippets/csharp/System.Net/WebRequest/BeginGetResponse/Project.csproj ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/WebRequest/BeginGetResponse/webrequest_begingetresponse.cs ✅Succeeded n/a (file deleted or renamed)
snippets/visualbasic/VS_Snippets_Remoting/HttpWebRequest_BeginGetResponse/VB/httpwebrequest_begingetresponse.vb ✅Succeeded n/a (file deleted or renamed)
snippets/visualbasic/VS_Snippets_Remoting/System.Net.HttpWebRequest.BeginGetResponse/VB/begingetresponse.vb ✅Succeeded n/a (file deleted or renamed)
xml/System.Net/HttpWebRequest.xml ✅Succeeded View
xml/System.Net/WebRequest.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link

Learn Build status updates of commit b3b4b42:

✅ Validation status: passed

File Status Preview URL Details
snippets/cpp/VS_Snippets_Remoting/System.Net.HttpWebRequest.BeginGetResponse/CPP/begingetresponse.cpp ✅Succeeded n/a (file deleted or renamed)
snippets/cpp/VS_Snippets_Remoting/WebRequest_BeginGetResponse/CPP/webrequest_begingetresponse.cpp ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/HttpWebRequest/Abort/begingetresponse.cs ✅Succeeded View
snippets/csharp/System.Net/HttpWebRequest/Abort/Project.csproj ✅Succeeded
snippets/csharp/System.Net/HttpWebRequest/EndGetResponse/httpwebrequest_begingetresponse.cs ✅Succeeded
snippets/csharp/System.Net/HttpWebRequest/EndGetResponse/Project.csproj ✅Succeeded
snippets/csharp/System.Net/WebRequest/BeginGetResponse/Project.csproj ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/WebRequest/BeginGetResponse/webrequest_begingetresponse.cs ✅Succeeded n/a (file deleted or renamed)
snippets/visualbasic/VS_Snippets_Remoting/HttpWebRequest_BeginGetResponse/VB/httpwebrequest_begingetresponse.vb ✅Succeeded n/a (file deleted or renamed)
snippets/visualbasic/VS_Snippets_Remoting/System.Net.HttpWebRequest.BeginGetResponse/VB/begingetresponse.vb ✅Succeeded n/a (file deleted or renamed)
xml/System.Net/HttpWebRequest.xml ✅Succeeded View
xml/System.Net/WebRequest.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link

Learn Build status updates of commit ea80921:

✅ Validation status: passed

File Status Preview URL Details
snippets/cpp/VS_Snippets_Remoting/System.Net.HttpWebRequest.BeginGetResponse/CPP/begingetresponse.cpp ✅Succeeded n/a (file deleted or renamed)
snippets/cpp/VS_Snippets_Remoting/WebRequest_BeginGetResponse/CPP/webrequest_begingetresponse.cpp ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/HttpWebRequest/Abort/begingetresponse.cs ✅Succeeded View
snippets/csharp/System.Net/HttpWebRequest/Abort/Project.csproj ✅Succeeded
snippets/csharp/System.Net/HttpWebRequest/EndGetResponse/httpwebrequest_begingetresponse.cs ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/HttpWebRequest/EndGetResponse/Project.csproj ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/WebRequest/BeginGetResponse/Project.csproj ✅Succeeded n/a (file deleted or renamed)
snippets/csharp/System.Net/WebRequest/BeginGetResponse/webrequest_begingetresponse.cs ✅Succeeded n/a (file deleted or renamed)
snippets/visualbasic/VS_Snippets_Remoting/HttpWebRequest_BeginGetResponse/VB/httpwebrequest_begingetresponse.vb ✅Succeeded n/a (file deleted or renamed)
snippets/visualbasic/VS_Snippets_Remoting/System.Net.HttpWebRequest.BeginGetResponse/VB/begingetresponse.vb ✅Succeeded n/a (file deleted or renamed)
xml/System.Net/HttpWebRequest.xml ✅Succeeded View
xml/System.Net/WebRequest.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

@gewarren gewarren 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!

@antonfirsov antonfirsov merged commit 22d17b0 into dotnet:main Mar 4, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpWebRequest.BeginGetResponse example doesn't correctly follow the APM pattern
3 participants