fix(pg-pool): honor connectionTimeoutMillis and don't leak timed-out connections

packages/pg-pool/index.js

@@ -249,14 +249,24 @@ class Pool extends EventEmitter {
     let timeoutHit = false
     if (this.options.connectionTimeoutMillis) {
       tid = setTimeout(() => {
+        // the native client reports itself connected before its connect callback fires
+        if (!client.connection && client.isConnected()) {
+          return
+        }
+
+        this.log('ending client due to timeout')
+        timeoutHit = true
+        this._clients = this._clients.filter((c) => c !== client)
+        if (!pendingItem.timedOut) {
+          pendingItem.timedOut = true
+          pendingItem.callback(new Error('Connection terminated due to connection timeout'), undefined, NOOP)
+        }
+        this._pulseQueue()
+        client.removeAllListeners('error')
+        client.on('error', () => {})
         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.connection.end()
+        } else {
           client.end()
         }
       }, this.options.connectionTimeoutMillis)
@@ -282,6 +292,11 @@ class Pool extends EventEmitter {
         if (!pendingItem.timedOut) {
           pendingItem.callback(err, undefined, NOOP)
         }
+      } else if (timeoutHit) {
+        this.log('client connected after timeout, discarding')
+        client.removeAllListeners('error')
+        client.on('error', () => {})
+        client.end()
       } else {
         this.log('new client connected')
 

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

@@ -227,6 +227,35 @@ describe('connection timeout', () => {
     })
   })
 
+  it('does not retain a connection that establishes after the timeout (#3543)', (done) => {
+    const Client = require('pg').Client
+    const orgConnect = Client.prototype.connect
+
+    // Force the first connection to finish establishing only *after* the
+    // connection timeout has already fired (200ms > 100ms).
+    let first = true
+    Client.prototype.connect = function (cb) {
+      if (first) {
+        first = false
+        return setTimeout(() => orgConnect.call(this, cb), 200)
+      }
+      return orgConnect.call(this, cb)
+    }
+
+    const pool = new Pool({ connectionTimeoutMillis: 100, max: 1 })
+    pool.connect((err, client) => {
+      Client.prototype.connect = orgConnect
+      // The checkout must fail with a timeout rather than silently resolving
+      // ~200ms later, and the timed-out connection must not be retained.
+      expect(err).to.be.an(Error)
+      expect(err.message).to.contain('timeout')
+      expect(client).to.be(undefined)
+      expect(pool.totalCount).to.equal(0)
+      expect(pool.idleCount).to.equal(0)
+      pool.end(done)
+    })
+  })
+
   it('should connect if timeout is passed, but native client in connected state', (done) => {
     const Client = require('pg').native.Client