async void method lacks await

Question

I am trying to do async programming for the first time but it is not working as I expect it. I have a button which loads a collection of urls (this is ommitted from the code snippets)

private async void btnLoad_Click(object sender, EventArgs e)
{
    foreach (var item in myCollectionOfUrls)
    {
        Uri tempUri = new Uri(item);
        Uri = tempUri; // Uri is a property
        string htmlCode = await LoadHtmlCodeAsync(Uri);
        LoadAllChaptersAsync(htmlCode, Path.GetFileNameWithoutExtension(item));
    }
}

LoadHtmlCodeAsync(Uri) works as intended:

private string LoadHtmlCode(string url)
{
    using (WebClient client = new WebClient())
    {
        try
        {
            System.Threading.Thread.Sleep(0);
            client.Encoding = Encoding.UTF8;
            client.Proxy = null;
            return client.DownloadString(url);
        }
        catch (Exception ex)
        {
            Logger.Log(ex.Message);
            throw;
        }
    }
}

But LoadAllChaptersAsync throws an error "this async method lacks await operators..."

private async void LoadAllChaptersAsync(string htmlCode, string mangaName)
{
    HtmlAgilityPack.HtmlDocument htmlDoc = new HtmlAgilityPack.HtmlDocument();
    htmlDoc.LoadHtml(htmlCode);

    var chapterLink = htmlDoc.DocumentNode.SelectNodes(@"//div[@id='chapterlist']//a/@href");
    var chapterName = htmlDoc.DocumentNode.SelectNodes(@"//div[@id='chapterlist']//a/@href/following-sibling::text()[1]").Reverse().ToList();
    for (int i = 0; i < chapterLink.Count; i++)
    {
        var link = "http://" + Uri.Host + chapterLink[i].GetAttributeValue("href", "not found");
        var chName = chapterName[i].OuterHtml.Replace(" : ", "");
        var chapterNumber = chapterLink[i].InnerText;
        Chapters.Add(new Tuple(link, chName, chapterNumber, mangaName));
    }
}

My expected result is that Chapters (a property of type List containing a Tuple) gets populated after I am done with extracting the information I need from the html source code. I want to do this asynchronously because for larger amounts of urls this process could take a while and I don't want to block the UI thread (it is a windows form app).

What did I do wrong?

Alex · Accepted Answer

But LoadAllChaptersAsync throws an error:
this async method lacks await operators...

That is because your LoadAllChaptersAsync method does not perform any asynchronous operations, and does not await any.

A common misconception is that using the async (or await) keyword in a method somehow magically creates a new task on a different thread. It does not.

I want to do this asynchronously because for larger amounts of urls this process could take a while and I don't want to block the UI thread (it is a windows form app).

You could change your method to return a new Task that performs work in the background, and that will return a new list with all of the newly created "chapters" on task completion. As in:

private Task>>
LoadAllChaptersAsync(string htmlCode, string mangaName)
{
    return Task.Run(() {
        var newChapters = new List>();

        // ...

        return newChapters;
    });
}

This task can be then awaited, there is no need to mark your method that does not do anything async as async.

var newChapters = await LoadAllChaptersAsync(htmlCode, Path.GetFileNameWithoutExtension(item));
Chapters.AddRange(newChapters);

Additional improvements

There are two improvements that we could make to the above solution. We can incorporate a few best practices for tasks that are primarily CPU bound and whose implementation does not include async/awaits.

Let the client (i.e. the caller) as high up in the call stack as possible decide whether to invoke the method as a separate task or to invoke it synchronously.
If the work done by the method could take a while you may want to be able to cancel it. So if it does its work in steps or a loop, make it support cooperative cancellation, by using a CancellationToken connected to a CancellationTokenSource.

For your code you might want to supply a "Stop Loading" button in the UI, and when clicked, use the following to cancel the work performed in the LoadAllChaptersAsync method:

private async void btnStopLoading_Click(object sender, EventArgs e)
{
    if (_loadChaptersCancelSource != null)
        _loadChaptersCancelSource.Cancel();
}

Then your original code could be changed to:

private async void btnLoad_Click(object sender, EventArgs e)
{
    if (_loadChaptersCancelSource == null)
    {
        var wasCancelled = false;
        _loadChaptersCancelSource = new CancellationTokenSource();
        try
        {
            var token = _loadChaptersCancelSource.Token;
            foreach (var item in myCollectionOfUrls)
            {
                // stop if cancellation was requested.
                token.ThrowIfCancellationRequested();

                Uri tempUri = new Uri(item);
                Uri = tempUri; // Uri is a property

                // also modified to be cancellable.
                string htmlCode = await LoadHtmlCodeAsync(Uri, token); 

                // client decides to run as a background task
                var newChapters = await Task.Run(() =>  
                    LoadAllChapters(htmlCode, Path.GetFileNameWithoutExtension(item), token), 
                    token);
                Chapters.AddRange(newChapters);
            }
        }
        catch (OperationCanceledException) 
        { 
            wasCancelled = true;
        }
        catch (AggregateException ex) 
        {
            if (!ex.InnerExceptions.Any(e => typeof(OperationCanceledException).IsAssignableFrom(e.GetType())))
                throw; // not cancelled, different error.
            wasCancelled = true;
        }
        finally
        {
            var cts = _loadChaptersCancelSource;
            _loadChaptersCancelSource = null;
            cts.Dispose();
        }
        if (wasCancelled)
            ; // Show a message ?
    }
}

And your LoadAllChapters could be a regular synchronous method, that allows for cooperative cancellation:

private List>
LoadAllChapters(string htmlCode, string mangaName, CancellationToken cancelToken)
{
    HtmlAgilityPack.HtmlDocument htmlDoc = new HtmlAgilityPack.HtmlDocument();
    htmlDoc.LoadHtml(htmlCode);

    // Don't continue if cancelation is requested
    cancelToken.ThrowIfCancellationRequested();

    var chapterLink = htmlDoc.DocumentNode.SelectNodes(@"//div[@id='chapterlist']//a/@href");
    var chapterName = htmlDoc.DocumentNode.SelectNodes(@"//div[@id='chapterlist']//a/@href/following-sibling::text()[1]").Reverse().ToList();
    var newChapters = new List>();

    for (int i = 0; i < chapterLink.Count; i++)
    {
        // Stop the loop if cancellation is requested.
        cancelToken.ThrowIfCancellationRequested();

        var link = "http://" + Uri.Host + chapterLink[i].GetAttributeValue("href", "not found");
        var chName = chapterName[i].OuterHtml.Replace(" : ", "");
        var chapterNumber = chapterLink[i].InnerText;

        newChapters.Add(new Tuple(link, chName, chapterNumber, mangaName));
    }
    return newChapters;
}

A very similar approach (that does involve async operations) with some additional explanations can be found here: "Async Cancellation: Bridging between the .NET Framework and the Windows Runtime".

async void method lacks await

Answers (2)

Related Questions