Add a Finite State Machine to accurately tell online/offline status

[TinyExplosions]

Currently, https://github.com/feedhenry/fh-sync-js/blob/9112991c954084d24eb001705f52f35252ba9872/src/sync-client.js#L319-L339

is a fairly brute force way to check if we think the device/browser is online or not. Adding in a finite state machine (via something like MachinaJS) would enable us to more accurately tell if the device is on or offline, and could also be extended to give a range of statuses (so we could tell if the user is on 3g etc etc and make decisions based on that)

A good example of this can be found at https://github.com/ifandelse/machina.js/tree/master/example/connectivity -works fine on cordova too!

[wtrocki]

@TinyExplosions - I love idea to give users choice to sync some elements when only on Wifi etc. I think that we may have 2 different actions from this:

• Extract offline check to method that can be overriden by customers to provide more suitable behavior for them.
• Provide more accurate and bulletproof implementation for default method (see examples above)

Thanks for contributing to sync!

[wei-lee]

@TinyExplosions Yeah, there are definitely rooms to improve this method. But also I think we need to be careful not to over-engineering things here. My question would be does it really worth it to include another library for just checking the status of the connectivity? Bear in mind this method is only meant to be used internally by sync, it is not meant to be used by other things to check the connectivy. Also for sync itself, we haven't come across a use case where things need to be done differently depending on the type of the network. So it seems to a bit of overkill to keep all the different statuses of the network while sync only cares about if the device is online or not. Do you have any use cases for this?

But definitely a good way to move forward is to allow developers to override the default implementation. In the end, sync only needs to know if the device is online or not, there are many different ways to implement this.

[wtrocki]

@wei-lee - this ticket is just follow up from slack where we hit some problems with offline state provider in sync (basically it was telling developer that browser is offline).

We do not use the most efficient method in the code: https://developer.mozilla.org/en-US/docs/Web/API/NavigatorOnLine/onLine

Ref: https://github.com/feedhenry/fh-sync-js/blob/9112991c954084d24eb001705f52f35252ba9872/src/sync-client.js#L323-L325

So aproach may be to just migrate to listeners as mentioned in docs and also allow developers to override it to provide more sophisticated ways to determine offline status.

[wei-lee]

@wtrocki Yes, I agree it's a good idea to allow developer to provide override. But I am still a bit confused about why it's reporting browser is offline? It is using the same method as described in the doc that you linked?

[wtrocki]

Method we used is considered unreliable (quote from docs above)

You could be getting false positives, such as in cases where the computer is running a virtualization software that has virtual ethernet adapters that are always "connected."`

Moving to listeners can resolve problem. I saw similar issue couple times. It may happen when testing in browser.

[wei-lee]

@wtrocki right, gotcha now. But I also couldn't find if the event listener is the reliable way? Seems like different browser vendors have different implementations for it as well.

[TinyExplosions]

I think a lot of it depends on the long term view of the sync engine: what it is, what it should be and how it should work. To my mind it should be equally at home on browser or mobile device, and as such should work reliably on each. If that's listening to browser events or failed http calls etc, then grand -I guess the FSM route allows for all that and extensibility.

we haven't come across a use case where things need to be done differently depending on the type of the network

That's a fair point, but I don't think it's beyond the realm of possibility where you might want to send back small payloads regardless of connection, but only send images etc if you're not on a data network -again, just making it as useful as possible (but that might not be the overall plan for the engine).

I fully get not embedding external dependancies, and I'm not keen on it either, but maybe we need to be a little smarter about when we attempt to sync etc

[evanshortiss]

Why not just make a call to the sys/info/ping or some similar endpoint occasionally to determine this? The navigator.online only means the device is connected to a network AFAIK, so it's similar to the cordova connection plugin - it does not indicate the reliability of said network only that we're connected to something.

Agree, this depends on how much flexibility you need. Adding an entire library might be overkill if you don't need the features. I wrote a simple module that monitors connectivity using an event emitter that calls sys/info/ping and emits a "connectionState" boolean with the result and even that could be argued to be overkill at ~5KB.

[wei-lee]

@TinyExplosions @evanshortiss @wtrocki now I think about it again, given that sync is designed to work in offline mode, do we really need to care if the device has internet connection or not? What if we just let it fire the sync request anyway? I think when there is no internet connection, the request should just fail, right? Will it consume a bit more battery?

If this works, down the road, I don't think sync needs to care if the device has internet or not, but rather the type of network the device is having. As suggested by @TinyExplosions, we can then only fire the sync request in certain network types.

BTW, @wtrocki has put in a lot of effort to make the module open source, now it's a good time to contribute code as well as raising issues 😃

[wtrocki]

? I think when there is no internet connection, the request should just fail, right? Will it consume a bit more battery?

This will have implications on sync times. Once mobile device become online we should force sync (I think)

@wei-lee I will contribute to this issue.

[evanshortiss]

@wei-lee I think the issue was that the navigator.online got "stuck" so even when the device was online it wasn't working - I agree otherwise sync "just works" regardless of connectivity.

[wei-lee]

@evanshortiss yeah, that's my point. We don't need to use navigator.online at all. Just let sync try make the call and it may success or fail.

@wtrocki can you explain a bit more about implications on sync times? What do you mean by that? I thought without the online check, the sync will ALWAYS happen at the specified interval?

[TinyExplosions]

Maybe I am just over complicating things with this suggestion! Certainly, we can just try to brute force a sync request every n seconds, but that seems somewhat inelegant, I would have thought it'd be better to sync on reconnection after a period offline, or to increase the polling timeout to avoid loads of requests that are doomed to fail, but yeah, possibly best to keep it simple and just send the request, who cares if it fails.

[wtrocki]

There are couple implications related with failed call: https://github.com/feedhenry/fh-sync-js/blob/9112991c954084d24eb001705f52f35252ba9872/src/sync-client.js#L701-L703

I have tested basic functionalities without isOnline method and everything works fine. However this may seem to be anti pattern for mobile development as it may drain battery.

My proposition to partially resolve this issue: https://github.com/feedhenry/fh-sync-js/pull/29

It may look really simple, but it will give users choice.

I would have thought it'd be better to sync on reconnection after a period offline,

Yeah - I was thinking about this actual problem. We have projects with sync times around 2 minutes. This 2 minutes means that in that period application needs to be in the foreground and online. Even if you become online for a 1 minute it may be not enough to be able to sync all requests. Thats possible to implement on user side - users will need to forceSync when they feel appropriate - for example when network is restored.

[wei-lee]

However this may seem to be anti pattern for mobile development as it may drain battery.

Yeah, fair point. My idea was a bit crazy 😸 . Actually forgot to ask, in which browser does navigator.onLine is failing? Can we check if the online/offline events work in that browser?

[TinyExplosions]

It's not easily reproducible, but have seen it in Chrome and Canary