Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Worse error from asynccontextmanager in Python 3.10 #90154

Closed
dimaqq mannequin opened this issue Dec 6, 2021 · 10 comments
Closed

Worse error from asynccontextmanager in Python 3.10 #90154

dimaqq mannequin opened this issue Dec 6, 2021 · 10 comments
Labels
3.10 only security fixes 3.11 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@dimaqq
Copy link
Mannequin

dimaqq mannequin commented Dec 6, 2021

BPO 45996
Nosy @ncoghlan, @asvetlov, @ambv, @dimaqq, @1st1, @graingert

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 2021-12-19.17:16:00.065>
created_at = <Date 2021-12-06.13:24:19.853>
labels = ['3.11', 'type-bug', '3.10', 'expert-asyncio']
title = 'Worse error from asynccontextmanager in Python 3.10'
updated_at = <Date 2021-12-20.08:04:18.860>
user = 'https://github.com/dimaqq'

bugs.python.org fields:

activity = <Date 2021-12-20.08:04:18.860>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2021-12-19.17:16:00.065>
closer = 'asvetlov'
components = ['asyncio']
creation = <Date 2021-12-06.13:24:19.853>
creator = 'Dima.Tisnek'
dependencies = []
files = []
hgrepos = []
issue_num = 45996
keywords = []
message_count = 10.0
messages = ['407799', '407811', '407812', '407813', '407814', '407817', '408939', '408940', '408941', '408948']
nosy_count = 6.0
nosy_names = ['ncoghlan', 'asvetlov', 'lukasz.langa', 'Dima.Tisnek', 'yselivanov', 'graingert']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45996'
versions = ['Python 3.10', 'Python 3.11']

@dimaqq
Copy link
Mannequin Author

dimaqq mannequin commented Dec 6, 2021

Consider this illegal code:

import logging
from asyncio import sleep, gather, run
from contextlib import asynccontextmanager

@asynccontextmanager
async def foo():
await sleep(1)
yield

async def test():
    f = foo()
    await gather(f.__aenter__(), f.__aenter__())

run(test())

If it's ran with Python 3.9, user gets a sensible error:

File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/contextlib.py", line 175, in __aenter__
return await self.gen.__anext__()
RuntimeError: anext(): asynchronous generator is already running

However, if it's ran with Python 3.10, user gets a cryptic error:

File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 197, in __aenter__
del self.args, self.kwds, self.func
AttributeError: args

Which makes it harder to pinpoint what's wrong when the stack is complex.
I've hit this with fastapi/starlette/mangum combo and a custom middleware.

@dimaqq dimaqq mannequin added 3.10 only security fixes 3.11 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error labels Dec 6, 2021
@graingert
Copy link
Mannequin

graingert mannequin commented Dec 6, 2021

I think AttributeError: args is the desired/expected behaviour

consider the sync version:

import logging
from asyncio import sleep, gather, run
from contextlib import asynccontextmanager, contextmanager

@contextmanager
def foo():
    yield


def test():
    f = foo()
    f.__enter__()
    f.__enter__()

test()
Traceback (most recent call last):
  File "/home/graingert/projects/example/sync.py", line 15, in <module>
    test()
  File "/home/graingert/projects/example/sync.py", line 13, in test
    f.__enter__()
  File "/usr/lib/python3.9/contextlib.py", line 117, in __enter__
    del self.args, self.kwds, self.func
AttributeError: args

@graingert
Copy link
Mannequin

graingert mannequin commented Dec 6, 2021

or consider the trio version:

import logging
import trio
from contextlib import asynccontextmanager

@asynccontextmanager
async def foo():
    await trio.sleep(1)
    yield


async def test():
    async with trio.open_nursery() as n:
        f = foo()
        n.start_soon(f.__aenter__)
        n.start_soon(f.__aenter__)

trio.run(test)
Traceback (most recent call last):
  File "/home/graingert/projects/examples/bar.py", line 17, in <module>
    trio.run(test)
  File "/home/graingert/.virtualenvs/testing39/lib/python3.9/site-packages/trio/_core/_run.py", line 1932, in run
    raise runner.main_task_outcome.error
  File "/home/graingert/projects/examples/bar.py", line 15, in test
    n.start_soon(f.__aenter__)
  File "/home/graingert/.virtualenvs/testing39/lib/python3.9/site-packages/trio/_core/_run.py", line 815, in __aexit__
    raise combined_error_from_nursery
  File "/usr/lib/python3.9/contextlib.py", line 179, in __aenter__
    del self.args, self.kwds, self.func
