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

Detailed TimeoutError message not captured when handling asyncio.TimeoutError in aiohttp #8133

Open
1 task done
a76yyyy opened this issue Feb 3, 2024 · 8 comments
Open
1 task done
Labels

Comments

@a76yyyy
Copy link

a76yyyy commented Feb 3, 2024

Describe the bug

When handling a timeout with aiohttp's asyncio.TimeoutError exception in an aiohttp request, the error message does not provide detailed information about the timeout. Despite attempting to print the exception within the except block, only The error msg is is displayed without any specifics of the timeout error.

To Reproduce

  1. Implement the following client code snippet:
import asyncio
import aiohttp

async def api_req():
    try:
        async with aiohttp.ClientSession() as client:
            res = await client.get('https://httpbin.org/delay/10', timeout=.1)
    except asyncio.TimeoutError as e:
        print(f'The error msg is {e}')
        raise e
    except Exception as e:
        print(f'This error ({e}) should not happen.')
        raise e
    print(res)
    return

asyncio.run(api_req())
  1. Run the script using your Python interpreter.
  2. Observe that when the API request times out, it prints "The error msg is" without any further details.

Expected behavior

I expect that upon encountering an asyncio.TimeoutError, the error message would contain detailed information about the timeout, such as the reason for the timeout or potentially the elapsed time before the timeout occurred.

For example:

The error msg is: asyncio.TimeoutError: Timed out after 0.1 seconds during connection establishment.
The error msg is: asyncio.TimeoutError: Timed out after 0.1 seconds during response processing.

Logs/tracebacks

The output from running the above code will be:

The error msg is

The following log traceback was generated when running the script with asyncio.run(api_req(), debug=True):

The error msg is 
Traceback (most recent call last):
  File "[YOUR_FILE_PATH]\test.py", line 19, in <module>
    asyncio.run(api_req(), debug=True)
  File "...\Python3.11.6\Lib\asyncio\runners.py", line 190, in run
    return runner.run(main)
  File "...\Python3.11.6\Lib\asyncio\runners.py", line 118, in run
    return self._loop.run_until_complete(task)
  File "...\Python3.11.6\Lib\asyncio\base_events.py", line 653, in run_until_complete
    return future.result()
  File "[YOUR_FILE_PATH]\test.py", line 12, in api_req
    raise e
  File "[YOUR_FILE_PATH]\test.py", line 9, in api_req
    res = await client.get('https://httpbin.org/delay/10', timeout=.1)
  File "...\site-packages\aiohttp\client.py", line 504, in _request
    with timer:
  File "...\site-packages\aiohttp\helpers.py", line 735, in __exit__
    raise asyncio.TimeoutError from None
TimeoutError

This traceback shows that an asyncio.TimeoutError occurred during the execution of client.get() on line 9 in the api_req function. Despite catching the exception, no detailed message about the timeout was printed before raising it again. The traceback confirms that the operation timed out as expected but does not provide additional context for the timeout event.

Python Version

$ python --version
Python 3.11.6

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.9.3
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Requires: aiosignal, attrs, frozenlist, multidict, yarl
Required-by:

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.5
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Requires: 
Required-by: aiohttp, yarl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.9.4
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Requires: idna, multidict
Required-by: aiohttp

OS

All

Related component

Client

Additional context

The environment leading to this issue includes a vanilla setup of Python and aiohttp used for making asynchronous HTTP requests. There are no proxy servers or other middleware involved that could affect the outcome. The expected behavior is to have access to more descriptive information about the TimeoutError in order to better understand the cause and troubleshoot timeouts in the application.

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@a76yyyy a76yyyy added the bug label Feb 3, 2024
@webknjaz
Copy link
Member

webknjaz commented Feb 3, 2024

Perhaps #8089 will help a bit.

@a76yyyy
Copy link
Author

a76yyyy commented Feb 3, 2024

Perhaps #8089 will help a bit.

I haven't read PR content carefully, but it doesn't seem to have been modified here

raise asyncio.TimeoutError from None

@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Feb 3, 2024

The request is for an error message to be added to the exception. However, in many places we use asyncio.timeout(), which doesn't support any parameters. It's not commonplace to include messages in TimeoutError, and given that we might want to replace the other instances with asyncio.timeout() later, I'd suggest we don't commit to this feature.

@a76yyyy
Copy link
Author

a76yyyy commented Feb 4, 2024

The request is for an error message to be added to the exception. However, in many places we use asyncio.timeout(), which doesn't support any parameters. It's not commonplace to include messages in TimeoutError, and given that we might want to replace the other instances with asyncio.timeout() later, I'd suggest we don't commit to this feature.

Appreciate your response to this issue. However, I would like to emphasize the importance of providing error types and details in timeout exceptions, and offer a more accurate comparison based on how requests and httpx handle timeouts.

Current Scenario:

