Fix brianc#2556 by keeping callback errors from interfering with cleanup (brianc#2753)

* Fix brianc#2556 (handleRowDescription of null) by keeping callback errors from interfering with cleanup

* Added regression test for brianc#2556

Author: prust

packages/pg/lib/query.js

@@ -135,7 +135,14 @@ class Query extends EventEmitter {
       return this.handleError(this._canceledDueToError, con)
     }
     if (this.callback) {
-      this.callback(null, this._results)
+      try {
+        this.callback(null, this._results)
+      }
+      catch(err) {
+        process.nextTick(() => {
+          throw err
+        })
+      }
     }
     this.emit('end', this._results)
   }

packages/pg/test/integration/gh-issues/2556-tests.js

@@ -0,0 +1,40 @@
+'use strict'
+var helper = require('./../test-helper')
+var assert = require('assert')
+
+var callbackError = new Error('TEST: Throw in callback')
+
+const suite = new helper.Suite()
+
+suite.test('it should cleanup client even if an error is thrown in a callback', (done) => {
+  // temporarily replace the test framework's uncaughtException handlers
+  // with a custom one that ignores the callbackError
+  let original_handlers = process.listeners('uncaughtException')
+  process.removeAllListeners('uncaughtException')
+  process.on('uncaughtException', (err) => {
+    if (err != callbackError) {
+      original_handlers[0](err)
+    }
+  })
+
+  // throw an error in a callback and verify that a subsequent query works without error
+  var client = helper.client()
+  client.query('SELECT NOW()', (err) => {
+    assert(!err)
+    setTimeout(reuseClient, 50)
+    throw callbackError
+  })
+
+  function reuseClient() {
+    client.query('SELECT NOW()', (err) => {
+      assert(!err)
+
+      // restore the test framework's uncaughtException handlers
+      for (let handler of original_handlers) {
+        process.on('uncaughtException', handler)
+      }
+
+      client.end(done)
+    })
+  }
+})