NotificationsYou must be signed in to change notification settings
Fork33.7k
Star70.4k

gh-111085 Fix logic order causing nicer error to never be raised in asyncio.Timeout#110910

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

Gobot1234 wants to merge4 commits intopython:mainfromGobot1234:patch-3

Closed

gh-111085 Fix logic order causing nicer error to never be raised in asyncio.Timeout#110910

Gobot1234 wants to merge4 commits intopython:mainfromGobot1234:patch-3

Conversation

Copy link

Contributor

Gobot1234 commentedOct 15, 2023•
edited
Loading

~~I don't think this needs a news entry or an issue~~

Issue:asyncio.Timeout raises AttributeError when not used in loop #111085

Fix logic order causing nicer error to never be raised

4071cd6

Gobot1234 requested review from1st1,asvetlov,gvanrossum,kumaraditya303 andwillingc ascode owners

October 15, 2023 21:33

bedevere-appbot added the awaiting review label

Oct 15, 2023

Gobot1234 changed the title~~Fix logic order causing nicer error to never be raised~~Fix logic order causing nicer error to never be raised in asyncio.Timeout

Oct 15, 2023

AlexWaygood added the topic-asyncio label

Oct 15, 2023

hauntsaninja approved these changes

Oct 18, 2023

View reviewed changes

Copy link

Contributor

hauntsaninja left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for fixing this! However, it does need a NEWS entry since exception type change is user visible behaviour

bedevere-appbot added awaiting merge and removed awaiting review labels

Oct 18, 2023

📜🤖 Added by blurb_it.

10317ea

Copy link

ContributorAuthor

Gobot1234 commentedOct 19, 2023

@hauntsaninja does this also need an issue then?

Copy link

Member

AlexWaygood commentedOct 19, 2023•
edited
Loading

@hauntsaninja does this also need an issue then?

Yeah, it's not possible to add a news entry without linking the news entry to a GitHub issue.

(Or, shouldn't be possible, anyway, since the news entry is meant to be linked to an issue :p )

Update and rename 2023-10-19-11-40-38.gh-issue-0.hBPpQr.rst to 2023-1…

35453e2

…0-19-11-40-38.gh-issue-111085.hBPpQr.rst

Gobot1234 changed the title~~Fix logic order causing nicer error to never be raised in asyncio.Timeout~~gh-111085 Fix logic order causing nicer error to never be raised in asyncio.Timeout

Oct 19, 2023

bedevere-appbot mentioned this pull request

Oct 19, 2023

asyncio.Timeout raises AttributeError when not used in loop#111085

Closed

Eclips4 approved these changes

Oct 19, 2023

View reviewed changes

Copy link

Member

Eclips4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM. Should it be backported to 3.12?

Copy link

ContributorAuthor

Gobot1234 commentedOct 19, 2023

Turns out the bug exists on 3.11 as well

Eclips4 added needs backport to 3.11

only security fixes

needs backport to 3.12only security fixes labels

Oct 19, 2023

Eclips4 reviewed

Oct 19, 2023

View reviewed changes

Misc/NEWS.d/next/Library/2023-10-19-11-40-38.gh-issue-111085.hBPpQr.rstShow resolvedHide resolved

Update ACKS

a2608eb

serhiy-storchaka self-requested a review

October 19, 2023 22:28

Copy link

Contributor

hauntsaninja commentedOct 20, 2023•
edited
Loading

I think this probably should not be backported. This effectively just changes a bad error message to a good one and has a chance of breaking code, so doesn't feel like good cost benefit.

Thanks again, I see Serhiy self requested review, so will wait for that before merging.

hauntsaninja removed needs backport to 3.11

only security fixes

needs backport to 3.12only security fixes labels

Oct 20, 2023

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request

Oct 20, 2023

pythongh-110910: Fix invalid state handling in TaskGroup and Timeout

62e5092

asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeErrorif they are improperly used.* When they are used without entering the context manager.* When they are used after finishing.* When the context manager is entered more than once (simultaneously or  sequentially).* If there is no current task when entering the context manager.They are now left in consistent state after raising an exception, sofollowing operations can be correctly performed (if they are allowed).

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request

Oct 20, 2023

pythongh-110910: Fix invalid state handling in TaskGroup and Timeout

7c9f4fd

asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeErrorif they are improperly used.* When they are used without entering the context manager.* When they are used after finishing.* When the context manager is entered more than once (simultaneously or  sequentially).* If there is no current task when entering the context manager.They are now left in consistent state after raising an exception, sofollowing operations can be correctly performed (if they are allowed).Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request

Oct 20, 2023

pythongh-110910: Fix invalid state handling in TaskGroup and Timeout

a9c546f

asyncio.TaskGroup and asyncio.Timeout classes now raise proper RuntimeErrorif they are improperly used.* When they are used without entering the context manager.* When they are used after finishing.* When the context manager is entered more than once (simultaneously or  sequentially).* If there is no current task when entering the context manager.They are now left in consistent state after raising an exception, sofollowing operations can be correctly performed (if they are allowed).Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>

Copy link

ContributorAuthor

Gobot1234 commentedOct 20, 2023

Closing in favour of#111111

Gobot1234 closed this

Oct 20, 2023

Copy link

Member

serhiy-storchaka commentedOct 20, 2023

While this change fixes an AttributeError in__enter__(), it left the object in inconsistent state, so other AttributeError can be raised in specific circumferences in other place. There are also other issues in the current code. It makes sense to fix them all at once.

In any case, thank you for your contribution. I included your changes in the other PR and added you as a co-author.