From d3aee3dfc8b698358eaf76587fd95f1f745494a0 Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Mon, 28 Oct 2019 11:45:20 -0500 Subject: [PATCH] Add additional pool test & deprecate .end There are no tests covering cursor.end(). It's also a weird method to have on the cursor as it terminates the connection to postgres internally. I don't recall why I added this method in the first place but its not correct to close a connection to postgres by calling .end on a cursor...seems like a pretty big footgun. If you have a pooled client and you call `cursor.end` it'll close the client internally and likely confuse the pool. Plus it's just weird to be able to close a connection by calling .end on a query or cursor. So...I'm deprecating that method. --- index.js | 6 ++++-- test/pool.js | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 8136c389..a6960081 100644 --- a/index.js +++ b/index.js @@ -154,13 +154,15 @@ Cursor.prototype._getRows = function(rows, cb) { this.connection.flush() } -Cursor.prototype.end = function(cb) { +// users really shouldn't be calling 'end' here and terminating a connection to postgres +// via the low level connection.end api +Cursor.prototype.end = util.deprecate(function(cb) { if (this.state !== 'initialized') { this.connection.sync() } this.connection.once('end', cb) this.connection.end() -} +}, 'Cursor.end is deprecated. Call end on the client itself to end a connection to the database.') Cursor.prototype.close = function(cb) { if (this.state === 'done') { diff --git a/test/pool.js b/test/pool.js index bd487807..80e0ddc1 100644 --- a/test/pool.js +++ b/test/pool.js @@ -83,4 +83,24 @@ describe('pool', function() { done() }) }) + + it('can close multiple times on a pool', async function() { + const pool = new pg.Pool({ max: 1 }) + const run = () => + new Promise(async resolve => { + const cursor = new Cursor(text) + const client = await pool.connect() + client.query(cursor) + cursor.read(25, function(err) { + assert.ifError(err) + cursor.close(function(err) { + assert.ifError(err) + client.release() + resolve() + }) + }) + }) + await Promise.all([run(), run(), run()]) + await pool.end() + }) })