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\$

closed as off-topic by Billal Begueradj, Graipher, Stephen Rauch, Toby Speight, Sᴀᴍ Onᴇᴌᴀ Jun 20 '18 at 15:06

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Billal Begueradj, Graipher, Stephen Rauch, Toby Speight, Sᴀᴍ Onᴇᴌᴀ
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Why not log the exception when you actually handle it? The traceback will show where it's from. \$\endgroup\$ – Janne Karila Apr 11 '13 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\$ – Droogans Apr 11 '13 at 20:09
  • \$\begingroup\$ Can you show the code that catches the exception? \$\endgroup\$ – Winston Ewert Apr 11 '13 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\$ – Quentin Pradet May 2 '13 at 7:13
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\$
  • \$\begingroup\$ I updated my question with more specific information about my problem. \$\endgroup\$ – Droogans Apr 11 '13 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\$

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