nested try..excepts don't work correctly for generators

[1st1]

BPO	25612
Nosy	@brettcannon, @ncoghlan, @pitrou, @scoder, @vstinner, @larryhastings, @benjaminp, @ned-deily, @njsmith, @asvetlov, @cjerdonek, @markshannon, @vadmium, @serhiy-storchaka, @1st1, @ajdavis
PRs	Move exc state to generator. Fixes bpo-25612 #1773 bpo-33612: Remove PyThreadState_Clear() assertion #7069 [3.7] bpo-33612: Remove PyThreadState_Clear() assertion (GH-7069) #7074 bpo-25612: Add minimal What's New in 3.7 entry #7656 [3.7] bpo-25612: Add minimal What's New in 3.7 entry (GH-7656) #7658
Files	gen_exc_1.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-10-30.11:37:04.064>
created_at = <Date 2015-11-12.20:59:18.932>
labels = ['interpreter-core', '3.7']
title = "nested try..excepts don't work correctly for generators"
updated_at = <Date 2018-06-29.12:41:13.832>
user = 'https://github.com/1st1'

bugs.python.org fields:

activity = <Date 2018-06-29.12:41:13.832>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-10-30.11:37:04.064>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2015-11-12.20:59:18.932>
creator = 'yselivanov'
dependencies = []
files = ['41026']
hgrepos = []
issue_num = 25612
keywords = ['patch']
message_count = 38.0
messages = ['254557', '254558', '254561', '254564', '254576', '254578', '254809', '254812', '254815', '254821', '254822', '254854', '254858', '254870', '254990', '254991', '254995', '254996', '255039', '255079', '256622', '287995', '287996', '304117', '304760', '304770', '304771', '305150', '305151', '305220', '305223', '317417', '317442', '319358', '319360', '319361', '319366', '320715']
nosy_count = 16.0
nosy_names = ['brett.cannon', 'ncoghlan', 'pitrou', 'scoder', 'vstinner', 'larry', 'benjamin.peterson', 'ned.deily', 'njs', 'asvetlov', 'chris.jerdonek', 'Mark.Shannon', 'martin.panter', 'serhiy.storchaka', 'yselivanov', 'emptysquare']
pr_nums = ['1773', '7069', '7074', '7656', '7658']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25612'
versions = ['Python 3.7']

[1st1]

Nested try..except statements with yields can loose reference to the current exception.

The following code:

    class MainError(Exception):
        pass

    class SubError(Exception):
        pass

    def main():
        try:
            raise MainError()
        except MainError:
            try:
                yield
            except SubError:
                print('got SubError')
            raise

    coro = main()
    coro.send(None)
    coro.throw(SubError())

prints:

    got SubError
    Traceback (most recent call last):
      File "t.py", line 19, in <module>
        coro.throw(SubError())
      File "t.py", line 15, in main
        raise
    RuntimeError: No active exception to reraise

[1st1]

This was originally discovered here: python/asyncio#287

[vstinner]

Is this issue related to the issue bpo-23353?

[vstinner]

I reverted the change of the issue bpo-23353 but it doesn't fix this example, so it looks like these issues are not related. Cool.

[1st1]

Another bug:

    class MainError(Exception):
        pass

    class SubError(Exception):
        pass

    def main():
        try:
            raise MainError()
        except MainError:
            yield

    coro = main()
    coro.send(None)
    coro.throw(SubError())

SubError will propagate, but won't have MainError in its __context__

[1st1]

Here's a patch the fixes the first problem (but __context__ bug is still open).

I'm not sure that the patch is correct :( But at least I've added new unittests (one still failing)

[1st1]

Can someone work with me on fixing this issue? I think it's important to ship 3.5.1 with this resolved (and 3.4.4 FWIW).

[gvanrossum]

You might have to ping python-dev. But in terms of priorities I think
it's not a blocker -- it's been broken for quite some time now, and
it's a fairly odd corner of the language.

[vadmium]

Regarding the second bug, did you consider that the exception thrown to the generator can already have __context__ set?

try:
try: raise ValueError("Context outside of generator")
except ValueError as ex: raise SubError() from ex
except SubError as ex:
coro.throw(ex) # ex.__context__ is a ValueError

I guess either one context could trump the other, or we could to follow the chain of contexts and append the other chain at the end. Both these ideas seem a bit ugly though.

[vstinner]

Can someone work with me on fixing this issue? I think it's important to ship 3.5.1 with this resolved (and 3.4.4 FWIW).

It don't consider this issue as a blocker for Python 3.5.1. This
release already contains a *lot* of bugfixes! It's important to get it
as soon as possible.

[vadmium]

Thinking about the __context__ thing some more, I guess it might make sense for __context__ to be overwritten by the generator context. The same way it gets overwritten when you execute “raise” inside an exception handler.

[1st1]

Thinking about the __context__ thing some more, I guess it might make sense for __context__ to be overwritten by the generator context. The same way it gets overwritten when you execute “raise” inside an exception handler.

Not sure I understand what you're saying here.

[vadmium]

I was making an analogy between how the “raise” statement works, and how the throw() method could work. In this example, there are three exception objects (MainError, SubError, and ValueError). I was suggesting that it is okay for the context to be set to the MainError instance, because that is how the analogous version using a “raise” statement works.

def main():
    try:
        raise MainError("Context inside generator")
    except MainError:
        yield  # __context__ could be changed to MainError

coro = main()
coro.send(None)
try:
    try: raise ValueError("Context outside of generator")
    except ValueError: raise SubError()
except SubError as ex:
    coro.throw(ex)  # __context__ is ValueError

# raise analogy:

try:
try: raise ValueError("Context outside of generator")
except ValueError as ex: raise SubError()
except SubError as ex:
saved = ex # __context__ is ValueError

try:
raise MainError("Context inside generator")
except MainError:
raise saved # Changes __context__ to MainError

===

Sorry I’m not really familiar with the code to quickly propose a patch or review your change though.