Handle the first /sync failure differently. #216
Conversation
A /sync request may spuriously fail on occasion, without the "connection" actually being lost. To avoid spurious "Connection Lost" warning messages we ignore the first /sync and immediately retry, and only if that fails do we enter an ERROR state.
| self._syncConnectionLost = true; | ||
| self._startKeepAlives().done(function() { | ||
| if (!self._syncConnectionLost) { | ||
| // This is the first failure, which may be spurious. To avoid unnecessary |
There was a problem hiding this comment.
"which may be spurious" - Can you give examples on this please as per RL discussion. Namely that things like restarting servers / load balancers / etc will knife any ongoing connection, and we want to effectively ignore that case.
After much musing, I guess it's probably fine. AFAIK clients have no real reason to know about this state, so you should not emit an event to say that you might have lost the connection. If it turns out we need it in the future we can add it then. Knee jerk reaction was no because it decouples when the LGTM |
|
LGTM! |
|
I'm backing this out because this will cause the client to sit around for another 1 minute and 50 seconds waiting for a connection to time out, without displaying an error message if the sync it triggers goes into the void. I suspect this is causing element-hq/element-web#2425 |
A
/syncrequest may spuriously fail on occasion, without the "connection" actually being lost. To avoid spurious "Connection Lost" warning messages we ignore the first/syncand immediately retry, and only if that fails do we enter an ERROR state.Is this sensible? Should we emit an event to indicate we may have lost a connection?