Why does ReSharper recommend the removal of its own fix?

Question

ReSharper recommends rethrowing an exception and then, when I do that, it says the entire catch clause is redundant anyway, and suggests it be removed.

I am using this code (from MethodMan here):

public static DataTable ExecuteDataSet(string sql, CommandType cmdType, params SqlParameter[] parameters)
{
    using (DataSet ds = new DataSet())
    using (SqlConnection connStr = new SqlConnection(UsageRptConstsAndUtils.CPSConnStr))
    using (SqlCommand cmd = new SqlCommand(sql, connStr))
    {
        cmd.CommandType = cmdType;
        foreach (var item in parameters)
        {
            cmd.Parameters.Add(item);
        }

        try
        {
            cmd.Connection.Open();
            new SqlDataAdapter(cmd).Fill(ds);
        }
        catch (SqlException ex)
        {
            throw;
        }
        return ds.Tables[0];
    }
}

When I have ReSharper Inspect > Code Issues in Solution, it wonders whether "exception rethrow possibly intended" here:

catch (SqlException ex)
{
    throw ex;
}

If I accept ReSharper's suggested fix ("rethrow exception"), Resharper removes the "ex":

catch (SqlException ex)
{
    throw;
}

...but then, on the next Inspection, it says, "catch clause is redundant" and suggests it be removed altogether.

But, of course, if I remove the catch block, it won't compile ("Expected catch or finally"). I could remove the try...but... if I change it to this:

                catch (SqlException sqlex)
                {
                    for (int i = 0; i < sqlex.Errors.Count; i++)
                    {
                        var sqlexDetail = String.Format("From
ExecuteDataSet(), SQL Exception #{0}{1}Source: {2}{1}   
Number: {3}{1}State: {4}{1}Class: {5}{1}Server: {6}{1}Message: {7}
{1}Procedure: {8}{1}LineNumber: {9}",
                            i + 1, // Users would get the fantods if they
saw #0
                            Environment.NewLine,
                            sqlex.Errors[i].Source,
                            sqlex.Errors[i].Number,
                            sqlex.Errors[i].State,
                            sqlex.Errors[i].Class,
                            sqlex.Errors[i].Server,
                            sqlex.Errors[i].Message,
                            sqlex.Errors[i].Procedure,
                            sqlex.Errors[i].LineNumber);
                        MessageBox.Show(sqlexDetail);
                    }
                }
                catch (Exception ex)
                {
                    String exDetail
String.Format(UsageRptConstsAndUtils.ExceptionFormatString, ex.Message, 

Environment.NewLine, ex.Source, ex.StackTrace);
                    MessageBox.Show(exDetail);
                }

...ReSharper's inspection doesn't complain about it.

Answer 1

catch (SqlException ex)
{
    throw ex;
}

Is a bad programming practice. You lose the original stack trace from the thrown exception.

catch (SqlException ex)
{
   throw;
}

Does absolutely nothing for you except waste time re-throwing the same identical exception. Re-sharper is expecting you to do something with the variable ex like log it.

Answer 2

When you do this, you're resetting the call stack, which loses valuable information about what threw the exception in the first place.

catch (SqlException ex)
{
    throw ex;
}

If ReSharper were being smarter, it would've told you to just remove that part in the first place, saving you the time it took to rewrite that portion of code.

The following code is better since it doesn't lose the stack trace info, but it's unnecessary.

catch (SqlException ex)
{
    throw;
}

Even if you omit the above, the exception is going to "bubble up" and can just be caught somewhere further up the stack in your program, wherever you're ready to actually deal with it (log it, or display a message, or take some alternative action, etc).