Skip to content

Commit db1ee01

Browse files
scottinetAschen
authored andcommitted
Do not try to reconnect if the browser is offline (#456)
# 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.
1 parent 05401c9 commit db1ee01

File tree

6 files changed

+155
-36
lines changed

6 files changed

+155
-36
lines changed

package-lock.json

Lines changed: 30 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
"eslint": "^5.16.0",
5555
"eslint-friendly-formatter": "^4.0.1",
5656
"eslint-loader": "^2.2.1",
57+
"lolex": "^5.1.1",
5758
"mocha": "6.2.0",
5859
"mock-require": "^3.0.3",
5960
"nyc": "^14.1.1",

src/protocols/abstract/realtime.js

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const
44
KuzzleAbstractProtocol = require('./common');
55

66
class RTWrapper extends KuzzleAbstractProtocol {
7-
87
constructor (host, options = {}) {
98
super(host, options);
109

@@ -52,27 +51,43 @@ class RTWrapper extends KuzzleAbstractProtocol {
5251
*
5352
* @param {Error} error
5453
*/
55-
clientNetworkError(error) {
54+
clientNetworkError (error) {
5655
this.state = 'offline';
5756
this.clear();
5857

5958
const connectionError = new Error(`Unable to connect to kuzzle server at ${this.host}:${this.port}`);
6059
connectionError.internal = error;
6160

6261
this.emit('networkError', connectionError);
62+
6363
if (this.autoReconnect && !this.retrying && !this.stopRetryingToConnect) {
6464
this.retrying = true;
6565

66+
if ( typeof window === 'object'
67+
&& typeof window.navigator === 'object'
68+
&& window.navigator.onLine === false
69+
) {
70+
window.addEventListener(
71+
'online',
72+
() => {
73+
this.retrying = false;
74+
this.connect().catch(err => this.clientNetworkError(err));
75+
},
76+
{ once: true });
77+
return;
78+
}
79+
6680
setTimeout(() => {
6781
this.retrying = false;
6882
this.connect(this.host).catch(err => this.clientNetworkError(err));
6983
}, this.reconnectionDelay);
70-
} else {
84+
}
85+
else {
7186
this.emit('disconnect');
7287
}
7388
}
7489

75-
isReady() {
90+
isReady () {
7691
return this.state === 'connected';
7792
}
7893
}

test/mocks/window.mock.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
const
2+
sinon = require('sinon'),
3+
KuzzleEventEmitter = require('../../src/eventEmitter');
4+
5+
// A class to mock the global window object
6+
class WindowMock extends KuzzleEventEmitter {
7+
constructor () {
8+
super();
9+
10+
if (typeof window !== 'undefined') {
11+
throw new Error('Cannot mock add a global "window" object: already defined');
12+
}
13+
14+
this.navigator = {
15+
onLine: true
16+
};
17+
18+
this.addEventListener = this.addListener;
19+
sinon.spy(this, 'addEventListener');
20+
}
21+
22+
static restore () {
23+
delete global.window;
24+
}
25+
26+
static inject () {
27+
Object.defineProperty(global, 'window', {
28+
value: new this(),
29+
enumerable: false,
30+
writable: false,
31+
configurable: true
32+
});
33+
}
34+
}
35+
36+
module.exports = WindowMock;

test/protocol/socketio.test.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
const
22
should = require('should'),
33
sinon = require('sinon'),
4-
SocketIO = require('../../src/protocols/socketio');
5-
6-
/**
7-
* @global window
8-
*/
4+
SocketIO = require('../../src/protocols/socketio'),
5+
windowMock = require('../mocks/window.mock');
96

107
describe('SocketIO networking module', () => {
118
let
@@ -15,6 +12,7 @@ describe('SocketIO networking module', () => {
1512

1613
beforeEach(() => {
1714
clock = sinon.useFakeTimers();
15+
1816
socketStub = {
1917
events: {},
2018
eventOnce: {},
@@ -74,11 +72,13 @@ describe('SocketIO networking module', () => {
7472
});
7573
socketIO.socket = socketStub;
7674

77-
window = {io: sinon.stub().returns(socketStub)}; // eslint-disable-line
75+
windowMock.inject();
76+
window.io = sinon.stub().returns(socketStub);
7877
});
7978

8079
afterEach(() => {
8180
clock.restore();
81+
windowMock.restore();
8282
});
8383

8484
it('should expose an unique identifier', () => {
@@ -225,8 +225,6 @@ describe('SocketIO exposed methods', () => {
225225

226226
socketIO = new SocketIO('address');
227227
socketIO.socket = socketStub;
228-
229-
window = {io: sinon.stub().returns(socketStub)}; // eslint-disable-line
230228
});
231229

232230
it('should be able to listen to an event just once', () => {

test/protocol/websocket.test.js

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
const
22
should = require('should'),
33
sinon = require('sinon'),
4+
lolex = require('lolex'),
45
NodeWS = require('ws'),
5-
WS = require('../../src/protocols/websocket');
6+
WS = require('../../src/protocols/websocket'),
7+
windowMock = require('../mocks/window.mock');
68

79
describe('WebSocket networking module', () => {
810
let
@@ -12,13 +14,13 @@ describe('WebSocket networking module', () => {
1214
clientStub;
1315

1416
beforeEach(() => {
15-
clock = sinon.useFakeTimers();
17+
clock = lolex.install();
1618
clientStub = {
1719
send: sinon.stub(),
1820
close: sinon.stub()
1921
};
2022

21-
window = 'foobar'; // eslint-disable-line
23+
windowMock.inject();
2224
WebSocket = function (...args) { // eslint-disable-line
2325
wsargs = args;
2426
return clientStub;
@@ -32,9 +34,9 @@ describe('WebSocket networking module', () => {
3234
});
3335

3436
afterEach(() => {
35-
clock.restore();
37+
clock.uninstall();
3638
WebSocket = undefined; // eslint-disable-line
37-
window = undefined; // eslint-disable-line
39+
windowMock.restore();
3840
});
3941

4042
it('should expose an unique identifier', () => {
@@ -156,6 +158,59 @@ describe('WebSocket networking module', () => {
156158
return should(promise).be.rejectedWith('foobar');
157159
});
158160

161+
it('should stop reconnecting if the browser goes offline', () => {
162+
const cb = sinon.stub();
163+
164+
websocket.retrying = false;
165+
websocket.addListener('networkError', cb);
166+
should(websocket.listeners('networkError').length).be.eql(1);
167+
168+
websocket.connect();
169+
websocket.connect = sinon.stub().rejects();
170+
clientStub.onopen();
171+
clientStub.onerror();
172+
173+
should(websocket.retrying).be.true();
174+
should(cb).be.calledOnce();
175+
should(websocket.connect).not.be.called();
176+
177+
window.navigator.onLine = false;
178+
179+
return clock.tickAsync(100)
180+
.then(() => {
181+
should(websocket.retrying).be.true();
182+
should(cb).be.calledTwice();
183+
should(websocket.connect).be.calledOnce();
184+
185+
should(window.addEventListener).calledWith(
186+
'online',
187+
sinon.match.func,
188+
{ once: true });
189+
190+
return clock.tickAsync(100);
191+
})
192+
.then(() => {
193+
// the important bit is there: cb hasn't been called since the last
194+
// tick because the SDK does not try to connect if the browser is
195+
// marked offline
196+
should(cb).be.calledTwice();
197+
198+
should(websocket.retrying).be.true();
199+
should(websocket.connect).be.calledOnce();
200+
201+
window.emit('online');
202+
return clock.tickAsync(100);
203+
})
204+
.then(() => {
205+
// And it started retrying to connect again now that the browser is
206+
// "online"
207+
should(cb).be.calledThrice();
208+
209+
should(websocket.retrying).be.true();
210+
should(websocket.connect).be.calledTwice();
211+
});
212+
});
213+
159214
it('should call listeners on a "disconnect" event', () => {
160215
const cb = sinon.stub();
161216

@@ -371,7 +426,7 @@ describe('WebSocket networking module', () => {
371426

372427
it('should fallback to the ws module if there is no global WebSocket API', () => {
373428
WebSocket = undefined; // eslint-disable-line
374-
window = undefined; // eslint-disable-line
429+
windowMock.restore();
375430

376431
const client = new WS('foobar');
377432

@@ -391,7 +446,7 @@ describe('WebSocket networking module', () => {
391446

392447
it('should initialize pass allowed options to the ws ctor when using it', () => {
393448
WebSocket = undefined; // eslint-disable-line
394-
window = undefined; // eslint-disable-line
449+
windowMock.restore();
395450

396451
let client = new WS('foobar');
397452

@@ -413,7 +468,7 @@ describe('WebSocket networking module', () => {
413468

414469
it('should throw if invalid options are provided', () => {
415470
WebSocket = undefined; // eslint-disable-line
416-
window = undefined; // eslint-disable-line
471+
windowMock.restore();
417472

418473
const invalidHeaders = ['foo', 'false', 'true', 123, []];
419474

0 commit comments

Comments
 (0)