Skip to content

Document exceptions on async Socket methods. #8525

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

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Oct 17, 2022

Fixes #1350.

@rzikm rzikm requested a review from a team as a code owner October 17, 2022 15:23
@ghost ghost assigned rzikm Oct 17, 2022
@ghost ghost added the area-System.Net label Oct 17, 2022
@ghost
Copy link

ghost commented Oct 17, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #1350.

Author: rzikm
Assignees: rzikm
Labels:

area-System.Net

Milestone: -

@opbld34
Copy link

opbld34 commented Oct 17, 2022

Learn Build status updates of commit d08cc96:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Sockets/SocketTaskExtensions.xml ⚠️Warning View Details
xml/System.Net.Sockets/Socket.xml ✅Succeeded View
xml/System.Net/IPEndPoint.xml ✅Succeeded View

xml/System.Net.Sockets/SocketTaskExtensions.xml

  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtension.AcceptAsync(System.Net.Sockets)'.
  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtensions.AcceptAsync(System.Net.Sockets.Socket, System.Net.Sockets.Socket)'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Learn Build.

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.

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:

@rzikm rzikm force-pushed the 1350-Needs-exceptions-documented branch from 4a4e019 to d08cc96 Compare October 19, 2022 12:47
@opbld32
Copy link

opbld32 commented Oct 19, 2022

Learn Build status updates of commit 4a4e019:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Sockets/SocketTaskExtensions.xml ⚠️Warning View Details
xml/System.Net.Http/WinHttpHandler.xml ✅Succeeded View
xml/System.Net.Sockets/Socket.xml ✅Succeeded View
xml/System.Net/IPEndPoint.xml ✅Succeeded View

xml/System.Net.Sockets/SocketTaskExtensions.xml

  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtension.AcceptAsync(System.Net.Sockets)'.
  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtensions.AcceptAsync(System.Net.Sockets.Socket, System.Net.Sockets.Socket)'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Learn Build.

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.

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:

@opbld31
Copy link

opbld31 commented Oct 19, 2022

Learn Build status updates of commit d08cc96:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Sockets/SocketTaskExtensions.xml ⚠️Warning View Details
xml/System.Net.Sockets/Socket.xml ✅Succeeded View
xml/System.Net/IPEndPoint.xml ✅Succeeded View

xml/System.Net.Sockets/SocketTaskExtensions.xml

  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtension.AcceptAsync(System.Net.Sockets)'.
  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtensions.AcceptAsync(System.Net.Sockets.Socket, System.Net.Sockets.Socket)'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Learn Build.

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.

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
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

We should be consistent with NotSupportedException, otherwise looks good.

We should also review & fix the sync and the Begin* methods, but I will do that together with #5045, once this PR is merged.

@opbld33
Copy link

opbld33 commented Nov 7, 2022

Learn Build status updates of commit 8533200:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/System.Net.Sockets/SocketTaskExtensions.xml ⚠️Warning View Details
xml/System.Net.Sockets/Socket.xml ✅Succeeded View
xml/System.Net/IPEndPoint.xml ✅Succeeded View

xml/System.Net.Sockets/SocketTaskExtensions.xml

  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtension.AcceptAsync(System.Net.Sockets)'.
  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'System.Net.Sockets.SocketTaskExtensions.AcceptAsync(System.Net.Sockets.Socket, System.Net.Sockets.Socket)'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Learn Build.

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.

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
Member

@liveans liveans left a comment

Choose a reason for hiding this comment

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

LGTM

@opbld33
Copy link

opbld33 commented Nov 7, 2022

Learn Build status updates of commit f3280b7:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Net.Sockets/Socket.xml ✅Succeeded View
xml/System.Net.Sockets/SocketTaskExtensions.xml ✅Succeeded View
xml/System.Net/IPEndPoint.xml ✅Succeeded View

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:

Copy link
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

LGTM

@rzikm rzikm merged commit 24aba1e into dotnet:main Nov 8, 2022
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.

Needs exceptions documented
7 participants