From ccc25f4915a3444845ce3c26ff142fae4bbc90e3 Mon Sep 17 00:00:00 2001 From: Brian C Date: Mon, 13 Jan 2020 13:00:18 -0600 Subject: [PATCH 1/9] Drop support for EOL versions of node (#2062) * Drop support for EOL versions of node * Re-add testing for node@8.x * Revert changes to .travis.yml * Update packages/pg-pool/package.json Co-Authored-By: Charmander <~@charmander.me> Co-authored-by: Charmander <~@charmander.me> --- packages/pg-pool/package.json | 2 +- packages/pg/Makefile | 4 +--- packages/pg/package.json | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/pg-pool/package.json b/packages/pg-pool/package.json index dc0275699..a1fa1465e 100644 --- a/packages/pg-pool/package.json +++ b/packages/pg-pool/package.json @@ -34,6 +34,6 @@ "pg-cursor": "^1.3.0" }, "peerDependencies": { - "pg": ">5.0" + "pg": ">=8.0" } } diff --git a/packages/pg/Makefile b/packages/pg/Makefile index 52d0545d3..a5b0bc1da 100644 --- a/packages/pg/Makefile +++ b/packages/pg/Makefile @@ -62,6 +62,4 @@ test-pool: lint: @echo "***Starting lint***" - node -e "process.exit(Number(process.versions.node.split('.')[0]) < 8 ? 0 : 1)" \ - && echo "***Skipping lint (node version too old)***" \ - || node_modules/.bin/eslint lib + node_modules/.bin/eslint lib diff --git a/packages/pg/package.json b/packages/pg/package.json index 91e27a887..5ca6f262f 100644 --- a/packages/pg/package.json +++ b/packages/pg/package.json @@ -51,6 +51,6 @@ ], "license": "MIT", "engines": { - "node": ">= 4.5.0" + "node": ">= 8.0.0" } } From 5b01eb062d99b494bdbc7ffe11ca6c3c56e33efa Mon Sep 17 00:00:00 2001 From: Brian C Date: Mon, 13 Jan 2020 14:31:48 -0600 Subject: [PATCH 2/9] Remove password from stringified outputs (#2066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove password from stringified outputs Theres a security concern where if you're not careful and you include your client or pool instance in console.log or stack traces it might include the database password. To widen the pit of success I'm making that field non-enumerable. You can still get at it...it just wont show up "by accident" when you're logging things now. The backwards compatiblity impact of this is very small, but it is still technically somewhat an API change so...8.0. * Implement feedback * Fix more whitespace the autoformatter changed * Simplify code a bit * Remove password from stringified outputs (#2070) * Keep ConnectionParameters’s password property writable `Client` writes to it when `password` is a function. * Avoid creating password property on pool options when it didn’t exist previously. * Allow password option to be non-enumerable to avoid breaking uses like `new Pool(existingPool.options)`. * Make password property definitions consistent in formatting and configurability. Co-authored-by: Charmander <~@charmander.me> --- .gitignore | 1 + packages/pg-pool/index.js | 12 +++++++ packages/pg/lib/client.js | 11 ++++++- packages/pg/lib/connection-parameters.js | 11 ++++++- packages/pg/lib/native/client.js | 10 +++++- .../test/integration/gh-issues/2064-tests.js | 32 +++++++++++++++++++ 6 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/2064-tests.js diff --git a/.gitignore b/.gitignore index df95fda07..bae2a20a1 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ package-lock.json *.swp dist .DS_Store +.vscode/ diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 1c7faf210..dd0d478d2 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -60,6 +60,18 @@ class Pool extends EventEmitter { constructor (options, Client) { super() this.options = Object.assign({}, options) + + if (options != null && 'password' in options) { + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + Object.defineProperty(this.options, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: options.password + }) + } + this.options.max = this.options.max || this.options.poolSize || 10 this.log = this.options.log || function () { } this.Client = this.options.Client || Client || require('pg').Client diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 93807e48c..c929d26f3 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -30,7 +30,16 @@ var Client = function (config) { this.database = this.connectionParameters.database this.port = this.connectionParameters.port this.host = this.connectionParameters.host - this.password = this.connectionParameters.password + + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + Object.defineProperty(this, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: this.connectionParameters.password + }) + this.replication = this.connectionParameters.replication var c = config || {} diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js index 0d5e0376d..c0f8498eb 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -54,7 +54,16 @@ var ConnectionParameters = function (config) { this.database = val('database', config) this.port = parseInt(val('port', config), 10) this.host = val('host', config) - this.password = val('password', config) + + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + Object.defineProperty(this, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: val('password', config) + }) + this.binary = val('binary', config) this.ssl = typeof config.ssl === 'undefined' ? useSsl() : config.ssl this.client_encoding = val('client_encoding', config) diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js index 6859bc2cc..d06166573 100644 --- a/packages/pg/lib/native/client.js +++ b/packages/pg/lib/native/client.js @@ -43,7 +43,15 @@ var Client = module.exports = function (config) { // for the time being. TODO: deprecate all this jazz var cp = this.connectionParameters = new ConnectionParameters(config) this.user = cp.user - this.password = cp.password + + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + const hiddenPassword = cp.password + Object.defineProperty(this, 'password', { + enumerable: false, + writable: true, + value: hiddenPassword + }) this.database = cp.database this.host = cp.host this.port = cp.port diff --git a/packages/pg/test/integration/gh-issues/2064-tests.js b/packages/pg/test/integration/gh-issues/2064-tests.js new file mode 100644 index 000000000..64c150bd0 --- /dev/null +++ b/packages/pg/test/integration/gh-issues/2064-tests.js @@ -0,0 +1,32 @@ + +"use strict" +const helper = require('./../test-helper') +const assert = require('assert') +const util = require('util') + +const suite = new helper.Suite() + +const password = 'FAIL THIS TEST' + +suite.test('Password should not exist in toString() output', () => { + const pool = new helper.pg.Pool({ password }) + const client = new helper.pg.Client({ password }) + assert(pool.toString().indexOf(password) === -1); + assert(client.toString().indexOf(password) === -1); +}) + +suite.test('Password should not exist in util.inspect output', () => { + const pool = new helper.pg.Pool({ password }) + const client = new helper.pg.Client({ password }) + const depth = 20; + assert(util.inspect(pool, { depth }).indexOf(password) === -1); + assert(util.inspect(client, { depth }).indexOf(password) === -1); +}) + +suite.test('Password should not exist in json.stringfy output', () => { + const pool = new helper.pg.Pool({ password }) + const client = new helper.pg.Client({ password }) + const depth = 20; + assert(JSON.stringify(pool).indexOf(password) === -1); + assert(JSON.stringify(client).indexOf(password) === -1); +}) From 8494cb449bdae057a2d1c6cb291a8c2901543aaa Mon Sep 17 00:00:00 2001 From: Brian C Date: Wed, 15 Jan 2020 14:12:48 -0600 Subject: [PATCH 3/9] Make `native` non-enumerable (#2065) * Make `native` non-enumerable Making it non-enumerable means less spurious "Cannot find module" errors in your logs when iterating over `pg` objects. `Object.defineProperty` has been available since Node 0.12. See https://github.com/brianc/node-postgres/issues/1894#issuecomment-543300178 * Add test for `native` enumeration Co-authored-by: Gabe Gorelick --- packages/pg/lib/index.js | 34 ++++++++++++------- .../test/integration/gh-issues/1992-tests.js | 11 ++++++ 2 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/1992-tests.js diff --git a/packages/pg/lib/index.js b/packages/pg/lib/index.js index de33c086d..3f8508734 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -44,20 +44,28 @@ if (typeof process.env.NODE_PG_FORCE_NATIVE !== 'undefined') { module.exports = new PG(Client) // lazy require native module...the native module may not have installed - module.exports.__defineGetter__('native', function () { - delete module.exports.native - var native = null - try { - native = new PG(require('./native')) - } catch (err) { - if (err.code !== 'MODULE_NOT_FOUND') { - throw err + Object.defineProperty(module.exports, 'native', { + configurable: true, + enumerable: false, + get() { + var native = null + try { + native = new PG(require('./native')) + } catch (err) { + if (err.code !== 'MODULE_NOT_FOUND') { + throw err + } + /* eslint-disable no-console */ + console.error(err.message) + /* eslint-enable no-console */ } - /* eslint-disable no-console */ - console.error(err.message) - /* eslint-enable no-console */ + + // overwrite module.exports.native so that getter is never called again + Object.defineProperty(module.exports, 'native', { + value: native + }) + + return native } - module.exports.native = native - return native }) } diff --git a/packages/pg/test/integration/gh-issues/1992-tests.js b/packages/pg/test/integration/gh-issues/1992-tests.js new file mode 100644 index 000000000..1832f5f8a --- /dev/null +++ b/packages/pg/test/integration/gh-issues/1992-tests.js @@ -0,0 +1,11 @@ + +"use strict" +const helper = require('./../test-helper') +const assert = require('assert') + +const suite = new helper.Suite() + +suite.test('Native should not be enumerable', () => { + const keys = Object.keys(helper.pg) + assert.strictEqual(keys.indexOf('native'), -1) +}) From e3a35e9dc5519ef9f8ce4fc92f52f57e77649513 Mon Sep 17 00:00:00 2001 From: Natalie Wolfe Date: Wed, 15 Jan 2020 13:10:59 -0800 Subject: [PATCH 4/9] Use class-extends to wrap Pool (#1541) * Use class-extends to wrap Pool * Minimize diff * Test `BoundPool` inheritance Co-authored-by: Charmander <~@charmander.me> Co-authored-by: Brian C --- packages/pg/lib/index.js | 17 ++++--------- .../test/integration/gh-issues/1542-tests.js | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/1542-tests.js diff --git a/packages/pg/lib/index.js b/packages/pg/lib/index.js index 3f8508734..c3062947d 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -7,25 +7,18 @@ * README.md file in the root directory of this source tree. */ -var util = require('util') var Client = require('./client') var defaults = require('./defaults') var Connection = require('./connection') var Pool = require('pg-pool') -const checkConstructor = require('./compat/check-constructor') const poolFactory = (Client) => { - var BoundPool = function (options) { - // eslint-disable-next-line no-eval - checkConstructor('pg.Pool', 'PG-POOL-NEW', () => eval('new.target')) - - var config = Object.assign({ Client: Client }, options) - return new Pool(config) + return class BoundPool extends Pool { + constructor (options) { + var config = Object.assign({ Client: Client }, options) + super(config) + } } - - util.inherits(BoundPool, Pool) - - return BoundPool } var PG = function (clientConstructor) { diff --git a/packages/pg/test/integration/gh-issues/1542-tests.js b/packages/pg/test/integration/gh-issues/1542-tests.js new file mode 100644 index 000000000..4d30d6020 --- /dev/null +++ b/packages/pg/test/integration/gh-issues/1542-tests.js @@ -0,0 +1,25 @@ + +"use strict" +const helper = require('./../test-helper') +const assert = require('assert') + +const suite = new helper.Suite() + +suite.testAsync('BoundPool can be subclassed', async () => { + const Pool = helper.pg.Pool; + class SubPool extends Pool { + + } + const subPool = new SubPool() + const client = await subPool.connect() + client.release() + await subPool.end() + assert(subPool instanceof helper.pg.Pool) +}) + +suite.test('calling pg.Pool without new throws', () => { + const Pool = helper.pg.Pool; + assert.throws(() => { + const pool = Pool() + }) +}) From 8c606ff50eb23d63b67e97d0148c3e5a822caee3 Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Fri, 17 Jan 2020 08:49:06 -0800 Subject: [PATCH 5/9] =?UTF-8?q?Continue=20support=20for=20creating=20a=20p?= =?UTF-8?q?g.Pool=20from=20another=20instance=E2=80=99s=20options=20(#2076?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add failing test for creating a `BoundPool` from another instance’s settings * Continue support for creating a pg.Pool from another instance’s options by dropping the requirement for the `password` property to be enumerable. --- packages/pg/lib/index.js | 3 +-- .../unit/connection-pool/configuration-tests.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 packages/pg/test/unit/connection-pool/configuration-tests.js diff --git a/packages/pg/lib/index.js b/packages/pg/lib/index.js index c3062947d..c73064cf2 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -15,8 +15,7 @@ var Pool = require('pg-pool') const poolFactory = (Client) => { return class BoundPool extends Pool { constructor (options) { - var config = Object.assign({ Client: Client }, options) - super(config) + super(options, Client) } } } diff --git a/packages/pg/test/unit/connection-pool/configuration-tests.js b/packages/pg/test/unit/connection-pool/configuration-tests.js new file mode 100644 index 000000000..10c991839 --- /dev/null +++ b/packages/pg/test/unit/connection-pool/configuration-tests.js @@ -0,0 +1,14 @@ +'use strict' + +const assert = require('assert') +const helper = require('../test-helper') + +test('pool with copied settings includes password', () => { + const original = new helper.pg.Pool({ + password: 'original', + }) + + const copy = new helper.pg.Pool(original.options) + + assert.equal(copy.options.password, 'original') +}) From 7aee26160a762f4eb6e0133f053f9a271ee2a834 Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Thu, 23 Jan 2020 09:03:50 -0800 Subject: [PATCH 6/9] Use user name as default database when user is non-default (#1679) Not entirely backwards-compatible. --- packages/pg/lib/connection-parameters.js | 5 ++++ packages/pg/lib/defaults.js | 2 +- .../integration/client/configuration-tests.js | 24 ++++++++++++++++++- .../connection-parameters/creation-tests.js | 7 +++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js index c0f8498eb..cd6d3b8a9 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -52,6 +52,11 @@ var ConnectionParameters = function (config) { this.user = val('user', config) this.database = val('database', config) + + if (this.database === undefined) { + this.database = this.user + } + this.port = parseInt(val('port', config), 10) this.host = val('host', config) diff --git a/packages/pg/lib/defaults.js b/packages/pg/lib/defaults.js index 120b8c7b5..eb58550d6 100644 --- a/packages/pg/lib/defaults.js +++ b/packages/pg/lib/defaults.js @@ -15,7 +15,7 @@ module.exports = { user: process.platform === 'win32' ? process.env.USERNAME : process.env.USER, // name of database to connect - database: process.platform === 'win32' ? process.env.USERNAME : process.env.USER, + database: undefined, // database user's password password: null, diff --git a/packages/pg/test/integration/client/configuration-tests.js b/packages/pg/test/integration/client/configuration-tests.js index 87bb52d47..a6756ddee 100644 --- a/packages/pg/test/integration/client/configuration-tests.js +++ b/packages/pg/test/integration/client/configuration-tests.js @@ -14,7 +14,7 @@ for (var key in process.env) { suite.test('default values are used in new clients', function () { assert.same(pg.defaults, { user: process.env.USER, - database: process.env.USER, + database: undefined, password: null, port: 5432, rows: 0, @@ -54,6 +54,28 @@ suite.test('modified values are passed to created clients', function () { }) }) +suite.test('database defaults to user when user is non-default', () => { + { + pg.defaults.database = undefined + + const client = new Client({ + user: 'foo', + }) + + assert.strictEqual(client.database, 'foo') + } + + { + pg.defaults.database = 'bar' + + const client = new Client({ + user: 'foo', + }) + + assert.strictEqual(client.database, 'bar') + } +}) + suite.test('cleanup', () => { // restore process.env for (var key in realEnv) { diff --git a/packages/pg/test/unit/connection-parameters/creation-tests.js b/packages/pg/test/unit/connection-parameters/creation-tests.js index 5d200be0a..fdb4e6627 100644 --- a/packages/pg/test/unit/connection-parameters/creation-tests.js +++ b/packages/pg/test/unit/connection-parameters/creation-tests.js @@ -16,8 +16,13 @@ test('ConnectionParameters construction', function () { }) var compare = function (actual, expected, type) { + const expectedDatabase = + expected.database === undefined + ? expected.user + : expected.database + assert.equal(actual.user, expected.user, type + ' user') - assert.equal(actual.database, expected.database, type + ' database') + assert.equal(actual.database, expectedDatabase, type + ' database') assert.equal(actual.port, expected.port, type + ' port') assert.equal(actual.host, expected.host, type + ' host') assert.equal(actual.password, expected.password, type + ' password') From b9f67a0d95772d5ee54462434d06d6e1556265c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Mon, 11 Feb 2019 21:18:53 +0100 Subject: [PATCH 7/9] Prevent requeuing a broken client If a client is not queryable, the pool should prevent requeuing instead of strictly enforcing errors to be propagated back to it. --- packages/pg-pool/index.js | 25 ++++++++++++++++++ packages/pg-pool/test/error-handling.js | 34 +++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index dd0d478d2..aed6656f8 100644 --- 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 index 72d97ede0..4bdb2f507 100644 --- 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 }) From 48ed1902987e49d07fcb802ca49bebd399d91b4f Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 23 Jan 2020 11:31:15 -0600 Subject: [PATCH 8/9] Write tests for change --- packages/pg-pool/index.js | 28 ++--------- packages/pg-pool/test/error-handling.js | 34 -------------- packages/pg-pool/test/releasing-clients.js | 54 ++++++++++++++++++++++ 3 files changed, 58 insertions(+), 58 deletions(-) create mode 100644 packages/pg-pool/test/releasing-clients.js diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index aed6656f8..e144bb83b 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -25,31 +25,10 @@ class PendingItem { } } -function throwOnRelease () { +function throwOnDoubleRelease () { 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 } @@ -281,7 +260,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 @@ -317,7 +296,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/error-handling.js b/packages/pg-pool/test/error-handling.js index 4bdb2f507..72d97ede0 100644 --- a/packages/pg-pool/test/error-handling.js +++ b/packages/pg-pool/test/error-handling.js @@ -181,40 +181,6 @@ 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 }) diff --git a/packages/pg-pool/test/releasing-clients.js b/packages/pg-pool/test/releasing-clients.js new file mode 100644 index 000000000..8fccde36e --- /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 bool + 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() + }) +}) From 98dd655ef40cda3e6918a6dc9dc9377559517d92 Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 23 Jan 2020 12:08:57 -0600 Subject: [PATCH 9/9] Use node 13.6 in travis Some weird behavior changed w/ async iteration in node 13.7...I'm not sure if this was an unintentional break or not but it definitely diverges in behavior from node 12 and earlier versions in node 13...so for now going to run tests on 13.6 to unblock the tests from running while I track this down. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 61a7a79af..7ea347e9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ env: node_js: - lts/dubnium - lts/erbium - - 13 + - 13.6 addons: postgresql: "10"