classification
Title: contextlib.contextmanager may incorrectly unchain RuntimeError
Type: behavior Stage: resolved
Components: Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Jelle Zijlstra, Mariatta, ncoghlan, rhettinger, serhiy.storchaka, svelankar
Priority: normal Keywords: 3.5regression

Created on 2017-03-02 10:11 by ncoghlan, last changed 2017-04-13 10:17 by Mariatta. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 891 closed svelankar, 2017-03-30 00:56
PR 949 merged svelankar, 2017-04-01 13:33
PR 1079 merged ncoghlan, 2017-04-11 09:30
PR 1105 merged Mariatta, 2017-04-13 03:55
PR 1107 merged Mariatta, 2017-04-13 09:53
Messages (10)
msg288793 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-03-02 10:11
As part of PEP 479, an extra check was added to contextlib._GeneratorContextManager to avoid getting confused when a StopIteration exception was raised in the body of the with statement, and hence thrown into the generator body implementing the context manager.

This extra check should only be used when the passed in exception is `StopIteration`, but that guard is currently missing, so it may unchain arbitrary RuntimeError exceptions if they set their `__cause__` to the originally passed in value.

Compare the current contextmanager behaviour:

```
>>> from contextlib import contextmanager
>>> @contextmanager
... def chain_thrown_exc():
...     try:
...         yield
...     except Exception as exc:
...         raise RuntimeError("Chained!") from exc
... 
>>> with chain_thrown_exc():
...     1/0
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero
```

To the expected inline behaviour:

```
>>> try:
...     1/0
... except Exception as exc:
...     raise RuntimeError("Chained!") from exc
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
RuntimeError: Chained!

```
msg290813 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-30 05:48
The __exit__() method doesn't conform PEP 8.

PEP 8: """Be consistent in return statements. Either all return statements in a function should return an expression, or none of them should. If any return statement returns an expression, any return statements where no value is returned should explicitly state this as return None , and an explicit return statement should be present at the end of the function (if reachable)."""

The __exit__() method has explicit "return False", bare "return", and implicit "return" at the end of the method. Together with different styles in different "except" clauses this makes it slightly hard to read.
msg291183 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-05 15:30
PEP 8's rule makes sense elsewhere, but for context managers I think an implicit return None is the norm and that making it explicit wouldn't improve readability.
msg291185 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-05 15:44
Then the exception for the __exit__ method should be documented in PEP 8.
msg291466 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-04-11 09:11
New changeset 00c75e9a45ff0366c185e9e8a2e23af5a35481b0 by Nick Coghlan (svelankar) in branch 'master':
bpo-29692: contextlib.contextmanager may incorrectly unchain RuntimeError (GH-949)
https://github.com/python/cpython/commit/00c75e9a45ff0366c185e9e8a2e23af5a35481b0
msg291468 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-04-11 09:19
This has been merged for 3.7, but cherry-picks to the other branches are still needed.

I also inadvertently missed adding svelankar's name (Siddharth Velankar) to Misc/ACKS, so that oversight will need to be tidied up as well.
msg291472 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-04-11 09:47
New changeset e8a6bb4f3936123f3eca0b6cea05e2875a2722bc by Nick Coghlan in branch 'master':
bpo-29692: Add missing ACKS entry (#1079)
https://github.com/python/cpython/commit/e8a6bb4f3936123f3eca0b6cea05e2875a2722bc
msg291594 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-04-13 09:50
New changeset 9b409ff41ceb2d7ea7e8d25a7bbf5eb7d46625f3 by Mariatta in branch '3.6':
[3.6] bpo-29692: contextlib.contextmanager may incorrectly unchain RuntimeError (GH-949) (#1105)
https://github.com/python/cpython/commit/9b409ff41ceb2d7ea7e8d25a7bbf5eb7d46625f3
msg291596 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-04-13 10:14
New changeset 4d015a40a7b9c3c1b8cfbe81453187d700a43163 by Mariatta in branch '3.5':
[3.5] bpo-29692: contextlib.contextmanager may incorrectly unchain RuntimeError (GH-949) (#1107)
https://github.com/python/cpython/commit/4d015a40a7b9c3c1b8cfbe81453187d700a43163
msg291597 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-04-13 10:17
PR has been backported into 3.5 and 3.6. Thanks all :)
History
Date User Action Args
2017-04-13 10:17:40Mariattasetstatus: open -> closed

stage: backport needed -> resolved
messages: + msg291597
versions: + Python 3.7
2017-04-13 10:14:55Mariattasetmessages: + msg291596
2017-04-13 09:53:56Mariattasetpull_requests: + pull_request1247
2017-04-13 09:50:23Mariattasetnosy: + Mariatta
messages: + msg291594
2017-04-13 03:55:16Mariattasetpull_requests: + pull_request1244
2017-04-11 09:47:41ncoghlansetmessages: + msg291472
2017-04-11 09:30:58ncoghlansetpull_requests: + pull_request1222
2017-04-11 09:19:12ncoghlansetassignee: ncoghlan
2017-04-11 09:19:01ncoghlansetresolution: fixed
stage: test needed -> backport needed
messages: + msg291468
versions: - Python 3.7
2017-04-11 09:11:15ncoghlansetmessages: + msg291466
2017-04-09 10:30:06svelankarsetnosy: + svelankar
2017-04-05 15:44:49serhiy.storchakasetmessages: + msg291185
2017-04-05 15:30:39rhettingersetnosy: + rhettinger
messages: + msg291183
2017-04-01 13:33:36svelankarsetpull_requests: + pull_request1131
2017-03-30 05:48:15serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg290813
2017-03-30 00:56:34svelankarsetpull_requests: + pull_request794
2017-03-02 23:31:56Jelle Zijlstrasetnosy: + Jelle Zijlstra
2017-03-02 10:11:35ncoghlancreate