What is the proper way to rethrow an exception in C#? [duplicate]

Question

Is it better to do this:

try
{
    ...
}
catch (Exception ex)
{
    ...
    throw;
}

Or this:

try
{
    ...
}
catch (Exception ex)
{
    ...
    throw ex;
}

Do they do the same thing? Is one better than the other?

Aufmann

Answer 1

You should always use the following syntax to rethrow an exception. Else you'll stomp the stack trace:

throw;

If you print the trace resulting from throw ex, you'll see that it ends on that statement and not at the real source of the exception.

Basically, it should be deemed a criminal offense to use throw ex.

---

If there is a need to rethrow an exception that comes from somewhere else (AggregateException, TargetInvocationException) or perhaps another thread, you also shouldn't rethrow it directly. Rather there is the ExceptionDispatchInfo that preserves all the necessary information.

try
{
    methodInfo.Invoke(...);
}
catch (System.Reflection.TargetInvocationException e)
{
    System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(e.InnerException).Throw();
    throw; // just to inform the compiler that the flow never leaves the block
}

Avebury

There is an fxcop rules for that: RethrowToPreserveStackDetails msdn.microsoft.com/en-us/library/ms182363(VS.80).aspx – Gintz

Is that actually true? I think the information is still all there inside innerException. OH I get it. You do lose information with throw ex but wouldn't (it will be inside inner exception) with throw new Exception( "foo", ex). – Shantishantung

Couldn't agree more. It's truly surprising how many developers with 5+ years experience who don't truly understand exception handling best practices. I can't explain how much I want to poke my eyes out when I see try/catches that either swallow the exception without logging or rethrow the exception without doing anything of value in the catch. – Ankeny

Interesting, doesn't work for me. Stack trace is lost. No difference between throw ex or throw, same result. – Strategic

I've tested all with a simple divide by zero and I do not find this one to be the best. You lose the line number of the exception. You will only get the line of the throw statement. Should use throw new Exception("Useful Message", ex); This creates an inner exception that preserves the original stack and line number. – Truesdale

From .Net 4.5, a must use is: ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); https://mcmap.net/q/40923/-how-to-rethrow-innerexception-without-losing-stack-trace-in-c – Cowpox

Building upon @daniloquio, one shall further use ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); throw; to satisfy the compiler. #57883 – Macnamara

Answer 2

My preferences is to use

try
{
}
catch (Exception ex)
{
     ...
     throw new Exception ("Add more context here", ex)
}

This preserves the original error, but it allows you to add more context, such as an object ID, a connection string, and stuff like that. Often my exception reporting tool will have five chained exceptions to report, each reporting more detail.

Meaningless

Typically I will be throwing specific BusinesssLayer Exceptions (e.g. BusinessObjectInstantionFailed sort of thing). I prefer chained Exceptions, because I would expect most other developers to assume exceptions might be chained, but they might not interrogate other properties. – Meaningless

This will not preserve the stack trace. – Gymno

@kami the original stacktrace is still available in the inner exception. Whether you wrap the exception or just throw; depends on the case. – Pseudocarp

Exception should have been marked abstract – Doddering

The ONLY time you should do this is if you truly have some valuable context to add. Most times just throw; is sufficient. When you make a new exception and make the original exception an inner, you add complexity. Someone debugging it has to now dig into an inner exception to find the error. – Ainslie

Agreed - but why would you even catch the exception in the first place unless you can perform a useful action like adding context to it? The question presupposes that the exception cannot be handled at this point. – Meaningless

I found this technique particularly useful to catch and re-throw an exception in some recursive XML deserialization code. If I catch and then do throw new Exception( string.Format("while deserializing the element {0}", element.Name), ex ); then if I get a crash deep in the XML parsing, the top level exception handler prints all of the InnerExceptions and i get a full trace of the names of all of the nested XML nodes that it currently inside, which wouldn't be evident just by looking at the callstack which just has a lot of calls to the recursive function. – Lawana

It hides exception type – Stodder

Answer 3

If you throw an exception without a variable (the first example) the stack trace will include the original method that threw the exception.

