Await state async

Question

For speed we sometimes return response to consumer before state is saved in DB. Sometimes (Mostly for our automated consumers) this can break because the want to make actions on the saved data before it is saved. I wrote this little helper

public async Task<TEntity> GetWithRetry<TEntity>(Expression<Func<TEntity, bool>> predicate, string errorOnTimeout) where TEntity : class
{
    var stopwatch = new Stopwatch();
    stopwatch.Start();

    do
    {
        var entity = await _context.DbSet<TEntity>().FirstOrDefaultAsync(predicate);
        if(entity == null)
            await Task.Delay(100);
        else
            return entity;
    } while(stopwatch.Elapsed < TimeSpan.FromSeconds(1000));

    throw new Exception(errorOnTimeout);
}

Used like

var existingBooking = await GetWithRetry<Booking>(b => b.BookingKey == confirmCommand.BookingKey, "Booking key not found");

Any pitfalls? Task.Delay should scale well since it returns the thread to the pool, and if the data exists in DB the first time around it should not give much more overhead than an extra wrapped task?

Answer 1

This seems good to me with a few remarks

public async Task<TEntity> GetWithRetry<TEntity> (Expression<Func<TEntity, bool>> predicate, string errorOnTimeout) where TEntity : class
{
    var stopwatch = new Stopwatch();
    stopwatch.Start();

    do
    {
        var entity = await _context.DbSet<TEntity>().FirstOrDefaultAsync(predicate);
        if(entity == null)
            await Task.Delay(100);
        else
            return entity;
    } while(stopwatch.Elapsed < TimeSpan.FromSeconds(1000));

    throw new Exception(errorOnTimeout);
}  

• You should use braces {} for single statement if and else to make your code less error prone.
• Do you really think that it could take more than 16 minutes until the state is saved ? Maybe TimeSpan.FromMilliseconds() with a value of 2000 would be the better choice.
• hardcoding the timeout value isn't that good. You should make it more flexible by passing in an argument with a default value.
• by reverting the condition the else would become redundant.
• By convention, you should append "Async" to the names of methods that have an Async or async modifier.

Applying the mentioned changes would lead to

public async Task<TEntity> GetWithRetryAsync<TEntity>(Expression<Func<TEntity, bool>> predicate, string errorOnTimeout, double timeoutMilliseconds = 2000D) where TEntity : class
{
    var stopwatch = new Stopwatch();
    stopwatch.Start();

    do
    {
        var entity = await _context.DbSet<TEntity>().FirstOrDefaultAsync(predicate);
        if (entity != null)
        {
            return entity;
        }

        await Task.Delay(100);

    } while (stopwatch.Elapsed < TimeSpan.FromMilliseconds(timeoutMilliseconds));

    throw new Exception(errorOnTimeout);
}  

You should also use a more specific exception type.