aiohttp and implementing a timeout

[jef41]

in the __init__.py for the aiohttp package there is a section that reads;

    # TODO: Implement timeouts

    async def _request(self, method, url, data=None, json=None, ssl=None, params=None, headers={}):
        redir_cnt = 0
            while redir_cnt < 2:
                reader = await self.request_raw(method, url, data, json, ssl, params, headers)
...

I am interested in implementing a timeout, partly curiosity, partly it could be useful for a current piece of work - though I would have to then build code to try and figure out why we timed out. Anyway... I have made a few small changes that seem to work for a timeout, for my use. I would be grateful of any insight as to whether this is a suitable implementation for incorporating into aiohttp, or perhaps I am not anticipating some knock-on effect?

in __init__.py change:

class ClientSession:
    def __init__(self, base_url="", headers={}, version=HttpVersion10, timeout=None):
...

to:

class ClientSession:
    def __init__(self, base_url="", headers={}, version=HttpVersion10, timeout=None):
        self._timeout = timeout
...

and below that change the _request method from that shown above to:

    async def _request(self, method, url, data=None, json=None, ssl=None, params=None, headers={}):
        redir_cnt = 0
        while redir_cnt < 2:
            try:
                if self._timeout:
                    reader = await asyncio.wait_for(
                        self.request_raw(method, url, data, json, ssl, params, headers),
                        self._timeout,
                    )
                else:
                    reader = await self.request_raw(method, url, data, json, ssl, params, headers)
            except asyncio.TimeoutError:
                raise
...

I can then call the session with a timeout, as per the examples;

async with aiohttp.ClientSession("http://httpbin.org", timeout=7) as session:

If I wrap that session in a try block I can then trap an except asyncio.TimeoutError . I tested this with both small and reasonable timeout values of 0.1 and 5 seconds and it does seem to work.
This change does mean that if there are redirects in place the timeout will be applied once per redirect, so up to 3 times the timeout.

[glenn20]

Your approach looks good and - You also seem to have tested it well (although, it would be interesting to see what it does when timeout is 0 or <0).

A few suggested notes of improvement:

• You should test for timeout is not None because timeout=0 has a different meaning to timeout=None,
• You don't need to trap the TimeoutError, as all you do is raise it again.
• asyncio.wait_for() just returns the awaitable if timeout is None, so you don't need the bare call to self.request_raw() if timeout is None.
• You can handle timeouts when there are redirects by tracking the remaining time.

Eg. (warning - untested code):

    async def _request(self, method, url, data=None, json=None, ssl=None, params=None, headers={}):
        redir_cnt = 0
        if self._timeout is not None:
            stop_time = time.time() + self._timeout
        while redir_cnt < 2:
            reader = await asyncio.wait_for(
                self.request_raw(method, url, data, json, ssl, params, headers),
                None if self._timeout is None else (stop_time - time.time())
            )
            ...

The first dot point should fix a potential bug. The second two are just suggested improvements, but your code works fine without them. The last one should ensure what I believe is expected behaviour when redirects are involved (but I am not 100% sure what the expected behaviour is here).

[jef41]

Blimey, that is such a helpful response, thank you. Your comments are useful in both improving the functionality and my code writing more generally.

With the addition of import time at the top of __init__.py your changes work just fine.

Testing with timeout=0 or a negative value the wait_for seems to return quickly (for a web response). In my test code bothtimeout= 0 and negative returned in 35ms, whatever the negative value was. That is I think, the same or similar behaviour to CPython where, I think, 0 assigns top priority and a negative value would be next in priority as a task running late. I have a feeling that MicroPython asyncio does not manage priority though I cannot immediately find a reference to that.

In trying to understand what should happen with redirects I took a look at Timeouts . From that I understand that we have implemented only a socket read timeout. A more fully featured approach would be to mimic that ClientTimeout data structure:

aiohttp.ClientTimeout(total=5*60, connect=None, sock_connect=None, sock_read=None, ceil_threshold=5)

then try to use each value in a similar wait_for on the appropriate methods, which requires a bit more work on my part.

A caveat that occurred to me is that wait_for uses integer seconds, so using wait_for_ms and ticks_ms would improve compatibility. When I implement that however, things get more complicated. In the following examples I am receiving a timeout for all responses, but the time taken to get a timeout is not remaining consistent in relation to the value set.

timeouts of 0, -1, -2 secs or ms all return in about 35ms
timeouts of less than 35 ms all take 35 ms
timeouts of 50, 70, 90 ms return in 52, 73, 92 ms
timeouts of 100, 200, 300, 400 ms return in 103, 204, 303, 404 ms, much the same as the line above
timeouts of 500, 600, 700, 800, 900ms return after 928, 957, 930, 914, 924 ms, odd
timeouts of 1000, 1100, 1200 etc return to more expected values 1001, 1103, 1204

