37

I'm currently writing a wrapper class. I want to be able to log exceptions properly but allow calling methods to be aware of exceptions which occur. My class looks like this:

import logging

log = logging.getLogger('module')

class MyAPIWrapper(library.APIClass):

    def __init__(self):
        self.log = logging.getLogger('module.myapiwrapper')


    def my_wrapper_method(self):
        try:
            response = self.call_api_method()
            return response.someData
        except APIException, e:
            self.log.exception('Oh noes!')
            raise e #Throw exception again so calling code knows it happened

I'm a bit dubious about catching and exception just to log it and then re-raising it so the calling code can do something about it. What's the proper pattern here?

Improve this question
2
  • possible duplicate of python exception logging Commented Sep 11, 2013 at 17:56
  • That's exactly what I am doing. Thanks for posting this question. Commented Mar 23, 2016 at 8:29

3 Answers 3

Reset to default
40

There is nothing wrong with catching to log. However, I'd recommend:

    try:
        response = self.call_api_method()
    except APIException, e:  # or 'as e' depending on your Python version
        self.log.exception('Oh noes!')
        raise #Throw exception again so calling code knows it happened
    else:
        return response.someData

By just doing a bare raise you preserve the full traceback info. It's also more explicit to put code that will only happen if you don't have an exception in the else clause, and it's clearer what line you're catching an exception from.

It would also be fine for the calling class to do the logging if it's handling the error anyway, but that may not be convenient for your app.

Edit: The documentation for try ... except ... else ... finally is under compound statements.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Is the last keyword supposed to be "except" instead of "else" or does that serve some purpose?
It's supposed to be else. The else clause happens only if there was no exception, just like the else clause on for and while loops only happen if you didn't break out.
12

That method is correct, although instead of raise e you should just use raise, which will automatically re-raise the last exception. This is also one of the rare cases where using a blanket except is considered acceptable.

Here is an example very similar to what you are doing from the Python docs on Handling Exceptions:

The last except clause may omit the exception name(s), to serve as a wildcard. Use this with extreme caution, since it is easy to mask a real programming error in this way! It can also be used to print an error message and then re-raise the exception (allowing a caller to handle the exception as well):

import sys

try:
    f = open('myfile.txt')
    s = f.readline()
    i = int(s.strip())
except IOError as (errno, strerror):
    print "I/O error({0}): {1}".format(errno, strerror)
except ValueError:
    print "Could not convert data to an integer."
except:
    print "Unexpected error:", sys.exc_info()[0]
    raise
Improve this answer

Comments

3

You can just extend the standard Exception class and add the logger into there.

Like this:

class LoggedException(Exception):
    """ An exception that also logs the msg to the given logger. """
    def __init__(self, logger: logging.Logger, msg: str):
        logger.error(msg)
        super().__init__(msg)
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.