Dapper icon indicating copy to clipboard operation
Dapper copied to clipboard

SqlMapper.GridReader.ReadUnbufferedAsync possibly hiding exceptions

Open DerPeit opened this issue 1 year ago • 0 comments

We're currently facing issues with querying multiple results and reading each of them unbuffered.

After investigating the Dapper source code I think we may have some exception that is being hidden by Dapper. Our stack trace reveals an exception being thrown in OnAfterGridAsync which is being called in a finally block. I understand that this block is necessary.

async Task Main()
{
    await foreach (var record in ReadUnbufferedAsync())
    {
        // ...
    }
}

private async Task OnAfterGridAsync()
{
    await Task.Yield();
    throw new InvalidOperationException("OnAfterGridAsync");
}

private async Task<bool> ReadAsync()
{
    await Task.Yield();
    throw new InvalidOperationException("ReadAsync"); // this exception will be hidden by the other
}

private string ConvertTo()
{
    return "Record";
}

public async IAsyncEnumerable<string> ReadUnbufferedAsync()
{
    try
    {
        while (await ReadAsync())
        {
            yield return ConvertTo();
        }
    }
    finally
    {
        await OnAfterGridAsync();
    }
}

However, wouldn't it be possible to apply refactoring to properly propagate exceptions to the call-site?

async Task Main()
{
    await foreach (var record in ReadUnbufferedAsync())
    {
        // ...
    }
}

private async Task OnAfterGridAsync()
{
    await Task.Yield();
    throw new InvalidOperationException("OnAfterGridAsync");
}

private async Task<bool> ReadAsync()
{
    await Task.Yield();
    throw new InvalidOperationException("ReadAsync");
}

private string ConvertTo()
{
    return "Record";
}

public async IAsyncEnumerable<string> ReadUnbufferedAsync()
{
    ExceptionDispatchInfo? exceptionCaught = null;
    
    try
    {
        while (true)
        {
            string record;
            
            try
            {
                var hasNext = await ReadAsync();
                
                if (!hasNext)
                {
                    break;
                }
                
                record = ConvertTo();
            }
            catch (Exception ex)
            {
                exceptionCaught = ExceptionDispatchInfo.Capture(ex);
                
                break;
            }
            
            yield return record;
        }
    }
    finally
    {
        try
        {
            await OnAfterGridAsync();
        }
        catch
        {
            if (exceptionCaught == null)
            {
                throw;
            } // else swallow this exception and propagate the other
        }
        
        exceptionCaught?.Throw();
    }
}

DerPeit avatar Jun 11 '24 11:06 DerPeit