2
\$\begingroup\$

Before I try to make this work, I wondered if anyone had tried this, whether it was a good idea or not, etc.

I find myself doing this a lot:

if some_result is None:
    try:
        raise ResultException
    except ResultException:
        logger.exception(error_msg)
        raise ResultException(error_msg)

I would rather try this:

if some_result is None:
    with ResultException:
        logger.exception(error_msg)

Where the __exit__() statement would always raise the exception in the context.

Would this be clearer? Is this possible?

I only ask because I imagine someone else has tried this, or at least attempted it. But I don't seem to find any common patterns for accomplishing this after doing a couple of Google searches.

UPDATE

Here's some more information on this particular issue.

I have a method that goes to a database and determines what the user's id is via their email. The problem is that if the user isn't found, it'll return None. This is okay for most calls, but when it's incorporated in higher-level methods, this can happen:

def change_some_setting(setting, email):
    user_id = user_id_by_email(email) # None
    _change_some_setting(setting, id)

Now when _change_some_setting attempts to change the setting for a user who's id is None, we get a very vague ProgrammingException from the database library.

This helps prevent that. Is there a better way?

\$\endgroup\$
4
  • \$\begingroup\$ Why not log the exception when you actually handle it? The traceback will show where it's from. \$\endgroup\$ Commented Apr 11, 2013 at 19:56
  • \$\begingroup\$ As far as this example is concerned, this is handling it. It catches result sets that are empty, logs the complaint, and then bails out of execution. \$\endgroup\$ Commented Apr 11, 2013 at 20:09
  • \$\begingroup\$ Can you show the code that catches the exception? \$\endgroup\$ Commented Apr 11, 2013 at 20:56
  • \$\begingroup\$ The fix is that user_id_by_email should raise an exception, as @JanneKarila said in his answer (however creating a wrapper is only acceptable if you really can't change user_id_by_email). \$\endgroup\$ Commented May 2, 2013 at 7:13

2 Answers 2

Reset to default
2
\$\begingroup\$

This:

if some_result is None:
    with ResultException:
        logger.exception(error_msg)

would seem to be equivalent (for most cases) to:

if some_result is None:
    logger.exception(error_msg)
    raise ResultException

And so using a context manager as you describe it would not seem to be helpful. That's probably why you can't find any instances of it. But this would not actually work because when you call logger.exception, you need to be in an exception handler.

You could do:

def raise_logged(exception):
    try:
         raise exception
    except: 
         logging.exception(str(exception))
         raise exception

Then use it like

 if some_result is None:
       raise_logged( ResultException("not so good") )

However, throwing the exception just to catch it and log it is a bit yucky. Do you really need to log the exception as this point? Usually, we log exceptions when they are caught.

\$\endgroup\$
1
  • \$\begingroup\$ I updated my question with more specific information about my problem. \$\endgroup\$ Commented Apr 11, 2013 at 20:52
2
\$\begingroup\$

Your issue is that a function returns None when raising an exception would be more useful to you. I would create a wrapper for the function for the sole purpose of raising the exception. Then catch the exception normally where it makes sense to you, and log it from there.

def checked_user_id_by_email(email):
    result = user_id_by_email(email)   
    if result is None:
        raise LookupError("User not found by email '%s'" % email)
    return result

You might use a decorator to wrap your functions like this.

\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.