Async & ContinueWith

I am implementing my own UnitOfWork for an EntityFramework-DataContext.

I want to call SaveChangesAsync and when this succeeds, clear a list. Is my approach with ContinueWith correct?

public async Task CommitAsync(int? commandTimeout = null, bool ignoreCustomLogic = false)
{
    try
    {
        return await DataContext.SaveChangesAsync().ContinueWith((antecedent) =>
        {
            if (!antecedent.IsFaulted)
            {
                _addedEntities.Clear();
                _changedEntities.Clear();
                _deletedEntities.Clear();
            }
            return antecedent.Result;
        });
    }
    catch (DbEntityValidationException dbEx)
    {
        RethrowException(dbEx);
        return default(int);
    }
}

private void RethrowException(DbEntityValidationException dbEx)
{
    string message = "ValidationExceptions: " + Environment.NewLine;
    foreach (var validationResult in dbEx.EntityValidationErrors)
    {
        foreach (var validationError in validationResult.ValidationErrors)
        {
            message += string.Format("Property: {0} Error: {1}{2}", validationError.PropertyName, validationError.ErrorMessage, Environment.NewLine);
        }
    }
    throw new Exception(message, dbEx);
}

I want to call SaveChangesAsync and when this succeeds, clear a list.

You don't need ContinueWith, you can just write it like this

public async Task CommitAsync()
{
    var result = await DataContext.SaveChangesAsync();
    _addedEntities.Clear();
    _changedEntities.Clear();
    _deletedEntities.Clear();
    return result;
}

In the original code, the catch block will never run.

Consider what happens when SaveChangesAsync throws a DbEntityValidationException: antecedent.Result will throw an AggregateException wrapping the DbEntityValidationException.

It's poor practice to throw an Exception as it means client code cannot be specific about the exceptions it catches. I would suggest changing RethrowException to a method that constructs the exception message you want, then calling it like this:

public async Task CommitAsync()
{
    try
    {
        var result = await DataContext.SaveChangesAsync();
        _addedEntities.Clear();
        _changedEntities.Clear();
        _deletedEntities.Clear();
        return result;
    }
    catch (DbEntityValidationException e)
    {
        var exceptionMessage = GetDbEntityValidationExceptionMessage(e);
        throw new DbEntityValidationException(exceptionMessage, e);
    }
}

This also removes the return default(int); which is just there to appease the compiler.

StackExchange Code Review Q#91799, answer score: 6