Question

I'm starting to notice this pattern throughout some of my code:

try:
    some_func()
except FirstException as err:  # known possible err
    # log err
    notify_user(err)
except SecondException as err:  # known possible err
    # log err
    notify_user(err)
except Exception as err:  # unexpected err to prevent crash
    # log err
    notify_user(err)
finally:
    # close logs

It should be noted that some_func does not explicitly raise these exceptions, but calls other methods that may propagate them back up the call stack. While its clean and clear that each exception caught will get its own logging (which presumably has a unique message), they all call the same notify_user. Therefore, this structure is not DRY, so I thought:

try:
    some_func()
except Exception as err:  # catch all (presumably) exceptions
    if isinstance(err, FirstException):
        # log err
    elif isinstance(err, SecondException):
        # log err
    else:
        # log err
    notify_user(err)
finally:
    # close logs

While also clear that logging is specific to the desired error caught, its now DRY.

But doesn't this defeat the point of knowing know exception types by trying to catch all exceptions and using conditionals to identify which is caught? Broad exception catching seems "okay" in concept, but I'm concerned it could lead to some unforeseen consequences later.

Was it helpful?

Solution

Catching a bare Exception is mostly bad practice, but this would solve your problem as I have understood it.

EXCEPTION_SPECIFIC_LOG_MSG = {
    FirstException: 'first exception',
    SecondException: 'second exception',
    Exception: 'bare exception'
}

try:
    some_func()
except (FirstException, SecondException, Exception) as e:
    print(EXCEPTION_SPECIFIC_LOG_MSG[type(e)])
    notify_user(e)

OTHER TIPS

Your solution looks ok to me. Alternatively (not necessarily "better"), here is a DRY version without using isinstance:

lasterr=None
try:
    some_func()
except FirstException as err:  # known possible err
    # log err
    lasterr=err
except SecondException as err:  # known possible err
    # log err
    lasterr=err
except Exception as err:  # unexpected err to prevent crash
    # log err
    lasterr=err
finally:
   if lasterr is not None:
      notify_user(err)
    # close logs

Of course, the statement lasterr=err is repeated here, but since this does not contain any real "logic", this does not count as a DRY violation. Those statements are usually not as likely to change as a function call notify_user(err), so the goal of DRY to reduce the number of places where changes will happen is fulfilled.

You did not show us the logging code, but it might be also a good idea not to do the real logging at all places where # log err occurs, only remember the specific logging message in a variable, and do the logging in the finally section as well.

Licensed under: CC-BY-SA with attribution
scroll top