Azure-functions-host: Add analyzer for compiled C# functions to disallow "async void"

Created on 31 May 2017  路  5Comments  路  Source: Azure/azure-functions-host

If you specify a compiled C# function (mine was queue triggered) as async void, eg:

public static async void Run(SomeQueueType queueItem, TraceWriter log) { ... }

Then the functions runtime will:

  • Incorrectly state that the function succeeded even if it fails (throwing exceptions, etc)
  • Will display logs in the portal monitoring view for the failed function execution instance incorrectly (won't show the exception log, because the event indicating function completion has already been logged).
  • Result in data loss: queue items will be consumed even if the function fails

Expected behavior:
Functions defined in this way are correctly monitored, or if they can't be correctly implemented due to C# language async semantics, they should be explicitly blocked by the SDK at compile or at least at run/bind time.

Workarounds:

Change "void" to "Task", so definition is:

public static async void Run(SomeQueueType queueItem, TraceWriter log) { ... }

Obviously took me quite a while to figure out what was going on here. I understand from the runtime perspective why this bug exists, but given it is non obvious behavior and results in data loss, the runtime should deal with this.

Repro'd locally on latest azure-functions-core-tools SDK (1.0.0-beta.97). Also believe this has been hitting me in production hosted on Azure.

improvement

Most helpful comment

The plan is to indeed add those to the tooling for pre-compiled functions. In those cases, we'd lean towards making some of those compilation errors, which is what we wanted to do for CSX but couldn't for backwards compatibility reasons.

All 5 comments

We have these async void analyzers for .csx functions - we'll need to see if something similar can be added for precompiled functions

https://github.com/Azure/azure-webjobs-sdk-script/blob/62c624e90018b12c671dda5d4cefc810a45c5bd7/src/WebJobs.Script/Description/DotNet/Compilation/CSharp/Analyzers/AsyncVoidAnalyzer.cs

The plan is to indeed add those to the tooling for pre-compiled functions. In those cases, we'd lean towards making some of those compilation errors, which is what we wanted to do for CSX but couldn't for backwards compatibility reasons.

FYI, @mamaso and @faviocav - I think the impact of this issue may be even more insidious than I thought. It appears that if you have a function deployed that hits this problem, and the function throws an exception, it crashes the function host process (understandable why - there's probably no try catch block controlling its scope anymore). I can repro this locally, and I assume it's happening hosted in Azure too.

Speculation: when that host process crashes, it can lose the recent log information of any other functions present in that functions app, even functions that don't have the same declaration problem. Yesterday I filed this issue which I thought was separate but I now believe actually isn't - it stopped repro'ing as soon as I deployed a work around to this issue by switching from void to Task.

@fabiocav YES, let's make it an error! I changed the titled to reflect the fix we'd make.

We need this analyzer extracted into a nuget package that can be referenced by the functions project created by the VS tooling

Was this page helpful?
0 / 5 - 0 ratings