From 025a6abdaad02155303d54904c4bb4fe49a685de Mon Sep 17 00:00:00 2001
From: Raul Ochoa <rochoaf@gmail.com>
Date: Mon, 1 Dec 2014 16:55:59 +0100
Subject: [PATCH] Closes stream responses on error. Fixes #188

---
 NEWS.md                     |  4 ++++
 app/models/formats/json.js  | 20 ++++++++++++++++----
 test/acceptance/app.test.js | 23 +++++++++++++++++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index df360bd6..b7309499 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,6 +1,10 @@
 1.19.1 - 2014-mm-dd
 -------------------
 
+Bug fixes:
+
+ * Closes stream responses on error (#188)
+
 
 1.19.0 - 2014-11-21
 -------------------
diff --git a/app/models/formats/json.js b/app/models/formats/json.js
index dfb662a9..8de5e842 100644
--- a/app/models/formats/json.js
+++ b/app/models/formats/json.js
@@ -1,9 +1,11 @@
 var pg  = require('./pg'),
     util = require('util'),
+    PgErrorHandler = require(global.settings.app_root + '/app/postgresql/error_handler'),
     _ = require('underscore');
 
 function JsonFormat() {
     this.buffer = '';
+    this.lastKnownResult = {};
 }
 
 JsonFormat.prototype = new pg('json');
@@ -11,6 +13,7 @@ JsonFormat.prototype = new pg('json');
 JsonFormat.prototype._contentType = "application/json; charset=utf-8";
 
 JsonFormat.prototype.formatResultFields = function(flds) {
+  flds = flds || [];
   var nfields = {};
   for (var i=0; i<flds.length; ++i) {
     var f = flds[i];
@@ -55,21 +58,23 @@ JsonFormat.prototype.startStreaming = function() {
     this._streamingStarted = true;
 };
 
-JsonFormat.prototype.handleQueryRow = function(row) {
+JsonFormat.prototype.handleQueryRow = function(row, result) {
     if ( ! this._streamingStarted ) {
         this.startStreaming();
     }
 
+    this.lastKnownResult = result;
+
     this.buffer += (this.total_rows++ ? ',' : '') + JSON.stringify(row);
 
-    if (this.total_rows % (this.opts.bufferedRows || 1000)) {
+    if (this.total_rows % (1||this.opts.bufferedRows || 1000)) {
         this.opts.sink.write(this.buffer);
         this.buffer = '';
     }
 };
 
 JsonFormat.prototype.handleQueryEnd = function(result) {
-    if ( this.error ) {
+    if (this.error && !this._streamingStarted) {
         this.callback(this.error);
         return;
     }
@@ -82,6 +87,8 @@ JsonFormat.prototype.handleQueryEnd = function(result) {
 
     this.opts.total_time = (Date.now() - this.start_time)/1000;
 
+    result = result || this.lastKnownResult || {};
+
     // Drop field description for skipped fields
     if (this.hasSkipFields) {
         var newfields = [];
@@ -99,9 +106,14 @@ JsonFormat.prototype.handleQueryEnd = function(result) {
         '],', // end of "rows" array
         '"time":', JSON.stringify(total_time),
         ',"fields":', JSON.stringify(this.formatResultFields(result.fields)),
-        ',"total_rows":', JSON.stringify(result.rowCount)
+        ',"total_rows":', JSON.stringify(result.rowCount || this.total_rows)
     ];
 
+    if (this.error) {
+        var pgErrorHandler = new PgErrorHandler(this.error);
+        out.push(',"error":', JSON.stringify([pgErrorHandler.getMessage()]));
+    }
+
 
     if ( result.notices && result.notices.length > 0 ) {
         var notices = {},
diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js
index 86bc882b..7ff75029 100644
--- a/test/acceptance/app.test.js
+++ b/test/acceptance/app.test.js
@@ -1441,4 +1441,27 @@ test('GET with callback must return 200 status error even if it is an error', fu
             });
     });
 
+    test('GET with callback must return 200 status error even if it is an error', function(done){
+        assert.response(
+            app,
+            {
+                url: "/api/v1/sql?" + querystring.stringify({
+                    q: "SELECT 100/(cartodb_id - 3) cdb_ratio FROM untitle_table_4"
+                }),
+                headers: {host: 'vizzuality.cartodb.com'},
+                method: 'GET'
+            },
+            {
+                status: 200
+            },
+            function(res) {
+                var parsedBody = JSON.parse(res.body);
+                assert.equal(parsedBody.rows.length, 2);
+                assert.deepEqual(parsedBody.fields, {"cdb_ratio": {"type": "number"}});
+                assert.deepEqual(parsedBody.error, ["division by zero"]);
+                done();
+            }
+        );
+    });
+
 });