Moved info about WorkflowAlreadyStartedException into the right position

[petrkoutnycz]

What was changed

Moved the notice about WorkflowAlreadyStartedException to the <exception /> declaration within XML docs.

Why?

It is more intuitive for .NET developers to have the information regarding exceptions defined with the correct XML tag.

[cretz]

Technically this method doesn't throw this exception when called, the task does when awaited. I know .NET docs often only use this tag if calling the method sans await would throw the exception which isn't the case here. For example, see how Task.WhenAny or Task.WhenAll document their exceptions. So if you had:

try
{
    var task = Workflow.StartChildWorkflowAsync(...);
}
catch (WorkflowAlreadyStartedException e)
{
    // ...
}

You would not catch it but this documentation now makes you think you would. This kind of code is common in workflows:

var handleTasks = Enumerable.Range(0, 10).Select(i => Workflow.StartChildWorkflowAsync(...)).ToList();
var handles = await Task.WhenAll(handleTasks);

Sometimes we use this tag in other SDK docs even if it only occurs on await because it's inconsequential to differentiate, but it's fairly important for deterministic coroutine reasons that we are accurate with workflow docs since invoking child workflow is often separated from awaiting it.

[petrkoutnycz]

Got your point, so we can still it phrase it like If awaited, can be thrown.... What do you think?

[cretz]

Sure, but that's basically what the paragraph above said. Not sure we want to set the precedent of using <exception tags in Workflow docs for cases where the exception is thrown in a potentially-separate coroutine. I suppose it's not a big deal, but what is there today follows what .NET does with their docs, not sure we need to intentionally deviate.

[petrkoutnycz]

Hmm, that is interesting. I thought exception is the thing that Microsoft does, do you have some sample link of what they do? Thank you

[cretz]

Hrmm, reviewing it, it seems even Microsoft is inconsistent here.

So like at https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.whenall, they document an exception that can happen if you call without await (ArgumentException, ArgumentNullException, etc) but not what happens when you do await which is separate from making the call (AggregateException).

But at https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.getasync they do use exception tags for even post-await exceptions.

But at https://learn.microsoft.com/en-us/dotnet/api/system.io.file.readallbytesasync they don't. They specifically use a paragraph to say what may be thrown async and have you look at the sync version for the exceptions that can come post async instead of using exception tag right there. They do the same on https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.copytoasync and others.

https://learn.microsoft.com/en-us/dotnet/api/system.threading.channels.channelreader-1.readasync uses exception tag and says "This exception is stored into the returned task".

So while it is mixed, I think for at least the core parts of the standard library, they are not using exception tags, but there are other more external parts where they are. I do think it's important to users to know which exceptions can occur as part of invocation and which ones can occur as part of waiting.

Maybe we can just use the exception tag but make it clear "This exception is stored into the returned task" like MS does in some places?

[petrkoutnycz]

Well, that makes sense.I will adjust it.

[petrkoutnycz]

Updated, sorry it took so long time.

[cretz]

Thanks!