From 64d6883a8173f4be5afe335af8d6f0090ada7f9f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 17 Mar 2014 18:13:50 +0100 Subject: [PATCH 1/6] Ensure connect callback is invoked on premature socket hangup Closes #534 --- lib/client.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/client.js b/lib/client.js index 490e1d1..73f910a 100644 --- a/lib/client.js +++ b/lib/client.js @@ -170,9 +170,17 @@ Client.prototype.connect = function(callback) { return self.emit('error', error); } callback(error); + callback = null; }); con.once('end', function() { + if ( callback ) { + // haven't received a connection message yet ! + var err = new Error("Stream unexpectedly ended before getting ready for query execution"); + callback(err); + callback = null; + return; + } if(self.activeQuery) { var disconnectError = new Error('Stream unexpectedly ended during query execution'); self.activeQuery.handleError(disconnectError); From e19235838da0aff961c8f0bfbe12baf0c3c04391 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 18 Mar 2014 13:19:07 +0100 Subject: [PATCH 2/6] Add unit test for callback on early postgresql disconnect Test adapted by that provided by Jess Sheneberger in #534 --- .../client/callback-on-early-disconnect.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test/unit/client/callback-on-early-disconnect.js diff --git a/test/unit/client/callback-on-early-disconnect.js b/test/unit/client/callback-on-early-disconnect.js new file mode 100644 index 0000000..edf5748 --- /dev/null +++ b/test/unit/client/callback-on-early-disconnect.js @@ -0,0 +1,22 @@ +var helper = require(__dirname + '/test-helper'); +var net = require('net'); +var pg = require('../../..//lib/index.js'); + +var server = net.createServer(function(c) { + console.log('server connected'); + c.destroy(); + console.log('server socket destroyed.'); + server.close(function() { console.log('server closed'); }); +}); + +server.listen(7777, function() { + console.log('server listening'); + var client = new pg.Client('postgres://localhost:7777'); + console.log('client connecting'); + client.connect(assert.calls(function(err) { + if (err) console.log("Error on connect: "+err); + else console.log('client connected'); + assert(err); + })); + +}); From d21b995726f4f7984c76716bbe0cc0b9e02c1e9f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 18 Mar 2014 13:36:57 +0100 Subject: [PATCH 3/6] Enable the test for #534 (needs special naming) This time, I hope, travis will confirm that the fix works with node-0.10 but not with node-0.8 --- ...{callback-on-early-disconnect.js => early-disconnect-tests.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/unit/client/{callback-on-early-disconnect.js => early-disconnect-tests.js} (100%) diff --git a/test/unit/client/callback-on-early-disconnect.js b/test/unit/client/early-disconnect-tests.js similarity index 100% rename from test/unit/client/callback-on-early-disconnect.js rename to test/unit/client/early-disconnect-tests.js From 96e4afdb1b7e5ebe74f92107801bc464a33247f4 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 18 Mar 2014 14:23:05 +0100 Subject: [PATCH 4/6] Have Connection also emit 'end' on stream 'close' event Should fix missing connect callback call with node-0.8 (#534) --- lib/connection.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/connection.js b/lib/connection.js index 0c5e96f..997b0a7 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -60,6 +60,13 @@ Connection.prototype.connect = function(port, host) { self.emit('end'); }); + this.stream.on('close', function() { + // TODO: avoid emitting 'end' twice ? + // node-0.10 emits both 'end' and 'close' + // for streams closed by the peer + self.emit('end'); + }); + if(!this.ssl) { return this.attachListeners(this.stream); } From e1b1c62e3e57e6b3b329baa1d48a89752c238b4f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 4 Apr 2014 10:07:07 +0200 Subject: [PATCH 5/6] Do not emit 'end' twice from Connection on close --- lib/connection.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 997b0a7..c5fc169 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -56,14 +56,10 @@ Connection.prototype.connect = function(port, host) { self.emit('error', error); }); - this.stream.on('end', function() { - self.emit('end'); - }); - this.stream.on('close', function() { - // TODO: avoid emitting 'end' twice ? - // node-0.10 emits both 'end' and 'close' - // for streams closed by the peer + // NOTE: node-0.10 emits both 'end' and 'close' + // for streams closed by the peer, while + // node-0.8 only emits 'close' self.emit('end'); }); From e72aff4cfbc7e70fe8e3ab8a4f4210bb1e94c44c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 4 Apr 2014 16:11:44 +0200 Subject: [PATCH 6/6] Have stream emit 'close' rather than 'end' for sake of testing --- test/unit/client/stream-and-query-error-interaction-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/client/stream-and-query-error-interaction-tests.js b/test/unit/client/stream-and-query-error-interaction-tests.js index 9b02caf..02d66c6 100644 --- a/test/unit/client/stream-and-query-error-interaction-tests.js +++ b/test/unit/client/stream-and-query-error-interaction-tests.js @@ -20,7 +20,7 @@ test('emits end when not in query', function() { assert.equal(client.queryQueue.length, 0); assert(client.activeQuery, 'client should have issued query'); process.nextTick(function() { - stream.emit('end'); + stream.emit('close'); }); }); });