Microsoft.Data.SqlClient.SqlConnection.GetOpenTdsConnection throws random exception invalid operation, connection closed

[daxu7509]

Describe the bug

posted on stackoverflow as well

I was using System.Data.Client and see above error, according to this fix, I should upgrade to Microsoft.Data.Cient, so I did that (.net framework 4.8 windows running environment)

However, from the log, I still see the same error, the only difference is it was System.Data.SqlClient.SqlConnection.GetOpenTdsConnection, now it is Microsoft.Data.SqlClient.SqlConnection.GetOpenTdsConnection.

For the fix mentioned in the previous ticket, does it apply to .net framework 4.8 on an azure windows app service?

[JRahnama]

@daxu7509 can you post a simple repro that we can investigate at our end?

[JRahnama]

Also, it would be nice if we can have the error stack trace please.

[daxu7509]

hi @JRahnama ,
It is a quite basic using block usage, it takes some user defined table types as parameter and open connection async, run reader async. So nothing really worth writing. The code is bit like this:
using (SqlCommand sqlCommand = new SqlCommand())
{
var connectionString = ConnectionStringHelper.GetConnectionString(wareHouseId);
using( var connection = new SqlConnection(connectionString))
sqlCommand.Connection = connection;

        await sqlCommand.Connection.OpenAsync();
       using(var sqlDataReader = await sqlCommand.ExecuteReaderAsync(CommandBehavior.CloseConnection))
       {
       }
    }

}
We saw this error on production system when using System.Data.Client, so we changed to Microsoft.Data.Client, however, we saw the same error happening as well.

One strange thing is that when this error happened on our azure testing site, our appinsight logged no request (as the exception was logged around 2am and no one was testing that).

AppInsight shows this:
"[{""id"":""28672654"",""outerId"":""0"",""type"":""System.AggregateException"",""message"":""A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread."",""severityLevel"":""Critical""},{""id"":""37200188"",""outerId"":""28672654"",""type"":""System.InvalidOperationException"",""message"":""Invalid operation. The connection is closed."",""parsedStack"":[{""level"":0,""method"":""Microsoft.Data.SqlClient.SqlConnection.GetOpenTdsConnection"",""assembly"":""Microsoft.Data.SqlClient, Version=4.1.0.0, Culture=neutral, PublicKeyToken=23ec7fc2d6eaa4a5"",""line"":0},{""level"":1,""method"":""Microsoft.Data.SqlClient.SqlCommand+<>c__DisplayClass201_1.b__0"",""assembly"":""Microsoft.Data.SqlClient, Version=4.1.0.0, Culture=neutral, PublicKeyToken=23ec7fc2d6eaa4a5"",""line"":0},{""level"":2,""method"":""System.Threading.Tasks.ContinuationTaskFromResultTask`1.InnerInvoke"",""assembly"":""mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"",""line"":0},{""level"":3,""method"":""System.Threading.Tasks.Task.Execute"",""assembly"":""mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"",""line"":0}],""severityLevel"":""Critical""}]"

[JRahnama]

@daxu7509 what connection string properties are being used?

[daxu7509]

"data source=xxxxx.xxxxxx7.database.windows.net;initial catalog=xxxx;user id=xx;Min Pool Size=30;Max Pool Size=300;TimeOut=25;TransparentNetworkIPResolution=False;ApplicationIntent=ReadOnly;TrustServerCertificate=true"
@JRahnama

[JRahnama]

@daxu7509 just to confirm a theory, if you take out CommandBehavior.CloseConnection everything works fine. correct?
Is it possible to post the UDT (user defined type) here as well?

[daxu7509]

hi @JRahnama I never tried this, don't know. Can I ask why you think that could make some difference? The most used UDT are these two:
CREATE TYPE [dbo].[Code] AS TABLE (
[Idx] INT NULL,
[TypeCode] VARCHAR (20) NULL,
[InstrType] VARCHAR (10) NULL,
[Universe] VARCHAR (20) NULL,
[Code] VARCHAR (20) NULL );

