Merge pull request #222 from CartoDB/issue-219

Closes stream responses on error
This commit is contained in:
Raul Ochoa 2015-05-13 18:40:40 +02:00
commit 7d62b60945
7 changed files with 139 additions and 26 deletions

View File

@ -1,6 +1,10 @@
1.22.1 - 2015-mm-dd
-------------------
Bug fixes:
* Close stream responses on error (#219)
Enhancements:
* Format files split into pg and ogr directories

View File

@ -1,6 +1,7 @@
var _ = require('underscore');
var pg = require('./../pg');
var PgErrorHandler = require('../../../postgresql/error_handler');
function GeoJsonFormat() {
this.buffer = '';
@ -54,7 +55,7 @@ GeoJsonFormat.prototype.handleQueryRow = function(row) {
};
GeoJsonFormat.prototype.handleQueryEnd = function(/*result*/) {
if (this.error) {
if (this.error && !this._streamingStarted) {
this.callback(this.error);
return;
}
@ -67,7 +68,15 @@ GeoJsonFormat.prototype.handleQueryEnd = function(/*result*/) {
this.startStreaming();
}
this.buffer += ']}'; // end of features
this.buffer += ']'; // end of features
if (this.error) {
var pgErrorHandler = new PgErrorHandler(this.error);
this.buffer += ',"error":' + JSON.stringify([pgErrorHandler.getMessage()]);
}
this.buffer += '}'; // end of root object
if (this.opts.callback) {
this.buffer += ')';
}

View File

@ -139,7 +139,7 @@ SvgFormat.prototype.handleQueryRow = function(row) {
};
SvgFormat.prototype.handleQueryEnd = function() {
if ( this.error ) {
if ( this.error && !this._streamingStarted) {
this.callback(this.error);
return;
}

View File

@ -1458,29 +1458,6 @@ it('GET with callback must return 200 status error even if it is an error', func
});
});
it('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();
}
);
});
it('too large rows get into error log', function(done){
var dbMaxRowSize = global.settings.db_max_row_size;

View File

@ -170,6 +170,31 @@ it('SVG format with missing "the_geom" field', function(done){
});
});
it('should close on error and error must be the only key in the body', function(done) {
assert.response(
app,
{
url: "/api/v1/sql?" + querystring.stringify({
q: "SELECT the_geom, 100/(cartodb_id - 3) cdb_ratio FROM untitle_table_4",
format: 'svg'
}),
headers: {
host: 'vizzuality.cartodb.com'
},
method: 'GET'
},
{
status: 400
},
function(res) {
var parsedBody = JSON.parse(res.body);
assert.deepEqual(Object.keys(parsedBody), ['error']);
assert.deepEqual(parsedBody.error, ["division by zero"]);
done();
}
);
});
});

View File

@ -239,4 +239,31 @@ it('null geometries', function(done){
);
});
it('should close on error and error must be the only key in the body', function(done) {
assert.response(
app,
{
url: "/api/v1/sql?" + querystring.stringify({
q: "SELECT the_geom, 100/(cartodb_id - 3) cdb_ratio FROM untitle_table_4",
format: 'topojson'
}),
headers: {
host: 'vizzuality.cartodb.com'
},
method: 'GET'
},
{
status: 400
},
function(res) {
var parsedBody = JSON.parse(res.body);
assert.deepEqual(Object.keys(parsedBody), ['error']);
assert.deepEqual(parsedBody.error, ["division by zero"]);
done();
}
);
});
});

View File

@ -0,0 +1,71 @@
require('../helper');
var app = require(global.settings.app_root + '/app/controllers/app')();
var assert = require('../support/assert');
var querystring = require('querystring');
describe('stream-responses', function() {
function createFailingQueryRequest(format) {
var params = {
q: "SELECT the_geom, 100/(cartodb_id - 3) cdb_ratio FROM untitle_table_4"
};
if (format) {
params.format = format;
}
return {
url: "/api/v1/sql?" + querystring.stringify(params),
headers: {
host: 'vizzuality.cartodb.com'
},
method: 'GET'
};
}
var okResponse = {
status: 200
};
describe('format-json', function() {
it('should close on error and error message must be part of the response', function(done) {
assert.response(
app,
createFailingQueryRequest(),
okResponse,
function(res) {
var parsedBody = JSON.parse(res.body);
assert.equal(parsedBody.rows.length, 2);
assert.deepEqual(parsedBody.fields, {
the_geom: { type: "geometry" },
cdb_ratio: { type: "number" }
});
assert.deepEqual(parsedBody.error, ["division by zero"]);
done();
}
);
});
});
describe('format-geojson', function() {
it('should close on error and error message must be part of the response', function(done) {
assert.response(
app,
createFailingQueryRequest('geojson'),
okResponse,
function(res) {
var parsedBody = JSON.parse(res.body);
assert.equal(parsedBody.features.length, 2);
assert.deepEqual(parsedBody.error, ["division by zero"]);
done();
}
);
});
});
});