fix: Connection timeout handling for native clients in connected state

Now, when using the native client with a connection timeout, the pool could incorrectly destroy or end a client that was already connected, due to not distinguishing between connection states. This caused issues such as brianc/node-pg-native#49, where a native client that had already established a connection could be forcefully closed if the connection callback was delayed past the timeout.

Author: maxbronnikov10

packages/pg-pool/index.js

@@ -241,10 +241,16 @@ class Pool extends EventEmitter {
     let timeoutHit = false
     if (this.options.connectionTimeoutMillis) {
       tid = setTimeout(() => {
-        this.log('ending client due to timeout')
-        timeoutHit = true
-        // force kill the node driver, and let libpq do its teardown
-        client.connection ? client.connection.stream.destroy() : client.end()
+        if (client.connection) {
+          this.log('ending client due to timeout')
+          timeoutHit = true
+          client.connection.stream.destroy()
+        } else if (!client.isConnected()) {
+          this.log('ending client due to timeout')
+          timeoutHit = true
+          // force kill the node driver, and let libpq do its teardown
+          client.end()
+        }
       }, this.options.connectionTimeoutMillis)
     }
 

packages/pg-pool/test/connection-timeout.js

@@ -226,4 +226,25 @@ describe('connection timeout', () => {
       })
     })
   })
+
+  it('should connect if timeout is passed, but native client in connected state', (done) => {
+    const Client = require('pg').native.Client
+
+    Client.prototype.connect = function (cb) {
+      this._connected = true
+
+      return setTimeout(() => {
+        cb()
+      }, 200)
+    }
+
+    const pool = new Pool({ connectionTimeoutMillis: 100, port: this.port, host: 'localhost' }, Client)
+
+    pool.connect((err, client, release) => {
+      expect(err).to.be(undefined)
+      expect(client).to.not.be(undefined)
+      expect(client.isConnected()).to.be(true)
+      done()
+    })
+  })
 })

packages/pg/lib/native/client.js

@@ -250,7 +250,10 @@ Client.prototype.end = function (cb) {
       cb = (err) => (err ? reject(err) : resolve())
     })
   }
+
   this.native.end(function () {
+    self._connected = false
+
     self._errorAllQueries(new Error('Connection terminated'))
 
     process.nextTick(() => {
@@ -306,3 +309,7 @@ Client.prototype.setTypeParser = function (oid, format, parseFn) {
 Client.prototype.getTypeParser = function (oid, format) {
   return this._types.getTypeParser(oid, format)
 }
+
+Client.prototype.isConnected = function () {
+  return this._connected
+}