Loading indicator
Skip to main content

Classic rock and async/await: Stop breaking the rules!

Vinyl record collection
Image: Vinyl by Dun.can, CC BY-SA 2.0

The universe demands some things must always occur in a certain order. Queen's "We Will Rock You" must be followed by "We Are The Champions." Same with Led Zeppelin's "Heartbreaker" -> "Living Loving Maid," Van Halen's "Eruption" -> "You Really Got Me," and Boston's "Foreplay" -> "Long Time." You have to. Every DJ knows this. It's the rule!

Async code in .NET has a similar rule. Calling a method that returns a Task must be awaited afterward. Like the Rule of We Will Rock You, the Rule of Awaiting Tasks is unwritten, but the compiler is a bad DJ—it provides no support in making sure you follow through.

We've seen our customers make this mistake too many times (we've even done it ourselves!). So we've updated NServiceBus to include a new Roslyn analyzer, which ensures this is one thing you can't screw up when using NServiceBus APIs. You should probably update to make sure you aren't making this mistake in your code right now.

The problem with async

Awaiting an async method isn't just a nice-to-have; it's mandatory. Otherwise, you open yourself up to a world of pain.

Consider this code:

public Task Handle(PlaceOrder message, IMessageHandlerContext context)
{
    db.SaveOrderDetails(message);
    return Task.CompletedTask;
}

So far so good. But now we want to publish an OrderPlaced event, so we add a line to do that:

public Task Handle(PlaceOrder message, IMessageHandlerContext context)
{
    db.SaveOrderDetails(message);
    context.Publish(new OrderPlaced { OrderId = message.OrderId });
    return Task.CompletedTask;
}

Now we have a problem. context.Publish() returns a Task, and that task needs to be awaited. If we forget, bad things start to happen:

  • The method execution returns to return Task.CompletedTask; before the Publish operation is completed.
  • The message being processed may complete (or not, randomly) before the Publish operation continues, meaning the current message context is no longer available.
  • The transaction wrapping the message handler completes before the message can be published.
  • The Publish operation fails with a transaction-related exception. Congratulations, you've now caused message loss.
  • The exception is never observed, so you don't find out what went wrong.
  • The compiler gives you absolutely no feedback that this could be a bad thing.

Worst of all, these problems become really hard to catch during development because it all boils down to when and in what order the scheduler decides to execute different async continuations. Forgetting an await is basically guaranteed to cause a race condition that you won't detect until you get production-level traffic. That's when you start observing weird and seemingly unrelated issues in production.

await just a minute!

But perhaps you’ll say that the compiler does provide feedback when you forget to await code. Well, that's true, but only if your method is already marked as async:

public async Task Handle(PlaceOrder message, IMessageHandlerContext context)
{
    db.SaveOrderDetails(message);
    // No await on context.Publish()
    context.Publish(new OrderPlaced { OrderId = message.OrderId });
}

Due to the addition of the async keyword on the method signature, this code generates the following compiler warning:

Warning CS2014: Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.

But for being so important, this is only a warning! Unless your project has been set up to treat all warnings as errors (and it probably should be), this code will still compile just fine.

Even though the async keyword will allow this situation to at least be detected, it's far too easy to forget it for the simple reason that it's Visual Studio's fault. When you use Visual Studio's tooling to implement the IHandleMessages<T> interface, this is what you get:

public Task Handle(PlaceOrder message, IMessageHandlerContext context)
{
    throw new NotImplementedException();
}

Notice anything? That's right, no async. Visual Studio doesn't bother to add the keyword that might save you.

While it seems like this is maybe a problem that Microsoft should be fixing in Visual Studio, we've seen too many customers get burned by this. With Roslyn analyzers, we have the tools to fix it and keep our users from experiencing this pain.

We're not awaiting any longer

We're now shipping a Roslyn analyzer directly in the NServiceBus package that will detect when you use one of our async API methods and you don't immediately await or assign the task to a variable. If you forget, you'll get this compile-time error:

ERROR NSB0001: A Task returned by an NServiceBus method is not awaited or assigned to a variable.

Problem solved. It will no longer be possible to forget to play "We Are The Champions" after "We Will Rock You." The compiler will make sure that you do.

We wanted to make this analyzer available for all our customers, regardless of which version of NServiceBus you're using. If you're on NServiceBus 6, you can use the NServiceBus 6.5 package to get the analzyer. If you've already updated to NServiceBus 7, it will be available in version 7.1.

One day Visual Studio may decide to fix this problem and require that tasks be assigned or awaited. If so, great! We'll happily remove the analyzer from NServiceBus at that time, content that we helped everyone to write better async code. But until that day, we've got your back.

Summary

Some may say the order in which you listen to songs by Queen doesn't matter. (I would not be one of those people.) But forgetting to await a Task definitely matters a whole lot. The production problems it causes are insidious and difficult to track down, yet they're entirely preventable.

That's why we're backporting this feature to NServiceBus 6, rather than just for NServiceBus 7. We think it's that important.

NServiceBus 6.5 and NServiceBus 7.1 are available on NuGet now. Check out the NServiceBus analyzer documentation, and then upgrade as soon as you can. You shouldn't have to worry about whether your async code is correct. We made that the compiler's job.

Unfortunately, the solution to the "We Will Rock You" problem still proves elusive…We'll get our engineers on it right away.


About the author: David Boike is a developer at Particular Software who loves Guns N' Roses but refuses to believe that they should be classified as classic rock.

Don't miss a thing. Sign up today and we'll send you an email when new posts come out.
Thank you for subscribing. We'll be in touch soon.
 
We collect and use this information in accordance with our privacy policy.
Need help getting started?