Diff for "ExceptionGuidelines"

Not logged in - Log In / Register

Differences between revisions 1 and 2
Revision 1 as of 2009-01-12 19:42:45
Size: 2595
Editor: abentley
Comment:
Revision 2 as of 2009-07-01 16:35:29
Size: 3794
Editor: adeuring
Comment:
Deletions are marked like this. Additions are marked like this.
Line 30: Line 30:
=== Catching all exceptions ===

This is in most cases a bad idea, since it means that we don't know
what exactly might happen in the code protected by {{{try:}}}.

As a developer, think again before you use {{{except Exception:}}}; as a
reviewer, ask the author why he thinks that it is reasonable
to do that.

Most importantly, we must not hide bugs this way. You must either
raise the exception again after doing some cleanup like calling
{{{transaction.abort()}}}, or you must log an OOPS.

{{{KeyboardInterrupt}}} and {{{SystemExit}}} must not be caught under any
circumstances.

If you really need to catch all exceptions, do it this way:

{{{
    try:
        # whatever.
    except (KeyboardInterrupt, SystemExit):
        # do some cleanup, if necessary.
        raise
    except Exception:
        # do some cleanup, if necessary.
        # log an OOPS or raise the exception again.
}}}

Note that from Python 2.5 on, {{{KeyboardInterrupt}}} and {{{SystemExit}}} do
no longer inherit from {{{Exception}}} but from {{{BaseException}}}, thus we
can remove the {{{except (KeyboardInterrupt, SystemExit):}}} clauses
once we have upgraded Launchpad to 2.5.

Catching exceptions

Never catch AssertionError or NameError or TypeError.

Only catch ValueError when you are trying to convert a value to a type using the builting "type conversion" functions such as int() and float().

    try:                                                                 
        foo = int(foo_str)
    except ValueError:    
        # Handle the non-int case.

You might want to catch TypeError when converting type, when the value may be a string or may be None. Resist this temptation! You should explicitly check for a None value before doing the conversion.

    if foo_str is None:                                       
        # Handle None value.
    try:                    
        foo = int(foo_str)
    except ValueError:    
        # Handle the non-int case.

Catching all exceptions

This is in most cases a bad idea, since it means that we don't know what exactly might happen in the code protected by try:.

As a developer, think again before you use except Exception:; as a reviewer, ask the author why he thinks that it is reasonable to do that.

Most importantly, we must not hide bugs this way. You must either raise the exception again after doing some cleanup like calling transaction.abort(), or you must log an OOPS.

KeyboardInterrupt and SystemExit must not be caught under any circumstances.

If you really need to catch all exceptions, do it this way:

    try:
        # whatever.
    except (KeyboardInterrupt, SystemExit):
        # do some cleanup, if necessary.
        raise
    except Exception:
        # do some cleanup, if necessary.
        # log an OOPS or raise the exception again.

Note that from Python 2.5 on, KeyboardInterrupt and SystemExit do no longer inherit from Exception but from BaseException, thus we can remove the except (KeyboardInterrupt, SystemExit): clauses once we have upgraded Launchpad to 2.5.

Raising exceptions

Raise AssertionError when code that calls your code has not met its part of the contract. Use AssertionError if your code has been passed code of the wrong type. Don't use TypeError for this.

The reason is that we're writing an application, not a framework, not a general purpose library. We live and die by the contracts between the pieces of our application. When the contracts are not adhered to, we need to know that unambiguously.

Application code should never raise TypeError or NameError.

There are very limited circumstances in which it's OK to raise ValueError. Usually, it's more appropriate to use a more specific exception. However, if you have a function which converts data from one form to another in a way similar to int() or float(), raising ValueError may be appropriate.

In general, don't make your specific exceptions derive from ValueError. ValueError is an error saying something about where in the structure of the code the error is assumed to be: in some argument to a function, or something like that. That assumption is often unhelpful or misleading.

We should be more interested in what condition occured than whether the error might have been passed as an argument to a function at some point or other. Reflect this in your choice of exceptions.

Breaking the rules

There are some occassions when infrastructure code needs to break these rules. These are special cases. They should be few, and specially commented.

ExceptionGuidelines (last edited 2021-12-10 15:16:24 by cjwatson)