System.Data.SqlClient.SqlInternalConnectionTds constructor purges original call stack when rethrowing an exception

[Dmitry-Me]

System.Data.SqlClient.SqlInternalConnectionTds currently goes like this:

try
{
    OpenLoginEnlist(timeout, connectionOptions, redirectedUserInstance);
    break;
}
catch (SqlException sqlex)
{
    if (something)
    {
         throw sqlex;
    }

throw sqlex causes original call stack of where the exception was thrown to be lost. If that's what it desired then it should be explained, otherwise just use throw;

[saurabh500]

@Dmitry-Me How will replacing throw sqlex be different from using throw in this case?

[Dmitry-Me]

@saurabh500 The difference will be in the Exception.StackTrace when the exception is caught again in the outer scope. With throw; the stack trace will point onto the place where the exception was originally thrown. With throw x; it will point onto the line with throw x;.

[JonHanna]

It might be nice if it in fact go explicitly rethrown further up. Having it rethrown at the public method that called into this would mean the stack trace went as far as the caller could directly deal with (rather than having to hunt through a trace to find the first point that made sense to them). This cuts out some of that, but not all, and is perhaps falling between the two stools of a trace that is clearer to the caller and a trace that is fully informative.

[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.

[GSPP]

It is important that internal stack information is not destroyed. This is very valuable when guessing the source of bugs. throw; is correct.

I have never seen code that intentionally deletes part of the stack in order to make it easier to interpret the stack. The entire framework does not do this. This would not be a good choice.