views:

160

answers:

5

I have a three layer structure
1. Presentation layer
2. Business layer
3. Data Layer
The Presentation layer interacts with the business layer via a service facade . Now I am confused on where should I throw my exceptions , where should I log them and where should I catch and swallow them . Currently I am swallowing my exception in my presentation layer after logging , whereas I am logging and throwing them everywhere else . This results in an exception getting logged thrice . That is not a huge concern , but I wanted to know the best practice regarding this .

For example : Presentation layer code :

try 
{
  UserService.GetAllUsers() ;
}
catch(Exception ex )
{
  Logger.log(ex) // exception gets logged here
  // redirect to a friendly user error page 
}

UserService Layer Code

try
{
  IUserDAO userDAO = ServiceRegistry.GetRegistry().GetDAOFactory().GetUserDAO() ;
  return userDao.GetAllUsers() ;
}
catch ( Exception ex)
{
  Logger.log(ex) ;  // and here !
  throw;
}

DAOLayer Code

try
{
  // All db interaction code 
}
catch(Exception ex)
{
  Logger.log(ex) //and here !!
  throw ;
}
+1  A: 

I can't really see why you're logging the exception and rethrowing it. Surely whichever process finally swallows it can do the logging. You can simply use ex.StackTrace to get the call stack if you want to record that information.

Generally, re-throwing the exception is what you would do when the issue causing the exception isn't something that can be addressed at the level that code is at. You'd typically use more specific catch blocks to catch these and respond appropriately (eg. if you got an exception due to a network timeout you could retry the connection or try to connect to a backup server). You'd want to log the exception at the point where you fixed it and just keep going.

If there's nothing you can do at the point where you get the exception, I'd only bother to catch and log it if I couldn't count on something further down the call stack to take care of the logging for me. Otherwise it just creates a rather pointless duplicate message.

Adam Luchjenbroers
The client of the layers might change , and if I am not logging in my layer , I am kind of becoming dependent on the client to do the logging , which if they dont .. I am in a soup .
NM
Well, that is a sensible reason to log in multiple places - since you can't count on the client taking care of the logging. Solvable errors should still be caught separately though.
Adam Luchjenbroers
+1  A: 

You should throw exceptions when exceptional things happen - on any layer... If you are expecting some error conditions to come up on a regular basis, code for them - show errors to the users and means to fix the error.

Log all excpetions, without exception! Good to have the data, and if you follow my previous advice, you won't have too many of those.

You should never swallow exceptions. See above. How will you know if things went wrong otherwise?

Oded
Swallowing the exception rather than allowing the application to fail can sometimes be the right call. It's trading Correctness for Robustness, and sometimes Robustness is more important.
Adam Luchjenbroers
Always log. You can decide if you want to propegate the exception or not depending on requirements.
Oded
+1  A: 

What I tend to do is to create an exception for each level, and I wrap the exception and pass it on, if the next layer up can't handle the exception.

For example, if you try to update the database, but the primary database is down, you may just need to go to the secondary database, so you try that, and if it works you log, but don't pass it on.

I tend to pass on exceptions that can't be handled, until I get to the top layer. There, for some errors pass them to the client, if it is an exception the user can do something about, but otherwise, log and swallow.

If it is something like out of memory, then let the user know, and prepare to gracefully die, before the application crashes.

James Black
+3  A: 

It's definitely unnecessary to put every piece of code in a try-catch block. For example, if an exception occurs in your 'DAOLayer' your log file will contain the same exception 3 times...

Keep in mind the golden rule with exceptions - exceptions should be exceptional! This usually means that you should only catch exceptions that you are able to handle - all other exceptions you should ignore - a handler at a higher level should handle these.

If you are concerned that an unhandled exception will crash your application (and rightfully so) you should take a look at the 2 events that .Net exposes for handling unhandled exception. The first is AppDomain.CurrentDomain.UnhandledException and the seconds is Application.ThreadException (which is in the Windows.Forms namespace).

Jaco Pretorius
A: 

Rather than wrapping exceptions in custom exceptions or relying solely on the StackTrace to give you all the information you need when you log the exception at the topmost level you can instead use ex.Data to add more information about the exception before passing it up the chain, e.g. ex.Data.Add("filename", filename). Your logging code can then dump out all these additional properties. This might give you enough information to understand why the exception was thrown and to to reproduce it.

Hightechrider