diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index ab514fa88..e3cf6dec8 100644 --- a/packages/pg-pool/index.js +++ b/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') diff --git a/packages/pg-pool/test/connection-timeout.js b/packages/pg-pool/test/connection-timeout.js index c4fd1832b..c4ae87dd7 100644 --- a/packages/pg-pool/test/connection-timeout.js +++ b/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