diff --git a/assets/render-timeout-fallback.mvt b/assets/render-timeout-fallback.mvt new file mode 100644 index 00000000..9ef336ae Binary files /dev/null and b/assets/render-timeout-fallback.mvt differ diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 208ac027..43dd9c3a 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -4,6 +4,7 @@ var step = require('step'); var cors = require('../middleware/cors'); var userMiddleware = require('../middleware/user'); var allowQueryParams = require('../middleware/allow-query-params'); +var vectorError = require('../middleware/vector-error'); var DataviewBackend = require('../backends/dataview'); var AnalysisStatusBackend = require('../backends/analysis-status'); @@ -50,7 +51,8 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.tile.bind(this) + this.tile.bind(this), + vectorError() ); app.get( @@ -58,7 +60,8 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.tile.bind(this) + this.tile.bind(this), + vectorError() ); app.get( @@ -67,7 +70,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, validateLayerRouteMiddleware, this.prepareContext, - this.layer.bind(this) + this.layer.bind(this), + vectorError() ); app.get( diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 55790bbf..a88c8592 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -6,6 +6,7 @@ var NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); var cors = require('../middleware/cors'); var userMiddleware = require('../middleware/user'); var allowQueryParams = require('../middleware/allow-query-params'); +var vectorError = require('../middleware/vector-error'); function NamedMapsController(prepareContext, namedMapProviderCache, tileBackend, previewBackend, surrogateKeysCache, tablesExtentApi, metadataBackend) { @@ -26,7 +27,8 @@ NamedMapsController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.tile.bind(this) + this.tile.bind(this), + vectorError() ); app.get( @@ -102,6 +104,7 @@ NamedMapsController.prototype.tile = function(req, res, next) { }, function handleImage(err, tile, headers, stats) { req.profiler.add(stats); + if (err) { err.label = 'NAMED_MAP_TILE'; next(err); diff --git a/lib/cartodb/middleware/context/locals.js b/lib/cartodb/middleware/context/locals.js index bb7d5ee2..0fdcce50 100644 --- a/lib/cartodb/middleware/context/locals.js +++ b/lib/cartodb/middleware/context/locals.js @@ -1,8 +1,6 @@ -const _ = require('underscore'); - module.exports = function localsMiddleware(req, res, next) { - _.extend(res.locals, req.params); - + // save req.params in res.locals + res.locals = Object.assign(req.params || {}, res.locals); + next(); }; - diff --git a/lib/cartodb/middleware/user.js b/lib/cartodb/middleware/user.js index ce3fdd29..adf06203 100644 --- a/lib/cartodb/middleware/user.js +++ b/lib/cartodb/middleware/user.js @@ -4,11 +4,5 @@ var cdbRequest = new CdbRequest(); module.exports = function userMiddleware(req, res, next) { res.locals.user = cdbRequest.userByReq(req); - // avoid a req.params.user equals to undefined - // overwrites res.locals.user - if(!req.params.user) { - delete req.params.user; - } - next(); }; diff --git a/lib/cartodb/middleware/vector-error.js b/lib/cartodb/middleware/vector-error.js new file mode 100644 index 00000000..f42f1c87 --- /dev/null +++ b/lib/cartodb/middleware/vector-error.js @@ -0,0 +1,30 @@ +const fs = require('fs'); + +const timeoutErrorVectorTile = fs.readFileSync(__dirname + '/../../../assets/render-timeout-fallback.mvt'); + +module.exports = function vectorError() { + return function vectorErrorMiddleware(err, req, res, next) { + if(req.params.format === 'mvt') { + + if (isTimeoutError(err)) { + res.set('Content-Type', 'application/x-protobuf'); + return res.status(429).send(timeoutErrorVectorTile); + } + } + + next(err); + }; +}; + + +function isRenderTimeoutError (err) { + return err.message === 'Render timed out'; +} + +function isDatasourceTimeoutError (err) { + return err.message && err.message.match(/canceling statement due to statement timeout/i); +} + +function isTimeoutError (err) { + return isRenderTimeoutError(err) || isDatasourceTimeoutError(err); +} diff --git a/scripts/mvt-timeout-error.py b/scripts/mvt-timeout-error.py new file mode 100755 index 00000000..1b2e66ee --- /dev/null +++ b/scripts/mvt-timeout-error.py @@ -0,0 +1,37 @@ +#!/usr/bin/env python + +import mapbox_vector_tile + +lines_list = [] + +# main diagonal line +lines_list.append({ "geometry":"LINESTRING (0 0, 4096 4096)"}) + +# diagonal lines +for i in range(4096/32, 4096, 4096/32): + start = i + end = 4096 - i + + lines_list.append({ "geometry":"LINESTRING (0 " + str(start) + ", " + str(end) + " 4096)" }) + lines_list.append({ "geometry":"LINESTRING (" + str(start) + " 0, 4096 " + str(end) + ")" }) + +# box lines +lines_list.append({ "geometry":"LINESTRING (0 0, 0 4096)"}) +lines_list.append({ "geometry":"LINESTRING (0 4096, 4096 4096)"}) +lines_list.append({ "geometry":"LINESTRING (4096 4096, 4096 0)"}) +lines_list.append({ "geometry":"LINESTRING (4096 0, 0 0)"}) + + +tile = mapbox_vector_tile.encode([ + { + "name": "errorTileSquareLayer", + "features": [{ "geometry":"POLYGON ((0 0, 0 4096, 4096 4096, 4096 0, 0 0))" }] + }, + { + "name": "errorTileStripesLayer", + "features": lines_list + } +]) + +with open('./assets/render-timeout-fallback.mvt', 'w+') as f: + f.write(tile) \ No newline at end of file diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 42c468fd..9893f00a 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -419,18 +419,23 @@ describe('user database timeout limit', function () { response: { status: 429, headers: { - 'Content-Type': 'application/json; charset=utf-8' + 'Content-Type': 'application/x-protobuf' } } }; - this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { + this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { assert.ifError(err); - - assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); - + + var tileJSON = tile.toJSON(); + assert.equal(Array.isArray(tileJSON), true); + assert.equal(tileJSON.length, 2); + assert.equal(tileJSON[0].name, 'errorTileSquareLayer'); + assert.equal(tileJSON[1].name, 'errorTileStripesLayer'); + done(); }); + }); }); }); diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 0bb45c2b..5942d045 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -232,7 +232,7 @@ describe('user render timeout limit', function () { response: { status: 429, headers: { - 'Content-Type': 'application/json; charset=utf-8' + 'Content-Type': 'application/x-protobuf' } } }; @@ -240,15 +240,12 @@ describe('user render timeout limit', function () { this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { assert.ifError(err); - assert.deepEqual(tile, { - errors: ['You are over platform\'s limits. Please contact us to know more details'], - errors_with_context: [{ - type: 'limit', - subtype: 'datasource', - message: 'You are over platform\'s limits. Please contact us to know more details' - }] - }); - + var tileJSON = tile.toJSON(); + assert.equal(Array.isArray(tileJSON), true); + assert.equal(tileJSON.length, 2); + assert.equal(tileJSON[0].name, 'errorTileSquareLayer'); + assert.equal(tileJSON[1].name, 'errorTileStripesLayer'); + done(); }); }); diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 41f0fb8c..b31b2c9b 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -95,8 +95,10 @@ if test x"$PREPARE_PGSQL" = xyes; then do # Strip PARALLEL labels for PostgreSQL releases before 9.6 if [ $PG_PARALLEL -eq 0 ]; then + TMPFILE=$(mktemp /tmp/$(basename $0).XXXXXXXX) sed -e 's/PARALLEL \= [A-Z]*,/''/g' \ - -e 's/PARALLEL [A-Z]*/''/g' -i sql/$i.sql + -e 's/PARALLEL [A-Z]*/''/g' sql/$i.sql > $TMPFILE + mv $TMPFILE sql/$i.sql fi cat sql/${i}.sql | sed -e 's/cartodb\./public./g' -e "s/''cartodb''/''public''/g" |