Prevent requeuing a broken client

johanneswuerbach · brianc · commit b9f67a0d9577 · 2020-01-23T11:12:39.000-06:00
If a client is not queryable, the pool should prevent requeuing instead
of strictly enforcing errors to be propagated back to it.
diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js
@@ -25,6 +25,31 @@ class PendingItem {
   }
 }
 
+function throwOnRelease () {
+  throw new Error('Release called on client which has already been released to the pool.')
+}
+
+function release (client, err) {
+  client.release = throwOnRelease
+  if (err || this.ending || !client._queryable || client._ending) {
+    this._remove(client)
+    this._pulseQueue()
+    return
+  }
+
+  // idle timeout
+  let tid
+  if (this.options.idleTimeoutMillis) {
+    tid = setTimeout(() => {
+      this.log('remove idle client')
+      this._remove(client)
+    }, this.options.idleTimeoutMillis)
+  }
+
+  this._idle.push(new IdleItem(client, tid))
+  this._pulseQueue()
+}
+
 function promisify (Promise, callback) {
   if (callback) {
     return { callback: callback, result: undefined }
diff --git a/packages/pg-pool/test/error-handling.js b/packages/pg-pool/test/error-handling.js
@@ -181,6 +181,40 @@ describe('pool error handling', function () {
     })
   })
 
+  describe('releasing a not queryable client', () => {
+    it('removes the client from the pool', (done) => {
+      const pool = new Pool({ max: 1 })
+      const connectionError = new Error('connection failed')
+
+      pool.once('error', () => {
+        // Ignore error on pool
+      })
+
+      pool.connect((err, client) => {
+        expect(err).to.be(undefined)
+
+        client.once('error', (err) => {
+          expect(err).to.eql(connectionError)
+
+          // Releasing the client should remove it from the pool,
+          // whether called with an error or not
+          client.release()
+
+          // Verify that the pool is still usuable and new client has been
+          // created
+          pool.query('SELECT $1::text as name', ['brianc'], (err, res) => {
+            expect(err).to.be(undefined)
+            expect(res.rows).to.eql([{ name: 'brianc' }])
+
+            pool.end(done)
+          })
+        })
+
+        client.emit('error', connectionError)
+      })
+    })
+  })
+
   describe('pool with lots of errors', () => {
     it('continues to work and provide new clients', co.wrap(function* () {
       const pool = new Pool({ max: 1 })
