cartodb4/Windshaft-cartodb
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #525 from CartoDB/fix-error-context

Browse Source 
Fix error context, replaced `turbo-carto` error type by `layer` type
This commit is contained in:
Daniel
2016-06-29 19:15:44 +02:00
committed by GitHub
parent 84c67f977e ed7f95a1a7
commit 0fd0974738
11 changed files with 312 additions and 228 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
NEWS.md
View File

@@ -68,6 +68,7 @@ Released 2016-06-21
Enhancements:
- Split turbo-carto adapter substitutions tokens query.
- Now errors with context have the same schema. #519
- Responses with error now return the layer-id to give more info to the user.
Announcements:
- Upgrades camshaft to [0.20.0](https://github.com/CartoDB/camshaft/releases/tag/0.20.0)

12
lib/cartodb/controllers/base.js
View File

@@ -242,11 +242,19 @@ function errorMessageWithContext(err) {
// See https://github.com/Vizzuality/Windshaft-cartodb/issues/68
var message = (_.isString(err) ? err : err.message) || 'Unknown error';
return {
var error = {
type: err.type || 'unknown',
message: stripConnectionInfo(message),
context: err.context || 'unknown'
};
for (var prop in err) {
// type & message are properties from Error's prototype and will be skipped
if (err.hasOwnProperty(prop)) {
error[prop] = err[prop];
}
}
return error;
}
module.exports.errorMessage = errorMessage;

10
lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js
View File

@@ -58,14 +58,12 @@ AnalysisMapConfigAdapter.prototype.getMapConfig = function(user, requestMapConfi
requestMapConfig = appendFiltersToNodes(requestMapConfig, dataviewsFiltersBySourceId);
function createAnalysis(analysisDefinition, index, done) {
function createAnalysis(analysisDefinition, done) {
self.analysisBackend.create(analysisConfiguration, analysisDefinition, function (err, analysis) {
if (err) {
var error = new Error(err.message);
error.type = 'analysis';
error.context = {};
error.context.layer = {
index: index,
error.analysis = {
id: analysisDefinition.id,
type: analysisDefinition.type
};
@@ -77,8 +75,8 @@ AnalysisMapConfigAdapter.prototype.getMapConfig = function(user, requestMapConfi
}
var analysesQueue = queue(requestMapConfig.analyses.length);
requestMapConfig.analyses.forEach(function(analysis, index) {
analysesQueue.defer(createAnalysis, analysis, index);
requestMapConfig.analyses.forEach(function(analysis) {
analysesQueue.defer(createAnalysis, analysis);
});
analysesQueue.awaitAll(function(err, analysesResults) {

19
lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js
View File

@@ -9,6 +9,8 @@ var turboCarto = require('turbo-carto');
var SubstitutionTokens = require('../../../utils/substitution-tokens');
var PostgresDatasource = require('../../../backends/turbo-carto-postgres-datasource');
var MapConfig = require('windshaft').model.MapConfig;
function TurboCartoAdapter() {
}
@@ -26,7 +28,8 @@ TurboCartoAdapter.prototype.getMapConfig = function (user, requestMapConfig, par
var parseCartoQueue = queue(layers.length);
layers.forEach(function(layer, index) {
parseCartoQueue.defer(self._parseCartoCss.bind(self), user, params, layer, index);
var layerId = MapConfig.getLayerId(requestMapConfig, index);
parseCartoQueue.defer(self._parseCartoCss.bind(self), user, params, layer, index, layerId);
});
parseCartoQueue.awaitAll(function (err, layers) {
@@ -65,7 +68,7 @@ var tokensQueryTpl = dot.template([
'from bbox_query, pixel_size_query, scale_denominator_query, zoom_query'
].join('\n'));
TurboCartoAdapter.prototype._parseCartoCss = function (username, params, layer, index, callback) {
TurboCartoAdapter.prototype._parseCartoCss = function (username, params, layer, layerIndex, layerId, callback) {
if (!shouldParseLayerCartocss(layer)) {
return callback(null, layer);
}
@@ -76,11 +79,13 @@ TurboCartoAdapter.prototype._parseCartoCss = function (username, params, layer,
if (err && err.name === 'TurboCartoError') {
var error = new Error('turbo-carto: ' + err.message);
error.http_status = 400;
error.type = 'turbo-carto';
error.context = err.context;
error.context.layer = {
index: index,
type: layer.type
error.type = 'layer';
error.subtype = 'turbo-carto';
error.layer = {
id: layerId,
index: layerIndex,
type: layer.type,
context: err.context
};
return callback(error);

468
npm-shrinkwrap.json generated
View File

File diff suppressed because it is too large Load Diff

2
package.json
View File

@@ -39,7 +39,7 @@
"step-profiler": "~0.3.0",
"turbo-carto": "0.12.1",
"underscore": "~1.6.0",
"windshaft": "2.3.0"
"windshaft": "2.4.0"
},
"devDependencies": {
"istanbul": "~0.4.3",

10
test/acceptance/analysis/error-cases.js
View File

@@ -204,9 +204,8 @@ describe('analysis-layers error cases', function() {
layergroupResult.errors_with_context[0].message,
'Analysis requires authentication with API key: permission denied.'
);
assert.equal(layergroupResult.errors_with_context[0].context.layer.index, 0);
assert.equal(layergroupResult.errors_with_context[0].context.layer.id, 'HEAD');
assert.equal(layergroupResult.errors_with_context[0].context.layer.type, 'buffer');
assert.equal(layergroupResult.errors_with_context[0].analysis.id, 'HEAD');
assert.equal(layergroupResult.errors_with_context[0].analysis.type, 'buffer');
testClient.drain(done);
});
@@ -258,9 +257,8 @@ describe('analysis-layers error cases', function() {
assert.equal(layergroupResult.errors_with_context[0].type, 'analysis');
assert.equal(layergroupResult.errors_with_context[0].message, 'Missing required param "radius"');
assert.equal(layergroupResult.errors_with_context[0].context.layer.index, 0);
assert.equal(layergroupResult.errors_with_context[0].context.layer.id, 'HEAD');
assert.equal(layergroupResult.errors_with_context[0].context.layer.type, 'buffer');
assert.equal(layergroupResult.errors_with_context[0].analysis.id, 'HEAD');
assert.equal(layergroupResult.errors_with_context[0].analysis.type, 'buffer');
testClient.drain(done);
});

2
test/acceptance/ported/attributes.js
View File

@@ -234,7 +234,7 @@ describe('attributes', function() {
'/**/ typeof test === \'function\' && ' +
'test({"errors":["Layer 0 has no exposed attributes"],' +
'"errors_with_context":[{' +
'"type":"unknown","message":"Layer 0 has no exposed attributes","context":"unknown"' +
'"type":"unknown","message":"Layer 0 has no exposed attributes"' +
'}]});'
);
return null;

2
test/acceptance/ported/multilayer_error_cases.js
View File

@@ -61,7 +61,7 @@ describe('multilayer error cases', function() {
'/**/ typeof test === \'function\' && ' +
'test({"errors":["Missing layers array from layergroup config"],' +
'"errors_with_context":[{"type":"unknown",' +
'"message":"Missing layers array from layergroup config","context":"unknown"}]});'
'"message":"Missing layers array from layergroup config"}]});'
);
done();
});

12
test/acceptance/turbo-cartocss/error-cases.js
View File

@@ -114,16 +114,18 @@ describe('turbo-carto error cases', function() {
assert.ok(layergroup.hasOwnProperty('errors'));
assert.equal(layergroup.errors_with_context.length, 1);
assert.equal(layergroup.errors_with_context[0].type, 'turbo-carto');
assert.equal(layergroup.errors_with_context[0].type, 'layer');
assert.equal(layergroup.errors_with_context[0].subtype, 'turbo-carto');
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.layer.index, 0);
assert.equal(layergroup.errors_with_context[0].context.layer.type, 'mapnik');
assert.equal(layergroup.errors_with_context[0].layer.id, 'layer0');
assert.equal(layergroup.errors_with_context[0].layer.index, 0);
assert.equal(layergroup.errors_with_context[0].layer.type, 'mapnik');
assert.equal(layergroup.errors_with_context[0].context.selector, '#populated_places_simple_reduced');
assert.deepEqual(layergroup.errors_with_context[0].context.source, {
assert.equal(layergroup.errors_with_context[0].layer.context.selector, '#populated_places_simple_reduced');
assert.deepEqual(layergroup.errors_with_context[0].layer.context.source, {
start: {
line: 10,
column: 3

2
test/acceptance/turbo-cartocss/regressions.js
View File

@@ -87,7 +87,7 @@ describe('turbo-carto regressions', function() {
var turboCartoError = layergroup.errors_with_context[0];
assert.ok(turboCartoError);
assert.equal(turboCartoError.type, 'turbo-carto');
assert.equal(turboCartoError.type, 'layer');
assert.ok(turboCartoError.message.match(/permission\sdenied\sfor\srelation\stest_table_private_1/));
done();