Skip to content

Add remarks about stored exceptions #8751

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 9, 2023
Merged

Add remarks about stored exceptions #8751

merged 4 commits into from
Jan 9, 2023

Conversation

gewarren
Copy link
Contributor

@gewarren gewarren commented Dec 30, 2022 •
edited
Loading

Adds boilerplate text that asynchronous methods store the same exceptions that their synchronous counterpart can throw, with a link to that counterpart.

Preview of remarks.

I used a utility to add these remarks to *Async methods that have a matching synchronous method (571 of 1136 *Async methods).

Contributes to #7840.

cc @stephentoub

@ghost ghost assigned gewarren Dec 30, 2022
@ghost ghost added the area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable. label Dec 30, 2022
@learn-build-service-prod

This comment was marked as outdated.

Sign in to view
@gewarren gewarren marked this pull request as ready for review December 30, 2022 01:56
@learn-build-service-prod

This comment was marked as outdated.

Sign in to view
@ManickaP
Copy link
Member

ManickaP commented Jan 2, 2023 •
edited
Loading

Just an idea, but could the link to the sync method point directly to the Exception sub-heading (#exceptions)? E.g.: https://review.learn.microsoft.com/en-us/dotnet/api/system.io.pipes.pipestream.flush?view=net-7.0&branch=pr-en-us-8751#exceptions

@gewarren
Copy link
Contributor Author

gewarren commented Jan 3, 2023

Just an idea, but could the link to the sync method point directly to the Exception dub-heading (#exceptions)? E.g.: https://review.learn.microsoft.com/en-us/dotnet/api/system.io.pipes.pipestream.flush?view=net-7.0&branch=pr-en-us-8751#exceptions

That's a good idea, except it's only effective for methods with a single overload. That's because all overloads are on a single page, and an xref link to a specific overload is accomplished using a bookmark, for example, https://learn.microsoft.com/en-us/dotnet/api/system.string.substring?view=net-7.0#system-string-substring(system-int32). By adding #exceptions to the xref link, the bookmark to the specific overload is superseded by the bookmark to the Exceptions heading (and since there's more than one Exceptions heading on a page with multiple overloads, it just points to the top of the page).

@stephentoub
Copy link
Member

Thanks for working on this.

@learn-build-service-prod
Copy link

Learn Build status updates of commit 8032194:

✅ Validation status: passed

File Status Preview URL Details
xml/Microsoft.Extensions.Caching.Distributed/DistributedCacheExtensions.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.Distributed/IDistributedCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.Distributed/MemoryDistributedCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.Redis/RedisCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.SqlServer/SqlServerCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.StackExchangeRedis/RedisCache.xml ✅Succeeded View
xml/Microsoft.Extensions.DependencyInjection/AsyncServiceScope.xml ✅Succeeded View
xml/Microsoft.Extensions.DependencyInjection/ServiceProvider.xml ✅Succeeded View
xml/Microsoft.Extensions.Hosting/HostingAbstractionsHostBuilderExtensions.xml ✅Succeeded View
xml/Microsoft.Extensions.Hosting/HostingAbstractionsHostExtensions.xml ✅Succeeded View
xml/Mono.Security.Interface/IMonoSslStream.xml ✅Succeeded View
xml/System.Activities/WorkflowInvoker.xml ✅Succeeded View
xml/System.CodeDom.Compiler/IndentedTextWriter.xml ✅Succeeded View
xml/System.CommandLine.Parsing/ParseResultExtensions.xml ✅Succeeded View
xml/System.CommandLine.Parsing/ParserExtensions.xml ✅Succeeded View
xml/System.CommandLine/CommandExtensions.xml ✅Succeeded View
xml/System.Data.Common/DbBatch.xml ✅Succeeded View
xml/System.Data.Common/DbCommand.xml ✅Succeeded View
xml/System.Data.Common/DbConnection.xml ✅Succeeded View
xml/System.Data.Common/DbDataReader.xml ✅Succeeded View
xml/System.Data.Common/DbDataSource.xml ✅Succeeded View
xml/System.Data.Common/DbTransaction.xml ✅Succeeded View
xml/System.Data.SqlClient/SqlBulkCopy.xml ✅Succeeded View
xml/System.Data.SqlClient/SqlCommand.xml ✅Succeeded View
xml/System.Data.SqlClient/SqlConnection.xml ✅Succeeded View

This comment lists only the first 25 files in the pull request.
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:

@gewarren
Copy link
Contributor Author

gewarren commented Jan 7, 2023

@stephentoub Can you take another look? Is this good to merge?

@gewarren gewarren enabled auto-merge (squash) January 9, 2023 23:10
@learn-build-service-prod
Copy link

Learn Build status updates of commit 55e5b69:

✅ Validation status: passed

File Status Preview URL Details
xml/Microsoft.Extensions.Caching.Distributed/DistributedCacheExtensions.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.Distributed/IDistributedCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.Distributed/MemoryDistributedCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.Redis/RedisCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.SqlServer/SqlServerCache.xml ✅Succeeded View
xml/Microsoft.Extensions.Caching.StackExchangeRedis/RedisCache.xml ✅Succeeded View
xml/Microsoft.Extensions.DependencyInjection/AsyncServiceScope.xml ✅Succeeded View
xml/Microsoft.Extensions.DependencyInjection/ServiceProvider.xml ✅Succeeded View
xml/Microsoft.Extensions.Hosting/HostingAbstractionsHostBuilderExtensions.xml ✅Succeeded View
xml/Microsoft.Extensions.Hosting/HostingAbstractionsHostExtensions.xml ✅Succeeded View
xml/Mono.Security.Interface/IMonoSslStream.xml ✅Succeeded View
xml/System.Activities/WorkflowInvoker.xml ✅Succeeded View
xml/System.CodeDom.Compiler/IndentedTextWriter.xml ✅Succeeded View
xml/System.CommandLine.Parsing/ParseResultExtensions.xml ✅Succeeded View
xml/System.CommandLine.Parsing/ParserExtensions.xml ✅Succeeded View
xml/System.CommandLine/CommandExtensions.xml ✅Succeeded View
xml/System.Data.Common/DbBatch.xml ✅Succeeded View
xml/System.Data.Common/DbCommand.xml ✅Succeeded View
xml/System.Data.Common/DbConnection.xml ✅Succeeded View
xml/System.Data.Common/DbDataReader.xml ✅Succeeded View
xml/System.Data.Common/DbDataSource.xml ✅Succeeded View
xml/System.Data.Common/DbTransaction.xml ✅Succeeded View
xml/System.Data.SqlClient/SqlBulkCopy.xml ✅Succeeded View
xml/System.Data.SqlClient/SqlCommand.xml ✅Succeeded View
xml/System.Data.SqlClient/SqlConnection.xml ✅Succeeded View

This comment lists only the first 25 files in the pull request.
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:

@gewarren gewarren merged commit 5d696e2 into dotnet:main Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephentoub stephentoub stephentoub approved these changes

@RussKie RussKie Awaiting requested review from RussKie

@merriemcgaw merriemcgaw Awaiting requested review from merriemcgaw merriemcgaw is a code owner

@Rick-Anderson Rick-Anderson Awaiting requested review from Rick-Anderson Rick-Anderson is a code owner

@kouvel kouvel Awaiting requested review from kouvel kouvel is a code owner

@HongGit HongGit Awaiting requested review from HongGit HongGit is a code owner

@mconnew mconnew Awaiting requested review from mconnew mconnew is a code owner

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@jkotalik jkotalik Awaiting requested review from jkotalik

@David-Engel David-Engel Awaiting requested review from David-Engel David-Engel is a code owner

@JRahnama JRahnama Awaiting requested review from JRahnama

@ajcvickers ajcvickers Awaiting requested review from ajcvickers

Assignees

@gewarren gewarren

Labels
area-Meta Concerns something that extends across runtime area boundaries, for example, IDisposable.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gewarren @ManickaP @stephentoub