Resharper Exception rethrow possibly intended

Question

Consider this method (pardon the sad attempt at Chuck Norris humor :) ):

public class ChuckNorrisException : Exception
{
    public ChuckNorrisException()
    {
    }

    public ChuckNorrisException(string message)
        : base(message)
    {
    }

    public ChuckNorrisException(string message, Exception cause)
        : base(message, cause)
    {
    }

    protected ChuckNorrisException(SerializationInfo info, StreamingContext context)
        : base(info, context)
    {
    }
}

static void ExceptionTest(double x)
{
    try
    {
        double y = 10 / x;
        Console.WriteLine("quotient = " + y);
    }
    catch (Exception e)
    {
        e = e is DivideByZeroException ? new ChuckNorrisException("Only Chuck Norris can divide by 0!", e) :
            e;
        throw e;
    }
}

In resharper, I get a warning on the "throw e" line saying "Exception rethrow possibly intended". But obviously in this case that's not the intention, since e could be wrapped in ChuckNorrisException, and if I just use "throw", that wrapped exception wouldn't get thrown.

I know I can suppress the resharper warning, but then it will be turned off for all scenarios if I'm not mistaken. I just wondered if anybody else had encountered this. The only workaround I've found is to make another exception variable (e2, for example), and throw that. That may be the best I can do here. Seems like resharper could detect this issue though and be smart enough to know that if e is modified, then throw e is ok.

Thanks.

[EDIT] Sorry, I forgot a step. Before the throw, I need to log the exception, so I can't just do:

e = e is DivideByZeroException ? new ChuckNorrisException("Only Chuck Norris can divide by 0!", e) :
            e;
throw e;

I have to do:

e = e is DivideByZeroException ? new ChuckNorrisException("Only Chuck Norris can divide by 0!", e) :
            e;
LogException(e);
throw e;

Why do you have to log the exception before throwing it (even if wrapped in another exception)? Why can the caller not log the exception, including the inner exception? — Adam Ralph, Feb 10 '17 at 01:37
Consider if this were a service method in a REST service, for example, and you wanted to log that exception on the server log file. So the caller (ex: web client) *could* log the exception, but it'd be much better to have access to that exception info on the server log itself. — dcp, Feb 10 '17 at 09:47
An exception raised in .NET code on the server cannot be caught by a client. The exception must be getting caught somewhere on the server otherwise the server would crash. — Adam Ralph, Feb 10 '17 at 09:49
@Adam Ralph - "An exception raised in .NET code on the server cannot be caught by a client" That's not exactly correct. For example, you can use a FaultException with a WCF service and that FaultException can be handled on the client. In our case, we log that exception on the server, and also catch the FaultException on the client and log it there as well. You can refer here for more info: https://msdn.microsoft.com/en-us/library/cc949036.aspx — dcp, Feb 10 '17 at 13:41

score 39 · Accepted Answer · edited May 23 '17 at 11:54

Maybe I'm not understanding the question, so please correct me if I've got the wrong end of the stick.

There's two cases going on here:

The first is that you catch the original exception. You then wrap it in a new exception instance as the inner exception, then throw the new one. No information is lost in this case (the inner exception preserves all information), so no warning is given.
The second is that you catch and re-throw the original exception. If you re-throw, you should never use "throw e", as it will tamper with the stack trace. This is why ReSharper is printing a warning. To re-throw the caught exception, you should use the "throw" keyword on its own.

The answer to this question explains it better than I can. Due to the subtle side effects and the sheer number of people who get this detail wrong, I personally view the re-throw syntax as flawed.

Anyway that's a description of why you're getting a warning. Here's what I'd do about it instead:

catch(DivideByZeroException e)
{
    // we don't catch any other exceptions because we weren't
    // logging them or doing anything with the exception before re-throwing
    throw new ChuckNorrisException("Useful information", e);
}

*Edit -- if you need to log exceptions, you can just do something like this instead. Note: This is my preferred solution as I think it reads better and is less likely to contain an error than querying for exception types yourself:

// catch most specific exception type first
catch(DivideByZeroException e)
{
    Log(e); 
    throw new ChuckNorrisException("Useful information", e);
} 
catch(Exception e) // log & re-throw all other exceptions
{
    Log(e);
    throw; // notice this is *not* "throw e"; this preserves the stack trace
}

Another alternative would be:

catch(Exception e)
{
    Log(e);
    if(e is DivideByZeroException)
    {
        // wrap + throw the exception with our new exception type
        throw new ChuckNorrisException("useful info", e);
    }

    // re-throw the original, preserving the stack trace
    throw;
}

Great answer! I did not know that about "throw e" messing with the stack trace. By the way, in case anyone else is interested here is a good link on this topic in case you want to learn more: http://www.tkachenko.com/blog/archives/000352.html One thing I didn't know either is that this behavior is different in C# vs. JAVA. Thanks again Mark! — dcp, Nov 21 '09 at 20:48

score 2 · Answer 2 · answered Nov 21 '09 at 20:11

2

This will have the same effect as the code you have posted and should not cause the warning.

catch (DivideByZeroException de)
    {
        throw new ChuckNorrisException("Only Chuck Norris can divide by 0!", de);
    }
}

answered Nov 21 '09 at 20:11

Adam Ralph

29,453
4
60
67

You are right, however, I left out a step in my original question (my fault). Sorry for that. – dcp Nov 21 '09 at 20:16

Resharper Exception rethrow possibly intended

2 Answers2