Should I factor out error reporting in catch statements by re-throwing errors?

I have an error/exception that is thrown by a particular method. Any time this error occurs, I want to log it. Would it be good practice to log the error within the initial method, and then re-throw it? That way I would not need to log it in the catch statements of any function that calls this method.

It would开发者_开发知识库 look something like this:

public void doSomething() throws Exception{
    try{
        someFunction(); // throws Exception
    } catch (Exception e){
        logger.fatal(e.getMessage()); // always log this Excpetion
        throw e;            
    }
}

My concern was the re-throwing of exact same error. I have not seen this done, and I wondered if it would be considered bad practice.

EDIT: I should add that this Exception should never ever happen. That might help understand this situation.

I would log information describing why your method is about to throw an exception rather than the exception itself. Let clients decide if they want to log the exception itself or handle it.

If you handle the exception I'm not sure it is the right thing to always log it. Depends on your application of course but in general it seems to me like it should be up to the client code to log the exception.

Thus, my recommendation, if the method throws the exception, let the client take care of the handling, even if it most often means logging it. (Unless of course you want the logging to be a documented side effect of the method.)

It really depends on your logging preferences. For example you database access layer may have a method that takes a SQL query as a String as an argument and executes it in the DB. Now you may want to log any SQLException in that layer and then wrap the SQLException by a custom exception [like a DatabaseException] and then re-throw it back to the parent layer. You may also want to encapsulate some additional behaviour like a boolean shouldRetry in your DatabaseException object so that the parent layer may retry the operation again. Logging the SQLException right away will let you debug the issue more easily later.

EDIT:

This approach makes more sense when your parent layer does an operation that might throw more than one kind of exception. For example, in the above scenario, the method that takes the SQL query might also throw a InvalidHostException if the database manager is unreachable or a ConnectionRefusedException, if the database manager refused connection due to overload. In such a case you might log the exception and then wrap it with the more general DatabaseException. Also if the method threw a ConnectionRefusedException then you might want to set the shouldRetry variable to true; while the same will be false for a InvalidHostException. This allows the caller API to try to call the method again after sometime if the value of shouldRetry was set to true.

NOTE: Except SQLException, the rest are creations of my imagination. :-)

I would log the error in the initial method only if there was pertinent information there that would not be available in the client code.

Maybe you could use aspect approach. So after each busines method you check the exception type that has been thrown and verify whether it matches criteria of exception that should be logged. If so you log it and rethrow exception otherwise you just rethrow exception without logging. This allow you to centralize your error logging and disable/enable it when necessary apart from business logic - this is typical cross-cutting concern not core concern.

public class MyLoggerInterceptor ... {

   @AroundInvoke
   public Object invoke(InvocationContext ic){
     try{
         ic.proceed();
     }catch(Exception e){
         if(exceptionShouldBeLogged(e)){
            logger.fatal(e);
         }
         throw e;
     }
   }
}       

There are already a lot of answers but I would say that you need to make the best decision with the information you have at the time, but you then need to monitor what you are logging when your application runs. (many developers don't do this, me included) If you think your logging is to verbose change it and if you find that trying to investigate an issue you don't have enough information then you add more logging in.

Its like method naming, its difficult to get right the first time but keep changing until you get it perfect.