0

I am implementing a message dispatcher/handler where each handler must implement an interface like Task<Result> HandleAsync(IEvent myevent)

For example:

public async Task<Result> HandleAsync(CustomerChangedEvent ev)
{
    await DoSomethingAsync();
    ...
    return myResult;
}

But sometimes I need to run something which is sync only so no await and that as you know will give you a compiler warning. What I have done is to add await Task.CompletedTask; in my code. Works fine AFAIK but is there any downside doing this?

Another alternative is to drop the async but in that case I need to return a Task, right? For example to return Task.FromResult(myResult).

Is there any benefits for one method over another (or different approach), especially from a performance perspective (like is there any substantial cost to declare a method as async. Personally, I prefer the first alternative. For some reason I find return Task.FromResult harder to read and understand even if the first method adds a dummy command.

EDIT:

Sometimes it is maybe better to rethink the logic. "funatparties" commented why run sync code with an async method.

Reason is that I am implementing an outbox message handler where I am processing a queue with different messages to be processed and some have async operations other not. For each type of message I create a handler class which implements an interface.

I want adding handle a new message type I want it to be as few steps as possible. So basically what you do is to create a new handler which implements a specific interface and register the handler with my dispatcher class (which I probably can do automatically as well using Reflection).

The interface require you to implement Task. When reading the queue it will run the HandleAsync method using the registered handler (depending on message type).

A better approach maybe is that the handler can define either an sync or an async method depending on what I need to do.

Magnus
  • 63
  • 6
  • 1
    I'm curious, what was the exact compiler warning you got? – Wyck Jan 18 '23 at 21:03
  • 1
    Why would you make a method async when it does not do an async operation? – funatparties Jan 18 '23 at 21:07
  • Read great article [eliding async await](https://blog.stephencleary.com/2016/12/eliding-async-await.html) by Stephen Cleary. Without seeing the code calling `HandleAsync` it is hard to tell if there is any actual difference or not. Note that there also options with `await Task.Yield` or `Task.Run` which can be handy in some cases (see [this also answer](https://stackoverflow.com/a/75110165/2501279)) – Guru Stron Jan 18 '23 at 21:12
  • `Task.CompletedTask` when you return `Task` and `Task.FromResult(yourResult)` when you return `Task` – Chronicle Jan 18 '23 at 23:21
  • 1
    @Wyck Warning is CS1998."This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread." – Magnus Jan 19 '23 at 09:18

2 Answers2

2

Actually, I recommend ignoring the compiler warning in this case. Most of the time this compiler warning is useful, so I don't recommend disabling it solution-wide, just here.

The problem with Task.FromResult is that you also have to handle exceptions - catch them and return Task.FromException (or Task.FromCanceled). The problem with await Task.CompletedTask is that it's an unnecessarily expensive NOOP.

So I recommend ignoring the compiler warning. If you have to do this in several places, you can write a simple helper type like this.

openshac
  • 4,966
  • 5
  • 46
  • 77
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Interesting... never thought about exceptions. Is it worth the overhead of building the state machine for an `async` method that's not using `await`? (although I guess it would rarely be noticeable) – Gabriel Luci Jan 19 '23 at 02:26
  • 1
    The state machine ends up being pretty much what you would write by hand. The code is longer but I believe it would be equivalent execution time and memory garbage. – Stephen Cleary Jan 19 '23 at 02:27
  • Is exception still a problem if the method catch and not rethrow any exceptions? – Magnus Jan 19 '23 at 09:44
  • What does await Task:Completed expensive? Is it the await (context-switch or something) or Task.CompletedTask by itself? If the latter would await Task.Delay(0) be better? Anyway suppressing the warning is maybe better anyway. – Magnus Jan 19 '23 at 09:47
  • Hi Stephen, I have a question about your book: https://stackoverflow.com/questions/75144953/using-task-wait-on-a-parallel-task-vs-asynchronous-task – David Klempfner Jan 19 '23 at 11:22
  • @Magnus: If the method never raises exceptions, then there's no problem with exceptions. But no method should do that. – Stephen Cleary Jan 19 '23 at 11:50
  • @openshac: "noop" is short for "no operation", which is code that does nothing. – Stephen Cleary Jan 19 '23 at 11:52
  • @StephenCleary How do you mean ? I of course handle the exception (logging etc) but do not propagate the exception. Why would that be bad? – Magnus Jan 19 '23 at 14:37
  • @Magnus: It's normal for unexpected exceptions to crash the process. Unexpected exceptions (by definition) leave your app in an undefined state, and attempting to proceed in that state can cause loss of user data and other problems. See [fail-fast](https://en.wikipedia.org/wiki/Fail-fast). – Stephen Cleary Jan 19 '23 at 15:05
  • @StephenCleary: In my use case which is to send messages to external systems (you can see the method executed a s a full "job") and/or doing some database stuff exceptions (failures is a better name) are expected. Some failures are transient and will be retried, others be flagged as failed and will not be retried. Application will eventually have things like circuit breakers to avoid overloading system when having many transient errors. There are few exceptions I can think of except for "out of memory" which are not recoverable and will probably cause process to crash in other parts. – Magnus Jan 20 '23 at 10:48
  • @Magnus I think that Mr Cleary means that it's bad behavior for an asynchronous method to throw exceptions directly (synchronously), instead of propagating the exceptions through the returned `Task`. The only exception to this rule is argument-validation errors, like passing a negative `delay` to `Task.Delay`. Only these errors should be surfaced synchronously. – Theodor Zoulias Jan 20 '23 at 13:42
0

I'm guessing the compiler warning you are getting is that you're not using await in an async method.

Using await Task.CompletedTask does nothing useful other than suppress the warning. The await keyword does nothing when it acts on a completed Task. You get no benefit, but there is still the overhead of managing an async method and all the background stuff that has to happen to accommodate it.

If you have to return a Task<Result>, but are not doing anything asynchronously, then drop the async and return Task.FromResult(myResult).

Gabriel Luci
  • 38,328
  • 4
  • 55
  • 84