Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm in the process of refactoring a fairly large web app. One of the major issues is inconsistent error handling and I'm trying to come up with a sensible strategy. I've created a custom error handler, via set_error_handler that essentially turns PHP errors in ErrorExceptions and a custom base exception class, that directly inherits from Exception.

On production I'm using a generic exception catch-all, via set_exception_handler, and I'm about to add exception logging* to the mix. My dilemma is where to do the actual logging, in the base exception class or in the catch-all.

I've thought of a couple of reasons to log it in the catch-all:

  • There are quite a few exceptions in the code that need to be converted to some appropriate child of the base exception class. Until that happens, not all exceptions will get logged.
  • It somehow feels more natural to do it in the catch-all, a base exception class shouldn't do more than being just that. (It might be a single responsibility principle thing, but it could just be a misguided feeling)

and one reason to log in the base exception class:

  • Currently the catch-all is only used on production. It would be easy to introduce it on our other environments (development, testing) but that would call for a few adjustments, as errors are handled differently per environment, as on production they are translated to 404/503 error pages.

Is there any acceptable practice for where to logging exceptions?

* Logging will involve writing to a text file at first, and it may evolve to sending mails for certain types of exceptions.


Some clarifications, prompted by @unholysampler's answer:

I'm facing a 2*10^6 sloc codebase, with a lot of third party stuff I have no control over, and some of the code I do have control over pre-dates exceptions in PHP. And there's also some crappy recent code, we are recovering from a long period of intense pressure where we virtually had to stop thinking and just hacked.

We are actively refactoring to address all the inconsistencies and introduce a sensible error handling approach, but that's going to take some time. I'm more interested in what to do until I reach the point where errors are handled appropriately. I'll probably ask another question on a sensible exception strategy at some point.

The main motivation behind logging is to get an email on my phone whenever something bad happens on production. I don't care if data dumps get huge, if they do I'll have a cron job deleting old ones every now and then.

share|improve this question

2 Answers 2

up vote 10 down vote accepted

In short, the only time you should log the existence of an exception is when you are handling it.

When you throw an exception, it is because your code has reached a state where it can not proceed correctly. By throwing an exception, you are representing a specific message to your program about the error that occurred. You should not catch an exception until you are at a point where it can properly be handled.

The code you write as part of your main application should be aware of the types of exceptions that can be thrown and when they might be thrown. If you can't do anything productive with an exception, don't catch it. Do not log an exception until it is being handled. Only the handling code knows what the exception means in the context of the program flow and how to respond to it. Writing a log message here may make sense here. If you use a logging framework, you can set a log level for the message and potentially filter them. This works well for exceptions that can occur, but are not critical and can be recovered from cleanly.

Your exception catch-all, is your last ditch effort to keep your code from crashing an ugly death. If you have gotten this far, you log all the state and error information you can. Then you do your best to nicely tell the user that the program is crashing before everything stops. Your goal should be to never have this code executed.

Embedding logging into the base class does not follow the above guidelines. The base class doesn't know anything about the state of the code. (Having the stack trace doesn't count because you are not going to write code that makes decisions based on parsing it.) The base class can't do anything to indicate the severity or how the exception might be handled. You don't want huge data dumps and stack traces every time there is a simple exception that you can handle and recover from cleanly.

share|improve this answer
    
I've added some clarifications on the question prompted by your answer. From what I gather, on the practical side of the question you propose to log in the catch-all? –  Yannis Rizos Nov 24 '11 at 5:22
    
@YannisRizos: Yes, you should implement the catch-all as your first step. What I said about the catch-all was more to ensure that you did not use it as a normal part of your code flow. Implementing an unhandled exception handler is important because it allows you to get lots of information every time your code does something every bad. –  unholysampler Nov 24 '11 at 12:16

If your language/runtime does not easily let you determine the source of the exception, there's a case for logging it at the throw. C++ and some JS engines do not expose the file+line or call stack of the exception by the time you've caught it / but this information is available at the time you construct the exception.

Our solution was to provide a mechanism that used runtime configuration to enable logging the type of the exception along with a cheap-stack when trying to diagnose these issues.

share|improve this answer
    
+1 That's definitely a good case for logging at the throw... PHP does provide a full stack trace at catch though, so I'll probably go the other way... –  Yannis Rizos Dec 1 '11 at 17:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.