29

I have trouble reconciling "best practices" and real-world approaches to handling exceptions. In my day to day routine, I find myself running into the following examples:

try:
    do_something()
except Exception:
    log_error()

and even

try:
    do_one_thing()
    try:
        do_another_thing()
    except Exception:
        repair_stuff()
        log_error()
    do_more_risky_stuff()
except:
    log_error()

The most obvious thing is catching the generic Exception type, which is a recurring theme in all "don't do this" programming books/articles on the subject. Furthermore, the nested example - I find it unreadable (or at least "could-be-more-readable"). Finally, having try..except blocks littered everywhere seems... plain wrong. I'm aware that I could be just beating a dead horse here.

I have brought up my concerns to my lead and they haven't exactly been welcomed. Not that they've been unreasonably dismissed (to my perception), it's rather that I can't offer any better approach.

So I have several questions on the matter:

  1. Is catching generic exceptions that wrong an approach? Had a lot of cases (been burnt trying to catch specific ones) where we did not know what to anticipate, while the behavior would be the same for all, e.g. log and continue with execution.
  2. Wrapping everything in try..except: log blocks is code repetition. Or is it? Solving this in any way that would not seem like over-engineering is beyond me.
  3. Handling nested try blocks could maybe be solved by separating them to their own scope (e.g. a function), however this hasn't proved itself as a reliable solution as oftentimes the caller might be desiring a different result on exception (empty result, alternative result, the exception itself etc...)