So if the timeout is in the upper half of a second the timeout is not triggered as precisely as if the timeout is an integer or in the lower half of a second. Maybe under the hood there is some rounding going on, maybe that relates to asyncio or clock speed (I am using a Pico 2 W board).
These things I hope to look at, but it will take a while, hence some notes here in case it is useful to anyone else in the mean time.

calling with millisecond timeout values this now looks like the snippet below, but sticking to integer seconds for a timeout might be simpler:

import time
...
        if self._timeout is not None:
            stop_time = time.ticks_ms() + self._timeout # int(1000*self._timeout)
        while redir_cnt < 2:
            reader = await asyncio.wait_for_ms(
                self.request_raw(method, url, data, json, ssl, params, headers),
                None if self._timeout is None else (time.ticks_diff(stop_time, time.ticks_ms()))
            )
...

[glenn20]

Blimey, that is such a helpful response, thank you. Your comments are useful in both improving the functionality and my code writing more generally.

You are very welcome. It's good to see people wanting to help improve micropython and its libraries.

Testing with timeout=0 or a negative value the wait_for seems to return quickly (for a web response). In my test code bothtimeout= 0 and negative returned in 35ms, whatever the negative value was. That is I think, the same or similar behaviour to CPython where, I think, 0 assigns top priority and a negative value would be next in priority as a task running late. I have a feeling that MicroPython asyncio does not manage priority though I cannot immediately find a reference to that.

This is not a priority, just a timeout. I believe the behaviour is that if timeout <= 0, it tries the call (request_raw()), and then immediately cancels it because the timeout is reached.

In trying to understand what should happen with redirects I took a look at Timeouts . From that I understand that we have implemented only a socket read timeout. A more fully featured approach would be to mimic that ClientTimeout data structure:

Ah, that does get more complicated. I didn't check out the api docs for aiohttp. CPython aiohttp is optimised for being very efficient with a large number of connections and tasks. I don't believe it is necessary for micropython to implement all of that (we aren't going to be trying to manage 100s of simultaneous connections on a micropython device), so I think it would be helpful to just implement the total form of the timeout.

A caveat that occurred to me is that wait_for uses integer seconds, so using wait_for_ms and ticks_ms would improve compatibility.

wait_for() can take integer or float seconds (or None) - in both CPython and MicroPython, so you don't need to bother with converting to and from milliseconds :-).

[jef41]

Thanks again, for the context and further help. I was converting to milliseconds because adding a float to time.time() does not seem to work, I think we get in to rounding errors print(f"{time.time()} {time.time()+0.2:f}")

So sticking with converting timeout to ms... I created a minimal test that assumes an internet connection, but is looking for a non-routable address, so should always timeout. This should report the response time with the timeout increasing from 0 to 2.9 seconds in 0.1seconds steps:

import time
import asyncio
import aiohttp

async def test_timeout(url, timeout):
    async with aiohttp.ClientSession(url, timeout=timeout) as session:
        async with session.get("/get") as resp:
            assert resp.status == 200
            rget = await resp.text()
            print(f"GET: {rget}")

for t in range(0, 30, 1):
    try:
        t1=time.ticks_ms()
        asyncio.run(test_timeout(url="http://10.255.255.1", timeout=t/10))
        print(f"reply received:{time.ticks_diff(time.ticks_ms(),t1)}")
    except asyncio.TimeoutError:
        print(f"timeout after:{time.ticks_diff(time.ticks_ms(),t1)}")

When running this, after typically 4 iterations I get an out of memory error. I guess the requests are left open and consuming memory - though that seems like a lot of memory. Tracking this down, if I catch an asyncio.CancelledError in __init__.request_raw around the line
reader, writer = await asyncio.open_connection(host, port, ssl=ssl)
the set self._request = None and use garbage collection then this does seem to work without running out of memory.

this approach, of wiping _request and using gc.collect() does seem somewhat hacky, but I cannot find a more elegant way of closing the failed/failing requests and freeing memory. I found it surprisingly tricky to pin down the correct place to catch the CancelledError, I suppose in hindsight I needed to find the awaitable that was actually the cause of the timeout - which is request_raw.

In case anyone else is interested or can help me improve on this the modfified aiohttp.__init__ is at:
https://gist.github.com/jef41/6a6b3d76de74af2b01a3c403285763bb
I am not sure this is worth even an attempt at a PR into micropython-lib while it requires importing gc

[glenn20]

Thanks again, for the context and further help. I was converting to milliseconds because adding a float to time.time() does not seem to work, I think we get in to rounding errors print(f"{time.time()} {time.time()+0.2:f}")

Oops, you are right, of course. I forgot that micropython time.time() returns an int (it returns a float in CPython).