In the second example, the stack trace will be changed to reflect the current method.

Example:

static string ReadAFile(string fileName) {
    string result = string.Empty;
    try {
        result = File.ReadAllLines(fileName);
    } catch(Exception ex) {
        throw;    // This will show ReadAllLines in the stack trace
        throw ex; // This will show ReadAFile in the stack trace
    }

Canebrake

Answer 4

The first preserves the original stack trace of the exception, the second replaces it with the current location.

Therefore the first is by far the better.

Availability

Answer 5

I'll agree that most of the time you either want to do a plain throw, to preserve as much information as possible about what went wrong, or you want to throw a new exception that may contain that as an inner-exception, or not, depending on how likely you are to want to know about the inner events that caused it.

There is an exception though. There are several cases where a method will call into another method and a condition that causes an exception in the inner call should be considered the same exception on the outer call.

One example is a specialised collection implemented by using another collection. Let's say it's a DistinctList<T> that wraps a List<T>, but refuses duplicate items.

If someone called ICollection<T>.CopyTo on your collection class, it might just be a straight call to CopyTo on the inner collection (if say, all the custom logic only applied to adding to the collection, or setting it up). Now, the conditions in which that call would throw are exactly the same conditions in which your collection should throw to match the documentation of ICollection<T>.CopyTo.

Now, you could just not catch the exception at all, and let it pass through. Here though, the user gets an exception from List<T> when they were calling something on a DistinctList<T>. It is not the end of the world, but you may want to hide those implementation details.

Or you could do your own checking:

public CopyTo(T[] array, int arrayIndex)
{
  if(array == null)
    throw new ArgumentNullException("array");
  if(arrayIndex < 0)
    throw new ArgumentOutOfRangeException("arrayIndex", "Array Index must be zero or greater.");
  if(Count > array.Length + arrayIndex)
    throw new ArgumentException("Not enough room in array to copy elements starting at index given.");
  _innerList.CopyTo(array, arrayIndex);
}

That's not the worse code because it's boilerplate and we can probably just copy it from some other implementation of CopyTo where it wasn't a simple pass-through and we had to implement it ourselves. Still, it's needlessly repeating the exact same checks that are going to be done in _innerList.CopyTo(array, arrayIndex), so the only thing it has added to our code is 6 lines where there could be a bug.

We could check and wrap:

public CopyTo(T[] array, int arrayIndex)
{
  try
  {
    _innerList.CopyTo(array, arrayIndex);
  }
  catch(ArgumentNullException ane)
  {
    throw new ArgumentNullException("array", ane);
  }
  catch(ArgumentOutOfRangeException aore)
  {
    throw new ArgumentOutOfRangeException("Array Index must be zero or greater.", aore);
  }
  catch(ArgumentException ae)
  {
    throw new ArgumentException("Not enough room in array to copy elements starting at index given.", ae);
  }
}

In terms of new code added that could potentially be buggy, this is even worse. And we don't gain a thing from the inner exceptions. If we pass a null array to this method and receive an ArgumentNullException, we're not going to learn anything by examining the inner exception and learning that a call to _innerList.CopyTo was passed a null array and threw an ArgumentNullException.

Here, we can do everything we want with:

public CopyTo(T[] array, int arrayIndex)
{
  try
  {
    _innerList.CopyTo(array, arrayIndex);
  }
  catch(ArgumentException ae)
  {
    throw ae;
  }
}

Every one of the exceptions that we expect to have to throw if the user calls it with incorrect arguments, will correctly be thrown by that rethrow. If there's a bug in the logic used here, it's in one of two lines - either we were wrong in deciding this was a case where this approach works, or we were wrong in having ArgumentException as the exception type looked for. It's the only two bugs that the catch block can possibly have.

Now. I still agree that most of the time you either want a plain throw; or you want to construct your own exception to more directly match the problem from the perspective of the method in question. There are cases like the above where rethrowing like this makes more sense, and there are plenty of other cases. E.g., to take a very different example, if an ATOM file reader implemented with a FileStream and an XmlTextReader receives a file error or invalid XML, then it will perhaps want to throw exactly the same exception it received from those classes, but it should look to the caller that it is AtomFileReader that is throwing a FileNotFoundException or XmlException, so they might be candidates for similarly rethrowing.

We can also combine the two:

public CopyTo(T[] array, int arrayIndex)
{
  try
  {
    _innerList.CopyTo(array, arrayIndex);
  }
  catch(ArgumentException ae)
  {
    throw ae;
  }
  catch(Exception ex)
  {
    //we weren't expecting this, there must be a bug in our code that put
    //us into an invalid state, and subsequently let this exception happen.
    LogException(ex);
    throw;
  }
}

Skyline

No. Just no. Do not hide "implementation details" when throwing exceptions. This will just mess with peoples heads, when they look in your source for explanation of the exception and find that it does not generate the exception itself. With the original stack trace, you can at least find the point where the exception is generated to see exactly why it was thrown. – Windywindzer

@SørenBoisen, why would they need to look at the source when the appropriate exception came from the appropriate method? If that isn't the case then you don't have one of the very rare cases where such throwing makes sense. – Skyline

Re "There is an exception though": Pun intended? – Attorney

What does "ATOM" in "an ATOM file reader" refer to? The text editor Atom? – Attorney

@PeterMortensen It refers to the XLM-based Atom web standard. en.wikipedia.org/wiki/Atom_(Web_standard) – Contravene

Answer 6

You should always use "throw;" to rethrow the exceptions in .NET,

Refer to the blog post Throw vs. Throw ex.

Basically, MSIL (CIL) has two instructions - "throw" and "rethrow" and C#'s "throw ex;" gets compiled into MSIL's "throw" and C#'s "throw;" - into MSIL "rethrow"! Basically I can see the reason why "throw ex" overrides the stack trace.

Savoury

Answer 7

The first is better. If you try to debug the second and look at the call stack you won't see where the original exception came from. There are tricks to keep the call-stack intact (try search, it's been answered before) if you really need to rethrow.

