5

Suppose I have the following code:

void foo() {
    /* ... */
    try {
        bar(param1);
    } catch (MyException e) {
        /* ??? */
    }
}

void bar(Object param1) throws MyException {
    /* ... */
    try {
       baz(param2);
    } catch (MyException e) {
        /* ??? */
    }
}

void baz(Object param2) throws MyException {
    /* ... */
    if (itsAllATerribleMistakeOhNo) {
        /* ??? */
        throw new MyException("oops, error.");
    }
}

I'm wondering where and how I should be logging the error.

  • Where the error occurs, below, in baz(), I know exactly what operation went awry and can log that fact.
  • At the top I have the most general context (e.g. what's the IP of the connection during whose handling we encountered the error.)
  • Along the way I might have some context which isn't known either at the top or at the bottom.

Another complication is that the error at the bottom might not really be considered an error when you look at it from the top (e.g. looking up something in a database fails; maybe you weren't sure ) - so I might choose to logger.WARN() instead of logger.ERROR().

So, above I described 3 locations (bottom, top, and along the way) - but it's not just a question of where to log, but also what to throw up. At every level in the middle, you have 2x2 options:

  • Log/Don't log a message
  • Throw the original exception / wrap the exception in a new exception with the added message.

What are the best practices, or some common wisdom, regarding these complex choices?

Note: I'm not asking about error handling/exception use in general, just about the dilemmae described above.

einpoklum
  • 118,144
  • 57
  • 340
  • 684

4 Answers4

3

When it comes to logging, I prefer to keep all my logging at the top at the application boundary. Usually I use an interceptor or filter to handle all logging in a general way. By this concept, I can guarantee that everything is logged once and only once.

In this case, you would log inside your foo() method or whatever the entry point to your application is (you mentioned the IP address, I suppose we are talking about a servlet container or application server).

Than, catch your own exception in the filter/interceptor and log it depending on your needs. Add a catch throwable to catch all other exceptions that you did not handle in your code and log them as an error, since obviously you missed something further down in the stack trace.

This concept requires some planning ahead. You will probably use your own ApplicationException that stores the Error Message (String) along with some severity level (probably in an Enum). You need this to choose the correct log level when you do the actual logging.

This works well for all cases and has the advantage that all logging is happening exactly once. However, there is one case where you still need logging in your code: if you can fully deal with an error somewhere in your code (that is, an exception happens and you can do something that allows you to continue working without (re)throwing an exception). Since your are not throwing an exception, nothing would be logged otherwise.

To sum it up:

  • Log at the topmost position in a general way, preferably using an interceptor or filter.
  • Wrap exceptions inside your own ApplicationExceptions and add severity plus other things of interest for logging in your application.
phisch
  • 4,571
  • 2
  • 34
  • 52
2

Some suggestions that I tend to follow:

Link for some best practices

1) Trace the exception where it occurs. As the point where the exception occurs if the class or API knows the context in which the exception occurs then tracing and providing a proper log is better. But if the API cannot handle or comment on the exact context then API should not log the event and leave it on the caller.

2) Wrapping the exceptions : When there are lot of exceptions that can be thrown and all exceptions form a similar group (SQLException) which provides single exception and lets you to extract information if needed. Otherwise there would have been an explosion of exceptions that the caller needs to handle.

3) Re-Throwing the exceptions: If the API logs the exception and user can take some actions on that then the Exception MUST be rethrown to tell the user that some error condition occured.

4) Proper cause of exception : The exception message should not be too techy for the caller to understand, the message itself should guide the user to understand the underlying reason for the exception.

UPDATE: Exception Management in Java

Community
  • 1
  • 1
Narendra Pathai
  • 41,187
  • 18
  • 82
  • 120
  • About your point 1: You suggest only logging at 1 point along the call chain. Can you elaborate on why you think this is best? About point 2: wrapping and using an exception subclass tree are two different things... About point 4: Good point but not relevant to my dilemma. – einpoklum Mar 04 '13 at 08:39
  • In case of the trace message, tracing at the point exception occurs proves helpful to the developer. As you tell the the chain adds on to the exception reason. Then according to me you should create separate checked exception to exactly point out the reason. If it is a developer or programming error then throwing runtime exception is better. For example `DBConnectivityNotAvailable` can be checked exception. Can you give an example of your case where you are confused? I mean it depends from case to case what strategy to use for logging. – Narendra Pathai Mar 04 '13 at 08:46
  • Mine is a complex case of some existing checked exceptions and some potential Runtime exceptions, where currently I have a catch-all block at the top of the call chain, and occasional loggings along the chains. There are also ERROR-vs-WARN issues, where the bottom of the chain can't know whether something is an ERROR or not. – einpoklum Mar 04 '13 at 08:52
  • No matter which approach you take `DOCUMENT IT`. – Narendra Pathai Mar 04 '13 at 09:55
  • in case of the error vs warn issue, if the bottom of the chain cannot know then it is best to throw a checked exception and let the caller of upper chain decide what makes sense in that context. Do not catch Runtime Exceptions unless it makes sense at that point (I mean to say that if the exception should not cross the context boundry). In that case catch all exceptions - log them in a proper manner or rethrow them. – Narendra Pathai Mar 04 '13 at 10:02
1

When I throw Exceptions in my code, I do not usually log anything. The exception is information enough. The only exception to this is, when I am at the border of my system, that is, when the exception will leave the boundary of my system, then I log as I am not sure what the other system will do with the error.

When I handle exceptions, I log them when I actively handle them, that means when I am in a catch clause which does something more then just rethrowing the exception. Usually this is rather at the top, but this depends on the situation.

Matthias
  • 3,582
  • 2
  • 30
  • 41
  • but what about: (1) When you partially-handle thing along the way? (2) Different contextual information available in different places in the call chain? – einpoklum Mar 04 '13 at 08:35
  • I have those situations rather seldom. More like I have to convert an exception because of a context change before rethrowing it. This then greatly depends on the situation but usually a log entry on a WARN level should be a good idea. – Matthias Mar 04 '13 at 08:42
  • For situation (2) This should influence your descision if you want to handle the exception or just to declare it is thrown by this method. If the context is enough to handle it, thats fine, if not just let the higher levels of the call stack handle things. – Matthias Mar 04 '13 at 08:45
1

When throwing an exception at the testing stage, you should remember:

  • Keep the exception message as clear as possible. Stack traces can be confusing at the best of times so ensure that what you are reading, at least, makes sense to you.
  • Ensure that the exception is relevant to the event. If the user types in the wrong value and you throw a NullPointerException, your code is illogical and loses it's value.
  • Ensure that it has as much information ABOUT THE EVENT as possible. That is, keep the message relevant. If a database call has gone wrong, print the connection string to the database, and the SQL query attempted. The state of every variable currently being used isn't necessary.
  • Don't waffle. It's tempting to type in technical jargon to make it look like you're hacking into the matrix. It doesn't help you in a stressful situation, and it certainly doesn't help anyone else using your code. Simple english words are always preferable.
  • Finally, NEVER IGNORE AN EXCEPTION. Always ensure you handle the exception, and you're outputting details in some way, following the rules I've stated above.
christopher
  • 26,815
  • 5
  • 55
  • 89
  • This is all good advice about working with exceptions, but it's not exactly what my question is about... – einpoklum Mar 04 '13 at 08:37