From f2b87e02f129a5508eca6ba5ff60f99ba6cdbe58 Mon Sep 17 00:00:00 2001 From: Brian C Date: Wed, 7 Jun 2017 22:58:03 -0500 Subject: [PATCH 1/3] Add client connectionString tests (#1310) * Remove redundant tests * Add client connectionString test Add test to ensure { connectionString } is respected as an argument to the client constructor * Add test for connection string property Also fixed some legacy require statements. --- lib/pool-factory.js | 1 - .../double-connection-tests.js | 2 +- .../ending-empty-pool-tests.js | 2 +- .../connection-pool/ending-pool-tests.js | 3 ++- .../connection-pool/error-tests.js | 4 +-- .../connection-pool/idle-timeout-tests.js | 2 +- .../connection-pool/max-connection-tests.js | 2 +- .../connection-pool/native-instance-tests.js | 2 +- .../connection-pool/optional-config-tests.js | 2 +- .../single-connection-tests.js | 2 +- .../single-pool-on-object-config-tests.js | 4 +-- .../waiting-connection-tests.js | 2 +- test/unit/client/configuration-tests.js | 11 ++++++++ test/unit/client/connection-string-tests.js | 27 ------------------- 14 files changed, 25 insertions(+), 41 deletions(-) delete mode 100644 test/unit/client/connection-string-tests.js diff --git a/lib/pool-factory.js b/lib/pool-factory.js index aa7bd0b1..85f0e6a5 100644 --- a/lib/pool-factory.js +++ b/lib/pool-factory.js @@ -3,7 +3,6 @@ var util = require('util'); var Pool = require('pg-pool'); module.exports = function(Client) { - var BoundPool = function(options) { var config = { Client: Client }; for (var key in options) { diff --git a/test/integration/connection-pool/double-connection-tests.js b/test/integration/connection-pool/double-connection-tests.js index ae7eb316..421e1f9e 100644 --- a/test/integration/connection-pool/double-connection-tests.js +++ b/test/integration/connection-pool/double-connection-tests.js @@ -1,2 +1,2 @@ -var helper = require(__dirname + "/test-helper") +var helper = require("./test-helper") helper.testPoolSize(2); diff --git a/test/integration/connection-pool/ending-empty-pool-tests.js b/test/integration/connection-pool/ending-empty-pool-tests.js index 4f5dd80a..d1acc6f2 100644 --- a/test/integration/connection-pool/ending-empty-pool-tests.js +++ b/test/integration/connection-pool/ending-empty-pool-tests.js @@ -1,4 +1,4 @@ -var helper = require(__dirname + '/test-helper') +var helper = require('./test-helper') var called = false; test('disconnects', function() { diff --git a/test/integration/connection-pool/ending-pool-tests.js b/test/integration/connection-pool/ending-pool-tests.js index 83f4b1bc..3a1ab46f 100644 --- a/test/integration/connection-pool/ending-pool-tests.js +++ b/test/integration/connection-pool/ending-pool-tests.js @@ -1,6 +1,7 @@ -var helper = require(__dirname + '/test-helper') +var helper = require('./test-helper') var called = false; + test('disconnects', function() { var sink = new helper.Sink(4, function() { called = true; diff --git a/test/integration/connection-pool/error-tests.js b/test/integration/connection-pool/error-tests.js index 2cf0501f..59121a86 100644 --- a/test/integration/connection-pool/error-tests.js +++ b/test/integration/connection-pool/error-tests.js @@ -1,5 +1,5 @@ -var helper = require(__dirname + "/../test-helper"); -var pg = require(__dirname + "/../../../lib"); +var helper = require("../test-helper"); +var pg = require("../../../lib"); //first make pool hold 2 clients pg.defaults.poolSize = 2; diff --git a/test/integration/connection-pool/idle-timeout-tests.js b/test/integration/connection-pool/idle-timeout-tests.js index 0a60ce50..b0908b6f 100644 --- a/test/integration/connection-pool/idle-timeout-tests.js +++ b/test/integration/connection-pool/idle-timeout-tests.js @@ -1,4 +1,4 @@ -var helper = require(__dirname + '/test-helper'); +var helper = require('./test-helper'); var _ = require('lodash') const config = _.extend({ }, helper.config, { idleTimeoutMillis: 50 }) diff --git a/test/integration/connection-pool/max-connection-tests.js b/test/integration/connection-pool/max-connection-tests.js index 68c0773f..944d2fb2 100644 --- a/test/integration/connection-pool/max-connection-tests.js +++ b/test/integration/connection-pool/max-connection-tests.js @@ -1,2 +1,2 @@ -var helper = require(__dirname + "/test-helper") +var helper = require("./test-helper") helper.testPoolSize(40); diff --git a/test/integration/connection-pool/native-instance-tests.js b/test/integration/connection-pool/native-instance-tests.js index 06fbdb45..314920c4 100644 --- a/test/integration/connection-pool/native-instance-tests.js +++ b/test/integration/connection-pool/native-instance-tests.js @@ -1,4 +1,4 @@ -var helper = require(__dirname + "/../test-helper") +var helper = require("./../test-helper") var pg = helper.pg var native = helper.args.native diff --git a/test/integration/connection-pool/optional-config-tests.js b/test/integration/connection-pool/optional-config-tests.js index f0ba2e76..be7063eb 100644 --- a/test/integration/connection-pool/optional-config-tests.js +++ b/test/integration/connection-pool/optional-config-tests.js @@ -1,4 +1,4 @@ -var helper = require(__dirname + '/test-helper'); +var helper = require('./test-helper'); //setup defaults helper.pg.defaults.user = helper.args.user; diff --git a/test/integration/connection-pool/single-connection-tests.js b/test/integration/connection-pool/single-connection-tests.js index 5ca0a888..89f6f069 100644 --- a/test/integration/connection-pool/single-connection-tests.js +++ b/test/integration/connection-pool/single-connection-tests.js @@ -1,2 +1,2 @@ -var helper = require(__dirname + "/test-helper") +var helper = require("./test-helper") helper.testPoolSize(1); diff --git a/test/integration/connection-pool/single-pool-on-object-config-tests.js b/test/integration/connection-pool/single-pool-on-object-config-tests.js index a28cbf5c..81cdf8e4 100644 --- a/test/integration/connection-pool/single-pool-on-object-config-tests.js +++ b/test/integration/connection-pool/single-pool-on-object-config-tests.js @@ -1,5 +1,5 @@ -var helper = require(__dirname + "/../test-helper"); -var pg = require(__dirname + "/../../../lib"); +var helper = require("../test-helper"); +var pg = require("../../../lib"); pg.connect(helper.config, assert.success(function(client, done) { assert.equal(Object.keys(pg._pools).length, 1); diff --git a/test/integration/connection-pool/waiting-connection-tests.js b/test/integration/connection-pool/waiting-connection-tests.js index f2519ec5..82572d1e 100644 --- a/test/integration/connection-pool/waiting-connection-tests.js +++ b/test/integration/connection-pool/waiting-connection-tests.js @@ -1,2 +1,2 @@ -var helper = require(__dirname + "/test-helper") +var helper = require("./test-helper") helper.testPoolSize(200); diff --git a/test/unit/client/configuration-tests.js b/test/unit/client/configuration-tests.js index 0204af22..5548e5fb 100644 --- a/test/unit/client/configuration-tests.js +++ b/test/unit/client/configuration-tests.js @@ -59,6 +59,17 @@ test('client settings', function() { test('initializing from a config string', function() { + test('uses connectionString property', function () { + var client = new Client({ + connectionString: 'postgres://brian:pass@host1:333/databasename' + }) + assert.equal(client.user, 'brian'); + assert.equal(client.password, "pass"); + assert.equal(client.host, "host1"); + assert.equal(client.port, 333); + assert.equal(client.database, "databasename"); + }) + test('uses the correct values from the config string', function() { var client = new Client("postgres://brian:pass@host1:333/databasename") assert.equal(client.user, 'brian'); diff --git a/test/unit/client/connection-string-tests.js b/test/unit/client/connection-string-tests.js deleted file mode 100644 index 9316daa9..00000000 --- a/test/unit/client/connection-string-tests.js +++ /dev/null @@ -1,27 +0,0 @@ -require(__dirname + '/test-helper'); - -/* - * Perhaps duplicate of test named 'initializing from a config string' in - * configuration-tests.js - */ - -test("using connection string in client constructor", function() { - var client = new Client("postgres://brian:pw@boom:381/lala"); - - test("parses user", function() { - assert.equal(client.user,'brian'); - }); - test("parses password", function() { - assert.equal(client.password, 'pw'); - }); - test("parses host", function() { - assert.equal(client.host, 'boom'); - }); - test('parses port', function() { - assert.equal(client.port, 381) - }); - test('parses database', function() { - assert.equal(client.database, 'lala') - }); -}); - From 76c59a01f26c06ada75f56b1b455317cd9e2d758 Mon Sep 17 00:00:00 2001 From: Brian Carlson Date: Fri, 9 Jun 2017 10:56:02 -0500 Subject: [PATCH 2/3] Emit error when backend unexpectedly disconnects --- lib/client.js | 48 +++++++--- lib/connection.js | 5 -- .../client/error-handling-tests.js | 22 ++++- .../client/network-partition-tests.js | 90 +++++++++++++++++++ ...error-handling-prepared-statement-tests.js | 15 ++-- ...tream-and-query-error-interaction-tests.js | 2 + 6 files changed, 157 insertions(+), 25 deletions(-) create mode 100644 test/integration/client/network-partition-tests.js diff --git a/lib/client.js b/lib/client.js index a0d7f21c..78d8bd7b 100644 --- a/lib/client.js +++ b/lib/client.js @@ -31,6 +31,9 @@ var Client = function(config) { var c = config || {}; this._types = new TypeOverrides(c.types); + this._ending = false; + this._connecting = false; + this._connectionError = false; this.connection = c.connection || new Connection({ stream: c.stream, @@ -50,6 +53,7 @@ util.inherits(Client, EventEmitter); Client.prototype.connect = function(callback) { var self = this; var con = this.connection; + this._connecting = true; if(this.host && this.host.indexOf('/') === 0) { con.connect(this.host + '/.s.PGSQL.' + this.port); @@ -107,6 +111,7 @@ Client.prototype.connect = function(callback) { //hook up query handling events to connection //after the connection initially becomes ready for queries con.once('readyForQuery', function() { + self._connecting = false; //delegate rowDescription to active query con.on('rowDescription', function(msg) { @@ -175,34 +180,52 @@ Client.prototype.connect = function(callback) { }); con.on('error', function(error) { - if(self.activeQuery) { + if(this.activeQuery) { var activeQuery = self.activeQuery; - self.activeQuery = null; + this.activeQuery = null; return activeQuery.handleError(error, con); } - if(!callback) { - return self.emit('error', error); + + if (this._connecting) { + // set a flag indicating we've seen an error during connection + // the backend will terminate the connection and we don't want + // to throw a second error when the connection is terminated + this._connectionError = true; } + + if(!callback) { + return this.emit('error', error); + } + con.end(); // make sure ECONNRESET errors don't cause error events callback(error); callback = null; - }); + }.bind(this)); con.once('end', function() { - if ( callback ) { - // haven't received a connection message yet ! + if (callback) { + // haven't received a connection message yet! var err = new Error('Connection terminated'); callback(err); callback = null; return; } - if(self.activeQuery) { + if(this.activeQuery) { var disconnectError = new Error('Connection terminated'); - self.activeQuery.handleError(disconnectError, con); - self.activeQuery = null; + this.activeQuery.handleError(disconnectError, con); + this.activeQuery = null; } - self.emit('end'); - }); + if (!this._ending) { + // if the connection is ended without us calling .end() + // on this client then we have an unexpected disconnection + // treat this as an error unless we've already emitted an error + // during connection. + if (!this._connectionError) { + this.emit('error', new Error('Connection terminated unexpectedly')); + } + } + this.emit('end'); + }.bind(this)); con.on('notice', function(msg) { @@ -342,6 +365,7 @@ Client.prototype.query = function(config, values, callback) { }; Client.prototype.end = function(cb) { + this._ending = true; this.connection.end(); if (cb) { this.connection.once('end', cb); diff --git a/lib/connection.js b/lib/connection.js index 87d4f274..b031ece1 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -87,9 +87,6 @@ Connection.prototype.connect = function(port, host) { }); this.stream.on('close', function() { - // NOTE: node-0.10 emits both 'end' and 'close' - // for streams closed by the peer, while - // node-0.8 only emits 'close' self.emit('end'); }); @@ -143,8 +140,6 @@ Connection.prototype.attachListeners = function(stream) { }; Connection.prototype.requestSsl = function() { - this.checkSslResponse = true; - var bodyBuffer = this.writer .addInt16(0x04D2) .addInt16(0x162F).flush(); diff --git a/test/integration/client/error-handling-tests.js b/test/integration/client/error-handling-tests.js index d3bf36c2..3fcd305b 100644 --- a/test/integration/client/error-handling-tests.js +++ b/test/integration/client/error-handling-tests.js @@ -1,6 +1,24 @@ var helper = require(__dirname + '/test-helper'); var util = require('util'); + + test('non-query error with callback', function () { + var client = new Client({ + user:'asldkfjsadlfkj' + }); + client.connect(assert.calls(function (err) { + assert(err); + })); + }); + + test('non-query error', function() { + var client = new Client({ + user:'asldkfjsadlfkj' + }); + assert.emits(client, 'error'); + client.connect(); + }); + var createErorrClient = function() { var client = helper.client(); client.once('error', function(err) { @@ -11,9 +29,8 @@ var createErorrClient = function() { return client; }; -test('error handling', function(){ +test('error handling', function() { test('within a simple query', function() { - var client = createErorrClient(); var query = client.query("select omfg from yodas_dsflsd where pixistix = 'zoiks!!!'"); @@ -77,7 +94,6 @@ test('error handling', function(){ }); test('non-query error', function() { - var client = new Client({ user:'asldkfjsadlfkj' }); diff --git a/test/integration/client/network-partition-tests.js b/test/integration/client/network-partition-tests.js new file mode 100644 index 00000000..944df510 --- /dev/null +++ b/test/integration/client/network-partition-tests.js @@ -0,0 +1,90 @@ +var co = require('co') + +var buffers = require('../../test-buffers') +var helper = require('./test-helper') + +var net = require('net') + +var Server = function(response) { + this.server = undefined + this.socket = undefined + this.response = response +} + +Server.prototype.start = function (cb) { + // this is our fake postgres server + // it responds with our specified response immediatley after receiving every buffer + // this is sufficient into convincing the client its connectet to a valid backend + // if we respond with a readyForQuery message + this.server = net.createServer(function (socket) { + this.socket = socket + if (this.response) { + this.socket.on('data', function (data) { + // deny request for SSL + if (data.length == 8) { + this.socket.write(new Buffer('N', 'utf8')) + // consider all authentication requests as good + } else if (!data[0]) { + this.socket.write(buffers.authenticationOk()) + // respond with our canned response + } else { + this.socket.write(this.response) + } + }.bind(this)) + } + }.bind(this)) + + var port = 54321 + + var options = { + host: 'localhost', + port: port, + } + this.server.listen(options.port, options.host, function () { + cb(options) + }) +} + +Server.prototype.drop = function () { + this.socket.end() +} + +Server.prototype.close = function (cb) { + this.server.close(cb) +} + +var testServer = function (server, cb) { + // wait for our server to start + server.start(function(options) { + // connect a client to it + var client = new helper.Client(options) + client.connect() + + // after 50 milliseconds, drop the client + setTimeout(function() { + server.drop() + }, 50) + + // blow up if we don't receive an error + var timeoutId = setTimeout(function () { + throw new Error('Client should have emitted an error but it did not.') + }, 5000) + + // return our wait token + client.on('error', function () { + clearTimeout(timeoutId) + server.close(cb) + }) + }) +} + +// test being disconnected after readyForQuery +const respondingServer = new Server(buffers.readyForQuery()) +testServer(respondingServer, function () { + process.stdout.write('.') + // test being disconnected from a server that never responds + const silentServer = new Server() + testServer(silentServer, function () { + process.stdout.write('.') + }) +}) diff --git a/test/integration/client/query-error-handling-prepared-statement-tests.js b/test/integration/client/query-error-handling-prepared-statement-tests.js index 15164658..77615ff3 100644 --- a/test/integration/client/query-error-handling-prepared-statement-tests.js +++ b/test/integration/client/query-error-handling-prepared-statement-tests.js @@ -29,12 +29,18 @@ test('query killed during query execution of prepared statement', function() { var client = new Client(helper.args); client.connect(assert.success(function() { var sleepQuery = 'select pg_sleep($1)'; - var query1 = client.query({ + + const queryConfig = { name: 'sleep query', text: sleepQuery, - values: [5] }, - assert.calls(function(err, result) { - assert.equal(err.message, 'terminating connection due to administrator command'); + values: [5], + }; + + // client should emit an error because it is unexpectedly disconnected + assert.emits(client, 'error') + + var query1 = client.query(queryConfig, assert.calls(function(err, result) { + assert.equal(err.message, 'terminating connection due to administrator command'); })); query1.on('error', function(err) { @@ -53,7 +59,6 @@ test('query killed during query execution of prepared statement', function() { })); }); - test('client end during query execution of prepared statement', function() { var client = new Client(helper.args); client.connect(assert.success(function() { diff --git a/test/unit/client/stream-and-query-error-interaction-tests.js b/test/unit/client/stream-and-query-error-interaction-tests.js index 02d66c62..a60e1f73 100644 --- a/test/unit/client/stream-and-query-error-interaction-tests.js +++ b/test/unit/client/stream-and-query-error-interaction-tests.js @@ -7,12 +7,14 @@ test('emits end when not in query', function() { stream.write = function() { //NOOP } + var client = new Client({connection: new Connection({stream: stream})}); client.connect(assert.calls(function() { client.query('SELECT NOW()', assert.calls(function(err, result) { assert(err); })); })); + assert.emits(client, 'error'); assert.emits(client, 'end'); client.connection.emit('connect'); process.nextTick(function() { From 58691218affa37b41529376c5167252846bb93fa Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Fri, 9 Jun 2017 12:33:55 -0500 Subject: [PATCH 3/3] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 590a9155..40a2c6ef 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pg", - "version": "6.2.3", + "version": "6.2.4", "description": "PostgreSQL client - pure javascript & libpq with the same API", "keywords": [ "postgres",