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

Option to periodicaly ping idle websockets to keep proxies happy #1640

Closed
takluyver opened this issue Feb 12, 2016 · 3 comments · Fixed by #1957
Closed

Option to periodicaly ping idle websockets to keep proxies happy #1640

takluyver opened this issue Feb 12, 2016 · 3 comments · Fixed by #1957

Comments

@takluyver
Copy link
Contributor

We have a mixin class in Jupyter which primarily adds periodic pinging to a websocket handler. This is useful when websockets are used behind proxies, as some proxy servers will kill the connection if it's unused for a set amount of time - 60 seconds in nginx (see #1070).

This seems like a generally applicable bit of functionality for websockets. Are you interested in including it in tornado? I imagine that this would be disabled by default, but could be enabled by a setting websocket_ping_interval. I am happy to work on this if you think it's in scope for tornado.

@bdarnell
Copy link
Member

Yes, I think it's a good idea. It's a common need, and it's useful both for keeping proxies from shutting down the connection and for quicker detection of dropped connections.

From a quick look at the linked code, my one quibble with Jupyter's implementation is that I prefer to specify intervals in either floating-point seconds or timedelta objects instead of milliseconds (with some legacy exceptions notably including PeriodicCallback).

takluyver added a commit to takluyver/tornado that referenced this issue Feb 15, 2016
Closes tornadowebgh-1640

As Ben requested on tornadoweb#1640, I've changed it to work in seconds rather
than milliseconds.

I'm not sure how we'd test something like this. I don't think we have
tests for it in Jupyter.
@awong1900
Copy link

Yes, very useful. I have a question, can add method ping() on WebSocketClientConnection class?

@takluyver
Copy link
Contributor Author

The server side of the pinging is awaiting review in #1642. :-)

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

Successfully merging a pull request may close this issue.

3 participants