From bfd3ae9a9e5880147e0c4b79473f46fbcc217a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Thu, 21 Dec 2023 16:19:11 +0100 Subject: [PATCH 1/7] Fix connection getting timeout while idle --- .../src/channel/browser/browser-channel.js | 29 +++++++++------ .../connection-provider-pooled.js | 4 +-- .../src/connection/connection-channel.js | 15 ++++++-- .../channel/browser/browser-channel.test.js | 35 +++++++++++++++++++ .../channel/browser/browser-channel.js | 29 +++++++++------ .../connection-provider-pooled.js | 4 +-- .../connection/connection-channel.js | 15 ++++++-- .../connection-provider-pooled.test.js | 25 +++++++++++++ .../test/internal/fake-connection.js | 13 ++++++- .../src/skipped-tests/browser.js | 5 +-- 10 files changed, 141 insertions(+), 33 deletions(-) diff --git a/packages/bolt-connection/src/channel/browser/browser-channel.js b/packages/bolt-connection/src/channel/browser/browser-channel.js index 945783a04..4d7fed9e8 100644 --- a/packages/bolt-connection/src/channel/browser/browser-channel.js +++ b/packages/bolt-connection/src/channel/browser/browser-channel.js @@ -54,6 +54,7 @@ export default class WebSocketChannel { this._receiveTimeout = null this._receiveTimeoutStarted = false this._receiveTimeoutId = null + this._closingPromise = null const { scheme, error } = determineWebSocketScheme(config, protocolSupplier) if (error) { @@ -163,17 +164,23 @@ export default class WebSocketChannel { * @returns {Promise} A promise that will be resolved after channel is closed */ close () { - return new Promise((resolve, reject) => { - this._clearConnectionTimeout() - if (this._ws && this._ws.readyState !== WS_CLOSED) { - this._open = false - this.stopReceiveTimeout() - this._ws.onclose = () => resolve() - this._ws.close() - } else { - resolve() - } - }) + if (this._closingPromise === null) { + this._closingPromise = new Promise((resolve, reject) => { + this._clearConnectionTimeout() + if (this._ws && this._ws.readyState !== WS_CLOSED) { + this._open = false + this.stopReceiveTimeout() + this._ws.onclose = () => { + resolve() + } + this._ws.close() + } else { + resolve() + } + }) + } + + return this._closingPromise } /** diff --git a/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js b/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js index d15de0cd9..cb3875bf4 100644 --- a/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js +++ b/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js @@ -226,11 +226,11 @@ export default class PooledConnectionProvider extends ConnectionProvider { } static _installIdleObserverOnConnection (conn, observer) { - conn._queueObserver(observer) + conn._setIdle(observer) } static _removeIdleObserverOnConnection (conn) { - conn._updateCurrentObserver() + conn._unsetIdle() } _handleSecurityError (error, address, connection) { diff --git a/packages/bolt-connection/src/connection/connection-channel.js b/packages/bolt-connection/src/connection/connection-channel.js index 43f4fe121..c35211119 100644 --- a/packages/bolt-connection/src/connection/connection-channel.js +++ b/packages/bolt-connection/src/connection/connection-channel.js @@ -125,6 +125,7 @@ export default class ChannelConnection extends Connection { ) { super(errorHandler) this._authToken = null + this._idle = false this._reseting = false this._resetObservers = [] this._id = idGenerator++ @@ -397,12 +398,19 @@ export default class ChannelConnection extends Connection { * * @param {any} observer */ - _queueObserver (observer) { + _setIdle (observer) { + this._idle = true + this._ch.stopReceiveTimeout() return this._protocol.queueObserverIfProtocolIsNotBroken(observer) } + _unsetIdle () { + this._idle = false + this._updateCurrentObserver() + } + hasOngoingObservableRequests () { - return this._protocol.hasOngoingObservableRequests() + return !this._idle && this._protocol.hasOngoingObservableRequests() } /** @@ -500,6 +508,9 @@ export default class ChannelConnection extends Connection { * @param {number} requestsNumber Ongoing requests number */ _handleOngoingRequestsNumberChange (requestsNumber) { + if (this._idle) { + return + } if (requestsNumber === 0) { this._ch.stopReceiveTimeout() } else { diff --git a/packages/bolt-connection/test/channel/browser/browser-channel.test.js b/packages/bolt-connection/test/channel/browser/browser-channel.test.js index 4cc9c4dac..13451e043 100644 --- a/packages/bolt-connection/test/channel/browser/browser-channel.test.js +++ b/packages/bolt-connection/test/channel/browser/browser-channel.test.js @@ -323,6 +323,41 @@ describe('WebSocketChannel', () => { fakeSetTimeout.uninstall() } }) + + it('should return always the same promise', async () => { + const fakeSetTimeout = setTimeoutMock.install() + try { + // do not execute setTimeout callbacks + fakeSetTimeout.pause() + const address = ServerAddress.fromUrl('bolt://localhost:8989') + const driverConfig = { connectionTimeout: 4242 } + const channelConfig = new ChannelConfig( + address, + driverConfig, + SERVICE_UNAVAILABLE + ) + webSocketChannel = new WebSocketChannel( + channelConfig, + undefined, + createWebSocketFactory(WS_OPEN) + ) + + const promise1 = webSocketChannel.close() + const promise2 = webSocketChannel.close() + + expect(promise1).toBe(promise2) + + await Promise.all([promise1, promise2]) + + const promise3 = webSocketChannel.close() + + expect(promise3).toBe(promise2) + + await promise3 + } finally { + fakeSetTimeout.uninstall() + } + }) }) describe('.setupReceiveTimeout()', () => { diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js index 22f743cea..1124d49a0 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js @@ -54,6 +54,7 @@ export default class WebSocketChannel { this._receiveTimeout = null this._receiveTimeoutStarted = false this._receiveTimeoutId = null + this._closingPromise = null const { scheme, error } = determineWebSocketScheme(config, protocolSupplier) if (error) { @@ -163,17 +164,23 @@ export default class WebSocketChannel { * @returns {Promise} A promise that will be resolved after channel is closed */ close () { - return new Promise((resolve, reject) => { - this._clearConnectionTimeout() - if (this._ws && this._ws.readyState !== WS_CLOSED) { - this._open = false - this.stopReceiveTimeout() - this._ws.onclose = () => resolve() - this._ws.close() - } else { - resolve() - } - }) + if (this._closingPromise === null) { + this._closingPromise = new Promise((resolve, reject) => { + this._clearConnectionTimeout() + if (this._ws && this._ws.readyState !== WS_CLOSED) { + this._open = false + this.stopReceiveTimeout() + this._ws.onclose = () => { + resolve() + } + this._ws.close() + } else { + resolve() + } + }) + } + + return this._closingPromise } /** diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js index 0476cc687..7f9a5ef17 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js @@ -226,11 +226,11 @@ export default class PooledConnectionProvider extends ConnectionProvider { } static _installIdleObserverOnConnection (conn, observer) { - conn._queueObserver(observer) + conn._setIdle(observer) } static _removeIdleObserverOnConnection (conn) { - conn._updateCurrentObserver() + conn._unsetIdle() } _handleSecurityError (error, address, connection) { diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js index eb6cc3777..a53897445 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js @@ -125,6 +125,7 @@ export default class ChannelConnection extends Connection { ) { super(errorHandler) this._authToken = null + this._idle = false this._reseting = false this._resetObservers = [] this._id = idGenerator++ @@ -397,12 +398,19 @@ export default class ChannelConnection extends Connection { * * @param {any} observer */ - _queueObserver (observer) { + _setIdle (observer) { + this._idle = true + this._ch.stopReceiveTimeout() return this._protocol.queueObserverIfProtocolIsNotBroken(observer) } + _unsetIdle () { + this._idle = false + this._updateCurrentObserver() + } + hasOngoingObservableRequests () { - return this._protocol.hasOngoingObservableRequests() + return !this._idle && this._protocol.hasOngoingObservableRequests() } /** @@ -500,6 +508,9 @@ export default class ChannelConnection extends Connection { * @param {number} requestsNumber Ongoing requests number */ _handleOngoingRequestsNumberChange (requestsNumber) { + if (this._idle) { + return + } if (requestsNumber === 0) { this._ch.stopReceiveTimeout() } else { diff --git a/packages/neo4j-driver/test/internal/connection-provider-pooled.test.js b/packages/neo4j-driver/test/internal/connection-provider-pooled.test.js index fb13d9271..e49e279a1 100644 --- a/packages/neo4j-driver/test/internal/connection-provider-pooled.test.js +++ b/packages/neo4j-driver/test/internal/connection-provider-pooled.test.js @@ -71,4 +71,29 @@ describe('#unit PooledConnectionProvider', () => { clock.uninstall() } }) + + it('_installIdleObserverOnConnection should set connection as idle', () => { + const connection = new FakeConnection() + const observer = { onCompleted: () => {} } + + PooledConnectionProvider._installIdleObserverOnConnection(connection, observer) + + expect(connection._idle).toBe(true) + expect(connection._idleObserver).toBe(observer) + }) + + it('_removeIdleObserverOnConnection should unset connection as idle', () => { + const connection = new FakeConnection() + const observer = { onCompleted: () => {} } + + PooledConnectionProvider._installIdleObserverOnConnection(connection, observer) + + expect(connection._idle).toBe(true) + expect(connection._idleObserver).toBe(observer) + + PooledConnectionProvider._removeIdleObserverOnConnection(connection) + + expect(connection._idle).toBe(false) + expect(connection._idleObserver).toBe(null) + }) }) diff --git a/packages/neo4j-driver/test/internal/fake-connection.js b/packages/neo4j-driver/test/internal/fake-connection.js index 82089b968..112133f5a 100644 --- a/packages/neo4j-driver/test/internal/fake-connection.js +++ b/packages/neo4j-driver/test/internal/fake-connection.js @@ -39,7 +39,8 @@ export default class FakeConnection extends Connection { this._databaseId = null this._requestRoutingInformationMock = null this._creationTimestamp = Date.now() - + this._idle = false + this._idleObserver = null this.resetInvoked = 0 this.releaseInvoked = 0 this.seenQueries = [] @@ -101,6 +102,16 @@ export default class FakeConnection extends Connection { return this._idleTimestamp } + _setIdle (observer) { + this._idle = true + this._idleObserver = observer + } + + _unsetIdle () { + this._idle = false + this._idleObserver = null + } + protocol () { // return fake protocol object that simply records seen queries and parameters return { diff --git a/packages/testkit-backend/src/skipped-tests/browser.js b/packages/testkit-backend/src/skipped-tests/browser.js index b8313e21d..45bf952b3 100644 --- a/packages/testkit-backend/src/skipped-tests/browser.js +++ b/packages/testkit-backend/src/skipped-tests/browser.js @@ -9,10 +9,11 @@ const skippedTests = [ ifEndsWith('test_should_check_multi_db_support'), ifEquals('stub.disconnects.test_disconnects.TestDisconnects.test_fail_on_reset'), ifEquals('stub.tx_begin_parameters.test_tx_begin_parameters.TestTxBeginParameters.test_impersonation_fails_on_v4x3'), - ifEquals('stub.session_run_parameters.test_session_run_parameters.TestSessionRunParameters.test_impersonation_fails_on_v4x3') + ifEquals('stub.session_run_parameters.test_session_run_parameters.TestSessionRunParameters.test_impersonation_fails_on_v4x3'), + ifEquals('stub.driver_parameters.test_liveness_check.TestLivenessCheck.test_timeout_recv_timeout') ), skip( - 'TLS Tests not implemented for browwer', + 'TLS Tests not implemented for browser', ifStartsWith('tls') ) ] From 4d48b6445a748dd03d388296d77522f19ed3d713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Fri, 22 Dec 2023 12:31:25 +0100 Subject: [PATCH 2/7] Add some tests --- .../connection/connection-channel.test.js | 143 +++++++++++++++++- 1 file changed, 139 insertions(+), 4 deletions(-) diff --git a/packages/bolt-connection/test/connection/connection-channel.test.js b/packages/bolt-connection/test/connection/connection-channel.test.js index e9cfead02..95edaa5bb 100644 --- a/packages/bolt-connection/test/connection/connection-channel.test.js +++ b/packages/bolt-connection/test/connection/connection-channel.test.js @@ -559,7 +559,7 @@ describe('ChannelConnection', () => { }) describe('.__handleOngoingRequestsNumberChange()', () => { - it('should call channel.stopReceiveTimeout when requets number equals to 0', () => { + it('should call channel.stopReceiveTimeout when requests number equals to 0', () => { const channel = { stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') @@ -571,7 +571,7 @@ describe('ChannelConnection', () => { expect(channel.stopReceiveTimeout).toHaveBeenCalledTimes(1) }) - it('should not call channel.startReceiveTimeout when requets number equals to 0', () => { + it('should not call channel.startReceiveTimeout when requests number equals to 0', () => { const channel = { stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') @@ -585,7 +585,7 @@ describe('ChannelConnection', () => { it.each([ [1], [2], [3], [5], [8], [13], [3000] - ])('should call channel.startReceiveTimeout when requets number equals to %d', (requests) => { + ])('should call channel.startReceiveTimeout when requests number equals to %d', (requests) => { const channel = { stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') @@ -599,7 +599,7 @@ describe('ChannelConnection', () => { it.each([ [1], [2], [3], [5], [8], [13], [3000] - ])('should not call channel.stopReceiveTimeout when requets number equals to %d', (requests) => { + ])('should not call channel.stopReceiveTimeout when requests number equals to %d', (requests) => { const channel = { stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') @@ -610,6 +610,68 @@ describe('ChannelConnection', () => { expect(channel.stopReceiveTimeout).toHaveBeenCalledTimes(0) }) + + it.each([ + [0], [1], [2], [3], [5], [8], [13], [3000] + ])('should not call channel.stopReceiveTimeout or startReceiveTimeout when requests number equals to %d and connection is idle', (requests) => { + const channel = { + stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), + startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') + } + const protocol = { + queueObserverIfProtocolIsNotBroken: jest.fn(() => {}) + } + const connection = spyOnConnectionChannel({ channel, protocolSupplier: () => protocol }) + connection._setIdle({}) + channel.stopReceiveTimeout.mockClear() + + connection._handleOngoingRequestsNumberChange(requests) + + expect(channel.stopReceiveTimeout).toHaveBeenCalledTimes(0) + expect(channel.startReceiveTimeout).toHaveBeenCalledTimes(0) + }) + + it.each([ + [1], [2], [3], [5], [8], [13], [3000] + ])('should call channel.startReceiveTimeout when requests number equals to %d and connection is not idle anymore', (requests) => { + const channel = { + stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), + startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') + } + const protocol = { + queueObserverIfProtocolIsNotBroken: jest.fn(() => {}), + updateCurrentObserver: jest.fn(() => {}) + } + const connection = spyOnConnectionChannel({ channel, protocolSupplier: () => protocol }) + connection._setIdle({}) + connection._unsetIdle() + channel.stopReceiveTimeout.mockClear() + + connection._handleOngoingRequestsNumberChange(requests) + + expect(channel.stopReceiveTimeout).toHaveBeenCalledTimes(0) + expect(channel.startReceiveTimeout).toHaveBeenCalledTimes(1) + }) + + it('should call channel.stopReceiveTimeout when requests number equals to 0 and connection is not idle anymore', () => { + const channel = { + stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout'), + startReceiveTimeout: jest.fn().mockName('startReceiveTimeout') + } + const protocol = { + queueObserverIfProtocolIsNotBroken: jest.fn(() => {}), + updateCurrentObserver: jest.fn(() => {}) + } + const connection = spyOnConnectionChannel({ channel, protocolSupplier: () => protocol }) + connection._setIdle({}) + connection._unsetIdle() + channel.stopReceiveTimeout.mockClear() + + connection._handleOngoingRequestsNumberChange(0) + + expect(channel.stopReceiveTimeout).toHaveBeenCalledTimes(1) + expect(channel.startReceiveTimeout).toHaveBeenCalledTimes(0) + }) }) describe('.resetAndFlush()', () => { @@ -1181,6 +1243,44 @@ describe('ChannelConnection', () => { }) describe('.hasOngoingObservableRequests()', () => { + it('should return false if connection is idle', () => { + const protocol = { + hasOngoingObservableRequests: jest.fn(() => true), + queueObserverIfProtocolIsNotBroken: jest.fn(() => {}) + } + const channel = { + stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout') + } + + const connection = spyOnConnectionChannel({ protocolSupplier: () => protocol, channel }) + connection._setIdle({}) + + const result = connection.hasOngoingObservableRequests() + + expect(result).toBe(false) + expect(protocol.hasOngoingObservableRequests).not.toBeCalledWith() + }) + + it('should redirect request to the protocol when connection is not idle anymore', () => { + const protocol = { + hasOngoingObservableRequests: jest.fn(() => true), + queueObserverIfProtocolIsNotBroken: jest.fn(() => {}), + updateCurrentObserver: jest.fn(() => {}) + } + const channel = { + stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout') + } + + const connection = spyOnConnectionChannel({ protocolSupplier: () => protocol, channel }) + connection._setIdle({}) + connection._unsetIdle() + + const result = connection.hasOngoingObservableRequests() + + expect(result).toBe(true) + expect(protocol.hasOngoingObservableRequests).toBeCalledWith() + }) + it('should call redirect request to the protocol', () => { const protocol = { hasOngoingObservableRequests: jest.fn(() => true) @@ -1195,6 +1295,41 @@ describe('ChannelConnection', () => { }) }) + describe('._setIdle()', () => { + it('should stop receive timeout and enqueue observer', () => { + const protocol = { + queueObserverIfProtocolIsNotBroken: jest.fn(() => {}) + } + const channel = { + stopReceiveTimeout: jest.fn().mockName('stopReceiveTimeout') + } + const observer = { + onComplete: () => {} + } + + const connection = spyOnConnectionChannel({ protocolSupplier: () => protocol, channel }) + + connection._setIdle(observer) + + expect(channel.stopReceiveTimeout).toBeCalledTimes(1) + expect(protocol.queueObserverIfProtocolIsNotBroken).toBeCalledWith(observer) + }) + }) + + describe('._unsetIdle()', () => { + it('should update current observer', () => { + const protocol = { + updateCurrentObserver: jest.fn(() => {}) + } + + const connection = spyOnConnectionChannel({ protocolSupplier: () => protocol }) + + connection._unsetIdle() + + expect(protocol.updateCurrentObserver).toBeCalledTimes(1) + }) + }) + function spyOnConnectionChannel ({ channel, errorHandler, From 2fdfe11f05db472e2c65c154e4b6c27f4f1cd884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Fri, 22 Dec 2023 13:23:07 +0100 Subject: [PATCH 3/7] Adjust code because of existing tests --- .../src/connection/connection-channel.js | 15 ++++++++++++++- .../connection/connection-channel.js | 15 ++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/bolt-connection/src/connection/connection-channel.js b/packages/bolt-connection/src/connection/connection-channel.js index c35211119..7806db219 100644 --- a/packages/bolt-connection/src/connection/connection-channel.js +++ b/packages/bolt-connection/src/connection/connection-channel.js @@ -394,7 +394,7 @@ export default class ChannelConnection extends Connection { } /** - * This method still here because it's used by the {@link PooledConnectionProvider} + * This method is used by the {@link PooledConnectionProvider} * * @param {any} observer */ @@ -404,11 +404,24 @@ export default class ChannelConnection extends Connection { return this._protocol.queueObserverIfProtocolIsNotBroken(observer) } + /** + * This method is used by the {@link PooledConnectionProvider} + * + */ _unsetIdle () { this._idle = false this._updateCurrentObserver() } + /** + * This method still here because of the connection-channel.tests.js + * + * @param {any} observer + */ + _queueObserver (observer) { + return this._protocol.queueObserverIfProtocolIsNotBroken(observer) + } + hasOngoingObservableRequests () { return !this._idle && this._protocol.hasOngoingObservableRequests() } diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js index a53897445..890192c28 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js @@ -394,7 +394,7 @@ export default class ChannelConnection extends Connection { } /** - * This method still here because it's used by the {@link PooledConnectionProvider} + * This method is used by the {@link PooledConnectionProvider} * * @param {any} observer */ @@ -404,11 +404,24 @@ export default class ChannelConnection extends Connection { return this._protocol.queueObserverIfProtocolIsNotBroken(observer) } + /** + * This method is used by the {@link PooledConnectionProvider} + * + */ _unsetIdle () { this._idle = false this._updateCurrentObserver() } + /** + * This method still here because of the connection-channel.tests.js + * + * @param {any} observer + */ + _queueObserver (observer) { + return this._protocol.queueObserverIfProtocolIsNotBroken(observer) + } + hasOngoingObservableRequests () { return !this._idle && this._protocol.hasOngoingObservableRequests() } From 6cc893e89a8c1ffc80764bf5ef340db8cbb13465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Wed, 27 Dec 2023 09:39:15 +0100 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Robsdedude --- packages/bolt-connection/src/connection/connection-channel.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/bolt-connection/src/connection/connection-channel.js b/packages/bolt-connection/src/connection/connection-channel.js index 7806db219..2b7a9690a 100644 --- a/packages/bolt-connection/src/connection/connection-channel.js +++ b/packages/bolt-connection/src/connection/connection-channel.js @@ -406,7 +406,6 @@ export default class ChannelConnection extends Connection { /** * This method is used by the {@link PooledConnectionProvider} - * */ _unsetIdle () { this._idle = false From 6533fa4ccbc91e3fd412d901f5a9b6ee4e3ca62a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Wed, 27 Dec 2023 10:16:26 +0100 Subject: [PATCH 5/7] remove unused return --- packages/bolt-connection/src/connection/connection-channel.js | 2 +- .../lib/bolt-connection/connection/connection-channel.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/bolt-connection/src/connection/connection-channel.js b/packages/bolt-connection/src/connection/connection-channel.js index 2b7a9690a..73ec2a821 100644 --- a/packages/bolt-connection/src/connection/connection-channel.js +++ b/packages/bolt-connection/src/connection/connection-channel.js @@ -401,7 +401,7 @@ export default class ChannelConnection extends Connection { _setIdle (observer) { this._idle = true this._ch.stopReceiveTimeout() - return this._protocol.queueObserverIfProtocolIsNotBroken(observer) + this._protocol.queueObserverIfProtocolIsNotBroken(observer) } /** diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js index 890192c28..3c927739c 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js @@ -406,7 +406,6 @@ export default class ChannelConnection extends Connection { /** * This method is used by the {@link PooledConnectionProvider} - * */ _unsetIdle () { this._idle = false From b29a35c15335d15b526b550a3784174b12d6b4af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Wed, 27 Dec 2023 10:34:51 +0100 Subject: [PATCH 6/7] sync deno --- .../lib/bolt-connection/connection/connection-channel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js index 3c927739c..5efd40da1 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js @@ -401,7 +401,7 @@ export default class ChannelConnection extends Connection { _setIdle (observer) { this._idle = true this._ch.stopReceiveTimeout() - return this._protocol.queueObserverIfProtocolIsNotBroken(observer) + this._protocol.queueObserverIfProtocolIsNotBroken(observer) } /** From 37468dcd3a2801e7da79f34042d304cca7a800c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Wed, 27 Dec 2023 11:07:50 +0100 Subject: [PATCH 7/7] Revert browser-channel changes --- .../src/channel/browser/browser-channel.js | 29 ++++++--------- .../channel/browser/browser-channel.test.js | 35 ------------------- .../channel/browser/browser-channel.js | 29 ++++++--------- 3 files changed, 22 insertions(+), 71 deletions(-) diff --git a/packages/bolt-connection/src/channel/browser/browser-channel.js b/packages/bolt-connection/src/channel/browser/browser-channel.js index 4d7fed9e8..945783a04 100644 --- a/packages/bolt-connection/src/channel/browser/browser-channel.js +++ b/packages/bolt-connection/src/channel/browser/browser-channel.js @@ -54,7 +54,6 @@ export default class WebSocketChannel { this._receiveTimeout = null this._receiveTimeoutStarted = false this._receiveTimeoutId = null - this._closingPromise = null const { scheme, error } = determineWebSocketScheme(config, protocolSupplier) if (error) { @@ -164,23 +163,17 @@ export default class WebSocketChannel { * @returns {Promise} A promise that will be resolved after channel is closed */ close () { - if (this._closingPromise === null) { - this._closingPromise = new Promise((resolve, reject) => { - this._clearConnectionTimeout() - if (this._ws && this._ws.readyState !== WS_CLOSED) { - this._open = false - this.stopReceiveTimeout() - this._ws.onclose = () => { - resolve() - } - this._ws.close() - } else { - resolve() - } - }) - } - - return this._closingPromise + return new Promise((resolve, reject) => { + this._clearConnectionTimeout() + if (this._ws && this._ws.readyState !== WS_CLOSED) { + this._open = false + this.stopReceiveTimeout() + this._ws.onclose = () => resolve() + this._ws.close() + } else { + resolve() + } + }) } /** diff --git a/packages/bolt-connection/test/channel/browser/browser-channel.test.js b/packages/bolt-connection/test/channel/browser/browser-channel.test.js index 13451e043..4cc9c4dac 100644 --- a/packages/bolt-connection/test/channel/browser/browser-channel.test.js +++ b/packages/bolt-connection/test/channel/browser/browser-channel.test.js @@ -323,41 +323,6 @@ describe('WebSocketChannel', () => { fakeSetTimeout.uninstall() } }) - - it('should return always the same promise', async () => { - const fakeSetTimeout = setTimeoutMock.install() - try { - // do not execute setTimeout callbacks - fakeSetTimeout.pause() - const address = ServerAddress.fromUrl('bolt://localhost:8989') - const driverConfig = { connectionTimeout: 4242 } - const channelConfig = new ChannelConfig( - address, - driverConfig, - SERVICE_UNAVAILABLE - ) - webSocketChannel = new WebSocketChannel( - channelConfig, - undefined, - createWebSocketFactory(WS_OPEN) - ) - - const promise1 = webSocketChannel.close() - const promise2 = webSocketChannel.close() - - expect(promise1).toBe(promise2) - - await Promise.all([promise1, promise2]) - - const promise3 = webSocketChannel.close() - - expect(promise3).toBe(promise2) - - await promise3 - } finally { - fakeSetTimeout.uninstall() - } - }) }) describe('.setupReceiveTimeout()', () => { diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js index 1124d49a0..22f743cea 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js @@ -54,7 +54,6 @@ export default class WebSocketChannel { this._receiveTimeout = null this._receiveTimeoutStarted = false this._receiveTimeoutId = null - this._closingPromise = null const { scheme, error } = determineWebSocketScheme(config, protocolSupplier) if (error) { @@ -164,23 +163,17 @@ export default class WebSocketChannel { * @returns {Promise} A promise that will be resolved after channel is closed */ close () { - if (this._closingPromise === null) { - this._closingPromise = new Promise((resolve, reject) => { - this._clearConnectionTimeout() - if (this._ws && this._ws.readyState !== WS_CLOSED) { - this._open = false - this.stopReceiveTimeout() - this._ws.onclose = () => { - resolve() - } - this._ws.close() - } else { - resolve() - } - }) - } - - return this._closingPromise + return new Promise((resolve, reject) => { + this._clearConnectionTimeout() + if (this._ws && this._ws.readyState !== WS_CLOSED) { + this._open = false + this.stopReceiveTimeout() + this._ws.onclose = () => resolve() + this._ws.close() + } else { + resolve() + } + }) } /**