Add onFailure to query#then (brianc#1082)

brianc · web-flow · commit b1b2801c710d · 2016-07-19T10:16:48.000-05:00
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 &amp; 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.
diff --git a/lib/index.js b/lib/index.js
@@ -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
diff --git a/lib/native/query.js b/lib/native/query.js
@@ -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) {
diff --git a/lib/query.js b/lib/query.js
@@ -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) {
diff --git a/package.json b/package.json
@@ -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",
diff --git a/test/integration/connection-pool/yield-support-body.js b/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
+  })
+})
diff --git a/test/integration/connection-pool/yield-support-tests.js b/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')
