Letting an exception to bubble up

Question

How to correctly let an exception to bubble up?
If I use Try-Catch when calling a method, is just throwing an exception inside a method like not trying to catch it at all?
For illustration: Are these approaches do the same work?

Example 1:

try
{
   MyFileHandlingMethod();             
}
catch (IOException ex)
{
   string recfilepath = "...
   string rectoadd = "RecDateTime=" + DateTime.Now.ToString()+ ...+ex.Message.ToString();
   File.AppendAllText(recfilepath, rectoadd);
}
catch (exception)
{
   throw;
}
...
MyFileHandlingMethod()
{
   ...
   TextReader tr2 = new StreamReader(nfilepath);
   resultN = tr2.ReadLine();
   tr2.Close();  
   ...
}

Example 2:

try
{
   MyFileHandlingMethod();             
}
catch (IOException ex)
{
   string recfilepath = "...
   string rectoadd = "RecDateTime=" + DateTime.Now.ToString()+ ...+ex.Message.ToString();
   File.AppendAllText(recfilepath, rectoadd);
}
catch (exception)
{
   throw;
}
...
MyFileHandlingMethod()
{
   ...
     try
     {
        TextReader tr2 = new StreamReader(nfilepath);
        resultN = tr2.ReadLine();
        tr2.Close();              
     }
     catch (Exception)
     {
        throw;     
     }       
   ...
}

Solution 1

Yes, those 2 approaches have almost the same effect; rethrowing will unwind the stack of the exception - meaning the stack frames "below" the method where the throw; will be discarded. They'll still be in the stack trace, but you won't be able to access their local variables in the debugger unless you break on thrown exceptions.

A catch/throw block like the one below where you don't do anything with the exception (like logging), is useless:

 catch (Exception)
 {
    throw;     
 } 

Remove it to clean up, in both your samples. In general, avoid entering a catch block if possible

---

And your method has another exception related problem, it does not free resources properly. The tr2.Close(); belongs in a finally clause but it's much easier to let the compiler handle that with a using() {} block :

void MyFileHandlingMethod()
{
   ...
   using (TextReader tr2 = new StreamReader(nfilepath))
   {
     resultN = tr2.ReadLine();         
   } //tr2.Dispose() inserted automatically here        
   ...
}

Solution 2

First of all you should use the using block with resources as this will take care of closing your resources correctly. The second example is pretty much useless as you don't do any work in the exception handler. Either you should remove it, or wrap it in another Exception to add some information.

Solution 3

Yes, the result is the same.

However, both will result in an unclosed stream if there is an error while reading it. You should use a using block or a try ... finally to make sure that the stream is closed:

using (TextReader tr2 = new StreamReader(nfilepath)) {
  resultN = tr2.ReadLine();
}

Note that there is no Close in this code. The using block will dispose the StreamReader, which will close the stream.

The using block is compiled into a try ... finally which it uses to make sure that the StreamReader is always disposed, but the exception will bubble up to the calling method.

Solution 4

I suggest you use your first example, with these changes:

try 
{
  MyFileHandlingMethod();              
} 
catch (IOException ex) 
{    
  string recfilepath = "...";
  string rectoadd = "RecDateTime=" + DateTime.Now.ToString()+ ex.Message.ToString();
  File.AppendAllText(recfilepath, rectoadd);
  throw;    // rethrow the same exception.
}
// no need for second catch}

You probably want to rethrow the exception once you have logged it, because you are not doing any actual recovery from the error.

View more solutions