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

error message when __aexit__ is not async #74108

Closed
TadhgMcDonald-Jensen mannequin opened this issue Mar 27, 2017 · 19 comments
Closed

error message when __aexit__ is not async #74108

TadhgMcDonald-Jensen mannequin opened this issue Mar 27, 2017 · 19 comments
Assignees
Labels
3.7 only security fixes 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@TadhgMcDonald-Jensen
Copy link
Mannequin

TadhgMcDonald-Jensen mannequin commented Mar 27, 2017

BPO 29922
Nosy @ned-deily, @bitdancer, @asvetlov, @markshannon, @serhiy-storchaka, @1st1, @ilevkivskyi, @RazerM, @miss-islington
PRs
  • bpo-29922: Improve error messages in 'async with' #6352
  • [3.7] bpo-29922: Improve error messages in 'async with' (GH-6352) #6353
  • [3.6] bpo-29922: Improve error messages in 'async with' (GH-6352) #6354
  • bpo-29922: Add more tests for error messages in 'async with'. #6370
  • [3.7] bpo-29922: Add more tests for error messages in 'async with'. (GH-6370) #6371
  • [3.6] bpo-29922: Add more tests for error messages in 'async with'. (GH-6370) #6373
  • 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 = 'https://github.com/1st1'
    closed_at = <Date 2018-04-04.17:39:22.753>
    created_at = <Date 2017-03-27.17:24:22.827>
    labels = ['interpreter-core', '3.8', 'type-bug', '3.7']
    title = 'error message when __aexit__ is not async'
    updated_at = <Date 2018-04-04.17:39:22.752>
    user = 'https://bugs.python.org/TadhgMcDonald-Jensen'

    bugs.python.org fields:

    activity = <Date 2018-04-04.17:39:22.752>
    actor = 'serhiy.storchaka'
    assignee = 'yselivanov'
    closed = True
    closed_date = <Date 2018-04-04.17:39:22.753>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2017-03-27.17:24:22.827>
    creator = 'Tadhg McDonald-Jensen'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29922
    keywords = ['patch']
    message_count = 19.0
    messages = ['290630', '290631', '290633', '290639', '290640', '308797', '314827', '314831', '314832', '314834', '314836', '314838', '314839', '314932', '314933', '314939', '314940', '314941', '314944']
    nosy_count = 11.0
    nosy_names = ['ned.deily', 'r.david.murray', 'asvetlov', 'Mark.Shannon', 'serhiy.storchaka', 'yselivanov', 'Chewy', 'levkivskyi', 'RazerM', 'Tadhg McDonald-Jensen', 'miss-islington']
    pr_nums = ['6352', '6353', '6354', '6370', '6371', '6373']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29922'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @TadhgMcDonald-Jensen
    Copy link
    Mannequin Author

    TadhgMcDonald-Jensen mannequin commented Mar 27, 2017

    When creating a asynchronous context manager if the __aexit__ method is not labeled as async (so it returns None instead of a coroutine) the error has a generic error message:

    TypeError: object NoneType can't be used in 'await' expression

    Would it be possible to change this so it indicates that it was the context that was invalid not an await statement? Since the traceback points to the last statement of the with block it can create very confusing errors if the last statement was an await.

    Example:

    import asyncio
    class Test():
        async def __aenter__(self):
            print("aenter used")
            value = asyncio.Future()
            value.set_result(True)
            return value
        #FORGOT TO MARK AS async !!
        def __aexit__(self, *errors):
            print("aexit used")
            return None
    
    async def my_test():
        async with Test() as x:
            print("inside async with, now awaiting on", x)
            await x

    my_test().send(None)

    Give the output:

    aenter used
    inside async with, now awaiting on <Future finished result=True>
    aexit used
    Traceback (most recent call last):
      File ".../test.py", line 19, in <module>
        my_test().send(None)
      File ".../test.py", line 16, in my_test
        await x
    TypeError: object NoneType can't be used in 'await' expression

    Which indicates to me that x was None when it was await-ed for.

    @TadhgMcDonald-Jensen TadhgMcDonald-Jensen mannequin added topic-asyncio 3.7 only security fixes type-bug An unexpected behavior, bug, or error labels Mar 27, 2017
    @bitdancer
    Copy link
    Member

    This is a specific example of the general problem reported in bpo-25538.

    @1st1
    Copy link
    Member

    1st1 commented Mar 27, 2017

    This is a specific example of the general problem reported in bpo-25538.

    It's a bit different code path/problem. But I agree, we should work on making both with and async with a bit more usable.

    Would it be possible to change this so it indicates that it was the context that was invalid not an await statement?

    I'm not sure if we can do anything about this. We compile 'async with' into a set of opcodes. The first ones resolve __aexit__, the latter ones await on it. The one that prepares to await on the aexit (GET_AWAITABLE) is the same that 'await' expression compiles to, and that opcode has no idea what exactly it awaits on.

    We could probably add GET_AEXIT_AWAITABLE opcode specifically to improve the error message (other than that it would be a copy of GET_AWAITABLE). I'll think about it.

    @1st1 1st1 self-assigned this Mar 27, 2017
    @TadhgMcDonald-Jensen
    Copy link
    Mannequin Author

    TadhgMcDonald-Jensen mannequin commented Mar 27, 2017

    This is a specific example of the general problem reported in bpo-25538.

    Definitely related, although the part of giving an error "... can't be used in 'await' expression" when no await expression is used isn't covered by that thread so I'm not sure I'd call this a direct duplicate.

    Currently when __anext__ return a non-awaitable this error is shown:

    TypeError: 'async for' received an invalid object from __anext__: NoneType

    So having __aenter__ and __aexit__ have similar error messages would be nice.

    Would it maybe make sense to implement this as adding an argument to GET_AWAITABLE to indicate whether it was called from await, __anext__, __aenter__ or __aexit__? (or others that may exist in future) I don't know enough about bytecode to know how it'd compare to an instruction for each case.

    @1st1
    Copy link
    Member

    1st1 commented Mar 27, 2017

    Would it maybe make sense to implement this as adding an argument to GET_AWAITABLE to indicate whether it was called from await, __anext__, __aenter__ or __aexit__?

    Yes, but it will make it a tad slower (which will also affect await performance). I'll run some benchmarks.

    @1st1 1st1 reopened this Mar 27, 2017
    @asvetlov
    Copy link
    Contributor

    It's Interpreter Core problem, not specific to asyncio bug.

    @asvetlov asvetlov added interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed topic-asyncio labels Dec 20, 2017
    @1st1 1st1 added 3.8 only security fixes and removed 3.7 only security fixes labels Dec 20, 2017
    @serhiy-storchaka
    Copy link
    Member

    This issue looks unrelated to bpo-25538.

    PR 6352 improves error messages without changing bytecode. It is so simple that could be backported to 3.7 if Ned approves this.

    But it conflicts with bpo-32949. This way will not work after merging any of current bpo-32949 PRs. We will have to find other way for bpo-32949.

    @ned-deily
    Copy link
    Member

    PR 6352 smells like a bug fix to me and I think it should be OK for 3.7.0b4.

    @ned-deily ned-deily added the 3.7 only security fixes label Apr 2, 2018
    @1st1
    Copy link
    Member

    1st1 commented Apr 2, 2018

    PR 6352 smells like a bug fix to me and I think it should be OK for 3.7.0b4.

    Can we backport it to 3.6 too?

    @ned-deily
    Copy link
    Member

    Can we backport it to 3.6 too?

    I suppose so. (Though you'll owe me if you break anything.)

    @serhiy-storchaka
    Copy link
    Member

    New changeset a68f2f0 by Serhiy Storchaka in branch 'master':
    bpo-29922: Improve error messages in 'async with' (GH-6352)
    a68f2f0

    @serhiy-storchaka
    Copy link
    Member

    It would be hard to backport this to 3.5. Wordcode allows scanning back.

    @1st1
    Copy link
    Member

    1st1 commented Apr 2, 2018

    3.5 is in security fixes only, so don't bother.

    @miss-islington
    Copy link
    Contributor

    New changeset fcd4e03 by Miss Islington (bot) in branch '3.7':
    bpo-29922: Improve error messages in 'async with' (GH-6352)
    fcd4e03

    @miss-islington
    Copy link
    Contributor

    New changeset 4fd6c27 by Miss Islington (bot) in branch '3.6':
    bpo-29922: Improve error messages in 'async with' (GH-6352)
    4fd6c27

    @serhiy-storchaka
    Copy link
    Member

    New changeset 2eeac26 by Serhiy Storchaka in branch 'master':
    bpo-29922: Add more tests for error messages in 'async with'. (GH-6370)
    2eeac26

    @miss-islington
    Copy link
    Contributor

    New changeset 785f36c by Miss Islington (bot) in branch '3.7':
    bpo-29922: Add more tests for error messages in 'async with'. (GH-6370)
    785f36c

    @miss-islington
    Copy link
    Contributor

    New changeset 1487cd1 by Miss Islington (bot) in branch '3.6':
    bpo-29922: Add more tests for error messages in 'async with'. (GH-6370)
    1487cd1

    @serhiy-storchaka
    Copy link
    Member

    Since BEFORE_ASYNC_WITH always is followed by GET_AWAITABLE, in future they can be merged into a single instruction (like GET_ANEXT or GET_YIELD_FROM_ITER).

    @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.7 only security fixes 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants