3
\$\begingroup\$

Some of my functions use a "fail_silently" flag. It is used in the following way:

def test(a, b, c=1, fail_silently = False)
    try:
        return int(a) + c
    except:
       if fail_silently:
           return None
       raise

Therefore, if there is an error, we catch it and fail gracefully. The key here is that it can be toggled dynamically by whoever is calling it.

I am using this for a many different functions and class methods and thought to make it a decorator.

There are a few problems:

  1. I want to be able name the flag "raise_exception" or "fail_silently" or "whatever"...
  2. The flag may or may not have a default value
  3. The flag may or may not be passed in (usually not)
  4. It needs to work with class methods too

Thus my decorator needs to look for (assuming the flag is called "fail_silently") the flag in the following locations in this order

  1. **kwargs (the passed in function arguments), simple dictionary get on flag
  2. *args - get the positional argument of the flag, then scan for index in args (which might not be there)
  3. Get the default value of the flag from the function

The problem is the code is now getting really messy and has many points of failure.

def try_except_response(parameter = 'fail_silently'):
    def real_decorator(func):
        @functools.wraps(func)
        def wrapper(*args, **kwargs):
            try:
                return func(*args, **kwargs) # the function itself, 
            except: # if it raises an Exception!
                # check to see if the Flag is in passed in kwargs!
                if kwargs.get(parameter)== True: 
                    return None
                elif kwargs.get(parameter) == False:
                    raise
                else:
                    # Flag is not in kwargs, check to see if it is in args
                    function_args, vargs, kewords, defaults = inspect.getargspec(func) # get the index of argument of interest
                    try:
                        argument_index = function_args.index(parameter)  # get the index of argument of interest
                    except ValueError:
                        raise ValueError('The inputted decorator for "fail_silently" was not found!  The behavior will not work.')

                    if len(args) > argument_index: # check to see if it is inputted

                        keyword = args[argument_index] # get the value!!
                        if keyword == True: 
                            return None
                        elif kwargs == False:
                            raise
                        else:
                            raise ValueError('The "fail_silently" flag did not return a value that we understand.  Must be True or False')

                    else:
                        # not in args either! let's go get the default value.
                        # TODO MORE STUFF!!!
                        raise
                    raise
                raise
        return wrapper

What is the best way this can be cleaned up? Or alternatives? I am thinking about implementing a version of locals() that will create a dictionary of all parameters passed into the function and then check from there...

There are simplifying assumptions we can make, but that would require the person writing the function to know something ('flag is always last argument, or default value is always False...'), but I am hoping to get the decorator to be as transparent as possible so it just "works" and lets the function be a function (or class method)

\$\endgroup\$
  • 6
    \$\begingroup\$ The simplest answer is "don't do this", ever, no matter how temptying it may seem at the time. Exceptions are there to help you find problems with your code. What you are doing is called "exception hiding", and is easily one of the most notorious coding anti-patterns. It results in code that is pretty much guaranteed to explode in your face and will be almost impossible to debug when it does. If exceptions need to be dealt with, let them be dealt with by whoever is calling the function in a manner appropriate for how they want to use it. \$\endgroup\$ – TheBlackCat Jun 29 '15 at 19:15
  • \$\begingroup\$ Here is a related discussion about your original case - int() raising an Exception, and an ignore_exception decorator which allows you to specify particular exceptions to be ignored: stackoverflow.com/questions/2262333/… \$\endgroup\$ – TessellatingHeckler Jun 29 '15 at 20:55
6
\$\begingroup\$

I'm not a fan of this design. @TheBlackCat is right. Don't do this.

If you are to do this, rethink your design. A more sensible one might look like

import functools
from contextlib import suppress

def try_except_response(func):
    @functools.wraps(func)
    def wrapper(*args, fail_silently, **kwargs):
        if fail_silently:
            with suppress(Exception):
                return func(*args, **kwargs)
        else:
            return func(*args, **kwargs)

    return wrapper

The key principle here is KISS.

Note that I'm suppressing Exception, not BaseException, since catching SystemExit and KeyboardInterrupt is almost always a bad idea.

If you want to make fail_silently a default argument, just use another wrapper:

def default_fail_silently(default):
    def decorator(func):
        @functools.wraps(func)
        def wrapper(*args, fail_silently=default, **kwargs):
            return func(*args, fail_silently=fail_silently, **kwargs)
        return wrapper
    return decorator

Then you can do

@default_fail_silently(False)
@try_except_response
def test(a, b, c=1)
    return int(a) + c

