WIP fixes 478, adds more error information when either analysis or turbo-carto is not well formed.

This commit is contained in:
Daniel García Aubert 2016-06-13 12:20:56 +02:00
parent 4a11115dd0
commit c0ce6e7a8a
5 changed files with 192 additions and 12 deletions

View File

@ -214,15 +214,19 @@ BaseController.prototype.sendError = function(req, res, err, label) {
statusCode = 200;
}
var errorResponseBody = { errors: allErrors.map(errorMessage) };
var errorResponseBody = {
errors: allErrors.map(errorMessage)
};
var errorsWithContext = allErrors.map(errorMessageWithContext).filter(function (err) { return !!err; });
if (errorsWithContext.length){
errorResponseBody.errors_with_context = errorsWithContext;
}
this.send(req, res, errorResponseBody, statusCode);
};
function errorMessage(err) {
// See https://github.com/Vizzuality/Windshaft-cartodb/issues/68
var message = (_.isString(err) ? err : err.message) || 'Unknown error';
function stripConnectionInfo(message) {
// Strip connection info, if any
return message
// See https://github.com/CartoDB/Windshaft/issues/173
@ -230,6 +234,27 @@ function errorMessage(err) {
// See https://travis-ci.org/CartoDB/Windshaft/jobs/20703062#L1644
.replace(/is the server.*encountered/im, 'encountered');
}
function errorMessage(err) {
// See https://github.com/Vizzuality/Windshaft-cartodb/issues/68
var message = (_.isString(err) ? err : err.message) || 'Unknown error';
return stripConnectionInfo(message);
}
function errorMessageWithContext(err) {
if (!err.context) {
return;
}
// See https://github.com/Vizzuality/Windshaft-cartodb/issues/68
var message = (_.isString(err) ? err : err.message) || 'Unknown error';
return {
message: stripConnectionInfo(message),
context: err.context
};
}
module.exports.errorMessage = errorMessage;
function findStatusCode(err) {

View File

@ -58,13 +58,27 @@ AnalysisMapConfigAdapter.prototype.getMapConfig = function(user, requestMapConfi
requestMapConfig = appendFiltersToNodes(requestMapConfig, dataviewsFiltersBySourceId);
function createAnalysis(analysisDefinition, done) {
self.analysisBackend.create(analysisConfiguration, analysisDefinition, done);
function createAnalysis(analysisDefinition, index, done) {
self.analysisBackend.create(analysisConfiguration, analysisDefinition, function (err, analysis) {
if (err) {
err.context = {
type: 'camshaft',
analysis: {
index: index,
id: analysisDefinition.id,
type: analysisDefinition.type
}
};
return done(err);
}
done(null, analysis);
});
}
var analysesQueue = queue(requestMapConfig.analyses.length);
requestMapConfig.analyses.forEach(function(analysis) {
analysesQueue.defer(createAnalysis, analysis);
requestMapConfig.analyses.forEach(function(analysis, index) {
analysesQueue.defer(createAnalysis, analysis, index);
});
analysesQueue.awaitAll(function(err, analysesResults) {

View File

@ -22,8 +22,8 @@ TurboCartoAdapter.prototype.getMapConfig = function (user, requestMapConfig, par
var parseCartoQueue = queue(layers.length);
layers.forEach(function(layer) {
parseCartoQueue.defer(self._parseCartoCss.bind(self), user, layer);
layers.forEach(function(layer, index) {
parseCartoQueue.defer(self._parseCartoCss.bind(self), user, layer, index);
});
parseCartoQueue.awaitAll(function (err, layers) {
@ -51,7 +51,7 @@ var pixelSizeTemplate = dot.template('40075017 * cos(ST_Y(ST_Centroid({{=it._bbo
var scaleDenominatorTemplate = dot.template('({{=it._pixelSize}} / 0.00028)::numeric');
TurboCartoAdapter.prototype._parseCartoCss = function (username, layer, callback) {
TurboCartoAdapter.prototype._parseCartoCss = function (username, layer, index, callback) {
if (!shouldParseLayerCartocss(layer)) {
return callback(null, layer);
}
@ -86,6 +86,13 @@ TurboCartoAdapter.prototype._parseCartoCss = function (username, layer, callback
if (err && err.name === 'TurboCartoError') {
err = new Error('turbo-carto: ' + err.message);
err.http_status = 400;
err.context = {
type: 'turbo-carto',
layer: {
index: index,
type: layer.type
}
};
return callback(err);
}

View File

@ -20,6 +20,13 @@ describe('analysis-layers error cases', function() {
}
};
var AUTH_ERROR_RESPONSE = {
status: 403,
headers: {
'Content-Type': 'application/json; charset=utf-8'
}
};
it('should handle missing analysis nodes for layers', function(done) {
var mapConfig = createMapConfig(
[
@ -64,4 +71,112 @@ describe('analysis-layers error cases', function() {
testClient.drain(done);
});
});
it('camshaft: should return error missing analysis nodes for layers with some context', function(done) {
var mapConfig = createMapConfig(
[
{
"type": "cartodb",
"options": {
"source": {
"id": "HEAD"
},
"cartocss": '#polygons { polygon-fill: red; }',
"cartocss_version": "2.3.0"
}
}
],
{},
[
{
"id": "HEAD",
"type": "buffer",
"params": {
"source": {
"id": "HEAD",
"type": "source",
"params": {
"query": "select * from populated_places_simple_reduced"
}
},
"radius": 50000
}
}
]
);
var testClient = new TestClient(mapConfig, 11111);
testClient.getLayergroup(AUTH_ERROR_RESPONSE, function(err, layergroupResult) {
assert.ok(!err, err);
assert.equal(layergroupResult.errors.length, 1);
assert.equal(
layergroupResult.errors[0],
'Analysis requires authentication with API key: permission denied.'
);
assert.equal(layergroupResult.errors_with_context[0].context.type, 'camshaft');
assert.equal(layergroupResult.errors_with_context[0].context.analysis.index, 0);
assert.equal(layergroupResult.errors_with_context[0].context.analysis.id, 'HEAD');
assert.equal(layergroupResult.errors_with_context[0].context.analysis.type, 'buffer');
testClient.drain(done);
});
});
it('camshaft: should return error: Missing required param "radius"; with context', function(done) {
var mapConfig = createMapConfig(
[
{
"type": "cartodb",
"options": {
"source": {
"id": "HEAD"
},
"cartocss": '#polygons { polygon-fill: red; }',
"cartocss_version": "2.3.0"
}
}
],
{},
[
{
"id": "HEAD",
"type": "buffer",
"params": {
"source": {
"id": "HEAD",
"type": "source",
"params": {
"query": "select * from populated_places_simple_reduced"
}
}
}
}
]
);
var testClient = new TestClient(mapConfig, 1234);
testClient.getLayergroup(ERROR_RESPONSE, function(err, layergroupResult) {
assert.ok(!err, err);
assert.equal(layergroupResult.errors.length, 1);
assert.equal(
layergroupResult.errors[0],
'Missing required param "radius"'
);
assert.equal(layergroupResult.errors_with_context[0].context.type, 'camshaft');
assert.equal(layergroupResult.errors_with_context[0].context.analysis.index, 0);
assert.equal(layergroupResult.errors_with_context[0].context.analysis.id, 'HEAD');
assert.equal(layergroupResult.errors_with_context[0].context.analysis.type, 'buffer');
testClient.drain(done);
});
});
});

View File

@ -106,4 +106,23 @@ describe('turbo-carto error cases', function() {
done();
});
});
it('turbo-carto: should return error invalid column from datasource with some context', function(done) {
this.testClient = new TestClient(makeMapconfig(null, 'ramp([wadus_column], (red, green, blue))'));
this.testClient.getLayergroup(ERROR_RESPONSE, function(err, layergroup) {
assert.ok(!err, err);
assert.ok(layergroup.hasOwnProperty('errors'));
assert.equal(layergroup.errors_with_context.length, 1);
assert.ok(layergroup.errors_with_context[0].message.match(/^turbo-carto/));
assert.ok(layergroup.errors_with_context[0].message.match(/unable\sto\scompute\sramp/i));
assert.ok(layergroup.errors_with_context[0].message.match(/wadus_column/));
assert.equal(layergroup.errors_with_context[0].context.type, 'turbo-carto');
assert.equal(layergroup.errors_with_context[0].context.layer.index, 0);
assert.equal(layergroup.errors_with_context[0].context.layer.type, 'mapnik');
done();
});
});
});