Skip to content

Commit

Permalink
Add onFailure to query#then (#1082)
Browse files Browse the repository at this point in the history
The promise adapter I had implemented wasn't spec compliant: it didn't accept both `onSuccess` and `onFailure` in the call to `query#then`.  This subtly broke yield & async/await because they both rely on `onError` being passed into `Promise#then`.  The pool was also not returning the promise after a client was acquired, which broke awaiting on `pool.connect` - this is also fixed now.
  • Loading branch information
brianc committed Jul 19, 2016
1 parent 1dc1dbc commit b1b2801
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/index.js
Expand Up @@ -68,13 +68,13 @@ PG.prototype.connect = function(config, callback) {

this._pools[poolName] = this._pools[poolName] || new this.Pool(config);
var pool = this._pools[poolName];
pool.connect(callback);
if(!pool.listeners('error').length) {
//propagate errors up to pg object
pool.on('error', function(e) {
this.emit('error', e, e.client);
}.bind(this));
}
return pool.connect(callback);
};

// cancel the query running on the given client
Expand Down
4 changes: 2 additions & 2 deletions lib/native/query.js
Expand Up @@ -34,8 +34,8 @@ var NativeQuery = module.exports = function(native) {

util.inherits(NativeQuery, EventEmitter);

NativeQuery.prototype.then = function(callback) {
return this.promise().then(callback);
NativeQuery.prototype.then = function(onSuccess, onFailure) {
return this.promise().then(onSuccess, onFailure);
};

NativeQuery.prototype.catch = function(callback) {
Expand Down
4 changes: 2 additions & 2 deletions lib/query.js
Expand Up @@ -40,8 +40,8 @@ var Query = function(config, values, callback) {

util.inherits(Query, EventEmitter);

Query.prototype.then = function(callback) {
return this.promise().then(callback);
Query.prototype.then = function(onSuccess, onFailure) {
return this.promise().then(onSuccess, onFailure);
};

Query.prototype.catch = function(callback) {
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -28,6 +28,7 @@
},
"devDependencies": {
"async": "0.9.0",
"co": "4.6.0",
"jshint": "2.5.2",
"lodash": "4.13.1",
"pg-copy-streams": "0.3.0",
Expand Down
28 changes: 28 additions & 0 deletions test/integration/connection-pool/yield-support-body.js
@@ -0,0 +1,28 @@
var helper = require('./test-helper')
var co = require('co')

var tid = setTimeout(function() {
throw new Error('Tests did not complete in tme')
}, 1000)

co(function * () {
var client = yield helper.pg.connect()
var res = yield client.query('SELECT $1::text as name', ['foo'])
assert.equal(res.rows[0].name, 'foo')

var threw = false
try {
yield client.query('SELECT LKDSJDSLKFJ')
} catch(e) {
threw = true
}
assert(threw)
client.release()
helper.pg.end()
clearTimeout(tid)
})
.catch(function(e) {
setImmediate(function() {
throw e
})
})
5 changes: 5 additions & 0 deletions test/integration/connection-pool/yield-support-tests.js
@@ -0,0 +1,5 @@
var semver = require('semver')
if (semver.lt(process.version, '1.0.0')) {
return console.log('yield is not supported in node <= v0.12')
}
require('./yield-support-body')

0 comments on commit b1b2801

Please sign in to comment.