From 7296e8180f59294062a259f0076e1a64856849ec Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 22 Jul 2021 16:06:33 +0200 Subject: [PATCH 01/15] Connection Hint: supporting connection.recv_timeout_seconds This value defines the time the client should wait for a response on a connection before it times out. Co-authored-by: Rouven Bauer --- .../src/channel/browser/browser-channel.js | 5 +++ .../src/channel/node/node-channel.js | 36 ++++++++++++++++--- .../src/connection/connection-channel.js | 12 ++++++- core/src/integer.ts | 9 ++++- testkit-backend/src/request-handlers.js | 5 ++- testkit-backend/src/skipped-tests.js | 6 ++++ 6 files changed, 66 insertions(+), 7 deletions(-) diff --git a/bolt-connection/src/channel/browser/browser-channel.js b/bolt-connection/src/channel/browser/browser-channel.js index 2c3a2d997..2e5acb921 100644 --- a/bolt-connection/src/channel/browser/browser-channel.js +++ b/bolt-connection/src/channel/browser/browser-channel.js @@ -171,6 +171,11 @@ export default class WebSocketChannel { }) } + /** + * @todo document it + */ + setupReceiveTimeout (receiveTimeout) {} + /** * Set connection timeout on the given WebSocket, if configured. * @return {number} the timeout id or null. diff --git a/bolt-connection/src/channel/node/node-channel.js b/bolt-connection/src/channel/node/node-channel.js index 9613814b4..f6e8189d4 100644 --- a/bolt-connection/src/channel/node/node-channel.js +++ b/bolt-connection/src/channel/node/node-channel.js @@ -305,12 +305,12 @@ export default class NodeChannel { _setupConnectionTimeout (config, socket) { const timeout = config.connectionTimeout if (timeout) { - socket.on('connect', () => { + const connectListener = () => { // connected - clear connection timeout socket.setTimeout(0) - }) + } - socket.on('timeout', () => { + const timeoutListener = () => { // timeout fired - not connected within configured time. cancel timeout and destroy socket socket.setTimeout(0) socket.destroy( @@ -319,12 +319,40 @@ export default class NodeChannel { config.connectionErrorCode ) ) - }) + } + + socket.on('connect', connectListener) + socket.on('timeout', timeoutListener) + + this._removeConnectionTimeoutListeners = () => { + this._conn.off('connect', connectListener) + this._conn.off('timeout', timeoutListener) + } socket.setTimeout(timeout) } } + /** + * @todo document it + */ + setupReceiveTimeout (receiveTimeout) { + if (this._removeConnectionTimeoutListeners) { + this._removeConnectionTimeoutListeners() + } + + this._conn.on('timeout', () => { + this._conn.destroy( + newError( + `Connection lost. Server didn't respond in ${receiveTimeout}ms`, + this._connectionErrorCode + ) + ) + }) + + this._conn.setTimeout(receiveTimeout) + } + /** * Write the passed in buffer to connection * @param {NodeBuffer} buffer - Buffer to write diff --git a/bolt-connection/src/connection/connection-channel.js b/bolt-connection/src/connection/connection-channel.js index 95a08de5e..defbedaea 100644 --- a/bolt-connection/src/connection/connection-channel.js +++ b/bolt-connection/src/connection/connection-channel.js @@ -18,7 +18,7 @@ */ import { Chunker, Dechunker, ChannelConfig, Channel } from '../channel' -import { newError, error, json, internal } from 'neo4j-driver-core' +import { newError, error, json, internal, toNumber } from 'neo4j-driver-core' import Connection from './connection' import Bolt from '../bolt' @@ -198,6 +198,16 @@ export default class ChannelConnection extends Connection { if (!this.databaseId) { this.databaseId = dbConnectionId } + + if ( + metadata.hints && + 'connection.recv_timeout_seconds' in metadata.hints + ) { + const receiveTimeout = + toNumber(metadata.hints['connection.recv_timeout_seconds']) * + 1000 + this._ch.setupReceiveTimeout(receiveTimeout) + } } resolve(self) } diff --git a/core/src/integer.ts b/core/src/integer.ts index 8941f51fd..03ff8b6ba 100644 --- a/core/src/integer.ts +++ b/core/src/integer.ts @@ -909,7 +909,14 @@ class Integer { * @expose */ static toNumber (val: Integerable): number { - return Integer.fromValue(val).toNumber() + switch (typeof val) { + case 'number': + return val + case 'bigint': + return Number(val) + default: + return Integer.fromValue(val).toNumber() + } } /** diff --git a/testkit-backend/src/request-handlers.js b/testkit-backend/src/request-handlers.js index fbbabf999..0f12d1927 100644 --- a/testkit-backend/src/request-handlers.js +++ b/testkit-backend/src/request-handlers.js @@ -237,7 +237,10 @@ export function StartTest (_, { testName }, wire) { export function GetFeatures (_context, _params, wire) { wire.writeResponse('FeatureList', { - features: ['AuthorizationExpiredTreatment'] + features: [ + 'AuthorizationExpiredTreatment', + 'ConfHint:connection.recv_timeout_seconds' + ] }) } diff --git a/testkit-backend/src/skipped-tests.js b/testkit-backend/src/skipped-tests.js index 708d3ae4e..df7768e55 100644 --- a/testkit-backend/src/skipped-tests.js +++ b/testkit-backend/src/skipped-tests.js @@ -84,6 +84,12 @@ const skippedTests = [ ifEndsWith( 'test_should_fail_when_reading_from_unexpectedly_interrupting_reader_using_session_run' ) + ), + skip( + 'Needs to implement GetRoutingTable tk protocol message', + ifStartsWith( + 'stub.configuration_hints.test_connection_recv_timeout_seconds.TestRoutingConnectionRecvTimeout' + ) ) ] From c0945de7fae2abf5c78146523aa9768956eb967a Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 22 Jul 2021 16:25:25 +0200 Subject: [PATCH 02/15] Migrating channel/browser test to bolt-connection --- .../channel}/browser/browser-channel.test.js | 10 +-- .../browser-host-name-resolver.test.js | 2 +- bolt-connection/test/timers-util.js | 67 +++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) rename {test/internal => bolt-connection/test/channel}/browser/browser-channel.test.js (96%) rename {test/internal => bolt-connection/test/channel}/browser/browser-host-name-resolver.test.js (92%) create mode 100644 bolt-connection/test/timers-util.js diff --git a/test/internal/browser/browser-channel.test.js b/bolt-connection/test/channel/browser/browser-channel.test.js similarity index 96% rename from test/internal/browser/browser-channel.test.js rename to bolt-connection/test/channel/browser/browser-channel.test.js index 3025598f0..37b119b63 100644 --- a/test/internal/browser/browser-channel.test.js +++ b/bolt-connection/test/channel/browser/browser-channel.test.js @@ -17,10 +17,10 @@ * limitations under the License. */ -import WebSocketChannel from '../../../bolt-connection/lib/channel/browser/browser-channel' -import ChannelConfig from '../../../bolt-connection/lib/channel/channel-config' +import WebSocketChannel from '../../../src/channel/browser/browser-channel' +import ChannelConfig from '../../../src/channel/channel-config' import { error, internal } from 'neo4j-driver-core' -import { setTimeoutMock } from '../timers-util' +import { setTimeoutMock } from '../../timers-util' const { serverAddress: { ServerAddress }, @@ -173,7 +173,7 @@ describe('#unit WebSocketChannel', () => { createWebSocketFactory(WS_CLOSED) ) - await expectAsync(channel.close()).toBeResolved() + await expect(channel.close()).resolves.not.toThrow() }) it('should resolve close when websocket is closed', async () => { @@ -186,7 +186,7 @@ describe('#unit WebSocketChannel', () => { createWebSocketFactory(WS_OPEN) ) - await expectAsync(channel.close()).toBeResolved() + await expect(channel.close()).resolves.not.toThrow() }) function testFallbackToLiteralIPv6 (boltAddress, expectedWsAddress) { diff --git a/test/internal/browser/browser-host-name-resolver.test.js b/bolt-connection/test/channel/browser/browser-host-name-resolver.test.js similarity index 92% rename from test/internal/browser/browser-host-name-resolver.test.js rename to bolt-connection/test/channel/browser/browser-host-name-resolver.test.js index 1995c096d..61653833b 100644 --- a/test/internal/browser/browser-host-name-resolver.test.js +++ b/bolt-connection/test/channel/browser/browser-host-name-resolver.test.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import BrowserHostNameResolver from '../../../bolt-connection/lib/channel/browser/browser-host-name-resolver' +import BrowserHostNameResolver from '../../../src/channel/browser/browser-host-name-resolver' describe('#unit BrowserHostNameResolver', () => { it('should resolve given address to itself', done => { diff --git a/bolt-connection/test/timers-util.js b/bolt-connection/test/timers-util.js new file mode 100644 index 000000000..01ad01742 --- /dev/null +++ b/bolt-connection/test/timers-util.js @@ -0,0 +1,67 @@ +/** + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +class SetTimeoutMock { + constructor () { + this._clearState() + } + + install () { + this._originalSetTimeout = global.setTimeout + global.setTimeout = (code, delay) => { + if (!this._paused) { + code() + this.invocationDelays.push(delay) + } + return this._timeoutIdCounter++ + } + + this._originalClearTimeout = global.clearTimeout + global.clearTimeout = id => { + this.clearedTimeouts.push(id) + } + + return this + } + + pause () { + this._paused = true + } + + uninstall () { + global.setTimeout = this._originalSetTimeout + global.clearTimeout = this._originalClearTimeout + this._clearState() + } + + setTimeoutOriginal (code, delay) { + return this._originalSetTimeout.call(null, code, delay) + } + + _clearState () { + this._originalSetTimeout = null + this._originalClearTimeout = null + this._paused = false + this._timeoutIdCounter = 0 + + this.invocationDelays = [] + this.clearedTimeouts = [] + } +} + +export const setTimeoutMock = new SetTimeoutMock() From fe5d296b91be0ee805c20f80be8a1eb4d948cda2 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 22 Jul 2021 16:51:34 +0200 Subject: [PATCH 03/15] Add tests to WebSocketChannel.setupReceiveTimeout --- .../channel/browser/browser-channel.test.js | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/bolt-connection/test/channel/browser/browser-channel.test.js b/bolt-connection/test/channel/browser/browser-channel.test.js index 37b119b63..53fa9f873 100644 --- a/bolt-connection/test/channel/browser/browser-channel.test.js +++ b/bolt-connection/test/channel/browser/browser-channel.test.js @@ -35,7 +35,7 @@ const WS_CLOSING = 2 const WS_CLOSED = 3 /* eslint-disable no-global-assign */ -describe('#unit WebSocketChannel', () => { +describe('WebSocketChannel', () => { let webSocketChannel afterEach(async () => { @@ -294,6 +294,39 @@ describe('#unit WebSocketChannel', () => { } }) + describe('.setupReceiveTimeout()', () => { + 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) + ) + }) + + it('should exists', () => { + expect(webSocketChannel).toHaveProperty('setupReceiveTimeout') + expect(typeof webSocketChannel.setupReceiveTimeout).toBe('function') + }) + + it('should not setTimeout', () => { + const fakeSetTimeout = setTimeoutMock.install() + try { + webSocketChannel.setupReceiveTimeout() + + expect(fakeSetTimeout._timeoutIdCounter).toEqual(0) + expect(webSocketChannel._connectionTimeoutId).toBe(null) + } finally { + fakeSetTimeout.uninstall() + } + }) + }) + function createWebSocketFactory (readyState) { const ws = {} ws.readyState = readyState From e2b69d172456ca622ce4d421d6bbd34889e994d2 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 22 Jul 2021 16:56:46 +0200 Subject: [PATCH 04/15] Add docs for Channel.setupReceiveTimeout --- bolt-connection/src/channel/browser/browser-channel.js | 7 ++++++- bolt-connection/src/channel/node/node-channel.js | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/bolt-connection/src/channel/browser/browser-channel.js b/bolt-connection/src/channel/browser/browser-channel.js index 2e5acb921..496889f75 100644 --- a/bolt-connection/src/channel/browser/browser-channel.js +++ b/bolt-connection/src/channel/browser/browser-channel.js @@ -172,7 +172,12 @@ export default class WebSocketChannel { } /** - * @todo document it + * Setup the receive timeout for the channel. + * + * Not supported for the browser channel. + * + * @param {number} receiveTimeout The amount of time the channel will keep without receive any data before timeout (ms) + * @returns {void} */ setupReceiveTimeout (receiveTimeout) {} diff --git a/bolt-connection/src/channel/node/node-channel.js b/bolt-connection/src/channel/node/node-channel.js index f6e8189d4..e236c60a4 100644 --- a/bolt-connection/src/channel/node/node-channel.js +++ b/bolt-connection/src/channel/node/node-channel.js @@ -334,7 +334,10 @@ export default class NodeChannel { } /** - * @todo document it + * Setup the receive timeout for the channel. + * + * @param {number} receiveTimeout The amount of time the channel will keep without receive any data before timeout (ms) + * @returns {void} */ setupReceiveTimeout (receiveTimeout) { if (this._removeConnectionTimeoutListeners) { From d1bd6fc389f445a88d44904ce48ce4b8824c5b3a Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 22 Jul 2021 17:05:50 +0200 Subject: [PATCH 05/15] Moving node-channel tests to bolt-connection --- .../test/channel}/node/node-channel.test.js | 8 ++++---- .../test/channel}/node/node-host-name-resolver.test.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) rename {test/internal => bolt-connection/test/channel}/node/node-channel.test.js (87%) rename {test/internal => bolt-connection/test/channel}/node/node-host-name-resolver.test.js (97%) diff --git a/test/internal/node/node-channel.test.js b/bolt-connection/test/channel/node/node-channel.test.js similarity index 87% rename from test/internal/node/node-channel.test.js rename to bolt-connection/test/channel/node/node-channel.test.js index 12b1f9df0..e7b2317a4 100644 --- a/test/internal/node/node-channel.test.js +++ b/bolt-connection/test/channel/node/node-channel.test.js @@ -16,8 +16,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import NodeChannel from '../../../bolt-connection/lib/channel/node/node-channel' -import ChannelConfig from '../../../bolt-connection/lib/channel/channel-config' +import NodeChannel from '../../../src/channel/node/node-channel' +import ChannelConfig from '../../../src/channel/channel-config' import { error, internal } from 'neo4j-driver-core' const { @@ -35,13 +35,13 @@ describe('#unit NodeChannel', () => { // Modify the connection to be closed channel._open = false - return expectAsync(channel.close()).toBeResolved() + return expect(channel.close()).resolves.not.toThrow() }) it('should resolve close when websocket is connected', () => { const channel = createMockedChannel(true) - return expectAsync(channel.close()).toBeResolved() + return expect(channel.close()).resolves.not.toThrow() }) }) diff --git a/test/internal/node/node-host-name-resolver.test.js b/bolt-connection/test/channel/node/node-host-name-resolver.test.js similarity index 97% rename from test/internal/node/node-host-name-resolver.test.js rename to bolt-connection/test/channel/node/node-host-name-resolver.test.js index aa5935b5d..c6cdbe206 100644 --- a/test/internal/node/node-host-name-resolver.test.js +++ b/bolt-connection/test/channel/node/node-host-name-resolver.test.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import NodeHostNameResolver from '../../../bolt-connection/lib/channel/node/node-host-name-resolver' +import NodeHostNameResolver from '../../../src/channel/node/node-host-name-resolver' import { internal } from 'neo4j-driver-core' const { From 89f5d8bd6125fc8f53fd4ba1983abe23ac88aab2 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 22 Jul 2021 17:50:28 +0200 Subject: [PATCH 06/15] Add tests to node-channel.js --- .../src/channel/node/node-channel.js | 4 +- .../test/channel/node/node-channel.test.js | 102 +++++++++++++++--- 2 files changed, 87 insertions(+), 19 deletions(-) diff --git a/bolt-connection/src/channel/node/node-channel.js b/bolt-connection/src/channel/node/node-channel.js index e236c60a4..5a0f17b19 100644 --- a/bolt-connection/src/channel/node/node-channel.js +++ b/bolt-connection/src/channel/node/node-channel.js @@ -148,7 +148,7 @@ const TrustStrategy = { * @param {function} onFailure - callback to execute on connection failure. * @return {*} socket connection. */ -function connect (config, onSuccess, onFailure = () => null) { +function _connect (config, onSuccess, onFailure = () => null) { const trustStrategy = trustStrategyName(config) if (!isEncrypted(config)) { const socket = net.connect( @@ -230,7 +230,7 @@ export default class NodeChannel { * Create new instance * @param {ChannelConfig} config - configuration for this channel. */ - constructor (config) { + constructor (config, connect = _connect) { const self = this this.id = _CONNECTION_IDGEN++ diff --git a/bolt-connection/test/channel/node/node-channel.test.js b/bolt-connection/test/channel/node/node-channel.test.js index e7b2317a4..096e3a918 100644 --- a/bolt-connection/test/channel/node/node-channel.test.js +++ b/bolt-connection/test/channel/node/node-channel.test.js @@ -18,7 +18,7 @@ */ import NodeChannel from '../../../src/channel/node/node-channel' import ChannelConfig from '../../../src/channel/channel-config' -import { error, internal } from 'neo4j-driver-core' +import { error, internal, newError } from 'neo4j-driver-core' const { serverAddress: { ServerAddress } @@ -26,7 +26,7 @@ const { const { SERVICE_UNAVAILABLE } = error -describe('#unit NodeChannel', () => { +describe('NodeChannel', () => { it('should resolve close if websocket is already closed', () => { const address = ServerAddress.fromUrl('bolt://localhost:9999') const channelConfig = new ChannelConfig(address, {}, SERVICE_UNAVAILABLE) @@ -43,28 +43,96 @@ describe('#unit NodeChannel', () => { return expect(channel.close()).resolves.not.toThrow() }) + + describe('.setupReceiveTimeout()', () => { + it('should call socket.setTimeout(receiveTimeout)', () => { + const receiveTimeout = 42 + const channel = createMockedChannel(true) + + channel.setupReceiveTimeout(receiveTimeout) + + expect(channel._conn.getCalls().setTimeout[1]).toEqual([receiveTimeout]) + }) + + it('should unsubscribe to the on connect and on timeout created on the create socket', () => { + const receiveTimeout = 42 + const channel = createMockedChannel(true) + + channel.setupReceiveTimeout(receiveTimeout) + + expect(channel._conn.getCalls().on.slice(0, 2)).toEqual( + channel._conn.getCalls().off + ) + }) + + it('should destroy the connection when time out', () => { + const receiveTimeout = 42 + const channel = createMockedChannel(true) + + channel.setupReceiveTimeout(receiveTimeout) + + const [event, listener] = channel._conn.getCalls().on[2] + expect(event).toEqual('timeout') + listener() + + expect(channel._conn.getCalls().destroy).toEqual([ + [ + newError( + "Connection lost. Server didn't respond in 42ms", + SERVICE_UNAVAILABLE + ) + ] + ]) + }) + + it('should not unsubscribe to the any on connect or on timeout if connectionTimeout is not set', () => { + const receiveTimeout = 42 + const channel = createMockedChannel(true, { connectionTimeout: 0 }) + + channel.setupReceiveTimeout(receiveTimeout) + + expect(channel._conn.getCalls().off).toEqual([]) + }) + }) }) -function createMockedChannel (connected) { +function createMockedChannel (connected, config = {}) { let endCallback = null const address = ServerAddress.fromUrl('bolt://localhost:9999') - const channelConfig = new ChannelConfig(address, {}, SERVICE_UNAVAILABLE) - const channel = new NodeChannel(channelConfig) - const socket = { - destroyed: false, - destroy: () => {}, - end: () => { - channel._open = false - endCallback() - }, - removeListener: () => {}, - on: (key, cb) => { - if (key === 'end') { - endCallback = cb + const channelConfig = new ChannelConfig(address, config, SERVICE_UNAVAILABLE) + const socketFactory = () => { + const on = [] + const off = [] + const setTimeout = [] + const destroy = [] + return { + destroyed: false, + destroy: () => { + destroy.push([...arguments]) + }, + end: () => { + channel._open = false + endCallback() + }, + removeListener: () => {}, + on: (key, cb) => { + on.push([...arguments]) + if (key === 'end') { + endCallback = cb + } + }, + off: () => { + off.push([...arguments]) + }, + setTimeout: () => { + setTimeout.push([...arguments]) + }, + getCalls: () => { + return { on, off, setTimeout, destroy } } } } - channel._conn = socket + const channel = new NodeChannel(channelConfig, socketFactory) channel._open = connected return channel } From ef2d95ad80d918dfaa134762f64e748f5e61c71e Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 23 Jul 2021 12:43:55 +0200 Subject: [PATCH 07/15] Add tests to connection-channel --- .../connection/connection-channel.test.js | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 bolt-connection/test/connection/connection-channel.test.js diff --git a/bolt-connection/test/connection/connection-channel.test.js b/bolt-connection/test/connection/connection-channel.test.js new file mode 100644 index 000000000..95d9576ee --- /dev/null +++ b/bolt-connection/test/connection/connection-channel.test.js @@ -0,0 +1,98 @@ +/** + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import ChannelConnection from '../../src/connection/connection-channel' +import { int, internal } from 'neo4j-driver-core' + +const { + serverAddress: { ServerAddress }, + logger: { Logger } +} = internal + +describe('ChannelConnection', () => { + describe('.connect()', () => { + it.each([ + [42000, 42n], + [21000, 21], + [12000, int(12)] + ])( + "should call this._ch.setupReceiveTimeout(%o) when onComplete metadata.hints['connection.recv_timeout_seconds'] is %o", + async (expected, receiveTimeout) => { + const channel = { + setupReceiveTimeout: jest.fn().mockName('setupReceiveTimeout') + } + const protocol = { + initialize: jest.fn(observer => + observer.onComplete({ + hints: { 'connection.recv_timeout_seconds': receiveTimeout } + }) + ) + } + const protocolSupplier = () => protocol + const connection = spyOnConnectionChannel({ channel, protocolSupplier }) + + await connection.connect('userAgent', {}) + + expect(channel.setupReceiveTimeout).toHaveBeenCalledWith(expected) + } + ) + + it.each([[undefined], [null], [{}], [{ hint: null }], [{ hint: {} }]])( + 'should call not call this._ch.setupReceiveTimeout() when onComplete metadata is %o', + async metadata => { + const channel = { + setupReceiveTimeout: jest.fn().mockName('setupReceiveTimeout') + } + const protocol = { + initialize: jest.fn(observer => observer.onComplete(metadata)) + } + const protocolSupplier = () => protocol + const connection = spyOnConnectionChannel({ channel, protocolSupplier }) + + await connection.connect('userAgent', {}) + + expect(channel.setupReceiveTimeout).not.toHaveBeenCalled() + } + ) + }) + + function spyOnConnectionChannel ({ + channel, + errorHandler, + address, + logger, + disableLosslessIntegers, + serversideRouting, + chuncker, + protocolSupplier + }) { + address = address || ServerAddress.fromUrl('bolt://localhost') + logger = logger || new Logger('info', () => {}) + return new ChannelConnection( + channel, + errorHandler, + address, + logger, + disableLosslessIntegers, + serversideRouting, + chuncker, + protocolSupplier + ) + } +}) From 23569ac7305f7abff027857269985b87e9a71110 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 23 Jul 2021 12:50:56 +0200 Subject: [PATCH 08/15] improve coverage in the bigint --- core/test/integer.test.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/core/test/integer.test.ts b/core/test/integer.test.ts index 7035a297c..32447e192 100644 --- a/core/test/integer.test.ts +++ b/core/test/integer.test.ts @@ -329,7 +329,7 @@ function forEachToNumberOrInfinityScenarios( } function forEachToNumberScenarios( - func: Consumer> + func: Consumer> ) { ;[ v('42', 42), @@ -337,6 +337,7 @@ function forEachToNumberScenarios( v('-999', -999), v('1000000000', 1000000000), v(1000000000, 1000000000), + v(BigInt(42), 42), v(Integer.MIN_SAFE_VALUE.toString(), Integer.MIN_SAFE_VALUE.toNumber()), v(Integer.MAX_SAFE_VALUE.toString(), Integer.MAX_SAFE_VALUE.toNumber()), v( @@ -988,7 +989,12 @@ function forEachFromStringScenarios( ].forEach(func) } -type Interable = Integer | number | { low: number; high: number } | string +type Interable = + | Integer + | number + | { low: number; high: number } + | string + | bigint function forEachFromValueScenarios( func: Consumer> ) { @@ -1003,9 +1009,13 @@ function forEachFromValueScenarios( function forEachStaticToNumberScenarios( func: Consumer> ) { - ;[v(Integer.ONE, 1), v('1', 1), v(1, 1), v({ low: 1, high: 0 }, 1)].forEach( - func - ) + ;[ + v(Integer.ONE, 1), + v('1', 1), + v(1, 1), + v({ low: 1, high: 0 }, 1), + v(BigInt(10), 10) + ].forEach(func) } function forEachStaticToStringScenarios( From a1e82f6d44f608add83eaaab3962d1c822e40b4d Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 23 Jul 2021 13:23:37 +0200 Subject: [PATCH 09/15] Test invalid connection hints --- .../src/connection/connection-channel.js | 25 ++++++--- .../connection-provider-routing.test.js | 1 - .../connection/connection-channel.test.js | 55 ++++++++++++++++++- 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/bolt-connection/src/connection/connection-channel.js b/bolt-connection/src/connection/connection-channel.js index defbedaea..fd51f0622 100644 --- a/bolt-connection/src/connection/connection-channel.js +++ b/bolt-connection/src/connection/connection-channel.js @@ -199,14 +199,23 @@ export default class ChannelConnection extends Connection { this.databaseId = dbConnectionId } - if ( - metadata.hints && - 'connection.recv_timeout_seconds' in metadata.hints - ) { - const receiveTimeout = - toNumber(metadata.hints['connection.recv_timeout_seconds']) * - 1000 - this._ch.setupReceiveTimeout(receiveTimeout) + if (metadata.hints) { + const receiveTimeoutRaw = + metadata.hints['connection.recv_timeout_seconds'] + if ( + receiveTimeoutRaw !== null && + receiveTimeoutRaw !== undefined + ) { + const receiveTimeoutInSeconds = toNumber(receiveTimeoutRaw) + if (receiveTimeoutInSeconds > 0) { + this._ch.setupReceiveTimeout(receiveTimeoutInSeconds * 1000) + } else { + this._log.info( + `Server located at ${this._address} supplied an invalid connection receive timeout value (${receiveTimeoutInSeconds}). ` + + 'Please, verify the server configuration and status because this can a symptom of a bigger issue.' + ) + } + } } } resolve(self) diff --git a/bolt-connection/test/connection-provider/connection-provider-routing.test.js b/bolt-connection/test/connection-provider/connection-provider-routing.test.js index ad0c34b44..f61b3c635 100644 --- a/bolt-connection/test/connection-provider/connection-provider-routing.test.js +++ b/bolt-connection/test/connection-provider/connection-provider-routing.test.js @@ -1719,7 +1719,6 @@ describe('#unit RoutingConnectionProvider', () => { database: 'databaseX' }) } catch (error) { - console.error('Message', error) expect(error instanceof Neo4jError).toBeTruthy() expect(error.code).toBe(SERVICE_UNAVAILABLE) expect(error.message).toContain( diff --git a/bolt-connection/test/connection/connection-channel.test.js b/bolt-connection/test/connection/connection-channel.test.js index 95d9576ee..5ece86084 100644 --- a/bolt-connection/test/connection/connection-channel.test.js +++ b/bolt-connection/test/connection/connection-channel.test.js @@ -19,6 +19,7 @@ import ChannelConnection from '../../src/connection/connection-channel' import { int, internal } from 'neo4j-driver-core' +import { add } from 'lodash' const { serverAddress: { ServerAddress }, @@ -53,7 +54,20 @@ describe('ChannelConnection', () => { } ) - it.each([[undefined], [null], [{}], [{ hint: null }], [{ hint: {} }]])( + it.each([ + [undefined], + [null], + [{}], + [{ hints: null }], + [{ hints: {} }], + [{ hints: { 'connection.recv_timeout_seconds': null } }], + [{ hints: { 'connection.recv_timeout_seconds': -1 } }], + [{ hints: { 'connection.recv_timeout_seconds': -1n } }], + [{ hints: { 'connection.recv_timeout_seconds': int(-1) } }], + [{ hints: { 'connection.recv_timeout_seconds': 0 } }], + [{ hints: { 'connection.recv_timeout_seconds': 0n } }], + [{ hints: { 'connection.recv_timeout_seconds': int(0) } }] + ])( 'should call not call this._ch.setupReceiveTimeout() when onComplete metadata is %o', async metadata => { const channel = { @@ -70,6 +84,45 @@ describe('ChannelConnection', () => { expect(channel.setupReceiveTimeout).not.toHaveBeenCalled() } ) + + it.each([ + [{ hints: { 'connection.recv_timeout_seconds': -1 } }], + [{ hints: { 'connection.recv_timeout_seconds': -1n } }], + [{ hints: { 'connection.recv_timeout_seconds': int(-1) } }], + [{ hints: { 'connection.recv_timeout_seconds': 0 } }], + [{ hints: { 'connection.recv_timeout_seconds': 0n } }], + [{ hints: { 'connection.recv_timeout_seconds': int(0) } }] + ])( + 'should call log an info when onComplete metadata is %o', + async metadata => { + const channel = { + setupReceiveTimeout: jest.fn().mockName('setupReceiveTimeout') + } + const protocol = { + initialize: jest.fn(observer => observer.onComplete(metadata)) + } + const address = ServerAddress.fromUrl('bolt://localhost') + const protocolSupplier = () => protocol + const loggerFunction = jest.fn().mockName('logger') + const logger = new Logger('info', loggerFunction) + const connection = spyOnConnectionChannel({ + channel, + protocolSupplier, + logger, + address + }) + + await connection.connect('userAgent', {}) + expect(loggerFunction).toHaveBeenCalledWith( + 'info', + `Connection [${ + connection._id + }][] Server located at ${address.asHostPort()} ` + + `supplied an invalid connection receive timeout value (${metadata.hints['connection.recv_timeout_seconds']}). ` + + 'Please, verify the server configuration and status because this can a symptom of a bigger issue.' + ) + } + ) }) function spyOnConnectionChannel ({ From c426699bbdc4bcbd5abb1efb8f8ef24ab407e01f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Fri, 23 Jul 2021 14:06:35 +0200 Subject: [PATCH 10/15] Update bolt-connection/src/channel/node/node-channel.js Co-authored-by: Robsdedude --- bolt-connection/src/channel/node/node-channel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt-connection/src/channel/node/node-channel.js b/bolt-connection/src/channel/node/node-channel.js index 5a0f17b19..c1bebe23a 100644 --- a/bolt-connection/src/channel/node/node-channel.js +++ b/bolt-connection/src/channel/node/node-channel.js @@ -336,7 +336,7 @@ export default class NodeChannel { /** * Setup the receive timeout for the channel. * - * @param {number} receiveTimeout The amount of time the channel will keep without receive any data before timeout (ms) + * @param {number} receiveTimeout How long the channel will wait for receiving data before timing out (ms) * @returns {void} */ setupReceiveTimeout (receiveTimeout) { From b10795c115047de4204876619aa315607961fec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Fri, 23 Jul 2021 14:08:05 +0200 Subject: [PATCH 11/15] Update bolt-connection/src/connection/connection-channel.js Co-authored-by: Robsdedude --- bolt-connection/src/connection/connection-channel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt-connection/src/connection/connection-channel.js b/bolt-connection/src/connection/connection-channel.js index fd51f0622..af4fe0524 100644 --- a/bolt-connection/src/connection/connection-channel.js +++ b/bolt-connection/src/connection/connection-channel.js @@ -212,7 +212,7 @@ export default class ChannelConnection extends Connection { } else { this._log.info( `Server located at ${this._address} supplied an invalid connection receive timeout value (${receiveTimeoutInSeconds}). ` + - 'Please, verify the server configuration and status because this can a symptom of a bigger issue.' + 'Please, verify the server configuration and status because this can be the symptom of a bigger issue.' ) } } From 3f9ba3d77148f823de87cb0cc17dd632d56a759a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Fri, 23 Jul 2021 14:08:32 +0200 Subject: [PATCH 12/15] Update bolt-connection/test/channel/node/node-channel.test.js Co-authored-by: Robsdedude --- bolt-connection/test/channel/node/node-channel.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt-connection/test/channel/node/node-channel.test.js b/bolt-connection/test/channel/node/node-channel.test.js index 096e3a918..bd649e854 100644 --- a/bolt-connection/test/channel/node/node-channel.test.js +++ b/bolt-connection/test/channel/node/node-channel.test.js @@ -85,7 +85,7 @@ describe('NodeChannel', () => { ]) }) - it('should not unsubscribe to the any on connect or on timeout if connectionTimeout is not set', () => { + it('should not unsubscribe from on connect nor from on timeout if connectionTimeout is not set', () => { const receiveTimeout = 42 const channel = createMockedChannel(true, { connectionTimeout: 0 }) From a4b29c5c3a6ebd65351b56f8da58d1fa2882b855 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 23 Jul 2021 14:12:33 +0200 Subject: [PATCH 13/15] Fix test string --- bolt-connection/test/connection/connection-channel.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt-connection/test/connection/connection-channel.test.js b/bolt-connection/test/connection/connection-channel.test.js index 5ece86084..4b861f75b 100644 --- a/bolt-connection/test/connection/connection-channel.test.js +++ b/bolt-connection/test/connection/connection-channel.test.js @@ -119,7 +119,7 @@ describe('ChannelConnection', () => { connection._id }][] Server located at ${address.asHostPort()} ` + `supplied an invalid connection receive timeout value (${metadata.hints['connection.recv_timeout_seconds']}). ` + - 'Please, verify the server configuration and status because this can a symptom of a bigger issue.' + 'Please, verify the server configuration and status because this can be the symptom of a bigger issue.' ) } ) From d77a6e41d1dc29985e9ce6f02863f699e8539d44 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 23 Jul 2021 14:51:26 +0200 Subject: [PATCH 14/15] Not allowing non-integer values --- bolt-connection/src/connection/connection-channel.js | 5 ++++- bolt-connection/test/connection/connection-channel.test.js | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/bolt-connection/src/connection/connection-channel.js b/bolt-connection/src/connection/connection-channel.js index af4fe0524..479765e77 100644 --- a/bolt-connection/src/connection/connection-channel.js +++ b/bolt-connection/src/connection/connection-channel.js @@ -207,7 +207,10 @@ export default class ChannelConnection extends Connection { receiveTimeoutRaw !== undefined ) { const receiveTimeoutInSeconds = toNumber(receiveTimeoutRaw) - if (receiveTimeoutInSeconds > 0) { + if ( + Number.isInteger(receiveTimeoutInSeconds) && + receiveTimeoutInSeconds > 0 + ) { this._ch.setupReceiveTimeout(receiveTimeoutInSeconds * 1000) } else { this._log.info( diff --git a/bolt-connection/test/connection/connection-channel.test.js b/bolt-connection/test/connection/connection-channel.test.js index 4b861f75b..93a3da3f3 100644 --- a/bolt-connection/test/connection/connection-channel.test.js +++ b/bolt-connection/test/connection/connection-channel.test.js @@ -86,12 +86,14 @@ describe('ChannelConnection', () => { ) it.each([ + [{ hints: { 'connection.recv_timeout_seconds': -1.5 } }], [{ hints: { 'connection.recv_timeout_seconds': -1 } }], [{ hints: { 'connection.recv_timeout_seconds': -1n } }], [{ hints: { 'connection.recv_timeout_seconds': int(-1) } }], [{ hints: { 'connection.recv_timeout_seconds': 0 } }], [{ hints: { 'connection.recv_timeout_seconds': 0n } }], - [{ hints: { 'connection.recv_timeout_seconds': int(0) } }] + [{ hints: { 'connection.recv_timeout_seconds': int(0) } }], + [{ hints: { 'connection.recv_timeout_seconds': 12.1 } }] ])( 'should call log an info when onComplete metadata is %o', async metadata => { From ad7ec103d3d757e5e0f425f5eb36d97a445a3f73 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 23 Jul 2021 17:13:54 +0200 Subject: [PATCH 15/15] Add support for the GetRoutingTable message --- .../src/rediscovery/routing-table.js | 14 ++++++++++-- test/internal/routing-table.test.js | 3 ++- testkit-backend/src/request-handlers.js | 22 +++++++++++++++++++ testkit-backend/src/skipped-tests.js | 6 ----- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/bolt-connection/src/rediscovery/routing-table.js b/bolt-connection/src/rediscovery/routing-table.js index 8184ad28b..c6918eb87 100644 --- a/bolt-connection/src/rediscovery/routing-table.js +++ b/bolt-connection/src/rediscovery/routing-table.js @@ -37,13 +37,21 @@ const MIN_ROUTERS = 1 * The routing table object used to determine the role of the servers in the driver. */ export default class RoutingTable { - constructor ({ database, routers, readers, writers, expirationTime } = {}) { + constructor ({ + database, + routers, + readers, + writers, + expirationTime, + ttl + } = {}) { this.database = database this.databaseName = database || 'default database' this.routers = routers || [] this.readers = readers || [] this.writers = writers || [] this.expirationTime = expirationTime || int(0) + this.ttl = ttl } /** @@ -139,6 +147,7 @@ export function createValidRoutingTable ( routerAddress, rawRoutingTable ) { + const ttl = rawRoutingTable.ttl const expirationTime = calculateExpirationTime(rawRoutingTable, routerAddress) const { routers, readers, writers } = parseServers( rawRoutingTable, @@ -153,7 +162,8 @@ export function createValidRoutingTable ( routers, readers, writers, - expirationTime + expirationTime, + ttl }) } diff --git a/test/internal/routing-table.test.js b/test/internal/routing-table.test.js index 378227bb4..f67c67af7 100644 --- a/test/internal/routing-table.test.js +++ b/test/internal/routing-table.test.js @@ -258,7 +258,8 @@ describe('#unit RoutingTable', () => { readers: readers.map(r => ServerAddress.fromUrl(r)), routers: routers.map(r => ServerAddress.fromUrl(r)), writers: writers.map(w => ServerAddress.fromUrl(w)), - expirationTime: calculateExpirationTime(currentTime, ttl) + expirationTime: calculateExpirationTime(currentTime, ttl), + ttl }) ) })) diff --git a/testkit-backend/src/request-handlers.js b/testkit-backend/src/request-handlers.js index 0f12d1927..5dfb37a72 100644 --- a/testkit-backend/src/request-handlers.js +++ b/testkit-backend/src/request-handlers.js @@ -270,3 +270,25 @@ export function ResolverResolutionCompleted ( const request = context.getResolverRequest(requestId) request.resolve(addresses) } + +export function GetRoutingTable (context, { driverId, database }, wire) { + const serverAddressToString = serverAddress => serverAddress.asHostPort() + const driver = context.getDriver(driverId) + const routingTable = + driver && + driver._getOrCreateConnectionProvider() && + driver._getOrCreateConnectionProvider()._routingTableRegistry && + driver._getOrCreateConnectionProvider()._routingTableRegistry.get(database) + + if (routingTable) { + wire.writeResponse('RoutingTable', { + database: routingTable.database, + ttl: routingTable.ttl, + readers: routingTable.readers.map(serverAddressToString), + writers: routingTable.writers.map(serverAddressToString), + routers: routingTable.routers.map(serverAddressToString) + }) + } else { + wire.writeError('Could not find routing table') + } +} diff --git a/testkit-backend/src/skipped-tests.js b/testkit-backend/src/skipped-tests.js index df7768e55..708d3ae4e 100644 --- a/testkit-backend/src/skipped-tests.js +++ b/testkit-backend/src/skipped-tests.js @@ -84,12 +84,6 @@ const skippedTests = [ ifEndsWith( 'test_should_fail_when_reading_from_unexpectedly_interrupting_reader_using_session_run' ) - ), - skip( - 'Needs to implement GetRoutingTable tk protocol message', - ifStartsWith( - 'stub.configuration_hints.test_connection_recv_timeout_seconds.TestRoutingConnectionRecvTimeout' - ) ) ]