New submission from Hrvoje Nikšić:

While using contextlib.ExitStack in our project, we noticed that its __exit__ 
method of contextlib.ExitStack suppresses the exception raised in any 
contextmanager's __exit__ except the outermost one. Here is a test case to 
reproduce the problem:

class Err:
  def __enter__(self): pass
  def __exit__(self, *exc): 1/0

class Ok:
  def __enter__(self): pass
  def __exit__(self, *exc): pass


import contextlib
s = contextlib.ExitStack()
s.push(Ok())
s.push(Err())
with s:
  pass

Since the inner context manager raises in __exit__ and neither context manager 
requests suppression, I would expect to see a ZeroDivisionError raised. What 
actually happens is that the exception is suppressed. This behavior caused us 
quite a few headaches before we figured out why we the exceptions raised in 
during __exit__ went silently undetected in production.

The problem is in ExitStack.__exit__, which explicitly propagates the exception 
only if it occurs in the outermost exit callback. The idea behind it appears to 
be to simply record the raised exception in order to allow exit callbacks of 
the outer context managers to see the it -- and get a chance to suppress it. 
The only exception that is directly re-raised is the one occurring in the 
outermost exit callback, because it certainly cannot be seen nor suppressed by 
anyone else.

But this reasoning is flawed because if an exception happens in an inner cm and 
none of the outer cm's chooses to suppress it, then there will be no one left 
to raise it. Simply returning True from ExitStack.__exit__ is of no help, as 
that only has an effect when an exception was passed into the function in the 
first place, and even then, the caller can only re-raise the earlier exception, 
not the exception that occurred in the exit callback. And if no exception was 
sent to ExitStack.__exit__, as is the case in the above code, then no exception 
will be re-raised at all, effectively suppressing it.

I believe the correct way to handle this is by keeping track of whether an 
exception actually occurred in one of the _exit_callbacks. As before, the 
exception is forwarded to next cm's exit callbacks, but if none of them 
suppresses it, then the exception is re-raised instead of returning from the 
function. I am attaching a patch to contextlib.py that implements this change.

The patch also makes sure that True is returned from ExitStack.__exit__ only if 
an exception was actually passed into it.

----------
components: Library (Lib)
files: exitstack.diff
keywords: patch
messages: 198411
nosy: hniksic
priority: normal
severity: normal
status: open
title: ExitStack.__exit__ incorrectly suppresses exceptions in __exit__ 
callbacks of inner context managers
type: behavior
versions: Python 3.4
Added file: http://bugs.python.org/file31872/exitstack.diff

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue19092>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to