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

asyncio.TimeoutError in default ClientSession #2

Closed
slavkoja opened this issue Mar 1, 2019 · 8 comments
Closed

asyncio.TimeoutError in default ClientSession #2

slavkoja opened this issue Mar 1, 2019 · 8 comments

Comments

@slavkoja
Copy link

slavkoja commented Mar 1, 2019

  • Python version: 3.7.2
  • Operating System: Linux
  • aiohttp version: 3.5.1

Description

The default aiohttp's ClientSession (created by EventSource constructor) has ont timeout set, then default ClientSession's timeout happens after 5 min of operation:

time python3 "tryasio.py"
MessageEvent(type='message', etc...)
...
Traceback (most recent call last):
  File "tryasio.py", line 20, in <module>
    asyncio.run(main())
  File "/usr/lib/python3.7/asyncio/runners.py", line 43, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.7/asyncio/base_events.py", line 584, in run_until_complete
    return future.result()
  File "tryasio.py", line 15, in main
    async for event in event_source:
  File "/home/censored/tmp/aiohttp-sse-client/aiohttp_sse_client/client.py", line 145, in __anext__
    async for line_in_bytes in self._response.content:
  File "/usr/lib/python3/dist-packages/aiohttp/streams.py", line 41, in __anext__
    rv = await self.read_func()
  File "/usr/lib/python3/dist-packages/aiohttp/streams.py", line 313, in readline
    await self._wait('readline')
  File "/usr/lib/python3/dist-packages/aiohttp/streams.py", line 281, in _wait
    await waiter
  File "/usr/lib/python3/dist-packages/aiohttp/helpers.py", line 567, in __exit__
    raise asyncio.TimeoutError from None
concurrent.futures._base.TimeoutError

real	5m0,595s
user	0m0,264s
sys	0m0,033s

I see, that i can post own ClientSession object to constructor, but IMO the timeout have to be disabled when no custom object is used. I solved this by adding ClientTimeout into aiohttp import and then prepare custom timeout with sock_read disabled:

timeout = ClientTimeout(sock_read=0)
self._session = ClientSession(timeout=timeout)
@lanfon72
Copy link

It seems you can just pass the timeout to EventSource such like

from aiohttp_sse_client.client import EventSource
evt = EventSource("the url", timeout=-1)

@awarecan
Copy link
Contributor

awarecan commented Oct 2, 2019

I think have a default timeout is better than none.

In your protocol's design, you should have a keepalive design to keep the socket open. Just like the recommendation in MDN, you can send a comment line to keep the socket alive.

If you still want to disable the timeout, @lanfon72 provides a good solution. Therefore, I am going to close this issue.

@awarecan awarecan closed this as completed Oct 2, 2019
@gimler
Copy link

gimler commented Oct 10, 2019

@lanfon72 this doesn't work for me python 3.6.3

this works for me

from aiohttp import ClientSession, ClientTimeout
...
timeout = ClientTimeout(sock_read=0)
session = ClientSession(timeout=timeout)
async with sse_client.EventSource(url, session=session) as event_source:

@tomwhoiscontrary
Copy link

To be clear, the default aiohttp timeout is being applied to the whole HTTP connection, not to individual reads from the connection, or to individual messages. So this suggestion:

In your protocol's design, you should have a keepalive design to keep the socket open. Just like the recommendation in MDN, you can send a comment line to keep the socket alive.

Doesn't work. It doesn't matter what keepalives you send, with the default timeout, the connection will be closed after five minutes.

Here is a simple but complete program, based on the example in the documentation, demonstrating this:

import asyncio
from aiohttp_sse_client import client as sse_client

async def stream_wikipedia():
    async with sse_client.EventSource('https://stream.wikimedia.org/v2/stream/recentchange') as event_source:
        try:
            async for event in event_source:
                print(event)
        except ConnectionError:
            pass

loop = asyncio.get_event_loop()
loop.run_until_complete(stream_wikipedia())

If you run this program, then after five minutes, it dies with a TimeoutError. This is not because people have stopped editing Wikipedia!

I don't think it makes sense to have a timeout like that in an SSE client. The whole point of SSE is to consume a potentially never-ending stream of events. And so, i think that this library should disable the read timeout by default.

@Lx
Copy link

Lx commented Jul 6, 2022

I wish I had found this issue earlier instead of needlessly debugging my server and my internet connection!

I think have a default timeout is better than none.

In your protocol's design, you should have a keepalive design to keep the socket open. Just like the recommendation in MDN, you can send a comment line to keep the socket alive.

If you still want to disable the timeout, @lanfon72 provides a good solution. Therefore, I am going to close this issue.

Although this issue is closed, for posterity, I would like to cast one more vote to have a default timeout of None because:

  • aiosseclient (one of the documented inspirations for this project) sets the default to None.
  • As @tomwhoiscontrary has already written above, the default 5-minute timeout is a hard limit, and as such, disregards any keep-alive activity anyway.
    • My server implementation sends a ping message every 15 seconds, yet aiohttp-sse-client still times out by default.

A 5-minute timeout (in total, disregarding all traffic and successful messages) simply doesn't make sense in the context of a long-running SSE stream. It makes sense for typical requests and responses, so it's a sensible aiohttp default—but as multiple people on this thread are suggesting, it's not a sensible aiohttp-sse-client one.

I think the aiohttp-sse-client maintainers maybe had different information about how the timeout works when they rejected the change earlier, and hopefully, with this new information, might reconsider now.

If the aiohttp-sse-client maintainers still aren't convinced that a default timeout of None isn't more sensible, perhaps placing a note similar to this in the README would help others avoid some lost debugging time:

Timeouts

Note that aiohttp requests have a default timeout of 5 minutes, inclusive of all traffic—that is, all aiohttp-sse-client requests will unconditionally time out after 5 minutes regardless of activity.

To prevent this behaviour, disable the timeout by passing a timeout argument to the underlying request call:

async with sse_client.EventSource(
    'https://stream.wikimedia.org/v2/stream/recentchange',
    timeout=None,
) as event_source:
    ...

@Lx
Copy link

Lx commented Jul 6, 2022

@slavkoja, as the issue author, do you care to re-open it in case the maintainers are not monitoring comments on closed issues?

@slavkoja
Copy link
Author

slavkoja commented Jul 6, 2022

@Lx be free to open new one, as i didn't close it, i cannot reopen it...

@rwayan
Copy link

rwayan commented Jul 17, 2024

@lanfon72 this doesn't work for me python 3.6.3

this works for me

from aiohttp import ClientSession, ClientTimeout
...
timeout = ClientTimeout(sock_read=0)
session = ClientSession(timeout=timeout)
async with sse_client.EventSource(url, session=session) as event_source:

session.close() need at the end

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

No branches or pull requests

7 participants