47

Is it possible to tell if there was an exception once you're in the finally clause? Something like:

try:
    funky code
finally:
    if ???:
        print('the funky code raised')

I'm looking to make something like this more DRY:

try:
    funky code
except HandleThis:
    # handle it
    raised = True
except DontHandleThis:
    raised = True
    raise
else:
    raised = False
finally:
    logger.info('funky code raised %s', raised)

I don't like that it requires to catch an exception, which you don't intend to handle, just to set a flag.


Since some comments are asking for less "M" in the MCVE, here is some more background on the use-case. The actual problem is about escalation of logging levels.

  • The funky code is third party and can't be changed.
  • The failure exception and stack trace does not contain any useful diagnostic information, so using logger.exception in an except block is not helpful here.
  • If the funky code raised then some information which I need to see has already been logged, at level DEBUG. We do not and can not handle the error, but want to escalate the DEBUG logging because the information needed is in there.
  • The funky code does not raise, most of the time. I don't want to escalate logging levels for the general case, because it is too verbose.

Hence, the code runs under a log capture context (which sets up custom handlers to intercept log records) and some debug info gets re-logged retrospectively:

try:
    with LogCapture() as log:
        funky_code()  # <-- third party badness
finally:
    # log events are buffered in memory. if there was an exception,
    # emit everything that was captured at a WARNING level
    for record in log.captured:
        if <there was an exception>:
            log_fn = mylogger.warning
        else:
            log_fn = getattr(mylogger, record.levelname.lower())
        log_fn(record.msg, record.args)
wim
  • 338,267
  • 99
  • 616
  • 750
  • 4
    Shouldn't that be "DRYer" instead of "more DRY" ;) – robinCTS Mar 05 '18 at 03:56
  • 2
    @robinCTS Should there even be a comparative for dry? – Raimund Krämer Mar 05 '18 at 08:56
  • 3
    Concerning the question: Shouldn't the logging of general exceptions (and perhaps rethrowing) be done in a general `except:` block, rather than finally? – Raimund Krämer Mar 05 '18 at 08:58
  • @RaimundKrämer Hmmm. I suppose either it's repeated and is therefore not DRY or it's not repeated and it *is* DRY ;) However, I think that it may not always be possible to completely remove any repetition, but only reduce it. Therefore, I suppose in that case, a comparative would be in order. – robinCTS Mar 05 '18 at 09:01
  • 9
    Can you give some clarification as to what you're trying to achieve here? The exceptions should contain the entire stacktrace, so you don't want to log them except at the point you actually handle them (at which point, you will still have the entire trace to know that it was caused by "funkycode"). From the question alone, I really struggle to see what benefit this brings - logging that you didn't handle an exception seems like a code-smell. –  Mar 05 '18 at 11:43
  • 1
    Agree with @Bilkokuya, it's not clear what you want to do. If you want to log exceptions, just do `except Exception as e: logger.exception(e)`. – Mark E. Haase Mar 05 '18 at 15:21
  • @Bilkokuya added some more background information about the use-case. – wim Mar 05 '18 at 16:41

6 Answers6

53

Using a contextmanager

You could use a custom contextmanager, for example:

class DidWeRaise:
    __slots__ = ('exception_happened', )  # instances will take less memory

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        # If no exception happened the `exc_type` is None
        self.exception_happened = exc_type is not None

And then use that inside the try:

try:
    with DidWeRaise() as error_state:
        # funky code
finally:
    if error_state.exception_happened:
        print('the funky code raised')

It's still an additional variable but it's probably a lot easier to reuse if you want to use it in multiple places. And you don't need to toggle it yourself.

Using a variable

In case you don't want the contextmanager I would reverse the logic of the trigger and toggle it only in case no exception has happened. That way you don't need an except case for exceptions that you don't want to handle. The most appropriate place would be the else clause that is entered in case the try didn't threw an exception:

exception_happened = True
try:
    # funky code
except HandleThis:
    # handle this kind of exception
else:
    exception_happened = False
