Document stored exceptions

[gewarren]

This issue is to discuss if and how to document "stored exceptions", that is, exceptions that are stored in the returned object by Task-returning methods.

With async methods, and more generally Task-returning methods, a majority of exceptions don't emerge synchronously from the call. Rather, any exception gets stored into the returned Task, and then later asking that Task for the exception (via await'ing it, .Wait()'ing it, accessing its .Exception), etc., will surface the exception. If you have a method:

public static async Task MethodAsync()
{
    await Task.Yield();
    throw new FormatException();
}

and you call it as:

Task t = MethodAsync();

the call to MethodAsync will never throw a FormatException. Rather, such an exception may emerge when you later await the task, e.g.

await t; // throws FormatException
t.GetResult().GetAwaiter(); // throws FormatException
t.Wait(); // throws an AggregateException wrapping FormatException
Exception e = Task.Exception; // returns an AggregateException wrapping FormatException
...

Historically, then, we haven't documented FormatException as an exception thrown out of MethodAsync, because technically it's not. Of course, in the 95% use case where a developer does:

await MethodAsync();

the distinction is irrelevant, as it's not directly observable whether the exception was thrown from MethodAsync or by awaiting the task it returned.

Originally posted by @stephentoub in https://github.com/dotnet/dotnet-api-docs/issues/7692#issuecomment-1034151896

[gewarren]

Asynchronous methods can throw argument validation exceptions synchronously, so it seems like we'd need to differentiate stored exceptions from synchronous exceptions.

We'd probably need to use a different XML tag in the /// comments (for example, <storedexception cref="FormatException">The format is incorrect.</storedexception>. These exceptions could then be added to a new "Stored exceptions" heading/section in the API reference docs, which would exist in addition to the existing Exceptions heading. Or we (mdoc) could automatically add boilerplate text to stored exceptions, saying that they are thrown asynchronously, but still list them in the existing Exceptions section.

Thoughts?

[stephentoub]

Asynchronous methods can throw argument validation exceptions synchronously, so it seems like we'd need to differentiate stored exceptions from synchronous exceptions.

Thanks, @gewarren. Yes, we would need some other mechanism to document the exceptions a method expects to store into the returned task.

[gewarren]

@carlossanlop What are your thoughts on this? Is it important enough to developers to know which stored exceptions are thrown by a method to try and move this proposal forward on both the .NET and docs side?

[wischi-chr]

It depends on what you mean with "important enough". In C# you can't "know" what exceptions are thrown in a method because the compiler doesn't enforce that the documentation matches the actual behaviour. But (as it is now) the documentation can be used to get a sense of that the author of the method expects you to handle or at least know about.

With stored exceptions that information is completely missing. If I as a consumer use a method and am not sure what exceptions to expect I check out the documentation. If stored exceptions are missing in the documentation I can only guess which cases I should handle when I await the task, or "Pokémon"-exception-handle them all.

IMHO there is no difference in importance between stored exceptions and synchronous exceptions. It doesn't really make sense to me why the reader of the documentation should only know about the synchronous exceptions, when basically the entire ecosystem moves towards asynchronous code with async / await.

[antonfirsov]

What makes this even worse is that the current BCL docs are very inconsistent on this. Looks like we don't even have a widely accepted guideline. A few random examples:

• HttpClient.SendAsync does document async HttpRequestException and TaskCanceledException
• SslStream.AuthenticateAs*Async does document async AuthenticationException, but does not document OperationCanceledException/TaskCanceledException
• FileStream.WriteAsync does not document OperationCanceledException or IOException
• TcpListener.AcceptTcpClientAsync does document SocketException, but id does not document OperationCanceledException (#7692)

[stephentoub]

Looks like we don't even have a widely accepted guideline.

We do, or at least we did. The documented exceptions have always been about the synchronous invocation of the method. If there are cases where that's not being followed, such as in the networking examples you highlight, that's a bug in those networking docs.

IMHO there is no difference in importance between stored exceptions and synchronous exceptions.

There is an important difference. If you just use await on the result of the method call directly, then yes, it's indistinguishable other than in the resulting Exception's call stack. But if you use Task.WhenAll/Any (or any other combinator), if you use ContinueWith, if you use a custom awaiter (e.g. that suppresses the exception being thrown), if you delay the await in order to perform some fast-path check on the resulting task, and so on, it's very distinguishable.

[antonfirsov]

Looks like we don't even have a widely accepted guideline.

We do, or at least we did. The documented exceptions have always been about the synchronous invocation of the method. If there are cases where that's not being followed, such as in the networking examples you highlight, that's a bug in those networking docs.

Then it's a very widespread bug/misunderstanding. /cc @rzikm

[stephentoub]

Then it's a very widespread bug/misunderstanding.

Are there lots of examples outside of networking?

[antonfirsov]

Are there lots of examples outside of networking?

Actually, I was only able to find OperationCanceledException s incorrectly misdocumented as sync, so it's likely not that common as I thought.

[gewarren]

The plan of action is:

1) Automatically link from async methods to their synchronous counterpart for exception info. 2) Automatically add the OperationCanceledException if a method has a CancellationToken parameter.

Then we'll check the clickthrough data for the exception links to see if there is enough interest to warrant adding a new section to docs for stored exceptions (and documenting them all in the ///).

[antonfirsov]

Automatically link from async methods to their synchronous counterpart for exception info.

Some of the async methods don't have synchronous counterpart; there are no sync methods at all in System.Net.Quic for example, meaning that there is no way to document valuable exception information today. See discussion on https://github.com/dotnet/runtime/issues/78751.

adding a new section to docs for stored exceptions

This would be great. Since some of the networking libraries "wrongly" document async exceptions, we could move those to the new section incrementally.

[wfurt]

I assume we can simply add remarks until we reach agreement here? As mentioned above, Quic does not have synchronous calls so reference to it is not option. We currently provide no guidance so callers do not know what failures should be handled.

Alternatively, we could put remark to exception description e.g. in existing list of exception we can indicate what is thrown directly from the method call and what is stored in resulting Task.

[gewarren]

@wfurt That works for me.

[gewarren]

The latest idea is to document each stored exception like a synchronous exception, but add the boilerplate text This exception is stored into the returned <see cref="T:System.Threading.Tasks.Task1" />.` at the end of each description. Thoughts @wfurt @antonfirsov?

[antonfirsov]

That text sounds very verbose, but it's better than the current state. Note that in some cases an exception could be thrown both synchronously and asynchronously. You are saying that there is absolutely no realistic way to extend the docs/tooling infrastructure to have a separate Asynchronous Exceptions block and an <async-exceptions> xmldoc tag for API-s which need it?

Also, if we introduce some convention, ideally it should mean that teams which own async API-s wold have to go through them systematically, identify and document async exceptions.

IMHO this is a significant debt in the .NET stack that needs higher level PM attention, a top-down story for .NET 9.0 and probably a proposal in https://github.com/dotnet/designs.

[gewarren]

@antonfirsov I wouldn't say there's absolutely no realistic way, but it won't happen any time soon. I'll add a customer suggestion to recognize a separate exception tag (or attribute on the existing tag) and display stored exceptions differently. If/when that's implemented, it should be possible to use automation to easily convert any exceptions that have the boilerplate text into the new tags.

[Edit] New customer suggestion is here: https://dev.azure.com/ceapex/Engineering/_workitems/edit/911335