Async/Await not awaiting when SQL server unavailable

I have a .NET Core worker service that fetches a list of procedures from DB that needs to execute on a regular time interval. For every procedure Task is created with a delay after procedure executed. Different procedures have different delays.

protected override async Task ExecuteAsync(CancellationToken cancellationToken)
{
    var proceduresToExec = await _sqlRepository.GetProceduresToExecute(cancellationToken);

    var workers = new List<Task>();

    foreach (var proc in proceduresToExec)
    {
        workers.Add(ExecuteProcedure(proc, cancellationToken));
    }

    // IMPORTANT to wait for all tasks
    await Task.WhenAll(workers);
}

Method that executes procedure is:

private async Task ExecuteProcedure(ProcToExec proc, CancellationToken cancellationToken)
{
    while (!cancellationToken.IsCancellationRequested)
    {
        try
        {
            // This line throws multiple exceptions per 1ms when SQL is unavailable
            await _sqlRepository.ExecuteStoredProcedureAsync(proc, cancellationToken);
            await Task.Delay(proc.Delay, cancellationToken);
        }
        catch (Exception ex)
        {
             // exceptions are logged here
            _logger.LogError(ex);
        }
    }
}

SQL access is:

public async Task ExecuteStoredProcedureAsync(ProcToExec proc, CancellationToken cancellationToken)
{

     var connStr = _configuration["DB_Conn_string"];

     using (SqlConnection conn = new SqlConnection(connStr))
     using (SqlCommand cmd = new SqlCommand($"dbo.{proc.ProcedureName}", conn))
     {
         await conn.OpenAsync(cancellationToken);

         cmd.CommandType = CommandType.StoredProcedure;

         await cmd.ExecuteNonQueryAsync(cancellationToken);
    }

    return;
}

Problem begins when SQL instance becomes unavailable.

Expected behaviour: Program tries to execute stored procedure, instance is unavailable. When timeout expires, throw error and then wait for delay (2000ms). After delay is done, retry.

Reality: When SQL instance becomes unavailable, logger logs multiple messages per 1ms. Average is ~8k logged errors per 1s.

Can anyone point me in direction what am I doing wrong?

Answer

See code comments:

private async Task ExecuteProcedure(ProcToExec proc, CancellationToken cancellationToken)
{
    while (!cancellationToken.IsCancellationRequested)
    {
        try
        {
            // This line throws multiple exceptions per 1ms when SQL is unavailable
            await _sqlRepository.ExecuteStoredProcedureAsync(proc, cancellationToken);
            // If above line throws, this following line 
            // WILL NOT BE EXECUTED!
            // Control flow will jump into the `catch` and go 
            // immediately into the next iteration.
            await Task.Delay(proc.Delay, cancellationToken);
        }
        catch (Exception ex)
        {
             // exceptions are logged here
            _logger.LogError(ex);
        }
    }
}

Also mind that this will only ever leave the loop IF cancellation is requested. If you want to return on successful execution, you have to actually make it do that.

According to your requirement “Expected behaviour: Program tries to execute stored procedure, instance is unavailable. When timeout expires, throw error and then wait for delay (2000ms). After delay is done, retry.” – I’d have expected to see something like this:

private async Task ExecuteProcedure(ProcToExec proc, CancellationToken cancellationToken)
{
    while (!cancellationToken.IsCancellationRequested)
    {
        try
        {
            await _sqlRepository.ExecuteStoredProcedureAsync(proc, cancellationToken);
            return;
        }
        catch (Exception ex)
        {
             // exceptions are logged here
            _logger.LogError(ex);
            await Task.Delay(proc.Delay, cancellationToken);
        }
    }
}

Maybe also consider Polly to introduce error handling policies (not affiliated to the project).

---

Another after-thought: Even in the last snippet, that should somewhat do what’s expected, you do not have a means to report to the caller if the procedure has been executed successfully or if the operation has been cancelled. Maybe think about whether you need that.