Skip to content

Commit 76c59a0

Browse files
committed
Emit error when backend unexpectedly disconnects
1 parent f2b87e0 commit 76c59a0

6 files changed

+156
-24
lines changed

lib/client.js

+35-11
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ var Client = function(config) {
3131
var c = config || {};
3232

3333
this._types = new TypeOverrides(c.types);
34+
this._ending = false;
35+
this._connecting = false;
36+
this._connectionError = false;
3437

3538
this.connection = c.connection || new Connection({
3639
stream: c.stream,
@@ -50,6 +53,7 @@ util.inherits(Client, EventEmitter);
5053
Client.prototype.connect = function(callback) {
5154
var self = this;
5255
var con = this.connection;
56+
this._connecting = true;
5357

5458
if(this.host && this.host.indexOf('/') === 0) {
5559
con.connect(this.host + '/.s.PGSQL.' + this.port);
@@ -107,6 +111,7 @@ Client.prototype.connect = function(callback) {
107111
//hook up query handling events to connection
108112
//after the connection initially becomes ready for queries
109113
con.once('readyForQuery', function() {
114+
self._connecting = false;
110115

111116
//delegate rowDescription to active query
112117
con.on('rowDescription', function(msg) {
@@ -175,34 +180,52 @@ Client.prototype.connect = function(callback) {
175180
});
176181

177182
con.on('error', function(error) {
178-
if(self.activeQuery) {
183+
if(this.activeQuery) {
179184
var activeQuery = self.activeQuery;
180-
self.activeQuery = null;
185+
this.activeQuery = null;
181186
return activeQuery.handleError(error, con);
182187
}
188+
189+
if (this._connecting) {
190+
// set a flag indicating we've seen an error during connection
191+
// the backend will terminate the connection and we don't want
192+
// to throw a second error when the connection is terminated
193+
this._connectionError = true;
194+
}
195+
183196
if(!callback) {
184-
return self.emit('error', error);
197+
return this.emit('error', error);
185198
}
199+
186200
con.end(); // make sure ECONNRESET errors don't cause error events
187201
callback(error);
188202
callback = null;
189-
});
203+
}.bind(this));
190204

191205
con.once('end', function() {
192-
if ( callback ) {
193-
// haven't received a connection message yet !
206+
if (callback) {
207+
// haven't received a connection message yet!
194208
var err = new Error('Connection terminated');
195209
callback(err);
196210
callback = null;
197211
return;
198212
}
199-
if(self.activeQuery) {
213+
if(this.activeQuery) {
200214
var disconnectError = new Error('Connection terminated');
201-
self.activeQuery.handleError(disconnectError, con);
202-
self.activeQuery = null;
215+
this.activeQuery.handleError(disconnectError, con);
216+
this.activeQuery = null;
203217
}
204-
self.emit('end');
205-
});
218+
if (!this._ending) {
219+
// if the connection is ended without us calling .end()
220+
// on this client then we have an unexpected disconnection
221+
// treat this as an error unless we've already emitted an error
222+
// during connection.
223+
if (!this._connectionError) {
224+
this.emit('error', new Error('Connection terminated unexpectedly'));
225+
}
226+
}
227+
this.emit('end');
228+
}.bind(this));
206229

207230

208231
con.on('notice', function(msg) {
@@ -342,6 +365,7 @@ Client.prototype.query = function(config, values, callback) {
342365
};
343366

344367
Client.prototype.end = function(cb) {
368+
this._ending = true;
345369
this.connection.end();
346370
if (cb) {
347371
this.connection.once('end', cb);

lib/connection.js

-5
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,6 @@ Connection.prototype.connect = function(port, host) {
8787
});
8888

8989
this.stream.on('close', function() {
90-
// NOTE: node-0.10 emits both 'end' and 'close'
91-
// for streams closed by the peer, while
92-
// node-0.8 only emits 'close'
9390
self.emit('end');
9491
});
9592

@@ -143,8 +140,6 @@ Connection.prototype.attachListeners = function(stream) {
143140
};
144141

145142
Connection.prototype.requestSsl = function() {
146-
this.checkSslResponse = true;
147-
148143
var bodyBuffer = this.writer
149144
.addInt16(0x04D2)
150145
.addInt16(0x162F).flush();

test/integration/client/error-handling-tests.js

+19-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,24 @@
11
var helper = require(__dirname + '/test-helper');
22
var util = require('util');
33

4+
5+
test('non-query error with callback', function () {
6+
var client = new Client({
7+
user:'asldkfjsadlfkj'
8+
});
9+
client.connect(assert.calls(function (err) {
10+
assert(err);
11+
}));
12+
});
13+
14+
test('non-query error', function() {
15+
var client = new Client({
16+
user:'asldkfjsadlfkj'
17+
});
18+
assert.emits(client, 'error');
19+
client.connect();
20+
});
21+
422
var createErorrClient = function() {
523
var client = helper.client();
624
client.once('error', function(err) {
@@ -11,9 +29,8 @@ var createErorrClient = function() {
1129
return client;
1230
};
1331

14-
test('error handling', function(){
32+
test('error handling', function() {
1533
test('within a simple query', function() {
16-
1734
var client = createErorrClient();
1835

1936
var query = client.query("select omfg from yodas_dsflsd where pixistix = 'zoiks!!!'");
@@ -77,7 +94,6 @@ test('error handling', function(){
7794
});
7895

7996
test('non-query error', function() {
80-
8197
var client = new Client({
8298
user:'asldkfjsadlfkj'
8399
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
var co = require('co')
2+
3+
var buffers = require('../../test-buffers')
4+
var helper = require('./test-helper')
5+
6+
var net = require('net')
7+
8+
var Server = function(response) {
9+
this.server = undefined
10+
this.socket = undefined
11+
this.response = response
12+
}
13+
14+
Server.prototype.start = function (cb) {
15+
// this is our fake postgres server
16+
// it responds with our specified response immediatley after receiving every buffer
17+
// this is sufficient into convincing the client its connectet to a valid backend
18+
// if we respond with a readyForQuery message
19+
this.server = net.createServer(function (socket) {
20+
this.socket = socket
21+
if (this.response) {
22+
this.socket.on('data', function (data) {
23+
// deny request for SSL
24+
if (data.length == 8) {
25+
this.socket.write(new Buffer('N', 'utf8'))
26+
// consider all authentication requests as good
27+
} else if (!data[0]) {
28+
this.socket.write(buffers.authenticationOk())
29+
// respond with our canned response
30+
} else {
31+
this.socket.write(this.response)
32+
}
33+
}.bind(this))
34+
}
35+
}.bind(this))
36+
37+
var port = 54321
38+
39+
var options = {
40+
host: 'localhost',
41+
port: port,
42+
}
43+
this.server.listen(options.port, options.host, function () {
44+
cb(options)
45+
})
46+
}
47+
48+
Server.prototype.drop = function () {
49+
this.socket.end()
50+
}
51+
52+
Server.prototype.close = function (cb) {
53+
this.server.close(cb)
54+
}
55+
56+
var testServer = function (server, cb) {
57+
// wait for our server to start
58+
server.start(function(options) {
59+
// connect a client to it
60+
var client = new helper.Client(options)
61+
client.connect()
62+
63+
// after 50 milliseconds, drop the client
64+
setTimeout(function() {
65+
server.drop()
66+
}, 50)
67+
68+
// blow up if we don't receive an error
69+
var timeoutId = setTimeout(function () {
70+
throw new Error('Client should have emitted an error but it did not.')
71+
}, 5000)
72+
73+
// return our wait token
74+
client.on('error', function () {
75+
clearTimeout(timeoutId)
76+
server.close(cb)
77+
})
78+
})
79+
}
80+
81+
// test being disconnected after readyForQuery
82+
const respondingServer = new Server(buffers.readyForQuery())
83+
testServer(respondingServer, function () {
84+
process.stdout.write('.')
85+
// test being disconnected from a server that never responds
86+
const silentServer = new Server()
87+
testServer(silentServer, function () {
88+
process.stdout.write('.')
89+
})
90+
})

test/integration/client/query-error-handling-prepared-statement-tests.js

+10-5
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,18 @@ test('query killed during query execution of prepared statement', function() {
2929
var client = new Client(helper.args);
3030
client.connect(assert.success(function() {
3131
var sleepQuery = 'select pg_sleep($1)';
32-
var query1 = client.query({
32+
33+
const queryConfig = {
3334
name: 'sleep query',
3435
text: sleepQuery,
35-
values: [5] },
36-
assert.calls(function(err, result) {
37-
assert.equal(err.message, 'terminating connection due to administrator command');
36+
values: [5],
37+
};
38+
39+
// client should emit an error because it is unexpectedly disconnected
40+
assert.emits(client, 'error')
41+
42+
var query1 = client.query(queryConfig, assert.calls(function(err, result) {
43+
assert.equal(err.message, 'terminating connection due to administrator command');
3844
}));
3945

4046
query1.on('error', function(err) {
@@ -53,7 +59,6 @@ test('query killed during query execution of prepared statement', function() {
5359
}));
5460
});
5561

56-
5762
test('client end during query execution of prepared statement', function() {
5863
var client = new Client(helper.args);
5964
client.connect(assert.success(function() {

test/unit/client/stream-and-query-error-interaction-tests.js

+2
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ test('emits end when not in query', function() {
77
stream.write = function() {
88
//NOOP
99
}
10+
1011
var client = new Client({connection: new Connection({stream: stream})});
1112
client.connect(assert.calls(function() {
1213
client.query('SELECT NOW()', assert.calls(function(err, result) {
1314
assert(err);
1415
}));
1516
}));
17+
assert.emits(client, 'error');
1618
assert.emits(client, 'end');
1719
client.connection.emit('connect');
1820
process.nextTick(function() {

0 commit comments

Comments
 (0)