Skip to content

Commit 56a5903

Browse files
committed
Make throws in query error callback not break client
If you receive an error while running a query and in user's callback they throw an exception it can disrupt the internal query queue and prevent a client from ever cleaning up properly
1 parent 9b1c4fa commit 56a5903

File tree

3 files changed

+33
-18
lines changed

3 files changed

+33
-18
lines changed

lib/client.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,9 @@ Client.prototype.connect = function(callback) {
173173
if(self.activeQuery.isPreparedStatement) {
174174
con.sync();
175175
}
176-
self.activeQuery.handleError(error);
176+
var activeQuery = self.activeQuery;
177177
self.activeQuery = null;
178+
activeQuery.handleError(error);
178179
}
179180
});
180181

lib/native/query.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ NativeQuery.prototype.handleError = function(error) {
5858
this._canceledDueToError = false;
5959
}
6060
if(this.callback) {
61-
this.callback(error);
61+
var cb = this.callback;
62+
//remove callback to prevent double call on readyForQuery
6263
this.callback = null;
64+
cb(error);
6365
} else {
6466
this.emit('error', error);
6567
}
Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,34 @@
11
var helper = require(__dirname + '/test-helper');
22
var util = require('util');
33

4-
test('error during query execution', function() {
5-
var client = new Client(helper.args);
6-
process.removeAllListeners('uncaughtException');
7-
assert.emits(process, 'uncaughtException', function() {
8-
assert.equal(client.activeQuery, null, 'should remove active query even if error happens in callback');
9-
client.query('SELECT * FROM blah', assert.success(function(result) {
10-
assert.equal(result.rows.length, 1);
11-
client.end();
12-
}));
13-
});
14-
client.connect(assert.success(function() {
15-
client.query('CREATE TEMP TABLE "blah"(data text)', assert.success(function() {
16-
var q = client.query('INSERT INTO blah(data) VALUES($1)', ['yo'], assert.success(function() {
17-
assert.emits(client, 'drain');
18-
throw new Error('WHOOOAAAHH!!');
4+
var withQuery = function(text, resultLength, cb) {
5+
test('error during query execution', function() {
6+
var client = new Client(helper.args);
7+
process.removeAllListeners('uncaughtException');
8+
assert.emits(process, 'uncaughtException', function() {
9+
console.log('got uncaught exception')
10+
assert.equal(client.activeQuery, null, 'should remove active query even if error happens in callback');
11+
client.query('SELECT * FROM blah', assert.success(function(result) {
12+
assert.equal(result.rows.length, resultLength);
13+
client.end();
14+
cb();
15+
}));
16+
});
17+
client.connect(assert.success(function() {
18+
client.query('CREATE TEMP TABLE "blah"(data text)', assert.success(function() {
19+
var q = client.query(text, ['yo'], assert.calls(function() {
20+
assert.emits(client, 'drain');
21+
throw new Error('WHOOOAAAHH!!');
22+
}));
1923
}));
2024
}));
21-
}));
25+
});
26+
}
27+
28+
//test with good query so our callback is called
29+
//as a successful callback
30+
withQuery('INSERT INTO blah(data) VALUES($1)', 1, function() {
31+
//test with an error query so our callback is called with an error
32+
withQuery('INSERT INTO asldkfjlaskfj eoooeoriiri', 0, function() {
33+
});
2234
});

0 commit comments

Comments
 (0)