Rick Strahl's Weblog  

Wind, waves, code and everything in between...
.NET • C# • Markdown • JavaScript • Angular
Contact   •   Articles   •   Products   •   Support   •   Search
Ad-free experience sponsored by:
ASPOSE - the market leader of .NET and Java APIs for file formats โ€“ natively work with DOCX, XLSX, PPT, PDF, images and more

Async/Await Calls Gotcha with the CSharp ? Null Propagator


:P
On this page:

So I ran into a bug in one of my projects this weekend. I've been working on updating Markdown Monster Editor to use the Chromium WebView Control. As part of that update I ended up going through an async/await Async Cascade scenario where a lot of code had to be converted from sync to async to accommodate the WebView's completely async-only interop APIs. This ended up touching a lot of methods just to get runnable code again. Eeek.

In the process I ended up with a lot of small method code like this:

/// <summary>
/// Focuses the Markdown editor in the Window
/// </summary>
public async Task SetEditorFocus()
{
    try
    {
    	// This!
        await EditorHandler?.JsEditorInterop?.SetFocus();
    }
    catch (Exception ex)
    {
        mmApp.Log("Handled: AceEditor.setfocus() failed", ex, logLevel: LogLevels.Warning);
    }
}

Looks like the await call should work, right? But it turns out it doesn't when JsEditorInterop is null.

When Markdown Monster starts up there might be several calls to this method when JsEditorInterop is still null, and with the code as it is it fails in that (common) scenario. That's expected and the call just doesn't happen as there's nothing to set focus to. With sync - no problem, but with async ๐Ÿ’ฃ.

So to summarize:

Sync code that works fine like this:

// sync code
EditorHandler?.JsEditorInterop?.SetFocus(); // works

doesn't work when using the same code with an await:

// async code
await EditorHandler?.JsEditorInterop?.SetFocus();  // boom

So why the tears with the await call here?

Why does this fail?

I didn't catch it right away and took to Twitter wondering why this failed with await vs. plain sync code, when @ssefaccan quickly picked up on what I missed:

namely that the async failure is not on the JsEditorInterop reference, but rather on the await call itself.

Let's break this down:

The ? (Elvis) operator in C# propagates the null value in JsEditorInterop and short circuits the call to SetFocus() which never happens in either version.

The first version works because null is a valid result for an expected result of a void method, or no result value from the method or assignment to a variable.

For the async code the same overall logic applies - JsEditorInterop returns null and SetFocus() is never called.

But... the expected result from an await call - even if there's no result value - is a Task object. Instead the result is null and the compiler generated async\await code internally can't deal with the null value. Hence the NullReferenceException when a value of Task is expected. And it goes Boom ๐Ÿ’ฃ

When you break it down like this the error makes perfect sense. But it's easy to miss, because of the different sync behavior which works as you would expect, while the async versions does - well, not so much...

Logical? Yes. But also very unintuitive!

Read the Signs!

Incidentally if I had paid better attention, Visual Studio (or Resharper really) would have been like: "Told you so!"

The IDE's Code Analysis is smart enough to actually catch the async null reference as invalid:

Then again... even if I had seen the warning before tracking the runtime error I would have probably ignored it as a false positive, and almost certainly wouldn't have drawn the right conclusion all the way to the null Task reference.

How to fix this?

The Twitter Thread has a bunch of different responses for approaches that can be used for this, but for me it seems simplest to use just an old school pre-checking for null:

public async Task SetFocus()
{
	var jsInterop = EditorHandler?.JsEditorInterop;
	if (jsInterop == null) return;
	
	try
	{
	    await jsInterop.SetFocus();
	}
	catch (Exception ex)
	{
	    mmApp.Log("Handled: AceEditor.setfocus() failed", ex, logLevel: LogLevels.Warning);
	}
}

Certainly more verbose, but it works.

The downside with this is that it's not very obvious on why the extra code is needed. It's easy to come through this code in the future and end up trying to refactor it back for null propagation, when that in fact won't work.

The sad part for me is that this code has just been bulk updated from sync code to async just in order to even get it to compile and I updated roughly 50 functions that all use a similar pattern. I now get to go back and hunt all of those down and adjust each method. I can hear the buzz of the Yak shavings in the distance...

Another Suggestion

Another suggestion that came up in the thread and perhaps an easier code fix is too double down on Elvis!

This code works too:

await (EditorHandler?.JsEditorInterop?.SetFocus() ?? Task.CompletedTask);

