aio-libs / aiohappyeyeballs

Happy Eyeballs for pre-resolved hosts
Other
8 stars 8 forks source link

start_connection raises an IndexError when socket creation fails #93

Open Moosapoor opened 1 week ago

Moosapoor commented 1 week ago

Describe the bug If creating the socket fails with an Exception other than an OSError, the start_connection method will raise an IndexError when it's trying to get the first exception.

To Reproduce

import aiohttp
import pytest
from unittest import mock

@pytest.mark.asyncio
async def test_session_will_raise_index_error():
    with mock.patch("socket.socket") as mock_socket:
        mock_socket.side_effect = Exception()
        session = aiohttp.ClientSession()
        await session.get("http://sample.com")
    await session.close()

Logs

.virtualenvs/api-R653k7wR-py3.11/lib/python3.11/site-packages/aiohttp/client.py:657: in _request
    conn = await self._connector.connect(
.virtualenvs/api-R653k7wR-py3.11/lib/python3.11/site-packages/aiohttp/connector.py:564: in connect
    proto = await self._create_connection(req, traces, timeout)
.virtualenvs/api-R653k7wR-py3.11/lib/python3.11/site-packages/aiohttp/connector.py:975: in _create_connection
    _, proto = await self._create_direct_connection(req, traces, timeout)
.virtualenvs/api-R653k7wR-py3.11/lib/python3.11/site-packages/aiohttp/connector.py:1319: in _create_direct_connection
    transp, proto = await self._wrap_create_connection(
.virtualenvs/api-R653k7wR-py3.11/lib/python3.11/site-packages/aiohttp/connector.py:1073: in _wrap_create_connection
    sock = await aiohappyeyeballs.start_connection(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

addr_infos = [(<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('5.22.145.16', 80)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('5.22.145.121', 80))]

    async def start_connection(
        addr_infos: Sequence[AddrInfoType],
        *,
        local_addr_infos: Optional[Sequence[AddrInfoType]] = None,
        happy_eyeballs_delay: Optional[float] = None,
        interleave: Optional[int] = None,
        loop: Optional[asyncio.AbstractEventLoop] = None,
    ) -> socket.socket:
        """
        Connect to a TCP server.

        Create a socket connection to a specified destination.  The
        destination is specified as a list of AddrInfoType tuples as
        returned from getaddrinfo().

        The arguments are, in order:

        * ``family``: the address family, e.g. ``socket.AF_INET`` or
            ``socket.AF_INET6``.
        * ``type``: the socket type, e.g. ``socket.SOCK_STREAM`` or
            ``socket.SOCK_DGRAM``.
        * ``proto``: the protocol, e.g. ``socket.IPPROTO_TCP`` or
            ``socket.IPPROTO_UDP``.
        * ``canonname``: the canonical name of the address, e.g.
            ``"www.python.org"``.
        * ``sockaddr``: the socket address

        This method is a coroutine which will try to establish the connection
        in the background. When successful, the coroutine returns a
        socket.

        The expected use case is to use this method in conjunction with
        loop.create_connection() to establish a connection to a server::

                socket = await start_connection(addr_infos)
                transport, protocol = await loop.create_connection(
                    MyProtocol, sock=socket, ...)
        """
        if not (current_loop := loop):
            current_loop = asyncio.get_running_loop()

        single_addr_info = len(addr_infos) == 1

        if happy_eyeballs_delay is not None and interleave is None:
            # If using happy eyeballs, default to interleave addresses by family
            interleave = 1

        if interleave and not single_addr_info:
            addr_infos = _interleave_addrinfos(addr_infos, interleave)

        sock: Optional[socket.socket] = None
        exceptions: List[List[OSError]] = []
        if happy_eyeballs_delay is None or single_addr_info:
            # not using happy eyeballs
            for addrinfo in addr_infos:
                try:
                    sock = await _connect_sock(
                        current_loop, exceptions, addrinfo, local_addr_infos
                    )
                    break
                except OSError:
                    continue
        else:  # using happy eyeballs
            sock, _, _ = await staggered.staggered_race(
                (
                    functools.partial(
                        _connect_sock, current_loop, exceptions, addrinfo, local_addr_infos
                    )
                    for addrinfo in addr_infos
                ),
                happy_eyeballs_delay,
                loop=current_loop,
            )

        if sock is None:
            all_exceptions = [exc for sub in exceptions for exc in sub]
            try:
>               first_exception = all_exceptions[0]
E               IndexError: list index out of range

.virtualenvs/api-R653k7wR-py3.11/lib/python3.11/site-packages/aiohappyeyeballs/impl.py:102: IndexError

Python version

$ python --version  
Python 3.11.3

aiohttp version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.10.5
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2

OS macOs

Additional context No additional context.

bdraco commented 1 week ago

This code comes from stdlib. Is it possible that creating a socket can raise something other than OSError?

Moosapoor commented 1 week ago

Is it possible that creating a socket can raise something other than OSError?

It happened in one of my projects and if I'm not wrong it was a RuntimeError.

bdraco commented 1 week ago

Can you post steps to recreate the issue without manually patching the exception, and post the exception trace you received in production?

Moosapoor commented 1 week ago

I don't know how to reproduce the issue in the real world tbh. It happened couple of times and I couldn't find any specific situation which could result the issue. This is the stack trace:

WouldBlock: null
  File "anyio/streams/memory.py", line 98, in receive
    return self.receive_nowait()
  File "anyio/streams/memory.py", line 93, in receive_nowait
    raise WouldBlock
EndOfStream: null
  File "starlette/middleware/base.py", line 159, in call_next
    message = await recv_stream.receive()
  File "anyio/streams/memory.py", line 118, in receive
    raise EndOfStream
IndexError: list index out of range
  ...
  File "starlette/middleware/base.py", line 165, in call_next
    raise app_exc
  File "starlette/middleware/base.py", line 151, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "starlette/middleware/exceptions.py", line 65, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "starlette/routing.py", line 756, in __call__
    await self.middleware_stack(scope, receive, send)
  File "starlette/routing.py", line 776, in app
    await route.handle(scope, receive, send)
  File "starlette/routing.py", line 297, in handle
    await self.app(scope, receive, send)
  File "starlette/routing.py", line 77, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "starlette/routing.py", line 72, in app
    response = await func(request)
  File "fastapi/routing.py", line 297, in app
    raw_response = await run_endpoint_function(
  File "fastapi/routing.py", line 210, in run_endpoint_function
    return await dependant.call(**values)
  ...
  File "contextlib.py", line 210, in __aenter__
    return await anext(self.gen)
  ...
  File "contextlib.py", line 210, in __aenter__
    return await anext(self.gen)
  ...
  File "tenacity/_asyncio.py", line 71, in __anext__
    do = self.iter(retry_state=self._retry_state)
  File "__init__.py", line 314, in iter
    return fut.result()
  File "concurrent/futures/_base.py", line 449, in result
    return self.__get_result()
  File "concurrent/futures/_base.py", line 401, in __get_result
    raise self._exception
  ...
  File "contextlib.py", line 650, in enter_async_context
    result = await _enter(cm)
  File "contextlib.py", line 210, in __aenter__
    return await anext(self.gen)
  ...
  File "aiohttp/client.py", line 1353, in __aenter__
    self._resp = await self._coro
  File "aiohttp/client.py", line 657, in _request
    conn = await self._connector.connect(
  File "aiohttp/connector.py", line 564, in connect
    proto = await self._create_connection(req, traces, timeout)
  File "aiohttp/connector.py", line 975, in _create_connection
    _, proto = await self._create_direct_connection(req, traces, timeout)
  File "aiohttp/connector.py", line 1319, in _create_direct_connection
    transp, proto = await self._wrap_create_connection(
  File "aiohttp/connector.py", line 1073, in _wrap_create_connection
    sock = await aiohappyeyeballs.start_connection(
  File "aiohappyeyeballs/impl.py", line 102, in start_connection
    first_exception = all_exceptions[0]
Dreamsorcerer commented 1 week ago

That traceback seems weirdly formatted, but I'm assuming the original exception comes from anyio, so maybe this is really a bug in anyio which should be raising some kind of OSError...?

gryevns commented 5 days ago

To give some more context, the exceptions for _ here are:

[
    RuntimeError('File descriptor 27 is used by transport <TCPTransport closed=False reading=True 0x2a4ce36edd00>'),
    RuntimeError('File descriptor 27 is used by transport <TCPTransport closed=False reading=True 0x2a4ce36edd00>')
]