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?

• Mathieu Guindon: In order to avoid answers addressing multiple different versions of the code, I rolled back your edit. If you want the final version reviewed, feel free to ask a new question.
• Anders: A bit unnecessary, but I don't mind. Its the task parts I want ideas around. Those haven't changed.

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.

• Anders: Thanks for feedback, thats a bug :D It should be milliseconds. The code is just a draft for the time being. The question is more about the Task parts.
• Anders: For exit early statements i like if (entity != null) return entity; on one line