Note that the parenthesis are required in order to keep the scope to the entire expression.

This code explicitly checks for a null value of the first expression and if so returns a completed Task instance. This works too, and it's a shorter implementation. It's more explicit too, as it effectively points at the reasoning why null can't be returned here - ie. a Task is required. Not a huge fan due to the required parenthesis, but that's more of a personal preference.

Summary

It's a bummer that null propagation doesn't work with Task only (no result) results on await calls in the same it does with synchronous calls.

Looks like there's a proposal in the CSharp language repository for a feature request that would make await calls behave similar to sync calls, which certainly seems like a good idea. I can't think of a downside to this. But regardless even if this comes to pass, it's likely a few versions off and certainly wouldn't help me in my full framework C#7 code.

As with so many things in .NET - things seem like they are not working but due to the language ambiguities we see behavior that might be unexpected - but correct. Knowing how it works helps working around issues like this next time they pop up.

Writing them down as a reminder does too... ๐Ÿ˜ƒ

this post created and published with the Markdown Monster Editor
Posted in Csharp  Dotnet  

The Voices of Reason


 

Mattias
May 17, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

Maybe introduce a little extension method? ๐Ÿ˜€

    cpp
    
        
    
 public static class TaskExtensions

{ public static Task EnsureAwaitable(this Task? task) // The name is so-so... ๐Ÿคจ { return task ?? Task.CompletedTask; } }

Now you can simply call:

    css
    
        
    
 await EditorHandler?.JsEditorInterop?.SetFocus().EnsureAwaitable();

Danthar
May 17, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

if i understand this all correctly. Its failing because the SetFocus function returns null instead of a Task, in that case then....

Your first solution does not work. SetFocus can still return a null task. thus explode on the await.

Only your second solution is correct. Since then you handled this case and await is always executed on a valid Task instance.

In my humble opinion the fact that SetFocus can return null instead of a task is a bug in the API


Rick Strahl
May 17, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

@Danthar - no, SetFocus() cannot return null, because it's an async method. How could you possibly return null from it? The compiler won't allow it. If you remove the async you can potentially do it but not with the generated C# code that goes with async.


Yury Schkatula
May 17, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

I would laugh if that syntax proposal will end up like that:

await? EditorHandler?.JsEditorInterop?.SetFocus()

Thomas Levesque
May 17, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

I was going to suggest the same solution as Mattias, but I think the usage example is wrong. It should be ...?.SetFocus().EnsureAwaitable(), not ...?.SetFocus()?.EnsureAwaitable(); otherwise EnsureAwaitable() won't be called on null.


Rick Strahl
May 17, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

Thanks @Thomas - fixed his comment.@Yury - I'd hope there's no change to the way this works compared to the sync call. If it's different then the whole language feature addition becomes pointless IMO.


Steve
May 18, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

I think Maybe<T> can also help with these scenarios...

https://blog.ploeh.dk/2018/03/26/the-maybe-functor/


James
June 03, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

I haven't tested it, but I suspect the syntax for the suggested TaskExtensions will still require parenthesis, albeit in a slightly nicer place

await (EditorHandler?.JsEditorInterop?.SetFocus()).EnsureAwaitable();

This is because the elvis operator stops at the first null in a chain - if EditorHandler was null, then it wouldn't call EnsureAwaitable without the parenthesis.


Honza
August 18, 2021

# re: Async/Await Calls Gotcha with the CSharp ? Null Propagator

I got here from the MM conversion post (thanks a lot for a great article BTW).

A few thoughts:

  • Anyone using <Nullable>enabled</Nullable> does not need to worry about awaiting nulls, the compiler will check and warn you about it.
  • Do prefer to call ConfigureAwait(false) in suitable locations, this would produce a compile-time error (since the returned Nullable<ConfiguredTaskAwaitable> does not have an extension method GetAwaiter and it being a struct thus produces the error. (I guess this won't apply to this conversion situation where you're re-writing code in bulk, but I thought it was worth mentioning.) I would recommend using an analyzer for situations where ConfigureAwait is not called ๐Ÿ˜‰ (you'd get a bunch more warnings though)
  • Problems with ConfigureAwait is that the call becomes much more ugly await (foo?.Bar() ?? Task.CompletedTask).ConfigureAwait(false);
  • Using a ValueTask may produce even more warnings/hints
  • I would refrain from using EnsureAwaitable as it is prone to mistakes.

West Wind  © Rick Strahl, West Wind Technologies, 2005 - 2021