Skip to content

Conversation

scottinet
Copy link
Contributor

Description

If autoReconnect is true, the SDK tries to reconnect every reconnectionDelay milliseconds.

If executed within a browser, the SDK can know if the browser has access to some kind of network: if it's marked as "offline", then there is no point retrying to connect, and this might even be harmful in some situations. For instance, browsers switch to offline if a laptop lid is closed, or if a mobile phone screen is turned off. So continuing to connect is just a useless consumption of battery power.

This PR stops the reconnection loop and instead waits for the browser to switch to "online" again before resuming its reconnection attempts.

@codecov
Copy link

codecov bot commented Oct 25, 2019

Codecov Report

Merging #456 into 6-dev will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            6-dev     #456      +/-   ##
==========================================
+ Coverage   96.02%   96.03%   +0.01%     
==========================================
  Files          33       33              
  Lines        1357     1362       +5     
==========================================
+ Hits         1303     1308       +5     
  Misses         54       54
Impacted Files Coverage Δ
src/protocols/abstract/realtime.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05401c9...3aa4cec. Read the comment docs.

Copy link
Contributor

@Aschen Aschen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand quite well this PR I think.

I see that you add a hook on the navigator online event to reconnect the SDK but shouldn't we have a hook on the offline event as well?

Co-Authored-By: Adrien Maret <amaret93@gmail.com>
@scottinet
Copy link
Contributor Author

scottinet commented Nov 4, 2019

@Aschen > About this PR's goal: assuming the SDK is run in a browser, if onLine is false, then we know for certain that the navigator don't have access to any kind of network, so we stop the reconnection loop until the navigator goes online again, potentially saving battery life on laptops, mobiles, ...
But the reverse is not true: a navigator onLine status set to true does not necessarily means that network is available (e.g. the device might be connected to a restricted, or badly configured, or faulty network).
So we don't rely on it entirely: we start the reconnection loop once the TCP socket is closed, which should happen before the "offline" event is triggered anyway (if one is triggered, which might not happen).

So we never need to listen to the "offline" event.

@Aschen Aschen merged commit db1ee01 into 6-dev Nov 4, 2019
@Aschen Aschen deleted the KZL-1401/6-stop-reconnecting branch November 4, 2019 09:43
@Aschen Aschen mentioned this pull request Nov 5, 2019
Aschen added a commit that referenced this pull request Nov 5, 2019
# [6.2.6](https://github.com/kuzzleio/sdk-javascript/releases/tag/6.2.6) (2019-11-05)


#### Enhancements

- [ [#456](#456) ] Do not try to reconnect if the browser is offline   ([scottinet](https://github.com/scottinet))
- [ [#451](#451) ] Remove local checks for arguments    ([Aschen](https://github.com/Aschen))
---
@scottinet scottinet mentioned this pull request Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants