diff --git a/lib/client.js b/lib/client.js index 3a8515a4..3104f70b 100644 --- a/lib/client.js +++ b/lib/client.js @@ -67,12 +67,12 @@ Client.prototype.connect = function(callback) { if(self.ssl) { con.requestSsl(); } else { - con.startup(self.getStartupConf()); + con.startup(self._getStartupConfiguration()); } }); con.on('sslconnect', function() { - con.startup(self.getStartupConf()); + con.startup(self._getStartupConfiguration()); }); function checkPgPass(cb) { @@ -122,7 +122,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._attachEventListeners(con) + self._attachEventListeners(con); //process possible callback argument to Client#connect if (callback) { @@ -134,16 +134,6 @@ Client.prototype.connect = function(callback) { self.emit('connect'); }); - if (!callback) { - return new global.Promise(function (resolve, reject) { - con.once('connect', () => { - con.removeListener('error', reject) - resolve() - }) - con.once('error', reject) - }) - } - con.on('error', function(error) { if(this.activeQuery) { var activeQuery = self.activeQuery; @@ -197,8 +187,22 @@ Client.prototype.connect = function(callback) { self.emit('notice', msg); }); + var result; + + if (!callback) { + result = new global.Promise(function (resolve, reject) { + con.once('connect', function () { + con.removeListener('error', reject) + resolve() + }) + this.once('error', reject) + }.bind(this)) + } + + return result; }; + // once a connection is established connect listeners Client.prototype._attachEventListeners = function(con) { var self = this; @@ -251,7 +255,7 @@ Client.prototype._attachEventListeners = function(con) { }); } -Client.prototype.getStartupConf = function() { +Client.prototype._getStartupConfiguration = function() { var params = this.connectionParameters; var data = { @@ -405,6 +409,9 @@ Client.prototype.query = function(config, values, callback) { Client.prototype.end = function(cb) { this._ending = true; + if (this.activeQuery) { + return this.connection.stream.end() + } this.connection.end(); if (cb) { this.connection.once('end', cb); diff --git a/lib/connection.js b/lib/connection.js index 6bda2f8b..c40639e0 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -186,7 +186,9 @@ Connection.prototype.password = function(password) { }; Connection.prototype._send = function(code, more) { - if(!this.stream.writable) { return false; } + if(!this.stream.writable) { + return false; + } if(more === true) { this.writer.addHeader(code); } else { @@ -308,11 +310,12 @@ Connection.prototype.sync = function() { this._send(0x53); }; +const END_BUFFER = new Buffer([0x58, 0x00, 0x00, 0x00, 0x04]); Connection.prototype.end = function() { //0x58 = 'X' this.writer.add(emptyBuffer); this._ending = true; - this._send(0x58); + return this.stream.end(END_BUFFER); }; Connection.prototype.close = function(msg, more) { diff --git a/lib/promise.js b/lib/promise.js new file mode 100644 index 00000000..4d308cb9 --- /dev/null +++ b/lib/promise.js @@ -0,0 +1,12 @@ +const util = require('util') +const deprecationMessage = 'Using the promise result as an event emitter is deprecated and will be removed in pg@8.0' +module.exports = function(emitter, callback) { + const promise = new global.Promise(callback) + promise.on = util.deprecate(function () { + emitter.on.apply(emitter, arguments) + }, deprecationMessage); + + promise.once = util.deprecate(function () { + emitter.once.apply(emitter, arguments) + }, deprecationMessage) +} diff --git a/test/integration/client/error-handling-tests.js b/test/integration/client/error-handling-tests.js index 08e78b33..4a2be1ee 100644 --- a/test/integration/client/error-handling-tests.js +++ b/test/integration/client/error-handling-tests.js @@ -1,103 +1,111 @@ +"use strict"; + var helper = require('./test-helper'); var util = require('util'); var pg = helper.pg - var createErorrClient = function() { var client = helper.client(); client.once('error', function(err) { - //console.log('error', util.inspect(err)); assert.fail('Client shoud not throw error during query execution'); }); client.on('drain', client.end.bind(client)); return client; }; -test('error handling', function() { - test('within a simple query', function() { - var client = createErorrClient(); +const suite = new helper.Suite('error handling') - var query = client.query(new pg.Query("select eeeee from yodas_dsflsd where pixistix = 'zoiks!!!'")); - - assert.emits(query, 'error', function(error) { - assert.equal(error.severity, "ERROR"); - }); - }); - - test('within a prepared statement', function() { - - var client = createErorrClient(); - - var q = client.query({text: "CREATE TEMP TABLE boom(age integer); INSERT INTO boom (age) VALUES (28);", binary: false}); - - test("when query is parsing", function() { - - //this query wont parse since there ain't no table named bang - - var ensureFuture = function(testClient) { - test("client can issue more queries successfully", function() { - var goodQuery = testClient.query(new pg.Query("select age from boom")); - assert.emits(goodQuery, 'row', function(row) { - assert.equal(row.age, 28); - }); - }); - }; - - var query = client.query(new pg.Query({ - text: "select * from bang where name = $1", - values: ['0'] - })); - - test("query emits the error", function() { - assert.emits(query, 'error', function(err) { - ensureFuture(client); - }); - }); - - test("when a query is binding", function() { - - var query = client.query(new pg.Query({ - text: 'select * from boom where age = $1', - values: ['asldkfjasdf'] - })); - - test("query emits the error", function() { - - assert.emits(query, 'error', function(err) { - test('error has right severity', function() { - assert.equal(err.severity, "ERROR"); - }) - - ensureFuture(client); - }); - }); - - //TODO how to test for errors during execution? - }); - }); - }); - - test('non-query error', function() { - var client = new Client({ - user:'asldkfjsadlfkj' - }); - assert.emits(client, 'error'); - client.connect(); - }); - - test('non-query error with callback', function() { - var client = new Client({ - user:'asldkfjsadlfkj' - }); - client.connect(assert.calls(function(error, client) { - assert.ok(error); +suite.test('query receives error on client shutdown', false, function(done) { + var client = new Client(); + client.connect(function(err) { + if (err) { + return done(err) + } + client.query('SELECT pg_sleep(5)', assert.calls(function(err, res) { + assert(err instanceof Error) + done() })); + setTimeout(() => { + client.end() + assert.emits(client, 'end'); + }, 50) }); - }); -test('non-error calls supplied callback', function() { +suite.test('within a simple query', (done) => { + var client = createErorrClient(); + + var query = client.query(new pg.Query("select eeeee from yodas_dsflsd where pixistix = 'zoiks!!!'")); + + assert.emits(query, 'error', function(error) { + assert.equal(error.severity, "ERROR"); + done(); + }); +}); + +(function () { + var client = createErorrClient(); + + var q = client.query({ text: "CREATE TEMP TABLE boom(age integer); INSERT INTO boom (age) VALUES (28);", binary: false }); + + var ensureFuture = function (testClient, done) { + var goodQuery = testClient.query(new pg.Query("select age from boom")); + assert.emits(goodQuery, 'row', function (row) { + assert.equal(row.age, 28); + done(); + }); + }; + + suite.test("when query is parsing", (done) => { + + //this query wont parse since there isn't a table named bang + var query = client.query(new pg.Query({ + text: "select * from bang where name = $1", + values: ['0'] + })); + + assert.emits(query, 'error', function (err) { + ensureFuture(client, done); + }); + }); + + suite.test("when a query is binding", function (done) { + + var query = client.query(new pg.Query({ + text: 'select * from boom where age = $1', + values: ['asldkfjasdf'] + })); + + assert.emits(query, 'error', function (err) { + assert.equal(err.severity, "ERROR"); + ensureFuture(client, done); + }); + }); +})(); + +suite.test('non-query error', function(done) { + var client = new Client({ + user:'asldkfjsadlfkj' + }); + client.on('error', (err) => { + assert(err instanceof Error) + done() + }); + client.connect(); +}); + +suite.test('non-query error with callback', function(done) { + var client = new Client({ + user:'asldkfjsadlfkj' + }); + client.connect(assert.calls(function(error, client) { + assert(error instanceof Error) + done() + })); +}); + +suite.test('non-error calls supplied callback', function(done) { var client = new Client({ user: helper.args.user, password: helper.args.password, @@ -108,75 +116,23 @@ test('non-error calls supplied callback', function() { client.connect(assert.calls(function(err) { assert.ifError(err); - client.end(); + client.end(done); })) }); -test('when connecting to invalid host', function() { - //this test fails about 30% on travis and only on travis... - //I'm not sure what the cause could be - if(process.env.TRAVIS) return false; - +suite.test('when connecting to invalid host with promise', function(done) { var client = new Client({ - user: 'aslkdjfsdf', - password: '1234', - host: 'asldkfjasdf!!#1308140.com' + host: 'asdlfkjasldkfjlaskdfj' }); - - var delay = 5000; - var tid = setTimeout(function() { - var msg = "When connecting to an invalid host the error event should be emitted but it has been " + delay + " and still no error event." - assert(false, msg); - }, delay); - client.on('error', function() { - clearTimeout(tid); - }) - client.connect(); + client.connect().catch((e) => done()); }); -test('when connecting to invalid host with callback', function() { +suite.test('when connecting to an invalid host with callback', function (done) { var client = new Client({ - user: 'brian', - password: '1234', host: 'asldkfjasdf!!#1308140.com' }); client.connect(function(error, client) { - assert(error); + assert(error instanceof Error); + done(); }); }); - -test('multiple connection errors (gh#31)', function() { - return false; - test('with single client', function() { - //don't run yet...this test fails...need to think of fix - var client = new Client({ - user: 'blaksdjf', - password: 'omfsadfas', - host: helper.args.host, - port: helper.args.port, - database: helper.args.database - }); - client.connect(); - assert.emits(client, 'error', function(e) { - client.connect(); - assert.emits(client, 'error'); - }); - }); - - test('with callback method', function() { - var badConString = "postgres://aslkdfj:oi14081@"+helper.args.host+":"+helper.args.port+"/"+helper.args.database; - return false; - }); -}); - -test('query receives error on client shutdown', function() { - var client = new Client(helper.config); - client.connect(assert.calls(function() { - client.query('SELECT pg_sleep(5)', assert.calls(function(err, res) { - assert(err); - })); - client.end(); - assert.emits(client, 'end'); - })); -}); - diff --git a/test/integration/client/promise-api-tests.js b/test/integration/client/promise-api-tests.js index 783fb46c..17094c9d 100644 --- a/test/integration/client/promise-api-tests.js +++ b/test/integration/client/promise-api-tests.js @@ -1,69 +1,9 @@ -const async = require('async') +'use strict'; + const helper = require('./test-helper') const pg = helper.pg; -class Test { - constructor(name, cb) { - this.name = name - this.action = cb - this.timeout = 5000 - } - - run(cb) { - try { - this._run(cb) - } catch (e) { - cb(e) - } - } - - _run(cb) { - if (!this.action) { - console.log(`${this.name} skipped`) - return cb() - } - if (!this.action.length) { - const result = this.action.call(this) - if ((result || 0).then) { - result - .then(() => cb()) - .catch(err => cb(err || new Error('Unhandled promise rejection'))) - } - } else { - this.action.call(this, cb) - } - } -} - -class Suite { - constructor() { - console.log('') - this._queue = async.queue(this.run.bind(this), 1) - this._queue.drain = () => { } - } - - run(test, cb) { - const tid = setTimeout(() => { - const err = Error(`test: ${test.name} did not complete withint ${test.timeout}ms`) - cb(err) - }, test.timeout) - test.run((err) => { - clearTimeout(tid) - if (err) { - console.log(test.name + ' FAILED!', err.stack) - } else { - console.log(test.name) - } - cb(err) - }) - } - - test(name, cb) { - this._queue.push(new Test(name, cb)) - } -} - -const suite = new Suite() +const suite = new helper.Suite() suite.test('valid connection completes promise', () => { const client = new pg.Client() @@ -93,11 +33,15 @@ suite.test('invalid connection rejects promise', (done) => { }) }) -suite.test('connected client does not reject promise after', (done) => { +suite.test('connected client does not reject promise after connection', (done) => { const client = new pg.Client() return client.connect() .then(() => { setTimeout(() => { + client.on('error', (e) => { + assert(e instanceof Error) + done() + }) // manually kill the connection client.connection.stream.end() }, 50) 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 e301ab68..a7bda9c7 100644 --- a/test/integration/client/query-error-handling-prepared-statement-tests.js +++ b/test/integration/client/query-error-handling-prepared-statement-tests.js @@ -2,7 +2,9 @@ var helper = require('./test-helper'); var Query = helper.pg.Query; var util = require('util'); -function killIdleQuery(targetQuery) { +var suite = new helper.Suite(); + +function killIdleQuery(targetQuery, cb) { var client2 = new Client(helper.args); var pidColName = 'procpid' var queryColName = 'current_query'; @@ -16,16 +18,16 @@ function killIdleQuery(targetQuery) { client2.query(killIdleQuery, [targetQuery], assert.calls(function(err, res) { assert.ifError(err); assert.equal(res.rows.length, 1); - client2.end(); + client2.end(cb); assert.emits(client2, 'end'); })); })); })); } -test('query killed during query execution of prepared statement', function() { +suite.test('query killed during query execution of prepared statement', function(done) { if(helper.args.native) { - return false; + return done(); } var client = new Client(helper.args); client.connect(assert.success(function() { @@ -56,11 +58,11 @@ test('query killed during query execution of prepared statement', function() { assert.fail('Prepared statement when executed should not return before being killed'); }); - killIdleQuery(sleepQuery); + killIdleQuery(sleepQuery, done); })); }); -test('client end during query execution of prepared statement', function() { +suite.test('client end during query execution of prepared statement', function(done) { var client = new Client(helper.args); client.connect(assert.success(function() { var sleepQuery = 'select pg_sleep($1)'; @@ -90,6 +92,6 @@ test('client end during query execution of prepared statement', function() { assert.fail('Prepared statement when executed should not return before being killed'); }); - client.end(); + client.end(done); })); }); diff --git a/test/suite.js b/test/suite.js new file mode 100644 index 00000000..37227fc9 --- /dev/null +++ b/test/suite.js @@ -0,0 +1,74 @@ +'use strict'; + +const async = require('async') + +class Test { + constructor(name, cb) { + this.name = name + this.action = cb + this.timeout = 5000 + } + + run(cb) { + try { + this._run(cb) + } catch (e) { + cb(e) + } + } + + _run(cb) { + if (!this.action) { + console.log(`${this.name} skipped`) + return cb() + } + if (!this.action.length) { + const result = this.action.call(this) + if ((result || 0).then) { + result + .then(() => cb()) + .catch(err => cb(err || new Error('Unhandled promise rejection'))) + } + } else { + this.action.call(this, cb) + } + } +} + +class Suite { + constructor(name) { + console.log('') + this._queue = async.queue(this.run.bind(this), 1) + this._queue.drain = () => { } + } + + run(test, cb) { + process.stdout.write(test.name + ' ') + if (!test.action) { + process.stdout.write('? - SKIPPED') + return cb() + } + + const tid = setTimeout(() => { + const err = Error(`test: ${test.name} did not complete withint ${test.timeout}ms`) + cb(err) + }, test.timeout) + + test.run((err) => { + clearTimeout(tid) + if (err) { + process.stdout.write(`FAILED!\n\n${err.stack}\n`) + process.exit(-1) + } else { + process.stdout.write('✔\n') + } + cb(err) + }) + } + + test(name, cb) { + this._queue.push(new Test(name, cb)) + } +} + +module.exports = Suite diff --git a/test/test-helper.js b/test/test-helper.js index f39cbfef..4cf6a455 100644 --- a/test/test-helper.js +++ b/test/test-helper.js @@ -240,6 +240,7 @@ var resetTimezoneOffset = function() { module.exports = { Sink: Sink, + Suite: require('./suite'), pg: require(__dirname + '/../lib/'), args: args, config: args,