callback in nextTick so it can't corrupt client

Author: twobitfool

lib/query.js

@@ -95,9 +95,17 @@ Query.prototype.handleReadyForQuery = function() {
   if(this._canceledDueToError) {
     return this.handleError(this._canceledDueToError);
   }
+
   if(this.callback) {
-    this.callback(null, this._result);
+    var that = {
+      callback: this.callback,
+      result:   this._result
+    };
+    process.nextTick(function () {
+      that.callback(null, that.result);
+    });
   }
+
   this.emit('end', this._result);
 };
 

test/integration/callback-crash-test.js

@@ -0,0 +1,45 @@
+var args = require(__dirname + '/../cli');
+var pg = require(__dirname + "/../../lib");
+
+process.on('uncaughtException', function(err) {
+  console.log(err.message);
+});
+
+var query = {
+  text: 'select * from person where id = $1'
+  , values: [1]
+};
+var queries = 0;
+
+pg.connect(args, function(err, client) {
+  setTimeout(function() {
+    throw new Error('#1');
+  }, 500);
+
+  setTimeout(function() {
+    client.query(query, function(err, result) {
+      console.log('ran first query');
+      queries++;
+      throw new Error('#2');
+    });
+  }, 1000);
+
+  setTimeout(function() {
+    client.query(query, function(err, result) {
+      console.log('ran second query');
+      queries++;
+    });
+  }, 1500);
+
+  setTimeout(function() {
+    client.end();
+    if (queries !== 2) {
+      throw new Error('test failed!');
+    }
+  }, 2000);
+
+  setTimeout(function() {
+    process.exit();
+  }, 2500);
+
+});

test/test-helper.js

@@ -10,6 +10,9 @@ var Connection = require(__dirname + '/../lib/connection');
 Client = require(__dirname + '/../lib').Client;
 
 process.on('uncaughtException', function(d) {
+  // quietly absorb fake errors
+  if (d.message === 'Fake Error') return;
+
   if ('stack' in d && 'message' in d) {
     console.log("Message: " + d.message);
     console.log(d.stack);
@@ -187,6 +190,9 @@ process.on('exit', function() {
 })
 
 process.on('uncaughtException', function(err) {
+  // quietly absorb fake errors
+  if (err.message === 'Fake Error') return;
+
   console.error("\n %s", err.stack || err.toString())
   //causes xargs to abort right away
   process.exit(255);

test/unit/client/simple-query-tests.js

@@ -121,6 +121,53 @@ test('executing query', function() {
       });
     });
 
+    test('handleReadyForQuery emits \'end\' even if callback throws an exception', function() {
+      var client = helper.client();
+      var query = client.query('whatever');
+
+      query.callback = function() {
+        throw new Error('Fake Error');
+      };
+      assert.emits(query, 'end');
+
+      try {
+        query.handleReadyForQuery();
+      }
+      catch (e) {}
+    });
+
+    test('handleError emits \'end\' even if callback throws an exception', function() {
+      var client = helper.client();
+      var query = client.query('whatever');
+
+      query.callback = function() {
+        throw new Error('Fake Error');
+      };
+      assert.emits(query, 'end');
+
+      try {
+        query.handleReadyForQuery();
+      }
+      catch (e) {}
+    });
+
+    test('client \'readyForQuery\' handler sets readyForQuery to true even if callback throws an exception', function() {
+      var client = helper.client();
+      var query = client.query('whatever');
+
+      query.callback = function() {
+        throw new Error('Fake Error');
+      };
+      client.activeQuery = query;
+      client.readyForQuery = false;
+      client._pulseQueryQueue = function() {};
+      try {
+        client.connection.emit('readyForQuery');
+      }
+      catch (e) {}
+      assert.equal(true, client.readyForQuery);
+    });
+
   });
 
 });