If our pinger fails to receive a response, properly disconnect #13
Conversation
| @@ -624,6 +624,7 @@ class ClientImplementation { | |||
|
|
|||
| case MESSAGE_TYPE.PINGRESP: | |||
| // We don't care whether the server is still there (yet) | |||
| this.sendPinger && this.sendPinger.reset(); | |||
There was a problem hiding this comment.
We shouldn't reset the pinger on ping response, because we might not have sent anything recently. Imagine:
keepAlive = 60.
T=0 Client sends ping
T = 40 Client receives ping response (after a long time, but still within keepAlive). Client resets timer, so we'll send our next ping at T=100.
T = 90 Server disconnects client because it hasn't received anything in 60*1.5.
There was a problem hiding this comment.
hmm thats a scary length of time to have no response, but if that happens then I'll have to rethink this.
There was a problem hiding this comment.
It is a long time, but not something we can rule out. There's also this from the spec
It is the responsibility of the Client to ensure that the interval between Control Packets being sent does not exceed the Keep Alive value.
Even if the response comes back in 1 second, resetting the timer on receipt will mean the time between PINGREQs would be at least keepAlive+1 seconds, which violates the spec. We'd usually get away with it because of the spec's keepalive + 50% grace, but it's not right.
|
Any updates guys? |
fixes #12