Right way of logging in async methods

Question

Two questions regarding the code from below:

is this implementation an implementation of an async method?

is it the right implemention of logging an exception that might appear during deletion a file?

public static async Task DeleteFiles(StorageFolder folder, Regex mask, LoggingChannel logger)
{
    var results = (from file in await folder.GetFilesAsync() where mask.IsMatch(file.Name) select file).Select(async f => await f.DeleteAsync());
    await Task.WhenAll(results);
    foreach (var failed in results.Where(r => r.Exception != null)) logger.LogMessage(failed.Exception.ToString(), LoggingLevel.Warning);
}

is it better (right?) way:

    public static async Task DeleteFiles(StorageFolder folder, Regex mask, LoggingChannel logger)
    {
        foreach(var f in (await folder.GetFilesAsync()).Where( f => mask.IsMatch(f.Name)))
        {
            try
            {
                await f.DeleteAsync();
            }
            catch(Exception ex)
            {
                logger.LogMessage(ex.ToString(), LoggingLevel.Warning);
            }
        }

    }

Where's the `catch` clause? `async/await` means that the code behaves just like synchronous code, ie it raises exceptions. This code will simply crash at the first error — Panagiotis Kanavos, May 31 '16 at 09:41
Also, using a `Select` clause to perform deletion is not a very good design. — Panagiotis Kanavos, May 31 '16 at 09:42
@Panagiotis Kanavos My idea was my results query returns an Enumerable> then I wait for completion of all Task and then I am taking only the IAsyncAction entities that contain a real exception and log them. But here are my two warries: such implementation is not an async and I am relay on the assumption that a task will be finished with either error on non. Msdn help doesnot specify an exception case clearly here https://msdn.microsoft.com/en-us/library/windows/apps/br227201.aspx — Yaugen Vlasau, May 31 '16 at 09:53
MSDN is very clear. `await` raises exceptions. No await, no exceptions. `await` doesn't make a method asynchronous, it awaits already asynchronous methods. You should *simplify* your code though. There is no reason to combine filename selection and deletion. — Panagiotis Kanavos, May 31 '16 at 09:56
@PanagiotisKanavos just added another implementation to the question. is that right one? — Yaugen Vlasau, May 31 '16 at 10:05

score 1 · Answer 1 · edited May 23 '17 at 12:02

The second implementation will delete the files one by one since every delete will be awaited giving a synchronous experience. So the first implementation might be quicker.

Proper implementation:

public static async Task DeleteFilesAsync(StorageFolder folder, Regex mask, LoggingChannel logger)
{
    var results = (from file in await folder.GetFilesAsync() where mask.IsMatch(file.Name) select file).Select(f => f.DeleteAsync());
    try
    {    
        await Task.WhenAll(results);
    }
    catch(Exception ex)
    {
        foreach (var failed in results.Where(r => r.Exception != null)) logger.LogMessage(failed.Exception.ToString(), LoggingLevel.Warning);
    }
}

I think exception handling should always be clearly identified in the code, hence the try/catch block.

Mind the .Select to get the tasks, I removed the unnecessary async/await there.

or do Task.WaitAll(results) and catch the AggregateException

See also Why doesn't await on Task.WhenAll throw an AggregateException?

score 0 · Answer 2 · answered Jun 02 '16 at 05:24

Another approach where logging can go in parallel. This is using rx and DataFlowBlock.

var actBlock = new ActionBlock<Task>(t => { var tsk = t.IsFaulted ? loggingTask : dummyTask; },new ExecutionDataflowBlockOptions
{
    MaxDegreeOfParallelism = configurableValue
});

var obsr = actBlock.AsObserver();
FileDeletionTasks.ToObservable().Subscribe(t => obsr.OnNext(t), async ex => await loggingTask);
actBlock.Complete();

Right way of logging in async methods

2 Answers2