finally:
    if exception_happened:
        print('the funky code raised')

And as already pointed out instead of having a "toggle" variable you could replace it (in this case) with the desired logging function:

mylog = mylogger.WARNING
try:
    with LogCapture() as log:
        funky_code()
except HandleThis:
    # handle this kind of exception
else:
    # In case absolutely no exception was thrown in the try we can log on debug level
    mylog = mylogger.DEBUG
finally:
    for record in log.captured:
        mylog(record.msg, record.args)

Of course it would also work if you put it at the end of your try (as other answers here suggested) but I prefer the else clause because it has more meaning ("that code is meant to be executed only if there was no exception in the try block") and may be easier to maintain in the long run. Although it's still more to maintain than the context manager because the variable is set and toggled in different places.

Using sys.exc_info (works only for unhandled exceptions)

The last approach I want to mention is probably not useful for you but maybe useful for future readers who only want to know if there's an unhandled exception (an exception that was not caught in any except block or has been raised inside an except block). In that case you can use sys.exc_info:

import sys

try:
    # funky code
except HandleThis:
    pass
finally:
    if sys.exc_info()[0] is not None:
        # only entered if there's an *unhandled* exception, e.g. NOT a HandleThis exception
        print('funky code raised')
MSeifert
  • 145,886
  • 38
  • 333
  • 352
  • 8
    I'd be pretty surprised if the cleanest method weren't rewriting the code so that the finally section doesn't need to know if an exception was raised. This is a close second, though. +1 from me too. – GrandOpener Mar 05 '18 at 08:39
  • The `sys.exc_info` method doesn't seem to work. This was my first thought, but it always seems to be `None` in the `finally` block (in Python 3.6 anyway). – Martin Tournoij Mar 05 '18 at 12:16
  • @Carpetsmoker Yes, the second approach only works for **unhandled** exceptions. With that I meant that the raised exception isn't cleared by any `except` clause. For example if you `raise ValueError` and have an `except TypeError`. I thought I made that clear in the answer (emphasis on "unhandled" and the inline code comment) but if that needs additional clarification please let me know. – MSeifert Mar 05 '18 at 17:10
  • The second approach was just what I was after - many thanks for including it! – Roger Heathcote May 14 '21 at 15:08
31
raised = True
try:
    funky code
    raised = False
except HandleThis:
    # handle it
finally:
    logger.info('funky code raised %s', raised)

Given the additional background information added to the question about selecting a log level, this seems very easily adapted to the intended use-case:

mylog = WARNING
try:
    funky code
    mylog = DEBUG
except HandleThis:
    # handle it
finally:
    mylog(...)
Jean-Paul Calderone
  • 47,755
  • 6
  • 94
  • 122
  • 5
    I sort of agree with this but I also find that forms that avoid a negation are also usually easier to understand. "raised" could well be a positive form and might avoid a negation. There's no way to know without knowing what's going to go into the real `finally` block. For the sample code, where the value is merely logged, I don't see it as a readability issue at all. – Jean-Paul Calderone Mar 04 '18 at 20:32
  • @DanielPryden But the actual comparison is `if not success` versus `if failure` and, really, the version without the negation is easier. (Not significantly easier, but your `if not has_zero_failures` is a straw man.) – David Richerby Mar 05 '18 at 10:47
  • 4
    conceptually, this code doesn't test whether an exception was raised, but whether the code block ran to completion. therefor, i'd also prefer a `success` or `completed` variable. – ths Mar 05 '18 at 13:00
3

You can easily assign your caught exception to a variable and use it in the finally block, eg:

>>> x = 1
>>> error = None
>>> try:
...     x.foo()
... except Exception as e:
...     error = e
... finally:
...     if error is not None:
...             print(error)
...
'int' object has no attribute 'foo'
Jonathan R
  • 3,652
  • 3
  • 22
  • 40
  • 3
    This seems to be the same as setting a flag, it's just using the exception instance as a flag. You still have to catch-all exception. – wim Mar 04 '18 at 20:05
  • 2
    @wim: Of course `error` _is_ a flag, but I find it a natural and appealing type of flag, as it preserves full information, which may be useful if the handling needs extending or during debugging. Maybe that is unnecessary here, but sometimes it is useful. – PJTraill Mar 07 '18 at 14:20