9
  • 31
    13 years old, and still relevant: Vexing exceptions – trentcl Feb 2 at 17:47
  • 4
    Just FYI: there is a big different between except: and except Exception:. The former will catch anything: >>> try: ... sys.exit(1) ... except: ... print("I don't think so!") ... I don't think so!. You never want a bare except:. If you need a "catch all" use except Exception:. – Bakuriu Feb 2 at 18:15
  • 2
    Related (or duplicate if you don't consider the language difference): Is catching general exceptions really a bad thing? and How is a nested Try/Catch (inside the try) not an anti-pattern? – Bernhard Barker Feb 2 at 19:24
  • 1
    Not very familiar with Python, but how well is it documented what exceptions your methods might throw, and how immediately accessible is that documentation when working on the code? Uncertainty about what might be thrown is one of the major reasons programmers catch generic exceptions. Java and IDEs like Eclipse (shudder) and IDEA set the gold standard for instant accessibility of documentation. Twenty years later, C# and Visual Studio are still a pale imitation. – Kevin Krumwiede Feb 4 at 1:44
  • 1
    Thank you for actually asking. The usual Python programmers' approach to exceptions seems to be to ignore them and let the user deal with them, which is horribly frustrating, especially when they are for common problems that can easily be anticipated (e.g. input file not found). – Michael Hampton Feb 4 at 12:48

10 Answers 10

63

This code:

try:
    do_something()
except Exception:
    log_error()

is dangerous. Not because you caught a generic Exception but because you suppressed the exception without doing any recovery or halting the system. Now the system is in an undefined state. It might be about to corrupt the database, format the hard drive, or send the president threatening emails. But hey, at least you logged the error first.

try:
    do_one_thing()
    do_another_thing()
    do_more_risky_stuff()
except specific_error1:
    log_error()
    recover_from_specific_problem_regardless_of_where_it_came_from()
except specific_error2:
    log_error()
    raise # Don't know how to recover at this level so kick it upstairs
    

Do it this way and either the problem is handled cleanly here or made into someone else's problem. If you really needed to know which methods those exceptions came from to recover then those methods should have recovered from the exceptions themselves.

Following the rule about keeping functions short should make this easy to debug.

7
  • 4
    Usually the problem here is to find out what specific exceptions can be thrown by the calls. Checked exceptions (when used correctly) or composite types could help here, but if you only have unchecked exceptions, all you can do is thoroughly examine documentation for each call, maybe look at its source, and then pray you didn't miss anything. – Malcolm Feb 3 at 9:45
  • 4
    @Malcolm And the much worse problem: The vast majority of programmers seem to not understand that adding new exceptions after the fact to a function is a breaking change, so chances are good that while it might work today, it might not work after a library update. – Voo Feb 3 at 12:29
  • 1
    What happens if recover_from_specific_problem_regardless_of_where_it_came_from() throws a specific_error2 exception (perhaps because it internally calls do_more_risky_stuff())? That exception won't get logged, right? – Schmuddi Feb 3 at 12:43
  • Undefined state is not undefined behaviour; just because you don't know where and why an exception was thrown doesn't mean anything could follow. – IS4 Feb 4 at 18:58
  • @is4 prove that and you’ll revolutionize computer security. I hear it’s equivalent to solving the halting problem in the general case. – candied_orange Feb 4 at 19:09
52

we did not know what to anticipate, while the behavior would be the same for all, e.g. log and continue with execution.

To me, this is your major problem. If an exception occurs and you don't know how to handle it, you should not continue with execution because you don't know what state your system is in. Just pass it up to the next level and let that handle it; at the very top level, you may want to log out the exception, but that should be at the top level, having aborted any other work that was in progress. At this point, you lose most of your repeated exception handling because it's all just handled at the top layer.

If there are specific exceptions you do know how to recover from, then absolutely catch the specific exception and have appropriate recovery logic. But don't try and do it for Exception because you don't actually know what went wrong in that case, so you can't correctly recover from it.

6
  • 1
    One reason to catch at a lower level might be to add context — e.g., to wrap the exception in domain-specific exception type that the next layer up will understand. (I suppose this could count as "recovering from" the exception — by throwing an exception!) But then you would just immediately raise the new exception, no logging required. – Chris Bouchard Feb 3 at 3:19
  • 3
    @ChrisBouchard This can be valuable if the code you're writing understands the consequences of a particular exception and is able to translate a low-level exception (an issue reading a file or similar) into a higher-level consequences-based exception (user creation failed), but there is a tendency to just blithely wrap every exception in an app-specific exception class with no interpretation (lookin' at you HibernateException). – James_pic Feb 3 at 10:47
  • 4
    This whole "never continue execution" always strikes me as academic and oversimplified - although reading the details I don't think that's what you mean anyhow. But by that logic SE should shut down a service every time a single unexpected error occurs during any request. I doubt they do that (ASP.NET Core doesn't make such a thing very easy either). In practice you try to structure your application in such a way that an unexpected exception cannot do major harm (hello transactions and structured cleanup of resources) and provide enough logging that you can trace and analyze the problem. – Voo Feb 3 at 12:34
  • 4
    @Voo, web servers (and even most applications) run in a different manner than, say, command line utilities which might compute a value and output it before exiting. In those cases, the framework/application itself would treat each incoming "event" (which may be a single button click or http request) as a standalone execution scope. Uncaught exceptions may end its parent scope abruptly without terminating the host application. Arguably, an exception could still mean "I did have my job and screwed up, don't let anyone else work", but I'd say that's the issue to fix. – TheRubberDuck Feb 3 at 14:23
  • 4
    @Voo it may just be a difference of interpretation. When I see "not continue with execution", I think "don't pretend things worked, but let the exception go" rather than "terminate the entire process". This answer is consistent with that idea (and thus, with itself) as it suggests handling things at a top level where changes have been aborted. – TheRubberDuck Feb 4 at 15:19
11

A lot depends on the context. A pattern like:

try:
    do_something()
except Exception:
    log_error()

wouldn't be that unusual in certain cases. If do_something is a user-supplied callback, or a request handler, or otherwise a "unit of self-contained work" being processed, this pattern could be sensible. By the time you catch the exception here, maybe do_something has already done whatever cleanup it can do, and all you need to do is log the error and move on to the next work unit.

If, however, do_something is not "self-contained", then you could be in trouble. For example, if do_something is responsible for doing some job -- say, creating a database object -- then when the exception is handled here, you may have no way of knowing whether that job was done. In that case, proceeding normally would be pretty dangerous.

This kind of "catch-all" handling code is very common (and totally reasonable, IMO) in frameworks which may be calling into unknown and unrelated code, and where the framework's correct functioning doesn't depend at all on whether the callee succeeds or fails. In application code, on the other hand, it's probably a mistake.

1
  • I run into this on benchmarking code, where a test run is set up and might fail for any number of unknown reasons, and the important thing is that you log that experiment #24 was a failure and just keep going to experiment #25. – ObscureOwl Feb 4 at 8:22
3

Use an adaptation of Joe Armstrong's "Let It Crash" philosophy.
(Adaptation, because the original design applies to extremely lightweight threads; here I am presenting a shortened, adapted-to-Java version.)

The approach is this:

  • Distinguish between exceptions that your code already knows how to deal with, and exceptions where your code doesn't.
    Just write the appropriate code in the former case. E.g. if you code asks the user for the name of a file to open, and you get a FileNotFoundException, you know what to do; if the file should be there (it's a configuration file that was written just five lines ago), then your code doesn't know what to do because an assumption is broken.
  • Don't try to handle don't-know-what-to-do exceptions, just assume that your code has failed and rethrow the exception.
    This is the "let it crash" part: If code fails, something unexpected happened that the code wasn't prepared to handle, and trying to patch up things usually just makes things worse, so just let it crash. Java-specific: If it's a checked exception, you usually can't rethrow; just wrap it like this:
    throw new RuntimeException("Expected file " + file + " to exist but it does not", ioException);
  • Since we don't want to crash the entire application, always have a higher level that deals with crashes.
    Different kinds of strategies exist, from simplest to most sophisticated:
    • Just log the error and abort the application.
    • An interactive application can log the error, tell the user the action failed and ask him what to do next. (It can also offer the user an easy way to ship the log file to the programmer.)
    • A server application that has mostly independent requests can simply return the error to the requesting machine. The payload of the error message can be as restricted or as complete as the situation demands (public services should give less information to prevent attackers from gathering information about error states that they might exploit, with strictly internal services it can be easier to send the full exception trace).
    • The application can retry the failed operation. Doesn't make sense in all cases, but if hardware or other components not under control of your software are involved, this can be a useful strategy. (E.g. networking issues.)
    • The application can try a simpler fallback algorithm that's less likely to fail.
    • If your application is truly complicated, you may find that having multiple layers of failure handlers is a win.

The outside-the-code part of the process is what you'll do when you receive reports of a failure.
Sometimes you make the code more resilient (make sure that the exception doesn't happen etc.), sometimes you just improve the log message and/or the user message, sometimes you add a failure handler, and sometimes you ignore the problem because it happens only once per decade and you have more important things to fix (the latter should be the rare exception, obviously, but it is an option).

0

Is catching generic exceptions that wrong an approach? Had a lot of cases (been burnt trying to catch specific ones) where we did not know what to anticipate, while the behavior would be the same for all, e.g. log and continue with execution.

If you just want to log the exceptions, that's fine. But you're not just logging in such a case, you're handling them. You're announcing "I don't know what went wrong but trust me, it wasn't important".

Wrapping everything in try..except: log blocks is code repetition. Or is it? Solving this in any way that would not seem like over-engineering is beyond me.

If your ambition is to log every exception that's raised by a framework (but still be able the handle some exceptions in a special way), the answer is simple. You can wrap the whole interface in a class that intercepts every function call, logs every exceptions, and then re-raises the exception. This is actually one of the correct use cases for catching generic exceptions, one of the factors being is that you are still allowing the exception to be handled properly as you are reraising it.

A benefit for this is you are NOT repeating yourself at all. And moreover, you have a bit of a guarantee that if an exception is raised, it is getting logged.

Wrapping interfaces to add logging is a relatively simple task, and I would not classify as over-engineering. It probably amounts to about 10 lines of python.

Handling nested try blocks could maybe be solved by separating them to their own scope (e.g. a function), however this hasn't proved itself as a reliable solution as oftentimes the caller might be desiring a different result on exception (empty result, alternative result, the exception itself etc...)

Well, a function can return anything you want. So you could very well return all those things. But it also forces you to think about what your post conditions are. In other words: "What are the sensible outcomes after this statement is run".

0

In theory:

try:
    do_something()
except Exception:
    log_error("logged in this function: " + except)
    **throw except**

Always throw it back up the chain if unable to safely handle it. If nothing else handles it, the program needs to crash. This could be a serious error. You could be reading random junk values. Users are not happy once all their valuable data get overwritten with junk. A crash is also a very obvious sign something important needs fixing. A log may end up ignored for years.

  1. Try to catch what you expect might go wrong (most likely first) before catching general exceptions. Not only does this make it obvious what you are expecting to go wrong, it also means you are handling that specific problem and not something you weren't expecting.
  2. It's not repetition if it's done properly. It's finding the best place to safely catch issues.
  3. The try catch blocks don't need to be in their own function. They shouldn't be very long. If they are, that's a potential sign something is wrong with the overall design.

In practice, you are modifying production code and breaking this is not an option. Follow your lead as everything is their responsibility. They know the system and know the potential risks. You can ask if there is anything you can do to help reduce risk when writing new code, but make it clear you are there to support and not trying to cause more problems.

If you are in a hostile workplace, put it in a non-aggressive / indirect email which you can keep (off site) if you are ever blamed.

2
  • 1
    The problem being that ... if you simply catch "every" exception ... every other part of the system is now dependent upon you. Far better is to be as very-specific as you possibly can be regarding which exceptions you catch. (Create a private method to reduce each exception-type to "common code.") You really don't want your code to be fingered-out as the one that "ate" an out-of-the-blue exception type that turned out to be extremely ... costly. – Mike Robinson Feb 3 at 19:16
  • @Mike Nothing is getting eaten above as it rethrows any exceptions it logs that can't be handled specifically. – David Feb 5 at 0:41
0

My experience in languages with exceptions is that you have to assume each line may fail in order to write code that is robust to failure, a bit like when you use transactions: at the end of the function you commit your changes or let the exceptions rollback them. For example you only build and validate an object locally before adding it to a container, etc (in other words, delay the use of externally visible side-effects).

When it is not possible and you have to repair some things if the code fails, I'd let the function repair it iself, for example in a finally block, before throwing the exception. For example in this code:

    try:
        do_another_thing()
    except Exception:
        repair_stuff()
        log_error()

It looks like every time you do_another_thing, you have to be careful about handling the exception and call repair_stuff, which is something a user of your API might forget to do. Unless repair_stuff is not related to what do_another_thing does, it should be called by do_another_thing itself.

Handling nested try blocks could maybe be solved by separating them to their own scope (e.g. a function), however this hasn't proved itself as a reliable solution as oftentimes the caller might be desiring a different result on exception (empty result, alternative result, the exception itself etc...)

The undo steps of the function that throws an exception should IMO be part of the function. Apart from the repairing code, yes, if you want to handle the situation differently you should have different handlers. However, the fact that a language provides exceptions does not mean all errors are best expressed as exceptions.

Sometimes functions accept an argument that says what to do in case of error (throw an exception or return an empty result). Or the functions are in fact methods for objects that contains user-defined handlers, in which case the code can use them, e.g.:

action = self.error_handler.on_network_failure()
if action is Actions.RETRY:
    continue
elif action is Actions.ABORT:
    return None
....

Is catching generic exceptions that wrong an approach? Had a lot of cases (been burnt trying to catch specific ones) where we did not know what to anticipate, while the behavior would be the same for all, e.g. log and continue with execution.

On some occasion you know that you want to check for network failures, or file system failures, etc. so you can handle them more appropriately. But generally speaking, you better have to assume that your code might be interrupted at any point.

You only catch some specific exceptions when you have an alterative execution path, but otherwise you can rely on the default mechanism and let exceptions bubble up.

So yes, having a general exception handler that logs and continue execution is fine, in case you are actually able to continue execution (ie. a server that failed to handle one request might be able to handle the next one).

Wrapping everything in try..except: log blocks is code repetition. Or is it?

It is code repetition and I think it is a code smell if there are too many try blocks.

Solving this in any way that would not seem like over-engineering is beyond me.

If you have a valid reason to handle your exceptions the same way you can decorate the functions:

@log_exceptions
def my_method(self):
  ...

I don't think writing a decorator is over-engineering in that case.

1
  • +1 for the decorator example, this is a great pattern to use if you find yourself constantly starting functions with try: and ending with except: log() or something similar (like returning a 500 with a general Internal Server Error message in a web-serving context). – zyd Feb 3 at 19:57
0

The best way to deal with exceptions, as a default behavior, is to simply ignore their existence. That is what exceptions are for in the first place.

If you write code like this (which, let us here acknowledge, may be necessary sometimes):

try:
    do_something()
except Exception:
    # deal with it somehow

then your logic would be just as easily written without exceptions:

result = do_something()
if (result == DO_SUCCESS):
   ...
else
   # deal with result here

The point of exceptions is so that, most of the time, you just do this:

do_something();

There is a handler which "has your back", but it's somewhere else.

Consider that in a high level language such as the one I think you are using, there is implicit memory management. When you evaluate an expression like [a, b, c], which is what I'm guessing might be the syntax to make a three-element array/list, that performs implicit memory allocation and could run out of memory. Yet, you do not litter your code with repetitions of this:

try:
   list = [ a, b, c ];
except OutOfMemory:
   # couldn't allocate list, oh  no!

You just do this:

list = [ a, b, c ]  # OOM handling is elsewhere (we pray).

The point I'm making is not that exceptions are just something to be swept under the rug, or a responsibility to be shirked and passed somewhere else, but rather that it's better to take a big picture view. OK, some unusual thing could happen here. What does that mean in the larger picture? Though it happens here, is here the best place to handle it? Is it already handled somewhere? What are the special circumstances which apply just to here, which won't be taken care of by that other elsewhere; will something be left in a bad state if we are abruptly bailed by an exception that we don't handle? Can we write the code in a way so that it doesn't care? What kind of documented contract do we impose on the code here, and how do exceptions figure into it?

For instance, suppose the code performs a transaction of some kind (not necessarily a database or remote server transaction; think of it in abstract terms: updating a data structure in memory while preserving certain invariants could be a transaction). There may be a way to write it more functionally: perhaps it can calculate a new version of the data structure and then as a last step, atomically replace it. In that case, you don't care about being interrupted by an exception. If the transaction is bailed by an exception, then it's like it never happened; the last step of actually replacing the object with the new version was not reached and so the untouched old version is still here.

The earlier you think about this kind of thing in the design (ideally from the beginning), the better the outcome will be in the code.

Dialects in the Lisp family of languages have something called unwind-protect: an operator for always executing some clean-up code if an unexpected non-local control transfer takes place. (Any kind of non-local transfer, not necessarily exception-related). It's like a try/finally in some Java-like languages. You have to use that operator more frequently than handling an actual exception. Yet, much of the time there is no need. It's strictly for situations in which something would be left in a bad leaking state.

0
  1. Is catching generic exceptions that wrong an approach? Had a lot of cases (been burnt trying to catch specific ones) where we did not know what to anticipate, while the behavior would be the same for all, e.g. log and continue with execution.

No. "Wrong" is a matter of context. It is wrong to catch generic exceptions when your code flow and subsequent decisions depend on the type, but when you simply don't care, there is no cleaner way.

Consider an application that reads a couple of file formats. When the user tries to open a file and every single parser you have fails to recognize the format (based on whatever, built-in heuristics, expected fields, signature bytes, etc.), you don't need to explain to the user what happened. Hundreds of file formats are out there, plus the file may be on a USB drive that just got disconnected, a file was locked by another application, dozens of things can go wrong. You can just catch everything and report that the file could not be opened, the format might not be recognizable or a problem occurred with the file system. Yes, you will annoy the user by not explaining everything but time is money and you cannot just spend it fine-tuning a response to every single thing that can go wrong. Besides, the users will be equally amused when they do manage to open their files after all, and do some important time-and-money-saving processing on their precious data.

  1. Wrapping everything in try..except: log blocks is code repetition. Or is it? Solving this in any way that would not seem like over-engineering is beyond me.

Only wrap something in a try block when the stack trace is going to be useful to you. Unwrapping the stack is costly and should be avoided in time-sensitive blocks, obviously. Consider a method that draws a 3-point circle on some canvas:

class Canvas
{
    void drawCircle(Point p1, Point p2, Point p3)`
}

What use could the stack trace possibly be of here? You know in advance almost anything that could go wrong. Invalid Point instances? Impossible, you catch them in their constructor anyway. Besides, the user is probably entering coordinates in textboxes where you gracefully forbid alphanumeric or invalid values. Or they create points by clicking on your Canvas, which means they cannot click on a point with invalid coordinates. Three points may be collinear, so the circle radius tends to infinity. Check for that in advance and inform the user appropriately (or draw an infinite straight line). Two (or all 3) points may be coincident. Check for that in advance and let the user know this cannot be done.

The point is that there is hardly anything that the stack trace in such a method would tell you, that you cannot predict in advance (unless your software is running on computers exposed to serious amounts of cosmic radiation).

  1. Handling nested try blocks could maybe be solved by separating them to their own scope (e.g. a function), however this hasn't proved itself as a reliable solution as oftentimes the caller might be desiring a different result on exception (empty result, alternative result, the exception itself etc...)

Every time a meaningful exception gets caught, you get the logs, see what happened and refactor your software in a way that what happened is no longer an exception. If you don't, then the logging part of the catch/except block is practically only for show. After you manage to handle 99% of what can go wrong, you won't really care anymore about the rest 1%, and you don't really care much about the except/catch block at all, you just leave it be, for the 1% of the stuff that can go wrong. Trying to "scope" specific exception cases smells like you are trying to use exceptions to control code execution flow.

A caller that "might be desiring a different result on exception", sounds like one that knows what to expect or want. Find a way to write the method(s), so that they return what is desired/expected then, why would you need exceptions? An exception is, by definition, an unexpected event that disrupts flow. If you know what to expect and what to do when you observe some sort of behavior, it's not an exception anymore. When you are calling code that you cannot change, neither know what exactly it does, documentation is your only hope and, of course, trying and catching exceptions would be advisable, but in those cases, you are simply doing code execution flow control, nothing is truly unexpected when you know what to expect (and, of course, do prepare for all that you can expect).

Want a typical example of when a try block would be absolutely necessary? Well, when you "try" to open a file, of course. Check this out:

if file.exists(filepath):
    content = read(filepath);

No matter how many times you check whether file.exists(filepath), the read(filepath) is on a different line of code and, therefore, semantically happens at a different time. Yes, the file may have disappeared in between. Will it, though? Well, in reality, it might disappear by any one of a series of very obscure reasons… the user may be moving a bunch of files and it happens to coincide with this check, or the filename was only just changed, or this is a remote filesystem and each request takes much time to be served, thus allowing for more time in-between, for obscure things to happen, etc.

99.9999% (or more) of the time, this won't be a problem, of course. When it happens, though, it will truly be an exception and, depending on the circumstances, the application may crash. For precisely that reason, you definitely need to wrap this around a try-except block. In a purely exceptional situation, such as this, you will find it impossible to know what on earth has happened, you will not really care about it anyway (you can get all the log details in the world but you will not be able to do anything to prevent the error from occurring out of very bad luck in another 10 years from now) and you will have absolutely no way of recovering whatever was going on in the try part of the block. This is the only meaningful situation that try-catch blocks are going to help, i.e. exogenous exceptions:

Always handle exceptions that indicate unexpected exogenous conditions; generally it is not worthwhile or practical to anticipate every possible failure. Just try the operation and be prepared to handle the exception.

0

An exception means the program is in an invalid state and cannot run forward from this point. So what to do now? The options are-

  • Stop the program (which happens by default).
  • Try to recover the program into a valid state. This is actually what being called "catching" and "handling" the exception. This can be done in three ways-
    1. Put some default values in the invalid parts of the state and carry on.
    2. Assuming next time it will be different from some external input, restart the relevant part of the program, which can re-initialize or repair the state. If we assume the program is a tree then we have to restart the branch which can repair the invalid state, while keeping other branches alive and running. Sometimes you may want to restart the full program, handling the exception at the root.
    3. Or just log the error and stop the erroneous branch of the program, keeping other branches alive and running.
  • Also as the programmer, check if the program logic/code is alright and not generating the invalid state itself.
  • And if you are coding a library and not the "end-programmer" (coding the executable program facing external inputs), then you can create and throw another exception, explaining the invalid state. Or you can just let the exception follow its way and let the end-programmer take the responsibility of handling it.

And all of these has to be on a case by case basis. Catch-all codes are not usually doing any of these steps. They will most probably just swallow/hide the error/exception. That's why it is not recommended.

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