Skip to content

Add support for connection.recv_timeout_seconds in the Browser Channel #955

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

Merged
merged 2 commits into from
Jul 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions packages/bolt-connection/src/channel/browser/browser-channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ export default class WebSocketChannel {
this._error = null
this._handleConnectionError = this._handleConnectionError.bind(this)
this._config = config
this._receiveTimeout = null
this._receiveTimeoutStarted = false
this._receiveTimeoutId = null

const { scheme, error } = determineWebSocketScheme(config, protocolSupplier)
if (error) {
Expand Down Expand Up @@ -84,6 +87,7 @@ export default class WebSocketChannel {
}
}
this._ws.onmessage = event => {
this._resetTimeout()
if (self.onmessage) {
const b = new ChannelBuffer(event.data)
self.onmessage(b)
Expand Down Expand Up @@ -111,7 +115,7 @@ export default class WebSocketChannel {
}

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

/**
* Stops the receive timeout for the channel.
*/
stopReceiveTimeout () {
if (this._receiveTimeout !== null && this._receiveTimeoutStarted) {
this._receiveTimeoutStarted = false
if (this._receiveTimeoutId != null) {
clearTimeout(this._receiveTimeoutId)
}
this._receiveTimeoutId = null
}
}

/**
* Start the receive timeout for the channel.
*/
startReceiveTimeout () {
if (this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
this._receiveTimeoutStarted = true
this._resetTimeout()
}
}

_resetTimeout () {
if (!this._receiveTimeoutStarted) {
return
}

if (this._receiveTimeoutId !== null) {
clearTimeout(this._receiveTimeoutId)
}

this._receiveTimeoutId = setTimeout(() => {
this._receiveTimeoutId = null
this._timedout = true
this.stopReceiveTimeout()
this._error = newError(
`Connection lost. Server didn't respond in ${this._receiveTimeout}ms`,
this._config.connectionErrorCode
)

this.close()
if (this.onerror) {
this.onerror(this._error)
}
}, this._receiveTimeout)
}

/**
Expand Down
226 changes: 225 additions & 1 deletion packages/bolt-connection/test/channel/browser/browser-channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

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

const {
Expand Down Expand Up @@ -360,6 +360,230 @@ describe('WebSocketChannel', () => {
})
})

describe('.startReceiveTimeout()', () => {
let fakeSetTimeout
beforeEach(() => {
const address = ServerAddress.fromUrl('http://localhost:8989')
const channelConfig = new ChannelConfig(
address,
{ connectionTimeout: 0 },
SERVICE_UNAVAILABLE
)
webSocketChannel = new WebSocketChannel(
channelConfig,
undefined,
createWebSocketFactory(WS_OPEN)
)
fakeSetTimeout = setTimeoutMock.install()
fakeSetTimeout.pause()
})

afterEach(() => {
fakeSetTimeout.uninstall()
})

describe('receive timeout is setup', () => {
const receiveTimeout = 1000
beforeEach(() => {
webSocketChannel.setupReceiveTimeout(receiveTimeout)
})

it('should call setTimeout(receiveTimeout) when it call first', () => {
webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
})

it('should call not setTimeout(receiveTimeout) when already started', () => {
webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])

webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
})

it('should call setTimeout(receiveTimeout) after stopped', () => {
webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])

webSocketChannel.stopReceiveTimeout()

webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(2)
expect(fakeSetTimeout.calls.length).toEqual(2)
expect(fakeSetTimeout.calls[1][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
})

describe('on times out', () => {
beforeEach(() => {
webSocketChannel.startReceiveTimeout()
})

it('should notify the error', () => {
webSocketChannel.onerror = jest.fn()

fakeSetTimeout.calls[0][0]()

expect(webSocketChannel.onerror).toBeCalledWith(newError(
'Connection lost. Server didn\'t respond in 1000ms',
webSocketChannel._config.connectionErrorCode
))
})

it('should close the connection', () => {
fakeSetTimeout.calls[0][0]()

expect(webSocketChannel._ws.readyState).toBe(WS_CLOSED)
})

it('should close the timedout', () => {
fakeSetTimeout.calls[0][0]()

expect(webSocketChannel._timedout).toBe(true)
})

describe('onmessage', () => {
it('should not reset the timer', () => {
fakeSetTimeout.calls[0][0]()

webSocketChannel._ws.onmessage('')

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
})
})
})

describe('onmessage', () => {
it('should reset the timer', () => {
webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])

webSocketChannel._ws.onmessage('')

expect(fakeSetTimeout._timeoutIdCounter).toEqual(2)
expect(fakeSetTimeout.calls.length).toEqual(2)
expect(fakeSetTimeout.calls[1][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
})
})
})

describe('receive timeout is not setup', () => {
it('should not call setTimeout(receiveTimeout) when not configured', () => {
// start
webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
})

describe('onmessage', () => {
it('should reset the timer', () => {
webSocketChannel.startReceiveTimeout()

webSocketChannel._ws.onmessage('')

expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
expect(fakeSetTimeout.calls.length).toEqual(0)
})
})
})
})

describe('.stopReceiveTimeout()', () => {
let fakeSetTimeout
beforeEach(() => {
const address = ServerAddress.fromUrl('http://localhost:8989')
const channelConfig = new ChannelConfig(
address,
{ connectionTimeout: 0 },
SERVICE_UNAVAILABLE
)
webSocketChannel = new WebSocketChannel(
channelConfig,
undefined,
createWebSocketFactory(WS_OPEN)
)
fakeSetTimeout = setTimeoutMock.install()
fakeSetTimeout.pause()
})

afterEach(() => {
fakeSetTimeout.uninstall()
})

describe('receive timeout is setup', () => {
const receiveTimeout = 1000
beforeEach(() => {
webSocketChannel.setupReceiveTimeout(receiveTimeout)
})

it('should not clear timeout when it is not started', () => {
webSocketChannel.stopReceiveTimeout()

expect(fakeSetTimeout.clearedTimeouts).toEqual([])
})

it('should clear timeout when it is started', () => {
webSocketChannel.startReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])

webSocketChannel.stopReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
})

describe('onmessage', () => {
it('should reset the timer when stoped', () => {
webSocketChannel.startReceiveTimeout()
webSocketChannel.stopReceiveTimeout()

webSocketChannel._ws.onmessage('')

expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
expect(fakeSetTimeout.calls.length).toEqual(1)
})
})
})

describe('receive timeout is not setup', () => {
it('should not call clearTimeout() when not configured', () => {
// start
webSocketChannel.startReceiveTimeout()
webSocketChannel.stopReceiveTimeout()

expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
})
})
})

function createWebSocketFactory (readyState) {
const ws = {}
ws.readyState = readyState
Expand Down
2 changes: 2 additions & 0 deletions packages/bolt-connection/test/timers-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class SetTimeoutMock {
code()
this.invocationDelays.push(delay)
}
this.calls.push([...arguments])
return this._timeoutIdCounter++
}

Expand Down Expand Up @@ -59,6 +60,7 @@ class SetTimeoutMock {
this._paused = false
this._timeoutIdCounter = 0

this.calls = []
this.invocationDelays = []
this.clearedTimeouts = []
}
Expand Down