Merge pull request #194 from CartoDB/188-close-stream-responses-on-error

Close stream responses on error

NEWS.md

@@ -3,8 +3,10 @@
 
 Bug fixes:
 
+ * Closes stream responses on error (#188)
  * Closes fd for log files on `kill -HUP` (#187)
 
+
 1.19.0 - 2014-11-21
 -------------------
 

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,11 +58,13 @@ 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)) {
@@ -69,7 +74,7 @@ JsonFormat.prototype.handleQueryRow = function(row) {
 };
 
 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 = {},

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('stream response is closed on error and error message is part of the response', 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();
+            }
+        );
+    });
+
 });