Skip to content

Commit 9b1c4fa

Browse files
committed
Make query callback exceptions not break client
If you throw an exception in a query callback the client will not pulse its internal query queue and therefor will never process any more queries or emit its own 'drain' event. I don't find this to be an issue in production code since I restart the process on exceptions, but it can break tests and cause things to 'hang'. My crude benchmarks show no noticable impact in perf from the try/catch/rethrow. :q
1 parent e95d28d commit 9b1c4fa

File tree

3 files changed

+42
-2
lines changed

3 files changed

+42
-2
lines changed

lib/client.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,22 @@ Client.prototype.connect = function(callback) {
143143
});
144144

145145
con.on('readyForQuery', function() {
146+
var error;
146147
if(self.activeQuery) {
147-
self.activeQuery.handleReadyForQuery();
148+
//try/catch/rethrow to ensure exceptions don't prevent the queryQueue from
149+
//being processed
150+
try{
151+
self.activeQuery.handleReadyForQuery();
152+
} catch(e) {
153+
error = e;
154+
}
148155
}
149156
self.activeQuery = null;
150157
self.readyForQuery = true;
151158
self._pulseQueryQueue();
159+
if(error) {
160+
throw error;
161+
}
152162
});
153163

154164
con.on('error', function(error) {

lib/native/index.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,23 @@ var clientBuilder = function(config) {
208208
});
209209

210210
connection.on('_readyForQuery', function() {
211+
var error;
211212
var q = this._activeQuery;
212213
//a named query finished being prepared
213214
if(this._namedQuery) {
214215
this._namedQuery = false;
215216
this._sendQueryPrepared(q.name, q.values||[]);
216217
} else {
217-
connection._activeQuery.handleReadyForQuery(connection._lastMeta);
218+
//try/catch/rethrow to ensure exceptions don't prevent the queryQueue from
219+
//being processed
220+
try{
221+
connection._activeQuery.handleReadyForQuery(connection._lastMeta);
222+
} catch(e) {
223+
error = e;
224+
}
218225
connection._activeQuery = null;
219226
connection._pulseQueryQueue();
227+
if(error) throw error;
220228
}
221229
});
222230
connection.on('copyInResponse', function () {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
var helper = require(__dirname + '/test-helper');
2+
var util = require('util');
3+
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!!');
19+
}));
20+
}));
21+
}));
22+
});

0 commit comments

Comments
 (0)