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

Jump to bottom

Incorrect import of TimeoutError while creating happy eyeballs connection #83310

Closed
tirkarthi opened this issue Dec 24, 2019 · 6 comments
Closed

Incorrect import of TimeoutError while creating happy eyeballs connection #83310

tirkarthi opened this issue Dec 24, 2019 · 6 comments
Labels
3.8 only security fixes 3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@tirkarthi
Copy link
Member

BPO 39129
Nosy @asvetlov, @1st1, @tirkarthi
PRs
  • bpo-39129: Fix import path for asyncio.TimeoutError #17691
  • [3.8] bpo-39129: Fix import path for asyncio.TimeoutError (GH-17691) #17693
    		•

    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 = None
created_at = <Date 2019-12-24.07:04:12.998>
labels = ['type-bug', '3.8', '3.9', 'expert-asyncio']
title = 'Incorrect import of TimeoutError while creating happy eyeballs connection'
updated_at = <Date 2020-01-08.19:00:16.275>
user = 'https://github.com/tirkarthi'

    bugs.python.org fields:

    activity = <Date 2020-01-08.19:00:16.275>
actor = 'xtreak'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2019-12-24.07:04:12.998>
creator = 'xtreak'
dependencies = []
files = []
hgrepos = []
issue_num = 39129
keywords = ['patch']
message_count = 5.0
messages = ['358841', '358845', '358847', '358848', '359625']
nosy_count = 3.0
nosy_names = ['asvetlov', 'yselivanov', 'xtreak']
pr_nums = ['17691', '17693']
priority = 'normal'
resolution = None
stage = 'test needed'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39129'
versions = ['Python 3.8', 'Python 3.9']
    @tirkarthi
    Copy link
    Member Author

    I guess the TimeoutError exception needs to be imported from asyncio.exceptions and not from asyncio.futures that causes AttributeError while instantiating a connection with happy eyeballs.

    ./python.exe -m asyncio
asyncio REPL 3.9.0a2+ (heads/master:068768faf6, Dec 23 2019, 18:35:26)
[Clang 10.0.1 (clang-1001.0.46.4)] on darwin
Use "await" directly instead of "asyncio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> conn = await asyncio.open_connection("localhost", port=8000, happy_eyeballs_delay=1)
Traceback (most recent call last):
  File "/Users/kasingar/stuff/python/cpython/Lib/concurrent/futures/_base.py", line 439, in result
    return self.__get_result()
  File "/Users/kasingar/stuff/python/cpython/Lib/concurrent/futures/_base.py", line 388, in __get_result
    raise self._exception
  File "<console>", line 1, in <module>
  File "/Users/kasingar/stuff/python/cpython/Lib/asyncio/streams.py", line 52, in open_connection
    transport, _ = await loop.create_connection(
  File "/Users/kasingar/stuff/python/cpython/Lib/asyncio/base_events.py", line 1041, in create_connection
    sock, _, _ = await staggered.staggered_race(
  File "/Users/kasingar/stuff/python/cpython/Lib/asyncio/staggered.py", line 144, in staggered_race
    raise d.exception()
  File "/Users/kasingar/stuff/python/cpython/Lib/asyncio/staggered.py", line 86, in run_one_coro
    with contextlib.suppress(futures.TimeoutError):
AttributeError: module 'asyncio.futures' has no attribute 'TimeoutError'

    @tirkarthi tirkarthi added 3.8 only security fixes 3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error labels Dec 24, 2019
    @asvetlov
    Copy link
    Contributor

    You are right.

    Hmm, I thought that Happy Eyeballs is better tested.

    @tirkarthi
    Copy link
    Member Author

    Is there a way this can be tested? I tried simulating OSError for IPv4 address and returning socket for ipv6 one like it resolves correctly. I guess that's the underlying idea of happy eyeballs but a test can be added for basic workflow since it lacks tests.

    @patch_socket
def test_create_connection_happy_eyeballs_exception(self, sock_mock):
    async def getaddrinfo(*args, **kw):
        return [(2, 1, 6, '', ('127.0.0.1', 8000)),
                (2, 1, 6, '', ('::1', 8000, 0, 0)),]

    def getaddrinfo_task(*args, **kwds):
        return self.loop.create_task(getaddrinfo(*args, **kwds))

    self.loop.getaddrinfo = getaddrinfo_task
    with mock.patch.object(self.loop, 'sock_connect',
                           side_effect=[OSError, sock_mock]):
        coro = self.loop.create_connection(MyProto, host='localhost', port=8000,
                                           happy_eyeballs_delay=1)
        transport, _ = self.loop.run_until_complete(coro)
        transport.close()

    @asvetlov
    Copy link
    Contributor

    The main problem of mock-based tests in asyncio is that the tests become unreliably very easy after mocked asyncio internal changes.

    I'm +0 on adding the proposed test. The better idea is serving on explicit AF_INET '127.0.0.1:<port>' address when the connection is established to AF_INET6 '[::1]:<port>'.

    @tirkarthi
    Copy link
    Member Author

    Sorry, I never got around writing a proper test for this so I am moving it to test needed if someone wants to volunteer for it. trio has some test cases for their happy eyeball implementation if it helps : https://github.com/python-trio/trio/blob/c5497c5ac4f6c457e3390c69cb8b5b62eca41979/trio/tests/test_highlevel_open_tcp_stream.py#L326

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @kumaraditya303
    Copy link
    Contributor

    Closing in favor of #98388

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.8 only security fixes 3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    asyncio
    Status: Done
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @asvetlov @tirkarthi @kumaraditya303