CREATE TYPE [dbo].[Setting] AS TABLE (
[Name]  NVARCHAR (50)  NULL,
[Value] NVARCHAR (50)  NULL);

[JRahnama]

@daxu7509 seems like the reader is disposed before getting the results back in an async call. When the reader is disposed the CommandBehavior.CloseConnection will make sure that the connection is closed that is why you are getting the message indicating the connection is closed.
{""id"":""37200188"",""outerId"":""28672654"",""type"":""System.InvalidOperationException"",""message"":""Invalid operation. The connection is closed."

[daxu7509]

in the production code, we wrap connection and command in polly retry, like this:

using (var cmd = new SqlCommand
{
CommandType = commandType,
CommandTimeout = taskTimeoutInSeconds, // in secs
CommandText = commandText
})
{
cmd.Parameters.AddRange(parameters.ToArray());
//Pass table Valued parameter to Store Procedure

            // this is the maxium amount of time to allow for any one attempt. If is exceeded then the request fails
            var timeoutPerTry = Policy
                .TimeoutAsync(context =>
                {
                    // ReSharper disable once AccessToDisposedClosure
                    var tryCount = CommonConnection.ProcessRetry(context);
                    var timeoutMs = taskTimeout * Math.Pow(Constants.RetryFactor, tryCount);
                    
                    taskTimeoutInSeconds= (int)(timeoutMs / 1000);
                    cmd.CommandTimeout = taskTimeoutInSeconds;

                    Log.LogVerbose(
                        $"Execute {commandText} try count of {tryCount} from context, timeout is now {timeoutMs} ms.");
                    return TimeSpan.FromMilliseconds(timeoutMs);
                }, TimeoutStrategy.Optimistic);

            // retry SqlException up to MaxRetries
            var retryPolicy = Policy
                .Handle<SqlException>()
                .RetryAsync(Constants.MaxRetries,
                    (response, calculatedWaitDuration, context) =>
                    {
                        Log.LogError(
                            $"Failed dynamic execution attempt. Retrying. {response.Message} - {response.StackTrace}");
                    });

            try
            {
                var combinedPolicy = retryPolicy.WrapAsync(timeoutPerTry);
                // ReSharper disable once AccessToDisposedClosure
                await combinedPolicy.ExecuteAsync(async () => {

                    var connectionString = ConnectionStringHelper.GetConnectionString(warehouseId);
                    using (var connection = new SqlConnection(connectionString))  // assumed no need for using block as closed by caller
                    {
                        await connection.OpenAsync();
                        cmd.Connection = connection;
                        
                        using (var reader = await cmd.ExecuteReaderAsync(CommandBehavior.CloseConnection))
                        {
                            results = mapper.Map<IDataReader, IEnumerable<T>>(reader);
                        }
                    }
                });

                //cmd.Connection = null;        //  https://stackoverflow.com/a/31234469/1033684
            }
            catch (SqlException ex) when (ex.Number == -2)  // -2 is a sql timeout
            {
                throw new ThunderTimeoutException(Constants.HttpResponseTimeoutSql);
            }
            catch (TimeoutRejectedException)
            {
                throw new ThunderTimeoutException(Constants.HttpResponseTimeoutTask);
            }

[daxu7509]

does that have any impact on the problem we see?

[JRahnama]

@daxu7509 Retry Logic is not compatible with the CommandBehavior.CloseConnection . For more information you can look into
issue #1454. We are in the process of updating our documentation for this matter.

[daxu7509]

oh, dear. I see where you coming from now. @JRahnama

I will remove CommandBehavior.CloseConnection and see what happens.

With the polly retry, as long as I put our things in using bracket, that should still handle all dispose and connection close of SqlConnection, SqlCommand and SqlDataReader? Do I need to do something special?

[JRahnama]

With the polly retry, as long as I put our things in using bracket, that should still handle all dispose and connection close of SqlConnection, SqlCommand and SqlDataReader? Do I need to do something special?

Correct. The Using block will take care of disposing and closing the connection and command.

Closing the issue due to inactivity. Feel free to open a new issue or comment here if there are more to discuss.