aiohttp Example: Currently, when a timeout occurs in aiohttp, it raises a generic asyncio.TimeoutError without specifying the type of timeout or providing detailed information.

async with aiohttp.ClientSession() as client:
    res = await client.get('https://postman-echo.com/delay/3', timeout=1)

Traceback (most recent call last):
  ...
TimeoutError: (No detailed message provided)

requests Example (Connection Timeout): The requests library does indeed provide a specific exception type ConnectTimeout along with detailed error information:

res = requests.get('https://postman-echo.com/delay/3', timeout=0.1)

Traceback (most recent call last):
  ...
requests.exceptions.ConnectTimeout: HTTPSConnectionPool(host='postman-echo.com', port=443): Max retries exceeded with url: /delay/3 (Caused by ConnectTimeoutError(<urllib3.connection.HTTPSConnection object at 0x107fe8d90>, 'Connection to postman-echo.com timed out. (connect timeout=0.1)'))

requests Example (Read Timeout): Similarly, for read timeouts, requests provides a distinct exception type ReadTimeout accompanied by a descriptive message:

res = requests.get('https://postman-echo.com/delay/3', timeout=1)

Traceback (most recent call last):
  ...
requests.exceptions.ConnectTimeout: HTTPSConnectionPool(host='postman-echo.com', port=443): Max retries exceeded with url: /delay/3 (Caused by ConnectTimeoutError(<urllib3.connection.HTTPSConnection object at 0x107fe8d90>, 'Connection to postman-echo.com timed out. (connect timeout=0.1)'))

httpx Example (Connection & Read Timeouts): While httpx offers separate exception types ConnectTimeout and ReadTimeout, it doesn't include as much detailed network context information directly as requests:

async with httpx.AsyncClient(verify=False) as client:
    res = await client.get('https://postman-echo.com/delay/3', timeout=.1)

Traceback (most recent call last):
  ...
httpx.ConnectTimeout

async with httpx.AsyncClient(verify=False) as client:
    res = await client.get('https://postman-echo.com/delay/3', timeout=1)

Traceback (most recent call last):
  ...
httpx.ReadTimeout

Considering aiohttp's future plans to utilize asyncio.timeout(), i suggest considering ways to refine aiohttp's timeout exceptions into different types of timeouts with corresponding context details. This would enable developers to quickly identify the root cause of problems, rather than just encountering a general TimeoutError. Such an improvement would maintain compatibility with asyncio while significantly enhancing users' ability to troubleshoot and resolve timeout-related issues efficiently.

Look forward to further discussing.

@Dreamsorcerer
Copy link
Member

However, I would like to emphasize the importance of providing error types and details in timeout exceptions

Then maybe also make a proposal to cpython? If they add a message argument or similar to asyncio.timeout(), then we can use that. Otherwise, we'd have to add a bunch of reraises in, which seems a little awkward to me:

try:
    with asyncio.timeout(...):
        ...
except TimeoutError:
    raise TimeoutError("...")

@a76yyyy
Copy link
Author

a76yyyy commented Feb 5, 2024

My understanding of this response is that you propose enhancing asyncio.Timeout by adding an exception_cls parameter and possibly **kwargs in its __init__() method. This would allow asyncio.timeout() to raise a custom exception such as exception_cls(**kwargs) instead of the default TimeoutError.

# Hypothetical usage after an update to asyncio.timeout()
class CustomTimeoutError(Exception):
    def __init__(self, message, **kwargs):
        self.url = url
        super().__init__(message)

try:
    with asyncio.timeout(5.0, exception_cls=CustomTimeoutError, url="https://example.com"):
        # Perform asynchronous operations
        ...
except CustomTimeoutError as e:
    print(f"Timed out while accessing {e.url}: {e}")

I wholeheartedly agree that implementing such a feature would be beneficial. It would significantly improve developers' ability to understand and troubleshoot timeout issues by providing contextually rich error messages.

Meanwhile, I acknowledge your concern about having to introduce awkward re-raising logic within aiohttp if this feature is not available. However, even without the direct support from asyncio, aiohttp could still internally handle and convert these exceptions by catching asyncio.TimeoutError instances and re-raising them as aiohttp-specific subclasses enriched with specific error causes and details.

@Dreamsorcerer
Copy link
Member

My understanding of this response is that you propose enhancing asyncio.Timeout by adding an exception_cls parameter and possibly **kwargs in its __init__() method.

That's probably more flexible that my initial thought, which was just to have a msg parameter that's passed to TimeoutError.

@webknjaz
Copy link
Member

I think, we should consider implementing the suggested context augmentation where possible, if the contributor is willing to present a PoC PR. I'm sure it's useful.

That https://github.com/aio-libs/aiohttp/blob/cdd5c6802e669c93f0077448430349b6dadd8580/aiohttp/helpers.py#L720C45-L720C49 place is an example where we force a from None, while the context is readily available in the local variable exc_val.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants