From d9297d54dece7ee1f867de4a2b9f2cba0575e808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 13 Jun 2016 16:14:01 +0200 Subject: [PATCH] made error_with_context non optional and adapted test's assertion --- lib/cartodb/controllers/base.js | 15 ++++----------- .../mapconfig/adapter/turbo-carto-adapter.js | 18 +++++++++--------- test/acceptance/dataviews/error-cases.js | 4 ++-- test/acceptance/limits.js | 4 ++-- test/acceptance/multilayer_server.js | 6 ++---- test/acceptance/named_layers.js | 9 +++------ test/acceptance/named_maps_authentication.js | 12 ++++++------ test/acceptance/named_maps_cache.js | 4 ++-- test/acceptance/ported/attributes.js | 6 +++++- test/acceptance/ported/blend_http_fallback.js | 8 +++----- test/acceptance/ported/external_resources.js | 7 +++---- .../ported/multilayer_error_cases.js | 15 +++++++++------ test/acceptance/ported/raster.js | 3 +-- test/acceptance/ported/regressions.js | 2 +- test/acceptance/ported/retina.js | 2 +- test/acceptance/ported/server.js | 2 +- test/acceptance/templates.js | 5 ++--- test/acceptance/turbo-cartocss/error-cases.js | 13 ++++++++++++- 18 files changed, 68 insertions(+), 67 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 8b36c095..b1b8bf6a 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -215,14 +215,10 @@ BaseController.prototype.sendError = function(req, res, err, label) { } var errorResponseBody = { - errors: allErrors.map(errorMessage) + errors: allErrors.map(errorMessage), + errors_with_context: allErrors.map(errorMessageWithContext) }; - var errorsWithContext = allErrors.map(errorMessageWithContext).filter(function (err) { return !!err; }); - if (errorsWithContext.length){ - errorResponseBody.errors_with_context = errorsWithContext; - } - this.send(req, res, errorResponseBody, statusCode); }; @@ -240,19 +236,16 @@ function errorMessage(err) { 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 { + type: err.type || 'unknown', message: stripConnectionInfo(message), - context: err.context + context: err.context || 'unknown' }; } module.exports.errorMessage = errorMessage; diff --git a/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js b/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js index dfdf6414..2756eb2d 100644 --- a/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js @@ -84,16 +84,16 @@ TurboCartoAdapter.prototype._parseCartoCss = function (username, layer, index, c this.turboCartoParser.process(username, layer.options.cartocss, sql, function (err, cartocss) { // Only return turbo-carto errors 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 - } + 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 }; - return callback(err); + + return callback(error); } // Try to continue in the rest of the cases diff --git a/test/acceptance/dataviews/error-cases.js b/test/acceptance/dataviews/error-cases.js index ebbfdc34..7e37e8ee 100644 --- a/test/acceptance/dataviews/error-cases.js +++ b/test/acceptance/dataviews/error-cases.js @@ -54,7 +54,7 @@ describe('histogram-dataview', function() { this.testClient.getLayergroup(ERROR_RESPONSE, function(err, errObj) { assert.ok(!err, err); - assert.deepEqual(errObj, { errors: [ '"dataviews" must be a valid JSON object: "string" type found' ] }); + assert.deepEqual(errObj.errors, [ '"dataviews" must be a valid JSON object: "string" type found' ]); done(); }); @@ -66,7 +66,7 @@ describe('histogram-dataview', function() { this.testClient.getLayergroup(ERROR_RESPONSE, function(err, errObj) { assert.ok(!err, err); - assert.deepEqual(errObj, { errors: [ '"dataviews" must be a valid JSON object: "array" type found' ] }); + assert.deepEqual(errObj.errors, [ '"dataviews" must be a valid JSON object: "array" type found' ]); done(); }); diff --git a/test/acceptance/limits.js b/test/acceptance/limits.js index 30a4e739..d0126623 100644 --- a/test/acceptance/limits.js +++ b/test/acceptance/limits.js @@ -106,7 +106,7 @@ describe('render limits', function() { }, function(res) { var parsed = JSON.parse(res.body); - assert.deepEqual(parsed, { errors: [ 'Render timed out' ] }); + assert.deepEqual(parsed.errors, [ 'Render timed out' ]); done(); } ); @@ -171,7 +171,7 @@ describe('render limits', function() { }, function(res) { var parsed = JSON.parse(res.body); - assert.deepEqual(parsed, { errors: ['Render timed out'] }); + assert.deepEqual(parsed.errors, ['Render timed out']); done(); } ); diff --git a/test/acceptance/multilayer_server.js b/test/acceptance/multilayer_server.js index 251ecc38..e4c13b54 100644 --- a/test/acceptance/multilayer_server.js +++ b/test/acceptance/multilayer_server.js @@ -228,7 +228,7 @@ describe('tests from old api translated to multilayer', function() { }, function(res) { var parsed = JSON.parse(res.body); - assert.deepEqual(parsed, { errors: [ 'Unexpected token W' ] }); + assert.deepEqual(parsed.errors, [ 'Unexpected token W' ]); done(); } @@ -334,9 +334,7 @@ describe('tests from old api translated to multilayer', function() { assert.ok(!res.headers.hasOwnProperty('x-cache-channel')); var parsed = JSON.parse(res.body); - assert.deepEqual(parsed, { - errors: ["fake error message"] - }); + assert.deepEqual(parsed.errors, ["fake error message"]); done(); } diff --git a/test/acceptance/named_layers.js b/test/acceptance/named_layers.js index 8788e5a6..2b1cf4ef 100644 --- a/test/acceptance/named_layers.js +++ b/test/acceptance/named_layers.js @@ -181,7 +181,7 @@ describe('named_layers', function() { } var parsedBody = JSON.parse(response.body); - assert.deepEqual(parsedBody, { errors: ["Template 'nonexistent' of user 'localhost' not found"] }); + assert.deepEqual(parsedBody.errors, ["Template 'nonexistent' of user 'localhost' not found"]); return null; }, @@ -234,10 +234,7 @@ describe('named_layers', function() { } var parsedBody = JSON.parse(response.body); - assert.deepEqual( - parsedBody, - { errors: [ "Unauthorized 'auth_valid_template' template instantiation" ] } - ); + assert.deepEqual(parsedBody.errors, [ "Unauthorized 'auth_valid_template' template instantiation" ]); return null; }, @@ -347,7 +344,7 @@ describe('named_layers', function() { } var parsedBody = JSON.parse(response.body); - assert.deepEqual(parsedBody, { errors: [ 'Nested named layers are not allowed' ] }); + assert.deepEqual(parsedBody.errors, ['Nested named layers are not allowed' ]); return null; }, diff --git a/test/acceptance/named_maps_authentication.js b/test/acceptance/named_maps_authentication.js index f984391c..ba791f37 100644 --- a/test/acceptance/named_maps_authentication.js +++ b/test/acceptance/named_maps_authentication.js @@ -169,8 +169,8 @@ describe('named maps authentication', function() { getNamedTile(nonexistentName, 0, 0, 0, { status: 404 }, function(err, res) { assert.ok(!err); assert.deepEqual( - JSON.parse(res.body), - { errors: ["Template '" + nonexistentName + "' of user '" + username + "' not found"] } + JSON.parse(res.body).errors, + ["Template '" + nonexistentName + "' of user '" + username + "' not found"] ); done(); }); @@ -179,7 +179,7 @@ describe('named maps authentication', function() { it('should return 403 if not properly authorized', function(done) { getNamedTile(tokenAuthTemplateName, 0, 0, 0, { status: 403 }, function(err, res) { assert.ok(!err); - assert.deepEqual(JSON.parse(res.body), { errors: ['Unauthorized template instantiation'] }); + assert.deepEqual(JSON.parse(res.body).errors, ['Unauthorized template instantiation']); done(); }); }); @@ -238,8 +238,8 @@ describe('named maps authentication', function() { getStaticMap(nonexistentName, { status: 404 }, function(err, res) { assert.ok(!err); assert.deepEqual( - JSON.parse(res.body), - { errors: ["Template '" + nonexistentName + "' of user '" + username + "' not found"] } + JSON.parse(res.body).errors, + ["Template '" + nonexistentName + "' of user '" + username + "' not found"] ); done(); }); @@ -248,7 +248,7 @@ describe('named maps authentication', function() { it('should return 403 if not properly authorized', function(done) { getStaticMap(tokenAuthTemplateName, { status: 403 }, function(err, res) { assert.ok(!err); - assert.deepEqual(JSON.parse(res.body), { errors: ['Unauthorized template instantiation'] }); + assert.deepEqual(JSON.parse(res.body).errors, ['Unauthorized template instantiation']); done(); }); }); diff --git a/test/acceptance/named_maps_cache.js b/test/acceptance/named_maps_cache.js index a2f2295e..5d121afa 100644 --- a/test/acceptance/named_maps_cache.js +++ b/test/acceptance/named_maps_cache.js @@ -124,8 +124,8 @@ describe('named maps provider cache', function() { getNamedTile({ statusCode: 404 }, function(err, res) { assert.ok(!err); assert.deepEqual( - JSON.parse(res.body), - { errors: ["Template 'template_with_color' of user 'localhost' not found"] } + JSON.parse(res.body).errors, + ["Template 'template_with_color' of user 'localhost' not found"] ); // add template again so it's clean in afterEach diff --git a/test/acceptance/ported/attributes.js b/test/acceptance/ported/attributes.js index 5151cc09..5478bbb6 100644 --- a/test/acceptance/ported/attributes.js +++ b/test/acceptance/ported/attributes.js @@ -231,7 +231,11 @@ describe('attributes', function() { assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); assert.equal( res.body, - '/**/ typeof test === \'function\' && test({"errors":["Layer 0 has no exposed attributes"]});' + '/**/ 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"' + + '}]});' ); return null; }, diff --git a/test/acceptance/ported/blend_http_fallback.js b/test/acceptance/ported/blend_http_fallback.js index 91e832f6..480dc5c0 100644 --- a/test/acceptance/ported/blend_http_fallback.js +++ b/test/acceptance/ported/blend_http_fallback.js @@ -138,11 +138,9 @@ describe('blend http fallback', function() { testClient.getTileLayer(mapConfig, tileRequest, expectedResponse, function(err, res) { assert.ok(!err); var parsedBody = JSON.parse(res.body); - assert.deepEqual(parsedBody, { - errors: [ - "Unable to fetch http tile: http://127.0.0.1:8033/error404/1/0/0.png [404]" - ] - }); + assert.deepEqual(parsedBody.errors, [ + "Unable to fetch http tile: http://127.0.0.1:8033/error404/1/0/0.png [404]" + ]); done(); }); }); diff --git a/test/acceptance/ported/external_resources.js b/test/acceptance/ported/external_resources.js index 6f73b52e..11ad5176 100644 --- a/test/acceptance/ported/external_resources.js +++ b/test/acceptance/ported/external_resources.js @@ -119,12 +119,11 @@ describe('external resources', function() { var mapConfig = testClient.defaultTableMapConfig('test_table_3', style); testClient.createLayergroup(mapConfig, { statusCode: 400 }, function(err, res) { - assert.deepEqual(JSON.parse(res.body), { - errors: ["Unable to download '" + url + "' for 'style0' (server returned 404)"] - }); + assert.deepEqual(JSON.parse(res.body).errors, [ + "Unable to download '" + url + "' for 'style0' (server returned 404)"] + ); done(); }); }); }); - diff --git a/test/acceptance/ported/multilayer_error_cases.js b/test/acceptance/ported/multilayer_error_cases.js index 11c11e63..9d59a5b3 100644 --- a/test/acceptance/ported/multilayer_error_cases.js +++ b/test/acceptance/ported/multilayer_error_cases.js @@ -31,7 +31,7 @@ describe('multilayer error cases', function() { }, {}, function(res) { assert.equal(res.statusCode, 400, res.body); var parsedBody = JSON.parse(res.body); - assert.deepEqual(parsedBody, {"errors":["layergroup POST data must be of type application/json"]}); + assert.deepEqual(parsedBody.errors, ["layergroup POST data must be of type application/json"]); done(); }); }); @@ -44,7 +44,7 @@ describe('multilayer error cases', function() { }, {}, function(res) { assert.equal(res.statusCode, 400, res.body); var parsedBody = JSON.parse(res.body); - assert.deepEqual(parsedBody, {"errors":["Missing layers array from layergroup config"]}); + assert.deepEqual(parsedBody.errors, ["Missing layers array from layergroup config"]); done(); }); }); @@ -58,7 +58,10 @@ describe('multilayer error cases', function() { assert.equal(res.statusCode, 200); assert.equal( res.body, - '/**/ typeof test === \'function\' && test({"errors":["Missing layers array from layergroup config"]});' + '/**/ 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"}]});' ); done(); }); @@ -83,7 +86,7 @@ describe('multilayer error cases', function() { }, {}, function(res) { assert.equal(res.statusCode, 400, res.body); var parsedBody = JSON.parse(res.body); - assert.deepEqual(parsedBody, {errors:["Missing cartocss_version for layer 0 options"]}); + assert.deepEqual(parsedBody.errors, ["Missing cartocss_version for layer 0 options"]); done(); }); }); @@ -355,7 +358,7 @@ describe('multilayer error cases', function() { var mapConfig = testClient.singleLayerMapConfig('select * from test_table', null, null, 'name'); testClient.getGrid(mapConfig, 1, 13, 4011, 3088, defaultErrorExpectedResponse, function(err, res) { - assert.deepEqual(JSON.parse(res.body), { errors: ["Layer '1' not found in layergroup"] }); + assert.deepEqual(JSON.parse(res.body).errors, ["Layer '1' not found in layergroup"]); done(); }); }); @@ -383,7 +386,7 @@ describe('multilayer error cases', function() { // FIXME: should be 404 assert.equal(res.statusCode, 400, res.statusCode + ':' + res.body); var parsed = JSON.parse(res.body); - assert.deepEqual(parsed, {"errors": ["Invalid or nonexistent map configuration token 'deadbeef'"]}); + assert.deepEqual(parsed.errors, ["Invalid or nonexistent map configuration token 'deadbeef'"]); return null; }, function finish(err) { diff --git a/test/acceptance/ported/raster.js b/test/acceptance/ported/raster.js index 2cea4bf4..fc26661b 100644 --- a/test/acceptance/ported/raster.js +++ b/test/acceptance/ported/raster.js @@ -148,11 +148,10 @@ describe('raster', function() { assert.ok(!err); checkCORSHeaders(res); var parsedBody = JSON.parse(res.body); - assert.deepEqual(parsedBody, { errors: [ 'Mapnik raster layers do not support interactivity' ] }); + assert.deepEqual(parsedBody.errors, [ 'Mapnik raster layers do not support interactivity' ]); done(); } ); }); }); - diff --git a/test/acceptance/ported/regressions.js b/test/acceptance/ported/regressions.js index 66c1262f..2a2188c1 100644 --- a/test/acceptance/ported/regressions.js +++ b/test/acceptance/ported/regressions.js @@ -29,7 +29,7 @@ describe('regressions', function() { contentType: 'application/json; charset=utf-8' }; requestTile('/0/0/0.png?testUnexpectedError=1', options, function(err, res) { - assert.deepEqual(JSON.parse(res.body), { "errors": ["test unexpected error"] }); + assert.deepEqual(JSON.parse(res.body).errors, ["test unexpected error"]); finish(done); }); }); diff --git a/test/acceptance/ported/retina.js b/test/acceptance/ported/retina.js index 1c4b16f1..0962619f 100644 --- a/test/acceptance/ported/retina.js +++ b/test/acceptance/ported/retina.js @@ -129,7 +129,7 @@ describe('retina support', function() { }, function(res, err) { assert.ok(!err, 'Failed to request 0/0/0' + scaleFactor + '.png tile'); - assert.deepEqual(JSON.parse(res.body), { errors: ["Tile with specified resolution not found"] } ); + assert.deepEqual(JSON.parse(res.body).errors, ["Tile with specified resolution not found"]); done(); } diff --git a/test/acceptance/ported/server.js b/test/acceptance/ported/server.js index 7d8f2e73..650cf2c5 100644 --- a/test/acceptance/ported/server.js +++ b/test/acceptance/ported/server.js @@ -112,7 +112,7 @@ describe('server', function() { } }; testClient.getGrid(mapConfig, 0, 13, 4011, 3088, expectedResponse, function(err, res) { - assert.deepEqual(JSON.parse(res.body), {"errors":["Tileset has no interactivity"]}); + assert.deepEqual(JSON.parse(res.body).errors, ["Tileset has no interactivity"]); done(); }); }); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index e8e0a2ca..7201b7fb 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -1927,9 +1927,8 @@ describe('template_api', function() { if (err) { return done(err); } - assert.deepEqual(JSON.parse(res.body), { - errors: ["Invalid or nonexistent map configuration token '" + nonexistentToken + "'"] - }); + assert.deepEqual(JSON.parse(res.body).errors, + ["Invalid or nonexistent map configuration token '" + nonexistentToken + "'"]); done(); }; diff --git a/test/acceptance/turbo-cartocss/error-cases.js b/test/acceptance/turbo-cartocss/error-cases.js index 23597dd7..8582d753 100644 --- a/test/acceptance/turbo-cartocss/error-cases.js +++ b/test/acceptance/turbo-cartocss/error-cases.js @@ -114,14 +114,25 @@ 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.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'); + assert.equal(layergroup.errors_with_context[0].context.selector, '#populated_places_simple_reduced'); + assert.deepEqual(layergroup.errors_with_context[0].context.source, { + start: { + line: 10, + column: 3 + }, + end: { + line: 10, + column: 56 + } + }); done(); }); });