Lictor

Answer 8

I found that if the exception is thrown in the same method that it is caught, the stack trace is not preserved, for what it's worth.

void testExceptionHandling()
{
    try
    {
        throw new ArithmeticException("illegal expression");
    }
    catch (Exception ex)
    {
        throw;
    }
    finally
    {
        System.Diagnostics.Debug.WriteLine("finally called.");
    }
}

Shantishantung

Answer 9

It depends. In a debug build, I want to see the original stack trace with as little effort as possible. In that case, "throw;" fits the bill.

In a release build, however, (a) I want to log the error with the original stack trace included, and once that's done, (b) refashion the error handling to make more sense to the user. Here "Throw Exception" makes sense. It's true that rethrowing the error discards the original stack trace, but a non-developer gets nothing out of seeing stack trace information, so it's okay to rethrow the error.

        void TrySuspectMethod()
        {
            try
            {
                SuspectMethod();
            }
#if DEBUG
            catch
            {
                //Don't log error, let developer see
                //original stack trace easily
                throw;
#else
            catch (Exception ex)
            {
                //Log error for developers and then
                //throw a error with a user-oriented message
                throw new Exception(String.Format
                    ("Dear user, sorry but: {0}", ex.Message));
#endif
            }
        }

The way the question is worded, pitting "Throw:" vs. "Throw ex;" makes it a bit of a red herring. The real choice is between "Throw;" and "Throw Exception," where "Throw ex;" is an unlikely special case of "Throw Exception."

Mancilla

-1 For usage of compiler conditionals. – Trapezium

It's also a very tight coupling to place UI messages in the Exception message. Bad for maintainability. – Cairngorm

@EricJ. If it's in an exception message, then "user" means the developers who use the class, rather than those who maintain it. That's precisely the audience for exception messages. – Skyline

Why do you need to catch the exception at this point, anyway? Just catch the exception at the root (or add a global handler), show a dialog to the user, log ex.ToString() to the application log, whatever. And you don't need to bother yourself with whether or not you are running a debug build. – Habitude