AttributeError: args

@graingert
Copy link
Mannequin

graingert mannequin commented Dec 6, 2021

ah I can repeat this on python3.8.10 trio but not python3.9.9 trio:

import logging
import trio
from contextlib import asynccontextmanager

@asynccontextmanager
async def foo():
    await trio.sleep(1)
    yield


async def test():
    async with trio.open_nursery() as n:
        f = foo()
        n.start_soon(f.__aenter__)
        n.start_soon(f.__aenter__)

trio.run(test)

Traceback (most recent call last):
  File "bar.py", line 17, in <module>
    trio.run(test)
  File "/home/graingert/.virtualenvs/osirium-main/lib/python3.8/site-packages/trio/_core/_run.py", line 1932, in run
    raise runner.main_task_outcome.error
  File "bar.py", line 15, in test
    n.start_soon(f.__aenter__)
  File "/home/graingert/.virtualenvs/osirium-main/lib/python3.8/site-packages/trio/_core/_run.py", line 815, in __aexit__
    raise combined_error_from_nursery
  File "/usr/lib/python3.8/contextlib.py", line 171, in __aenter__
    return await self.gen.__anext__()
RuntimeError: anext(): asynchronous generator is already running

@graingert
Copy link
Mannequin

graingert mannequin commented Dec 6, 2021

I see the change here: 1c5c9c8#diff-e00601a380ba6c916ba4333277fe6ea43d2477804002ab1ae64480f80fec8e3aR177-R179

this is intentionally implementing https://bugs.python.org/issue30306 for asynccontextmanagers that was initially missing

@graingert
Copy link
Mannequin

graingert mannequin commented Dec 6, 2021

you can see the analogous sync contextmanager issue on python3.6 with:

import logging
from contextlib import contextmanager

@contextmanager
def foo():
    yield


def test():
    f = foo()
    f.__enter__()
    f.__enter__()

test()

on python3.7+ you get the bpo-30306 behaviour

Traceback (most recent call last):
  File "sync.py", line 14, in <module>
    test()
  File "sync.py", line 12, in test
    f.__enter__()
  File "/usr/lib/python3.8/contextlib.py", line 111, in __enter__
    del self.args, self.kwds, self.func
AttributeError: args

and python3.6 you get the same sort of error you see now for asynccontextmanagers:

Traceback (most recent call last):
  File "sync.py", line 14, in <module>
    test()
  File "sync.py", line 12, in test
    f.__enter__()
  File "/usr/lib/python3.6/contextlib.py", line 83, in __enter__
    raise RuntimeError("generator didn't yield") from None
RuntimeError: generator didn't yield

@dimaqq
Copy link
Mannequin Author

dimaqq mannequin commented Dec 20, 2021

Andrew, I see that you've closed this issue as "fixed".

I'm a little confused by that.

If you mean that 3.10 behaviour is better than 3.9, than perhaps "not a bug / rejected / wont fix" would make more sense.

Actually I don't agree with Thomas's logic... his argument feels like consistency for its own sake.

I agree with the intention of the change for the arguments to be released (https://bugs.python.org/issue30306) but I disagree with the cryptic error.

I'm sure we could have both argument release and readable exception, e.g.:

try:
del self.args, ...
except AttributeError:
raise RuntimeError("anext(): asynchronous generator is already running")

@graingert
Copy link
Mannequin

graingert mannequin commented Dec 20, 2021

Actually I don't agree with Thomas's logic... his argument feels like consistency for its own sake.

Do you expect sync and async contextmanagers to act differently?

Why would sync contextmanagers raise AttributeError and async contextmanagers raise a RuntimeError?

If it's sensible to guard against invalid re-entry for async contextmanagers then I think it's sensible to apply the same guard to sync contextmanagers.

@dimaqq
Copy link
Mannequin Author

dimaqq mannequin commented Dec 20, 2021

I'm fine with guarding both.

@asvetlov
Copy link
Contributor

Sorry, I closed it because async behavior reflects sync version now.

If you want to improve both -- you are welcome! Perhaps it is worth another issue with another problem description.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes 3.11 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant