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

WebSocketResponse's timeout does not apply to send_bytes/send_json/send_str calls. #2310

Open
frederikaalund opened this issue Oct 10, 2017 · 2 comments
Milestone

Comments

@frederikaalund
Copy link
Contributor

Long story short

This is more of a question. Is this behaviour by design?

We have to timeout-related parameters on WebSocketResponse:

  • timeout
  • receive_timeout

The receive_timeout works for all receive operations. Consequently, I assumed that the timeout parameter worked for all send operations. Apparently, I was mistaken. The documentation is not so clear about this.

From the look of the implementation, timeout is actually only used in the close() call.

Expected/Actual behaviour

I expected that the timeout parameter applied to the send_bytes/send_json/send_str calls. It does not.

Discussion

Currently, I simply wrap my send_x calls in with Timeout(...). It's not that much effort but it would be nice to see this directly implemented in aiohttp.

I can see two options:

  • Let timeout parameter apply to the send_x calls
  • Add a send_timeout parameter to the WebSocketResponse that applied a timeout to the send_x calls

This also relates to #2309. In my actual implementation, the send_x calls are submitted to the event loop in a fire-and-forget fashion. Consequently, if the connection is abruptly cut, said send_x calls are left lingering and never cancelled. By using a timeout, said calls will time out and not fill up in memory.

Your environment

Aiohttp 2.2.3 and Python 3.6.1.

@asvetlov asvetlov added this to the 3.0 milestone Oct 19, 2017
@asvetlov
Copy link
Member

asvetlov commented Nov 1, 2017

Hmm. Perhaps we need receive_timeout, close_timeout and send_timeout.
At deprecation period timeout should be an alias for close_timeout.
After deprecation expiring and timeout removal (or just at time of aiohttp 4.0 release) we could resurrect timeout as default mutable exclusive value for all three params.
@frederikaalund if your would provide a Pull Request -- you are welcome!
I love to see it in aiohttp 3.0 -- we need a time for deprecation period.
Otherwise timeout cannot be reused with proper meaning for a long.

@frederikaalund
Copy link
Contributor Author

I'm on it.

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

No branches or pull requests

3 participants