System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning() silently swallows error conditions

[Dmitry-Me]

System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning() has a Debug.Assert check that the errors connection is not empty and also this excellent if:

 SqlConnection exception = null;
 //later...
 if (temp != null && temp.Count > 0)
      //craft exception
 }

 //later ...
 if (exception != null ) {
     //do a lot of cool stuff
 } // else do nothing

which means that allowing to have this function called when there're no elements in temp leads to a logical error - no error indication is given and the connection is just silently closed.

Those Debug.Asserts should instead throw something like InvalidOperationException.

[saurabh500]

@Dmitry-Me What is the issue here? Are you asking to change what the Debug assert failure does because there is some chance that this function can be incorrectly invoked from a location where there may be nothing in temp ?

[Dmitry-Me]

@saurabh500 Yes, the current implementation allows for error hiding. With assertions disabed it can called in scenarios where it silently does nothing.

[JonHanna]

Can you show a repro where that happens?

[Dmitry-Me]

@JonHanna No, I cannot. This is "what if the Moon in in wrong phase" kind of defect.

[divega]

As recently announced in the .NET Blog, focus on new SqlClient features an improvements is moving to the new Microsoft.Data.SqlClient package. For this reason, we are moving this issue to the new repo at https://github.com/dotnet/SqlClient. We will still use https://github.com/dotnet/corefx to track issues on other providers like System.Data.Odbc and System.Data.OleDB, and general ADO.NET and .NET data access issues.

[David-Engel]

Closing as not planned.