This does mean there's more boilerplate, since it exists for each piece of functionality you add, but the logic itself is an order of magnitude simpler.

\$\endgroup\$
  • \$\begingroup\$ i totally agree here. trying to simply the logic. is there a way it can be simplified where the default_fail_silently flag can be checked at function evaluation? that way each caller can set their own whether or not to supress \$\endgroup\$ – user1639926 Jun 29 '15 at 21:50
  • 1
    \$\begingroup\$ @user1639926 "that way each caller can set their own whether or not to supress" → I'm not sure what you mean by this. Why is what I gave not sufficient? You do, say, test(a, b, fail_silently=False). \$\endgroup\$ – Veedrac Jun 29 '15 at 21:58
  • \$\begingroup\$ Note that contextlib.suppress is new in python 3.4 \$\endgroup\$ – Eric Jul 27 '15 at 21:54
2
\$\begingroup\$

Your code looks really nice, I saw a few parts that might be improved:

  1. if kwargs.get(parameter)== True:: binary operators are suggested, by PEP8, to have a space pre&proceeding the operator =
  2. function_args, vargs, kewords,: is that last one a misspell?

  3. if keyword == True:: If you're testing whether the variable is empty, as PEP8 says, you can use if keyword


Other than that, your code looks really nice, good work!

\$\endgroup\$
1
\$\begingroup\$

I'm trying to get this code to work (Python 2.7) and it doesn't work copied from your question. try_except_response needs to return real_decorator, I think, as well as include the imports.

Now I've tried it - it seems unhelpful that a function can have 'fail_silently' as an argument which this looks for. If you are editing the function to put that in the argument list, why not edit it to put the silencer in the function while you are there?

Anyway, saying nothing of whether this is a good idea or a good approach, the code you have posted has some unnecessary comments which repeats what the line of code says - this is no help to someone who can read Python - and no help to someone who can't:

except: # if it raises an Exception!

keyword = args[argument_index] # get the value!!

argument_index = function_args.index(parameter)  # get the index of argument of interest

And some which don't explain clearly, or are misleading:

if len(args) > argument_index: # check to see if it is inputted
#Check ... what?

function_args, vargs, kewords, defaults = inspect.getargspec(func) # get the index of argument of interest
#This doesn't get an index.

Regarding the level of indentation; you can take this structure:

            if kwargs.get(parameter)== True: 
                return None
            elif kwargs.get(parameter) == False:
                raise
            else:
                # All this code is indented
                # ...

and make it:

            # Was the parameter passed in as a kwarg?
            if parameter in kwargs:
                if kwargs.get(parameter):
                    return None
                else:
                    raise

            # All this code is not indented

You can take:

if len(args) > argument_index:
    # big indented block



else:
    raise

and reverse the logic:

# Was the parameter provided to the function call?
if len(args) <= argument_index: # no
    raise

# not-indented code

After searching for the parameter value in the keyword args and not finding it, if you find the value in args, why do you call it ... keyword?

And you can probably remove the "except ValueError" code if you finish writing the TODO behaviour for if searching fails.

            # Is the parameter a named parameter in the function definition?
            function_args, vargs, kewords, defaults = inspect.getargspec(func)
            if not parameter in argument_index:
                raise

Then the inner wrapper becomes a try / except, and four clearly separate sections.

    def wrapper(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except:
            # Was the parameter passed in as a kwarg?
            if parameter in kwargs:
                if kwargs.get(parameter):
                    return None
                else:
                    raise

            # Is the parameter one of the normal function parameters?
            function_args, vargs, kewords, defaults = inspect.getargspec(func)
            if not parameter in function_args:
                raise

            # Was the parameter provided to the function call?
            argument_index = function_args.index(parameter)
            if len(args) <= argument_index: # no
                raise

            # Can we use the provided value?
            keyword = args[argument_index]
            if keyword is True: 
                return None
            elif kwargs is False:
                raise
            else:
                raise ValueError('The "fail_silently" flag did not return a value that we understand.  Must be True or False')

            # If we could not do anything else, re-raise by default.
            raise
\$\endgroup\$
1
\$\begingroup\$

Following on from a previous answer, we can simplify even further to:

from contextlib import *

def try_except_response(func):
    @functools.wraps(func)
    def wrapper(*args, fail_silently, **kwargs):
        with suppress(Exception) if fail_silently else ExitStack():
            return func(*args, **kwargs)

    return wrapper
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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