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

Improve lost connection detection #473

Closed
marci4 opened this issue May 3, 2017 · 16 comments
Closed

Improve lost connection detection #473

marci4 opened this issue May 3, 2017 · 16 comments

Comments

@marci4
Copy link
Collaborator

marci4 commented May 3, 2017

E.g. using this lib on a smartphone, you may lose the connection.

Related issues:
#471
#180

@marci4 marci4 self-assigned this May 3, 2017
@marci4 marci4 added this to the Release 1.3.4 milestone May 3, 2017
@marci4
Copy link
Collaborator Author

marci4 commented May 3, 2017

Looking for some input here!

I implemented a check with a timer.
It sends pings in a definied interval (default: 30s)
If there was an error sending this ping, the websocket gets closed
You can deactivate this check setting the interval below 1.

Check out the code here

Did I miss something?
Should I add anything?

Greetings
marci4

@zhoulifu
Copy link
Contributor

zhoulifu commented May 4, 2017

Hello @marci4
In my opinion, it's not enough to detecting the disconnection just sending ping.
For example, one client was shut down suddenly, without sending closeframe or closing the connection, the server could still send message to this disconnected connection without any errors due to the tcp retransmission. Drop all outgoing packages from client side using iptables can simulate this scenario:

iptables -A OUTPUT -p tcp --sport {CLIENT_PORT} -j DROP

So it's necessary to check whether the corresponding pong was received within a certain amount of time. But this would bring more pressure to server when there are plenty of clients.

@marci4
Copy link
Collaborator Author

marci4 commented May 5, 2017

Thx for your input @zhoulifu :)

This will add a bit more pressure but I think it is fine, especially since you have the possibility to turn it off.

Greetings
marci4

@marci4
Copy link
Collaborator Author

marci4 commented May 5, 2017

Ok I added a check for the pong response.

Default check interval is 30 seconds, is this to often?

Could you please check if for you this is working as well?

Greetings
marci4

@zhoulifu
Copy link
Contributor

zhoulifu commented May 8, 2017

Hey @marci4, i've tested it and it works 👍
30s is okay i think since pong arrived within hundreds of milliseconds in general.

@marci4
Copy link
Collaborator Author

marci4 commented May 8, 2017

Hello @zhoulifu,
thx for your feedback.
Gonna leave this issue open and not yet merch any changes.
Just to give other users time to give feedback as well!

Greetings
marci4

@marci4 marci4 mentioned this issue May 11, 2017
@marci4
Copy link
Collaborator Author

marci4 commented May 11, 2017

Merged changes into master branch.

Changed timeout check to 60 seconds.

Closing issue.
Greetings
marci4

@marci4 marci4 closed this as completed May 11, 2017
@marci4
Copy link
Collaborator Author

marci4 commented May 15, 2017

@qhchen2
Copy link

qhchen2 commented Jun 1, 2017

Could you create an example that uses this lost detection connection? Im trying to follow the wiki and My Server that extends WebSocketServer doesn't recognize the method myServer.setConnectionLostTimeout(15). The wiki is pretty short so I'm not exactly sure where I'm going wrong. Im using 1.3.3

@marci4
Copy link
Collaborator Author

marci4 commented Jun 1, 2017

Lost connection is not yet released as a jar. It will come with the version 1.3.4 (no release date planned sorry)

You can also use the master branch which already includes the lost connection detection!

Do you have any idea how I can extend the wiki?

Greetings
marci4

@qhchen2
Copy link

qhchen2 commented Jun 1, 2017

Thanks for the quick reply, maybe examples on what methods to call in the js client and what methods in the server and to put it all in an example. Even if 1.3.4 was released the wiki is unfortunately so short I would not have understood the example(The same with trying to do wss, had alot of problems trying to make that work. As for me, Im forced to use 1.3.3 for now, so Im trying to implement the ping/pong thing myself. What is not clear for me from the wiki what ping and pong are exactly, strings, objects etc? also some kind of interaction diagram would be handy. So right now im unsure what to send to the client from the server and what method to use. Im guessing I could use websocket.send(String) but maybe thats not ideal because of the overhead.

Greetings,
Q

@qhchen2
Copy link

qhchen2 commented Jun 2, 2017

Im using a javascript client and am wondering how to check for pings from a server in a regular interval there. Did you use JS also and have found a solution? JS does not appear to have multithreading

Greetings,
Q

@marci4
Copy link
Collaborator Author

marci4 commented Jun 2, 2017

Hello @qhchen2,

I am gonna extend the wiki a bit more then (Sorry, for me these things are pretty clear ;) )

Let me explain some stuff for you!
A ping is specified by https://tools.ietf.org/html/rfc6455#section-5.5.2
A pong is specified by https://tools.ietf.org/html/rfc6455#section-5.5.3
If an endpoint recieves a ping frame it must respond with a pong frame. That is the basic idea behind this feature!
If you are using a valid endpoint implementing the spec correctly (what you can assume from modern browsers), you don't have to do anything (most modern browser don't even show ping/pong frames).

Greetings
marci4

@qhchen2
Copy link

qhchen2 commented Jun 2, 2017

Hey marci, just a quick question on some code I found in AbstractWebSocket class in the master, namely

/**
* Attribute for the lost connection check interval
*/
private int connectionLostTimeout = 60;

/**
 * Get the interval checking for lost connections
 * Default is 60 seconds
 * @return the interval
 */
public int getConnectionLostTimeout() {
    return connectionLostTimeout;
}

Is this your code that is not part of 1.3.3, or is it code that has been part of the project for a long time? cause now it seems I can just use these builtin functions and variables to make the pingpong work.

Greetings,
Q

@marci4
Copy link
Collaborator Author

marci4 commented Jun 4, 2017

Hello @qhchen2,

the code is part of my connection lost detection!
The master branch is always a representation of the current development status!

Greetings
marci4

@NitzDKoder
Copy link

NitzDKoder commented Feb 27, 2019

@marci4 on Android

Client:

Request is done from android webview level web-socket as client.

but after webview is frozen it fails to send pong.

https://developers.google.com/web/updates/2018/07/page-lifecycle-api#developer-recommendations-for-each-state

Server:
we are getting reason: The connection was closed because the other endpoint did not respond with a pong in time. For more information

say if server is setConnectionLostTimeout(30secs), Server will auto detect and wait for message from client? Client has to retry to the same server port?

Thanks
Nithin

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

4 participants