1

Okay, so what it sounds like you actually just want to either modify your existing context manager, or use a similar approach: logbook actually has something called a FingersCrossedHandler that would do exactly what you want. But you could do it yourself, like:

@contextmanager
def LogCapture():
    # your existing buffer code here
    level = logging.WARN
    try:
        yield
    except UselessException:
        level = logging.DEBUG
        raise  # Or don't, if you just want it to go away
    finally:
        # emit logs here

Original Response

You're thinking about this a bit sideways.

You do intend to handle the exception - you're handling it by setting a flag. Maybe you don't care about anything else (which seems like a bad idea), but if you care about doing something when an exception is raised, then you want to be explicit about it.

The fact that you're setting a variable, but you want the exception to continue on means that what you really want is to raise your own specific exception, from the exception that was raised:

class MyPkgException(Exception):  pass
class MyError(PyPkgException): pass  # If there's another exception type, you can also inherit from that

def do_the_badness():
    try:
        raise FileNotFoundError('Or some other code that raises an error')
    except FileNotFoundError as e:
        raise MyError('File was not found, doh!') from e
    finally:
        do_some_cleanup()

try:
    do_the_badness()
except MyError as e:
    print('The error? Yeah, it happened')

This solves:

  • Explicitly handling the exception(s) that you're looking to handle
  • Making the stack traces and original exceptions available
  • Allowing your code that's going to handle the original exception somewhere else to handle your exception that's thrown
  • Allowing some top-level exception handling code to just catch MyPkgException to catch all of your exceptions so it can log something and exit with a nice status instead of an ugly stack trace
wim
  • 338,267
  • 99
  • 616
  • 750
Wayne Werner
  • 49,299
  • 29
  • 200
  • 290
  • 2
    +1 for the frame challenge - anything that involves "if the `try` failed..." sounds like an `except` block. – Soron Mar 05 '18 at 15:31
  • 2
    You're making too many assumptions about the use case here, and have ended up answering a different question. The actual exception is not interesting, and I don't want to raise a custom error or enable exception chaining. – wim Mar 05 '18 at 16:19
  • 1
    This still means you have to repeat `raise MyError` in every `except` block. – Barmar Mar 06 '18 at 20:21
  • @Barmar `except (ThisError, AnotherError, FinalError) as e:`? – Wayne Werner Mar 07 '18 at 05:35
  • 1
    @WayneWerner That will only work if you want to use the same handling for all those types. – Barmar Mar 07 '18 at 15:43
  • The OP explicitly said they didn't want to have to handle the exception to set a flag - but *setting a flag is handling the exception*. You can't say, "I want to do something when an exception happens, but I don't want to do anything when the exception happens". It just so happens that in the OPs case, "doing something" is recording that an exception happened. Then at that point the options are a) re-raise the exception b) raise a new exception (from the current one) and b) silence all exceptions. If you have specific handling to perform for exception types A, B, and C, but you want different – Wayne Werner Mar 07 '18 at 16:10
  • approaches for E and F, you can just use `isinstance(e, (E, F))` (or vise versa). Practically I've never had that issue, and I can't really imagine the circumstance where setting a flag but not handling an exception makes sense. If you're not catching the exception, what good is the flag? – Wayne Werner Mar 07 '18 at 16:14
  • *setting a flag is handling the exception* - disagree. Wanting to know whether an exception occurred is **not** handling the exception, it's something entirely orthogonal. I have no intention of handling the exception (hence not wanting to catch it). – wim Mar 07 '18 at 17:11
0

If it was me, I'd do a little re-ordering of your code.

raised = False
try:
    # funky code
except HandleThis:
    # handle it
    raised = True
except Exception as ex:
    # Don't Handle This 
    raise ex
