AspNetCoreDiagnosticScenarios icon indicating copy to clipboard operation
AspNetCoreDiagnosticScenarios copied to clipboard

Returning async/await advice contradicts other MSFT advice

Open jnsn opened this issue 4 years ago • 6 comments

In the AsyncGuidance document, there's a section that states that async/await is preferred over directly returning Task.

In his talk Correcting Common Async/Await Mistakes in .NET, Brandon Minnick advises to directly return the Task over async/await. (The specific bit is around 24:27.)

Since this is contradictory advice given by two MSFT employees, I feel like you guys should align this somehow, or clarify this use-case a bit more. :-)

jnsn avatar Jul 12 '19 06:07 jnsn

The guidance here to never just return the Task seems like a safer default to use. In that video it looks like he mentions the troubles you can have if you return the Task from within a try/catch block, but there are more ways you can trip yourself up if you aren't awaiting the Task. This article from Stephen Cleary https://blog.stephencleary.com/2016/12/eliding-async-await.html mentions more of the pitfalls, and at the bottom has some recommendations that basically say don't just return the Task unless it is a very simple method that just passes through to another Task returning method and will not throw any exceptions or anything.

dferretti avatar Jul 12 '19 12:07 dferretti

Agreed, the guidance here is a better general reference for people, default to the style that will be less surprising.

It even mentions the caveat that for high-performance scenarios it is then worth considering eliding, but only if you want/need that perf gain.

slang25 avatar Jul 12 '19 12:07 slang25

While reading through MSDN I found another directly contradictory example: The very last code block in https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/local-functions#local-functions-and-exceptions

that basically say don't just return the Task unless it is a very simple method that just passes through to another Task returning method and will not throw any exceptions or anything.

It's directly contradictory to this advice too as it basically is a wrapper which throws exceptions.

georg-jung avatar Dec 10 '19 15:12 georg-jung

Useful notes on sync contexts when using async/await - https://devblogs.microsoft.com/dotnet/configureawait-faq/

davidglassborow avatar Dec 13 '19 19:12 davidglassborow

... has some recommendations that basically say don't just return the Task unless it is a very simple method that just passes through to another Task returning method and will not throw any exceptions or anything.

The example given in the article in the OP saying not to return the Task directly seems to be exactly the type of method this other advice says is safe to return the Task directly.

public Task<int> DoSomethingAsync()
{
    return CallDependencyAsync();
}

So I’m still confused!

stevendarby avatar Jun 18 '20 04:06 stevendarby

Having something like this:

public class C
{
  public Task M()
  {
    return FooAsync(); //a Task returning method.
  }
}

It's totally fine if you do not care about handling exceptions in FooAsync().

In regard as to why it is faster to pass through the Task instead of awaiting. Check out the generated code to support async/await vs just passing through the Task:

https://sharplab.io/#v2:D4AQDABCCMCsDcBYAUCAzFATBAwhA3ihMVBiAGwQAKAhgM50AqAFgE4CuAFAJQFEkCQAdijkAdABEApgBsaAT07RuSZAIC+/YltJQAHKIgBBAO40AlgBcpAEx47CagSRABOUZNkKlKnZuTqQA===

epignosisx avatar Jun 18 '20 18:06 epignosisx