Skip to content

Commit 5294b99

Browse files
bigmontzbigmontz
authored
Add support for connection.recv_timeout_seconds in the Browser Channel (#955)
The implementation of the timeout connection hint has changed. The control of when start and stop the timeout has moved to the `channel-channel` instead of relying on the socket timeout configuration. This change enables the `browser-channel` to implement the `receive timeout` which was not possible before since the `WebSocket` doesn't have a timeout configuration.
1 parent c6164d7 commit 5294b99

File tree

3 files changed

+271
-3
lines changed

3 files changed

+271
-3
lines changed

‎packages/bolt-connection/src/channel/browser/browser-channel.js

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ export default class WebSocketChannel {
5353
this._error = null
5454
this._handleConnectionError = this._handleConnectionError.bind(this)
5555
this._config = config
56+
this._receiveTimeout = null
57+
this._receiveTimeoutStarted = false
58+
this._receiveTimeoutId = null
5659

5760
const { scheme, error } = determineWebSocketScheme(config, protocolSupplier)
5861
if (error) {
@@ -84,6 +87,7 @@ export default class WebSocketChannel {
8487
}
8588
}
8689
this._ws.onmessage = event => {
90+
this._resetTimeout()
8791
if (self.onmessage) {
8892
const b = new ChannelBuffer(event.data)
8993
self.onmessage(b)
@@ -111,7 +115,7 @@ export default class WebSocketChannel {
111115
}
112116

113117
// onerror triggers on websocket close as well.. don't get me started.
114-
if (this._open) {
118+
if (this._open && !this._timedout) {
115119
// http://stackoverflow.com/questions/25779831/how-to-catch-websocket-connection-to-ws-xxxnn-failed-connection-closed-be
116120
this._error = newError(
117121
'WebSocket connection failure. Due to security ' +
@@ -181,18 +185,56 @@ export default class WebSocketChannel {
181185
* @param {number} receiveTimeout The amount of time the channel will keep without receive any data before timeout (ms)
182186
* @returns {void}
183187
*/
184-
setupReceiveTimeout (receiveTimeout) {}
188+
setupReceiveTimeout (receiveTimeout) {
189+
this._receiveTimeout = receiveTimeout
190+
}
185191

186192
/**
187193
* Stops the receive timeout for the channel.
188194
*/
189195
stopReceiveTimeout () {
196+
if (this._receiveTimeout !== null && this._receiveTimeoutStarted) {
197+
this._receiveTimeoutStarted = false
198+
if (this._receiveTimeoutId != null) {
199+
clearTimeout(this._receiveTimeoutId)
200+
}
201+
this._receiveTimeoutId = null
202+
}
190203
}
191204

192205
/**
193206
* Start the receive timeout for the channel.
194207
*/
195208
startReceiveTimeout () {
209+
if (this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
210+
this._receiveTimeoutStarted = true
211+
this._resetTimeout()
212+
}
213+
}
214+
215+
_resetTimeout () {
216+
if (!this._receiveTimeoutStarted) {
217+
return
218+
}
219+
220+
if (this._receiveTimeoutId !== null) {
221+
clearTimeout(this._receiveTimeoutId)
222+
}
223+
224+
this._receiveTimeoutId = setTimeout(() => {
225+
this._receiveTimeoutId = null
226+
this._timedout = true
227+
this.stopReceiveTimeout()
228+
this._error = newError(
229+
`Connection lost. Server didn't respond in ${this._receiveTimeout}ms`,
230+
this._config.connectionErrorCode
231+
)
232+
233+
this.close()
234+
if (this.onerror) {
235+
this.onerror(this._error)
236+
}
237+
}, this._receiveTimeout)
196238
}
197239

198240
/**

‎packages/bolt-connection/test/channel/browser/browser-channel.test.js

Lines changed: 225 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
import WebSocketChannel from '../../../src/channel/browser/browser-channel'
2121
import ChannelConfig from '../../../src/channel/channel-config'
22-
import { error, internal } from 'neo4j-driver-core'
22+
import { error, internal, newError } from 'neo4j-driver-core'
2323
import { setTimeoutMock } from '../../timers-util'
2424

2525
const {
@@ -360,6 +360,230 @@ describe('WebSocketChannel', () => {
360360
})
361361
})
362362

363+
describe('.startReceiveTimeout()', () => {
364+
let fakeSetTimeout
365+
beforeEach(() => {
366+
const address = ServerAddress.fromUrl('http://localhost:8989')
367+
const channelConfig = new ChannelConfig(
368+
address,
369+
{ connectionTimeout: 0 },
370+
SERVICE_UNAVAILABLE
371+
)
372+
webSocketChannel = new WebSocketChannel(
373+
channelConfig,
374+
undefined,
375+
createWebSocketFactory(WS_OPEN)
376+
)
377+
fakeSetTimeout = setTimeoutMock.install()
378+
fakeSetTimeout.pause()
379+
})
380+
381+
afterEach(() => {
382+
fakeSetTimeout.uninstall()
383+
})
384+
385+
describe('receive timeout is setup', () => {
386+
const receiveTimeout = 1000
387+
beforeEach(() => {
388+
webSocketChannel.setupReceiveTimeout(receiveTimeout)
389+
})
390+
391+
it('should call setTimeout(receiveTimeout) when it call first', () => {
392+
webSocketChannel.startReceiveTimeout()
393+
394+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
395+
expect(fakeSetTimeout.calls.length).toEqual(1)
396+
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
397+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
398+
})
399+
400+
it('should call not setTimeout(receiveTimeout) when already started', () => {
401+
webSocketChannel.startReceiveTimeout()
402+
403+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
404+
expect(fakeSetTimeout.calls.length).toEqual(1)
405+
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
406+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
407+
408+
webSocketChannel.startReceiveTimeout()
409+
410+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
411+
expect(fakeSetTimeout.calls.length).toEqual(1)
412+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
413+
})
414+
415+
it('should call setTimeout(receiveTimeout) after stopped', () => {
416+
webSocketChannel.startReceiveTimeout()
417+
418+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
419+
expect(fakeSetTimeout.calls.length).toEqual(1)
420+
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
421+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
422+
423+
webSocketChannel.stopReceiveTimeout()
424+
425+
webSocketChannel.startReceiveTimeout()
426+
427+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(2)
428+
expect(fakeSetTimeout.calls.length).toEqual(2)
429+
expect(fakeSetTimeout.calls[1][1]).toEqual(receiveTimeout)
430+
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
431+
})
432+
433+
describe('on times out', () => {
434+
beforeEach(() => {
435+
webSocketChannel.startReceiveTimeout()
436+
})
437+
438+
it('should notify the error', () => {
439+
webSocketChannel.onerror = jest.fn()
440+
441+
fakeSetTimeout.calls[0][0]()
442+
443+
expect(webSocketChannel.onerror).toBeCalledWith(newError(
444+
'Connection lost. Server didn\'t respond in 1000ms',
445+
webSocketChannel._config.connectionErrorCode
446+
))
447+
})
448+
449+
it('should close the connection', () => {
450+
fakeSetTimeout.calls[0][0]()
451+
452+
expect(webSocketChannel._ws.readyState).toBe(WS_CLOSED)
453+
})
454+
455+
it('should close the timedout', () => {
456+
fakeSetTimeout.calls[0][0]()
457+
458+
expect(webSocketChannel._timedout).toBe(true)
459+
})
460+
461+
describe('onmessage', () => {
462+
it('should not reset the timer', () => {
463+
fakeSetTimeout.calls[0][0]()
464+
465+
webSocketChannel._ws.onmessage('')
466+
467+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
468+
expect(fakeSetTimeout.calls.length).toEqual(1)
469+
})
470+
})
471+
})
472+
473+
describe('onmessage', () => {
474+
it('should reset the timer', () => {
475+
webSocketChannel.startReceiveTimeout()
476+
477+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
478+
expect(fakeSetTimeout.calls.length).toEqual(1)
479+
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
480+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
481+
482+
webSocketChannel._ws.onmessage('')
483+
484+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(2)
485+
expect(fakeSetTimeout.calls.length).toEqual(2)
486+
expect(fakeSetTimeout.calls[1][1]).toEqual(receiveTimeout)
487+
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
488+
})
489+
})
490+
})
491+
492+
describe('receive timeout is not setup', () => {
493+
it('should not call setTimeout(receiveTimeout) when not configured', () => {
494+
// start
495+
webSocketChannel.startReceiveTimeout()
496+
497+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
498+
})
499+
500+
describe('onmessage', () => {
501+
it('should reset the timer', () => {
502+
webSocketChannel.startReceiveTimeout()
503+
504+
webSocketChannel._ws.onmessage('')
505+
506+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
507+
expect(fakeSetTimeout.calls.length).toEqual(0)
508+
})
509+
})
510+
})
511+
})
512+
513+
describe('.stopReceiveTimeout()', () => {
514+
let fakeSetTimeout
515+
beforeEach(() => {
516+
const address = ServerAddress.fromUrl('http://localhost:8989')
517+
const channelConfig = new ChannelConfig(
518+
address,
519+
{ connectionTimeout: 0 },
520+
SERVICE_UNAVAILABLE
521+
)
522+
webSocketChannel = new WebSocketChannel(
523+
channelConfig,
524+
undefined,
525+
createWebSocketFactory(WS_OPEN)
526+
)
527+
fakeSetTimeout = setTimeoutMock.install()
528+
fakeSetTimeout.pause()
529+
})
530+
531+
afterEach(() => {
532+
fakeSetTimeout.uninstall()
533+
})
534+
535+
describe('receive timeout is setup', () => {
536+
const receiveTimeout = 1000
537+
beforeEach(() => {
538+
webSocketChannel.setupReceiveTimeout(receiveTimeout)
539+
})
540+
541+
it('should not clear timeout when it is not started', () => {
542+
webSocketChannel.stopReceiveTimeout()
543+
544+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
545+
})
546+
547+
it('should clear timeout when it is started', () => {
548+
webSocketChannel.startReceiveTimeout()
549+
550+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
551+
expect(fakeSetTimeout.calls.length).toEqual(1)
552+
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
553+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
554+
555+
webSocketChannel.stopReceiveTimeout()
556+
557+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
558+
expect(fakeSetTimeout.calls.length).toEqual(1)
559+
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
560+
})
561+
562+
describe('onmessage', () => {
563+
it('should reset the timer when stoped', () => {
564+
webSocketChannel.startReceiveTimeout()
565+
webSocketChannel.stopReceiveTimeout()
566+
567+
webSocketChannel._ws.onmessage('')
568+
569+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
570+
expect(fakeSetTimeout.calls.length).toEqual(1)
571+
})
572+
})
573+
})
574+
575+
describe('receive timeout is not setup', () => {
576+
it('should not call clearTimeout() when not configured', () => {
577+
// start
578+
webSocketChannel.startReceiveTimeout()
579+
webSocketChannel.stopReceiveTimeout()
580+
581+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
582+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
583+
})
584+
})
585+
})
586+
363587
function createWebSocketFactory (readyState) {
364588
const ws = {}
365589
ws.readyState = readyState

‎packages/bolt-connection/test/timers-util.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class SetTimeoutMock {
2828
code()
2929
this.invocationDelays.push(delay)
3030
}
31+
this.calls.push([...arguments])
3132
return this._timeoutIdCounter++
3233
}
3334

@@ -59,6 +60,7 @@ class SetTimeoutMock {
5960
this._paused = false
6061
this._timeoutIdCounter = 0
6162

63+
this.calls = []
6264
this.invocationDelays = []
6365
this.clearedTimeouts = []
6466
}

0 commit comments

Comments
 (0)