-
Notifications
You must be signed in to change notification settings - Fork 1
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
adds timeout to socket event on client side #263
Conversation
potential problem: what if the network's just really slow and it takes more than 20 seconds to get a response from the socket? then everything would get out of sync... not sure how to solve this though... |
I haven’t quite got my head around what’s going on with the Goals app, as I’ve been on leave/not paying as much attention as I would like, but if it’s helpful to talk about the logic/behaviour side of things, please remember that I’m a resource for you to pull on
From: Emily Bertwistle [mailto:notifications@github.com]
Sent: 23 April 2017 21:13
To: CYPIAPT-LNDSE/goals-app
Cc: Subscribed
Subject: Re: [CYPIAPT-LNDSE/goals-app] adds timeout to socket event on client side (#263)
potential problem: what if the network's just really slow and it takes more than 20 seconds to get a response from the socket? then everything would get out of sync... not sure how to solve this though...
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#263 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AP5t--wBS4GsROl5EplHM14U7SNIEDIkks5ry7C8gaJpZM4NFhSX>.
---------------------------------------------------------------------------------------
This email has been scanned for email related threats and delivered safely by Mimecast.
For more information please visit http://www.mimecast.com
---------------------------------------------------------------------------------------
|
@MHemsley I think this timeout is okay, 20 seconds is a long time for a very small piece of data, so I think it is enough. Mostly, this timeout will only come into play if there is an error on the server or a serious connectivity problem. In this case, the client will retry until it succeeds.. |
@des-des Agreed, we'll soon find out if there are issues. Is there a limit to the retries? |
for reference: socketio/socket.io-client-java#309 |
@MHemsley I think for MVP this is robust enough. There are more complex ways of handling this sort of problem. For example if our number of users was in the tens of thousands, this kind of retry behaviour might overload a server... If we have a couple of user connected at any one time, even if both of them are retrying every 20s 24 hours a day, this is not problem (and should not happen anyway).. |
builds on #256
if the browser doesn't get a response from the socket within 20 seconds of emitting a 'goal' event, the update is cancelled and 'pendingSync' is set to false.
related #152