finally:
    if raised:
        logger.info('funky code was raised')

I've placed the raised boolean assignment outside of the try statement to ensure scope and made the final except statement a general exception handler for exceptions that you don't want to handle.

This style determines if your code failed. Another approach might me to determine when your code succeeds.

success = False
try:
    # funky code
    success = True
except HandleThis:
    # handle it
    pass
except Exception as ex:
    # Don't Handle This 
    raise ex
finally:
    if success:
        logger.info('funky code was successful')
    else:
        logger.info('funky code was raised')
Mike Peder
  • 728
  • 4
  • 8
  • 8
    This changes the logic (you get wrong result from the "Don't Handle" branch) – wim Mar 04 '18 at 20:04
  • Baloney! This works how the author intended. Instead of starting off by stating falsely that an exception was raised (or code succeeded), this is using a boolean to accurately reflect the state of code blocks. Note that the `success` flag is not set true until the entire `try` block completes. Similarly, the `raised` flag isn't set true until a specific exception occurs (an exception to be post-processed). The author is using the `raised` flag to also control which exception triggers post-processing by setting it judiciously. Don't forget `Exception` casts a wide net, otherwise `else` does it. – ingyhere Jan 28 '20 at 21:23
-1

If exception happened --> Put this logic in the exception block(s).
If exception did not happen --> Put this logic in the try block after the point in code where the exception can occur.

Finally blocks should be reserved for "cleanup actions," according to the Python language reference. When finally is specified the interpreter proceeds in the except case as follows: Exception is saved, then the finally block is executed first, then lastly the Exception is raised.

ingyhere
  • 11,818
  • 3
  • 38
  • 52
  • Just to be clear, the original question is predicated on poor architecture -- Don't promulgate a hack when there are better solutions that comport with architectural standards, the Python Language Reference, etc. – ingyhere Jan 28 '20 at 21:05
  • The question is about escalating logging levels and you didn't address that at all. If this pattern is a poor architecture, then what architecture exactly would you suggest as a better flow-control pattern for implementing something like a [`FingersCrossedHandler`](https://logbook.readthedocs.io/en/stable/api/handlers.html#logbook.FingersCrossedHandler) for logging/monitoring? Your (late) answer hasn't contributed anything new or useful here. – wim Jan 28 '20 at 21:12
  • Have you considered using AOP to isolate the log statements from the troubled code, something like [aspectlib](https://pypi.org/project/aspectlib/)? You could also possibly isolate the troubled code into its own class which has its own dedicated logger to see those exceptions by themselves. – ingyhere Jan 30 '20 at 03:04
  • As mentioned in the post, we can't modify the code under question because it is a third party code with its own loggers. I don't understand how aspectlib works or how it could help here, never used it, but if you think it would be useful then perhaps edit into your answer to demonstrate how. – wim Jan 30 '20 at 05:01
  • AOP in Python, aka Monkey Patching, involves dynamically injecting new method/property values into the target source *without modifying the target code*. So conceivably you could replace the loggers with your own config logger that hits a configured file. You could even replace a `logger` variable with a method that wraps two loggers -- the original logger and a new one. – ingyhere Jan 31 '20 at 05:31
  • But I don’t have any problem capturing the logs or monkeypatching out loggers. That part is already fine. How does your suggestion help to retrospectively escalate previous log events *only in the case that an exception was encountered*? Can you add a code sample demonstrating your idea? This is the important part of the question. – wim Jan 31 '20 at 05:53
  • You're right, of course, that's where the details become critical. My suggestion assumes that logging levels are applied correctly and that exceptions are logged appropriately, therefore basic filters on the log statements work for the capture. Possibly there are keywords to filter. For uncaught exceptions, one could always filter syslogs. By monkeypatching the logs you could apply a lower log level to the "funky code," then perhaps filter for exceptions, thereby not overloading the root logger with spurious statements. I'll think about it but my belief is monkey patching may offer a solution. – ingyhere Jan 31 '20 at 06:25