diff --git a/.travis.yml b/.travis.yml index 61a7a79af..b00d6e695 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,10 @@ env: node_js: - lts/dubnium - lts/erbium - - 13 + # node 13.7 seems to have changed behavior of async iterators exiting early on streams + # if 13.8 still has this problem when it comes down I'll talk to the node team about the change + # in the mean time...peg to 13.6 + - 13.6 addons: postgresql: "10" diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 1c7faf210..83ec51e09 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -25,6 +25,10 @@ class PendingItem { } } +function throwOnDoubleRelease () { + throw new Error('Release called on client which has already been released to the pool.') +} + function promisify (Promise, callback) { if (callback) { return { callback: callback, result: undefined } @@ -244,7 +248,7 @@ class Pool extends EventEmitter { client.release = (err) => { if (released) { - throw new Error('Release called on client which has already been released to the pool.') + throwOnDoubleRelease() } released = true @@ -280,7 +284,8 @@ class Pool extends EventEmitter { _release (client, idleListener, err) { client.on('error', idleListener) - if (err || this.ending) { + // TODO(bmc): expose a proper, public interface _queryable and _ending + if (err || this.ending || !client._queryable || client._ending) { this._remove(client) this._pulseQueue() return diff --git a/packages/pg-pool/test/releasing-clients.js b/packages/pg-pool/test/releasing-clients.js new file mode 100644 index 000000000..da8e09c16 --- /dev/null +++ b/packages/pg-pool/test/releasing-clients.js @@ -0,0 +1,54 @@ +const Pool = require('../') + +const expect = require('expect.js') +const net = require('net') + +describe('releasing clients', () => { + it('removes a client which cannot be queried', async () => { + // make a pool w/ only 1 client + const pool = new Pool({ max: 1 }) + expect(pool.totalCount).to.eql(0) + const client = await pool.connect() + expect(pool.totalCount).to.eql(1) + expect(pool.idleCount).to.eql(0) + // reach into the client and sever its connection + client.connection.end() + + // wait for the client to error out + const err = await new Promise((resolve) => client.once('error', resolve)) + expect(err).to.be.ok() + expect(pool.totalCount).to.eql(1) + expect(pool.idleCount).to.eql(0) + + // try to return it to the pool - this removes it because its broken + client.release() + expect(pool.totalCount).to.eql(0) + expect(pool.idleCount).to.eql(0) + + // make sure pool still works + const { rows } = await pool.query('SELECT NOW()') + expect(rows).to.have.length(1) + await pool.end() + }) + + it('removes a client which is ending', async () => { + // make a pool w/ only 1 client + const pool = new Pool({ max: 1 }) + expect(pool.totalCount).to.eql(0) + const client = await pool.connect() + expect(pool.totalCount).to.eql(1) + expect(pool.idleCount).to.eql(0) + // end the client gracefully (but you shouldn't do this with pooled clients) + client.end() + + // try to return it to the pool + client.release() + expect(pool.totalCount).to.eql(0) + expect(pool.idleCount).to.eql(0) + + // make sure pool still works + const { rows } = await pool.query('SELECT NOW()') + expect(rows).to.have.length(1) + await pool.end() + }) +})