Fix BrowserChannel close when is already closing (#1169)

WebSockets can take a while for closing and the `WebSocket.close` is called when status is `WS_CLOSING`, an error is thrown. This situation can happen when the driver is being closed while session still releasing connections back to the pool or after a receive timeout while closing the session. BrowserChannel should wait for the original close socket finish whenever a new close request happens to avoid this kind of error and provides a graceful shutdown for driver and session.
neo4j · Dec 27, 2023 · 56d77c1 · 56d77c1
1 parent ce8bc52
commit 56d77c1
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 22 deletions.
diff --git 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/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
@@ -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
   }
 
   /**