Rainbow.Dapper.Database.Dispose(true) should never throw an exception

[dmytro-gokun]

The current implementation of this method is the following:

       public void Dispose()
        {
            if (_connection.State != ConnectionState.Closed)
            {
                _transaction?.Rollback();

                _connection.Close();
                _connection = null;
            }
        }

Here's one problem. If this method is called in "using" statement or as a part of an explicit "finally" block it may happen so that it will fail and throw and exception of its own, thus, hiding the original exception.

Here's one case i'm getting from time to time:

System.InvalidOperationException: This SqlTransaction has completed; it is no longer usable.
   at System.Data.SqlClient.SqlTransaction.ZombieCheck()
   at System.Data.SqlClient.SqlTransaction.Rollback()
   at Dapper.Database`1.Dispose() in C:\projects\dapper\Dapper.Rainbow\Database.cs:line 473
   at MyClass.MyMethod() in D:\MyClass.cs:line 666

Here, as we can see the original exception is lost forever. The only work-around I can invent is to do it like this (pseudo-code):

using (db = NewDb())
{
    try
    {
        db.BeginTransaction();
        // Useful work
        db.CommitTransaction();
    }
    catch (ex)
    {
        Log(ex);
        throw;
    }
}

which kind of sucks, especially taking into account that we need to put this boiler plate in every place we use Rainbow.Database.

Actually, it's a good practice to not throw from Dispose(true).

Thoughts?

I can provide a PR if it's going to be accepted.

[dmytro-gokun]

In fact, here https://docs.microsoft.com/en-us/dotnet/api/system.data.common.dbconnection.close?view=netstandard-2.0 we can read:

"The Close method rolls back any pending transactions. It then releases the connection to the connection pool, or closes the connection if connection pooling is disabled."

So, I'd imagine that the correct implementation for "Dispose" should be something like this:

        public virtual void Dispose()
        {
                var connection = _connection;
                _connection = null;
                _transaction = null;
                connection?.Close();
        }

[dmytro-gokun]

Does anybody experience this issue at all? Or is it just me? I'm running .NET Core 2.2, just in case.