From de9d5e3cd5466bda6554fd631c51196c78b80816 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 16 Jan 2013 10:11:55 +0100 Subject: [PATCH 1/3] Cleanly handle missing stream error on COPY operation. Closes #241 --- lib/query.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/query.js b/lib/query.js index a3330231..1185a2ad 100644 --- a/lib/query.js +++ b/lib/query.js @@ -170,9 +170,11 @@ p.prepare = function(connection) { this.getRows(connection); }; p.streamData = function (connection) { - this.stream.startStreamingToConnection(connection); + if ( this.stream ) this.stream.startStreamingToConnection(connection); + else this.handleError(new Error('No destination stream defined')); }; p.handleCopyFromChunk = function (chunk) { - this.stream.handleChunk(chunk); + if ( this.stream ) this.stream.handleChunk(chunk); + else this.handleError(new Error('error', 'No source stream defined')); } module.exports = Query; From a39e0d7cc967eaf5a3f63eff9ed48621f2bf45d3 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 16 Jan 2013 12:04:28 +0100 Subject: [PATCH 2/3] Rework handling of missing stream object for copy ops This version works better (doesn't throw) but also doesn't report any error, which is not good --- lib/query.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/query.js b/lib/query.js index 1185a2ad..2f80411d 100644 --- a/lib/query.js +++ b/lib/query.js @@ -17,7 +17,7 @@ var Query = function(config, values, callback) { this.types = config.types; this.name = config.name; this.binary = config.binary; - this.stream = config.stream; + this.stream = config.stream; //use unique portal name each time this.portal = config.portal || "" this.callback = config.callback; @@ -171,10 +171,17 @@ p.prepare = function(connection) { }; p.streamData = function (connection) { if ( this.stream ) this.stream.startStreamingToConnection(connection); - else this.handleError(new Error('No destination stream defined')); + else { + connection.endCopyFrom(); // send an EOF to connection + // TODO: signal the problem somehow + //this.handleError(new Error('No source stream defined')); + } }; p.handleCopyFromChunk = function (chunk) { if ( this.stream ) this.stream.handleChunk(chunk); - else this.handleError(new Error('error', 'No source stream defined')); + else { + // TODO: signal the problem somehow + //this.handleError(new Error('error', 'No destination stream defined')); + } } module.exports = Query; From 2fc22de21a7649e473ad17c77536452c1e805912 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 16 Jan 2013 12:40:59 +0100 Subject: [PATCH 3/3] Send backend a CopyFail when no stream is defined to copy from --- lib/connection.js | 5 +++++ lib/query.js | 6 +----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 49722c2d..ccc5571b 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -267,6 +267,11 @@ p.sendCopyFromChunk = function (chunk) { p.endCopyFrom = function () { this.stream.write(this.writer.add(emptyBuffer).flush(0x63)); } +p.sendCopyFail = function (msg) { + //this.stream.write(this.writer.add(emptyBuffer).flush(0x66)); + this.writer.addCString(msg); + this._send(0x66); +} //parsing methods p.setBuffer = function(buffer) { if(this.lastBuffer) { //we have unfinished biznaz diff --git a/lib/query.js b/lib/query.js index 2f80411d..5f22a66b 100644 --- a/lib/query.js +++ b/lib/query.js @@ -171,11 +171,7 @@ p.prepare = function(connection) { }; p.streamData = function (connection) { if ( this.stream ) this.stream.startStreamingToConnection(connection); - else { - connection.endCopyFrom(); // send an EOF to connection - // TODO: signal the problem somehow - //this.handleError(new Error('No source stream defined')); - } + else connection.sendCopyFail('No source stream defined'); }; p.handleCopyFromChunk = function (chunk) { if ( this.stream ) this.stream.handleChunk(chunk);