From 0684c1b9d34ffdee103bfee9f0cdb0697e04f7d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 5 Jul 2017 19:08:19 +0200 Subject: [PATCH 01/66] Work in progress: get timeout from redis --- lib/cartodb/api/user_limits_api.js | 88 ++++++++++++++++--- .../provider/create-layergroup-provider.js | 2 +- .../mapconfig/provider/map-store-provider.js | 2 +- .../mapconfig/provider/named-map-provider.js | 2 +- test/acceptance/user-timeout-limit.js | 82 +++++++++++++++++ 5 files changed, 163 insertions(+), 13 deletions(-) create mode 100644 test/acceptance/user-timeout-limit.js diff --git a/lib/cartodb/api/user_limits_api.js b/lib/cartodb/api/user_limits_api.js index ae9cd46b..7f244e7d 100644 --- a/lib/cartodb/api/user_limits_api.js +++ b/lib/cartodb/api/user_limits_api.js @@ -1,3 +1,5 @@ +var step = require('step'); + /** * * @param metadataBackend @@ -13,16 +15,82 @@ function UserLimitsApi(metadataBackend, options) { module.exports = UserLimitsApi; -UserLimitsApi.prototype.getRenderLimits = function (username, callback) { +UserLimitsApi.prototype.getRenderLimits = function (username, apiKey, callback) { var self = this; - this.metadataBackend.getTilerRenderLimit(username, function handleTilerLimits(err, renderLimit) { - if (err) { - return callback(err); - } - return callback(null, { - cacheOnTimeout: self.options.limits.cacheOnTimeout || false, - render: renderLimit || self.options.limits.render || 0 - }); - }); + var limits = { + cacheOnTimeout: self.options.limits.cacheOnTimeout || false, + render: self.options.limits.render || 0 + }; + + step( + function getTilerLimit() { + var next = this; + self.getTilerRenderLimit(username, function (err, tilerRenderLimit) { + if (err) { + return callback(err); + } + + if (Number.isFinite(tilerRenderLimit)) { + limits.render = tilerRenderLimit; + return callback(null, limits); + } + + return next(); + }); + }, + function getTimeoutLimit() { + self.getTimeoutRenderLimit(username, apiKey, function (err, timeoutRenderLimit) { + if (err) { + return callback(err); + } + + if (timeoutRenderLimit && timeoutRenderLimit.render) { + if (Number.isFinite(timeoutRenderLimit.render)) { + limits.render = timeoutRenderLimit.render; + } + } + + return callback(null, limits); + }); + } + ); +}; + +UserLimitsApi.prototype.getTilerRenderLimit = function (username, callback) { + this.metadataBackend.getTilerRenderLimit(username, callback); +}; + +UserLimitsApi.prototype.getTimeoutRenderLimit = function (username, apiKey, callback) { + var self = this; + + step( + function isAuthorized() { + var next = this; + + if (!apiKey) { + return next(null, false); + } + + self.metadataBackend.getUserMapKey(username, function (err, userApiKey) { + if (err) { + return next(err); + } + + return next(null, userApiKey === apiKey); + }); + }, + function getUserTimeoutRenderLimits(err, authorized) { + if (err) { + return next(err); + } + self.metadataBackend.getUserTimeoutRenderLimits(username, authorized, this); + }, + function setTilerRenderLimit(err, timeoutRenderLimit) { + if (err) { + return callback(err); + } + return callback(null, timeoutRenderLimit); + } + ); }; diff --git a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js index af5a775b..340073b5 100644 --- a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js +++ b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js @@ -26,7 +26,7 @@ CreateLayergroupMapConfigProvider.prototype.getMapConfig = function(callback) { var context = {}; step( function prepareContextLimits() { - self.userLimitsApi.getRenderLimits(self.user, this); + self.userLimitsApi.getRenderLimits(self.user, self.params.api_key, this); }, function handleRenderLimits(err, renderLimits) { assert.ifError(err); diff --git a/lib/cartodb/models/mapconfig/provider/map-store-provider.js b/lib/cartodb/models/mapconfig/provider/map-store-provider.js index c07f9abc..177322d4 100644 --- a/lib/cartodb/models/mapconfig/provider/map-store-provider.js +++ b/lib/cartodb/models/mapconfig/provider/map-store-provider.js @@ -27,7 +27,7 @@ MapStoreMapConfigProvider.prototype.getMapConfig = function(callback) { var context = {}; step( function prepareContextLimits() { - self.userLimitsApi.getRenderLimits(self.user, this); + self.userLimitsApi.getRenderLimits(self.user, self.params.api_key, this); }, function handleRenderLimits(err, renderLimits) { assert.ifError(err); diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 594ec247..17c66c29 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -114,7 +114,7 @@ NamedMapMapConfigProvider.prototype.getMapConfig = function(callback) { function prepareContextLimits(err, _mapConfig) { assert.ifError(err); mapConfig = _mapConfig; - self.userLimitsApi.getRenderLimits(self.owner, this); + self.userLimitsApi.getRenderLimits(self.owner, self.params.api_key, this); }, function cacheAndReturnMapConfig(err, renderLimits) { self.err = err; diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js new file mode 100644 index 00000000..b595cd0e --- /dev/null +++ b/test/acceptance/user-timeout-limit.js @@ -0,0 +1,82 @@ +require('../support/test_helper'); + +var assert = require('../support/assert'); +var TestClient = require('../support/test-client'); +var testHelper = require('../support/test_helper'); + +var redis = require('redis'); +var keysToDelete; + +function withUserTimeoutRenderLimit(redisClient, user, userTimeoutLimit, callback) { + redisClient.SELECT(5, function(err) { + if (err) { + return callback(err); + } + + var userTimeoutLimitsKey = 'limits:timeout:' + user; + var redisParams = [ + userTimeoutLimitsKey, + 'render', userTimeoutLimit, + 'render_public', userTimeoutLimit + ]; + + redisClient.hmset(redisParams, function (err) { + if (err) { + return callback(err); + } + keysToDelete[userTimeoutLimitsKey] = 5; + return callback(); + }); + }); +} + +function createMapConfig (cartocss) { + return { + version: '1.6.0', + layers: [{ + type: "cartodb", + options: { + sql: [ + 'SELECT', + ' pg_sleep(1),', + ' 1 cartodb_id,', + ' \'SRID=3857;POINT(0 0)\'::geometry the_geom_webmercator' + ].join('\n'), + cartocss: cartocss, + cartocss_version: '2.3.0', + interactivity: 'cartodb_id' + } + }] + }; +} + + + +describe('user timeout limits', function () { + var redisClient = redis.createClient(global.environment.redis.port); + + beforeEach(function() { + keysToDelete = {}; + }); + + afterEach(function (done) { + testHelper.deleteRedisKeys(keysToDelete, done); + }); + + it('layergroup creation works even if test tile is slow', function (done) { + withUserTimeoutRenderLimit(redisClient, 'localhost', 1, function (err) { + if (err) { + return done(err); + } + + var mapConfig = createMapConfig(TestClient.CARTOCSS.POINTS); + var testClient = new TestClient(mapConfig, 1234); + testClient.getTile(4, 4, 4, {}, function (err, res, tile) { + assert.ok(err, err); + // TODO: check timeout tile + + testClient.drain(done) + }); + }); + }); +}); From 37c970903e5a940d40884488034a402a20a81b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 5 Jul 2017 19:09:14 +0200 Subject: [PATCH 02/66] Avoid uncaught exception when layergroup is not present --- test/support/test-client.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 659d6686..d499e9d5 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -579,8 +579,10 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { }); }, function finish(err, res, image) { - self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; - self.keysToDelete['user:localhost:mapviews:global'] = 5; + if (layergroupId) { + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; + } return callback(err, res, image); } ); From 23aab7a09fa2e7595eadf83bac4d0a9b9b2e4786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 6 Jul 2017 16:23:42 +0200 Subject: [PATCH 03/66] User feature branch of cartodb-redis --- package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 463848c5..dac5756a 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "camshaft": "0.55.6", "cartodb-psql": "0.8.0", "cartodb-query-tables": "0.2.0", - "cartodb-redis": "0.13.2", + "cartodb-redis": "cartodb/node-cartodb-redis#timeout-limits", "debug": "~2.2.0", "dot": "~1.0.2", "express": "~4.13.3", diff --git a/yarn.lock b/yarn.lock index 8409e6e1..b80967ae 100644 --- a/yarn.lock +++ b/yarn.lock @@ -262,9 +262,9 @@ cartodb-query-tables@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/cartodb-query-tables/-/cartodb-query-tables-0.2.0.tgz#b4d672accde04da5b890a5d56a87b761fa7eec44" -cartodb-redis@0.13.2: - version "0.13.2" - resolved "https://registry.yarnpkg.com/cartodb-redis/-/cartodb-redis-0.13.2.tgz#de5214fa5c3ab336c4da978133efa8f908b3691c" +cartodb-redis@cartodb/node-cartodb-redis#timeout-limits: + version "0.13.3" + resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/0c0e5c410243bd4a16e9d94b52821e9d6533eb16" dependencies: dot "~1.0.2" redis-mpool "~0.4.1" @@ -1745,7 +1745,7 @@ repeat-string@^1.5.2: version "1.6.1" resolved "https://registry.yarnpkg.com/repeat-string/-/repeat-string-1.6.1.tgz#8dcae470e1c88abc2d600fff4a776286da75e637" -request@2.x, request@^2.55.0, request@~2.79.0: +request@2.x, request@^2.55.0, request@^2.69.0, request@~2.79.0: version "2.79.0" resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" dependencies: @@ -1770,7 +1770,7 @@ request@2.x, request@^2.55.0, request@~2.79.0: tunnel-agent "~0.4.1" uuid "^3.0.0" -request@^2.69.0, request@^2.81.0: +request@^2.81.0: version "2.81.0" resolved "https://registry.yarnpkg.com/request/-/request-2.81.0.tgz#c6928946a0e06c5f8d6f8a9333469ffda46298a0" dependencies: From 32bcf9ca89bf33b72fc00dd90f4a727a74e8f982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 6 Jul 2017 16:24:18 +0200 Subject: [PATCH 04/66] Fix jshint typo --- test/acceptance/user-timeout-limit.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index b595cd0e..cb5fe20e 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -71,11 +71,10 @@ describe('user timeout limits', function () { var mapConfig = createMapConfig(TestClient.CARTOCSS.POINTS); var testClient = new TestClient(mapConfig, 1234); - testClient.getTile(4, 4, 4, {}, function (err, res, tile) { + testClient.getTile(4, 4, 4, {}, function (err, /* res, tile */) { assert.ok(err, err); // TODO: check timeout tile - - testClient.drain(done) + testClient.drain(done); }); }); }); From ad1506ae976c4976ff6a8b9c6e304d75c61f9b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 6 Jul 2017 16:24:50 +0200 Subject: [PATCH 05/66] Remove empty lines --- test/acceptance/user-timeout-limit.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index cb5fe20e..0c75dce2 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -50,8 +50,6 @@ function createMapConfig (cartocss) { }; } - - describe('user timeout limits', function () { var redisClient = redis.createClient(global.environment.redis.port); From 669707b26c05ab6ef88d35587cd5f0499d345ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 18 Jul 2017 11:56:54 +0200 Subject: [PATCH 06/66] Fix typo --- lib/cartodb/api/user_limits_api.js | 5 ++++- test/acceptance/user-timeout-limit.js | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/api/user_limits_api.js b/lib/cartodb/api/user_limits_api.js index 7f244e7d..7e004e31 100644 --- a/lib/cartodb/api/user_limits_api.js +++ b/lib/cartodb/api/user_limits_api.js @@ -81,10 +81,13 @@ UserLimitsApi.prototype.getTimeoutRenderLimit = function (username, apiKey, call }); }, function getUserTimeoutRenderLimits(err, authorized) { + var next = this; + if (err) { return next(err); } - self.metadataBackend.getUserTimeoutRenderLimits(username, authorized, this); + + self.metadataBackend.getUserTimeoutRenderLimits(username, authorized, next); }, function setTilerRenderLimit(err, timeoutRenderLimit) { if (err) { diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index 0c75dce2..c83db965 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -69,7 +69,7 @@ describe('user timeout limits', function () { var mapConfig = createMapConfig(TestClient.CARTOCSS.POINTS); var testClient = new TestClient(mapConfig, 1234); - testClient.getTile(4, 4, 4, {}, function (err, /* res, tile */) { + testClient.getTile(4, 4, 4, {}, function (err /*, res, tile */) { assert.ok(err, err); // TODO: check timeout tile testClient.drain(done); From 87eb5407a8f9dcfa5937fb88e60c8291d9cb2c13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 18 Jul 2017 20:50:31 +0200 Subject: [PATCH 07/66] WIP: implement timeout limit for raster --- test/acceptance/user-timeout-limit.js | 118 ++++++++++++++------------ test/support/test-client.js | 55 ++++++++---- test/support/test_helper.js | 19 ++++- 3 files changed, 120 insertions(+), 72 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index c83db965..1d588f74 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -1,48 +1,25 @@ require('../support/test_helper'); -var assert = require('../support/assert'); -var TestClient = require('../support/test-client'); -var testHelper = require('../support/test_helper'); +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); -var redis = require('redis'); -var keysToDelete; +const timeoutErrorTilePath = `${process.cwd()}/assets/render-timeout-fallback.png`; -function withUserTimeoutRenderLimit(redisClient, user, userTimeoutLimit, callback) { - redisClient.SELECT(5, function(err) { - if (err) { - return callback(err); - } +var pointSleepSql = ` + SELECT + pg_sleep(0.5), + 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, + 1 cartodb_id +`; - var userTimeoutLimitsKey = 'limits:timeout:' + user; - var redisParams = [ - userTimeoutLimitsKey, - 'render', userTimeoutLimit, - 'render_public', userTimeoutLimit - ]; - - redisClient.hmset(redisParams, function (err) { - if (err) { - return callback(err); - } - keysToDelete[userTimeoutLimitsKey] = 5; - return callback(); - }); - }); -} - -function createMapConfig (cartocss) { +function createMapConfig (sql = pointSleepSql, cartocss = TestClient.CARTOCSS.POINTS) { return { version: '1.6.0', layers: [{ - type: "cartodb", + type: 'cartodb', options: { - sql: [ - 'SELECT', - ' pg_sleep(1),', - ' 1 cartodb_id,', - ' \'SRID=3857;POINT(0 0)\'::geometry the_geom_webmercator' - ].join('\n'), - cartocss: cartocss, + sql, + cartocss, cartocss_version: '2.3.0', interactivity: 'cartodb_id' } @@ -51,28 +28,63 @@ function createMapConfig (cartocss) { } describe('user timeout limits', function () { - var redisClient = redis.createClient(global.environment.redis.port); + describe('with onTileErrorStrategy ENABLED', function () { + let onTileErrorStrategy; - beforeEach(function() { - keysToDelete = {}; + before(function () { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = true; + }); + + after(function () { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + }); + + it('layergroup creation works if test tile is fast but tile request fails if they are slow', function (done) { + var testClient = new TestClient(createMapConfig(), 1234); + + testClient.setUserRenderTimeoutLimit('localhost', 50, function (err) { + assert.ifError(err); + + testClient.getTile(0, 0, 0, {}, function (err, res, tile) { + assert.ifError(err); + + assert.imageIsSimilarToFile(tile, timeoutErrorTilePath, 0.05, function (err) { + assert.ifError(err); + testClient.drain(done); + }); + }); + }); + }); }); - afterEach(function (done) { - testHelper.deleteRedisKeys(keysToDelete, done); - }); + describe('with onTileErrorStrategy DISABLED', function() { + var onTileErrorStrategy; - it('layergroup creation works even if test tile is slow', function (done) { - withUserTimeoutRenderLimit(redisClient, 'localhost', 1, function (err) { - if (err) { - return done(err); - } + beforeEach(function() { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = false; + }); - var mapConfig = createMapConfig(TestClient.CARTOCSS.POINTS); - var testClient = new TestClient(mapConfig, 1234); - testClient.getTile(4, 4, 4, {}, function (err /*, res, tile */) { - assert.ok(err, err); - // TODO: check timeout tile - testClient.drain(done); + afterEach(function() { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + }); + + it('layergroup creation works even if test tile is slow', function (done) { + var testClient = new TestClient(createMapConfig(), 1234); + testClient.setUserRenderTimeoutLimit('localhost', 50, function (err) { + assert.ifError(err); + var params = { + status: 400, + contentType: 'application/json; charset=utf-8' + }; + + testClient.getTile(0, 0, 0, params, function (err, res, tile) { + assert.ifError(err); + + assert.equal(tile.errors[0], 'Render timed out'); + testClient.drain(done); + }); }); }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index dbaf466d..80bc9223 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -14,13 +14,13 @@ var helper = require('./test_helper'); var CartodbWindshaft = require('../../lib/cartodb/server'); var serverOptions = require('../../lib/cartodb/server_options'); serverOptions.analysis.batch.inlineExecution = true; -var server = new CartodbWindshaft(serverOptions); function TestClient(config, apiKey) { this.mapConfig = isMapConfig(config) ? config : null; this.template = isTemplate(config) ? config : null; this.apiKey = apiKey; this.keysToDelete = {}; + this.server = new CartodbWindshaft(serverOptions); } module.exports = TestClient; @@ -97,7 +97,7 @@ TestClient.prototype.getWidget = function(widgetName, params, callback) { step( function createLayergroup() { var next = this; - assert.response(server, + assert.response(self.server, { url: url, method: 'POST', @@ -156,7 +156,7 @@ TestClient.prototype.getWidget = function(widgetName, params, callback) { url = '/api/v1/map/' + layergroupId + '/0/widget/' + widgetName + '?' + qs.stringify(urlParams); - assert.response(server, + assert.response(self.server, { url: url, method: 'GET', @@ -208,7 +208,7 @@ TestClient.prototype.widgetSearch = function(widgetName, userQuery, params, call step( function createLayergroup() { var next = this; - assert.response(server, + assert.response(self.server, { url: url, method: 'POST', @@ -265,7 +265,7 @@ TestClient.prototype.widgetSearch = function(widgetName, userQuery, params, call } url = '/api/v1/map/' + layergroupId + '/0/widget/' + widgetName + '/search?' + qs.stringify(urlParams); - assert.response(server, + assert.response(self.server, { url: url, method: 'GET', @@ -332,7 +332,7 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { step( function createLayergroup() { var next = this; - assert.response(server, + assert.response(self.server, { url: url, method: 'POST', @@ -385,7 +385,7 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { } url = '/api/v1/map/' + layergroupId + '/dataview/' + dataviewName + '?' + qs.stringify(urlParams); - assert.response(server, + assert.response(self.server, { url: url, method: 'GET', @@ -441,7 +441,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { params.placeholders = params.placeholders || {}; - assert.response(server, + assert.response(self.server, { url: urlNamed + '?' + qs.stringify({ api_key: self.apiKey }), method: 'POST', @@ -473,7 +473,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { urlNamed + '/' + templateId + '?' + qs.stringify({api_key: self.apiKey}) : url; - assert.response(server, + assert.response(self.server, { url: path, method: 'POST', @@ -569,20 +569,22 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { expectedResponse.headers['Content-Type'] = 'application/json; charset=utf-8'; } - assert.response(server, request, expectedResponse, function(res, err) { + if (params.contentType) { + expectedResponse.headers['Content-Type'] = 'application/json; charset=utf-8'; + } + + assert.response(self.server, request, expectedResponse, function(res, err) { assert.ifError(err); var obj; if (isPng) { obj = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); - } - else if (isMvt) { + } else if (isMvt) { if (res.body) { obj = new mapnik.VectorTile(z, x, y); obj.setDataSync(new Buffer(res.body, 'binary')); } - } - else { + } else { obj = JSON.parse(res.body); } @@ -618,7 +620,7 @@ TestClient.prototype.getLayergroup = function(expectedResponse, callback) { url += '?' + qs.stringify({api_key: this.apiKey}); } - assert.response(server, + assert.response(self.server, { url: url, method: 'POST', @@ -662,7 +664,7 @@ TestClient.prototype.getNodeStatus = function(nodeName, callback) { step( function createLayergroup() { var next = this; - assert.response(server, + assert.response(self.server, { url: url, method: 'POST', @@ -723,7 +725,7 @@ TestClient.prototype.getNodeStatus = function(nodeName, callback) { } }; - assert.response(server, request, expectedResponse, function(res, err) { + assert.response(self.server, request, expectedResponse, function(res, err) { assert.ifError(err); next(null, res, JSON.parse(res.body)); }); @@ -741,6 +743,10 @@ TestClient.prototype.drain = function(callback) { }; module.exports.getStaticMap = function getStaticMap(templateName, params, callback) { + var self = this; + + self.server = new CartodbWindshaft(serverOptions); + if (!callback) { callback = params; params = null; @@ -771,9 +777,22 @@ module.exports.getStaticMap = function getStaticMap(templateName, params, callba // this could be removed once named maps are invalidated, otherwise you hits the cache var server = new CartodbWindshaft(serverOptions); - assert.response(server, requestOptions, expectedResponse, function (res, err) { + assert.response(self.server, requestOptions, expectedResponse, function (res, err) { helper.deleteRedisKeys({'user:localhost:mapviews:global': 5}, function() { return callback(err, mapnik.Image.fromBytes(new Buffer(res.body, 'binary'))); }); }); }; + +TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimit, callback) { + const userTimeoutLimitsKey = `limits:timeout:${user}`; + const params = [ + userTimeoutLimitsKey, + 'render', userTimeoutLimit, + 'render_public', userTimeoutLimit + ]; + + this.keysToDelete[userTimeoutLimitsKey] = 5; + + helper.configureMetadata('hmset', params, callback); +} diff --git a/test/support/test_helper.js b/test/support/test_helper.js index de9a6e3c..e8b057c9 100644 --- a/test/support/test_helper.js +++ b/test/support/test_helper.js @@ -166,12 +166,29 @@ function rmdirRecursiveSync(dirname) { } } +function configureMetadata(action, params, callback) { + redisClient.SELECT(5, function (err) { + if (err) { + return callback(err); + } + + redisClient[action](params, function (err) { + if (err) { + return callback(err); + } + + return callback(); + }); + }); +} + module.exports = { deleteRedisKeys: deleteRedisKeys, lzma_compress_to_base64: lzma_compress_to_base64, checkNoCache: checkNoCache, checkSurrogateKey: checkSurrogateKey, checkCache: checkCache, - rmdirRecursiveSync: rmdirRecursiveSync + rmdirRecursiveSync: rmdirRecursiveSync, + configureMetadata }; From 8907082a85ef965d2c0de7e4d81592f656273a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 19 Jul 2017 12:24:37 +0200 Subject: [PATCH 08/66] Parse body relying on content-type header --- test/support/test-client.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 80bc9223..48ef0eb5 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -575,20 +575,18 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { assert.response(self.server, request, expectedResponse, function(res, err) { assert.ifError(err); - var obj; + var body; - if (isPng) { - obj = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); - } else if (isMvt) { - if (res.body) { - obj = new mapnik.VectorTile(z, x, y); - obj.setDataSync(new Buffer(res.body, 'binary')); - } - } else { - obj = JSON.parse(res.body); + if (res.headers['content-type'] === 'image/png') { + body = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); + } else if (res.headers['content-type'] === 'application/x-protobuf') { + body = new mapnik.VectorTile(z, x, y); + body.setDataSync(new Buffer(res.body, 'binary')); + } else if (res.headers['content-type'] === 'application/json; charset=utf-8') { + body = JSON.parse(res.body); } - next(null, res, obj); + next(null, res, body); }); }, function finish(err, res, image) { From e9bc0732c0aea7d687864c8c1608497c387dd201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 19 Jul 2017 12:33:57 +0200 Subject: [PATCH 09/66] Use switch statement instead of if ... else if ... else ... --- test/support/test-client.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 48ef0eb5..9d1d1d7e 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -575,15 +575,22 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { assert.response(self.server, request, expectedResponse, function(res, err) { assert.ifError(err); - var body; - if (res.headers['content-type'] === 'image/png') { - body = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); - } else if (res.headers['content-type'] === 'application/x-protobuf') { - body = new mapnik.VectorTile(z, x, y); - body.setDataSync(new Buffer(res.body, 'binary')); - } else if (res.headers['content-type'] === 'application/json; charset=utf-8') { - body = JSON.parse(res.body); + var body; + switch (res.headers['content-type']) { + case 'image/png': + body = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); + break; + case 'application/x-protobuf': + body = new mapnik.VectorTile(z, x, y); + body.setDataSync(new Buffer(res.body, 'binary')); + break; + case 'application/json; charset=utf-8': + body = JSON.parse(res.body); + break; + default: + body = res.body + break; } next(null, res, body); From eee4fc815ebad60a948e0112e2641691987b8c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 25 Jul 2017 19:11:56 +0200 Subject: [PATCH 10/66] Do not expose database error details --- lib/cartodb/models/dataview/base.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index f8eb1027..7c3dbe6f 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -9,9 +9,9 @@ BaseDataview.prototype.getResult = function(psql, override, callback) { var self = this; this.sql(psql, override, function(err, query) { psql.query(query, function(err, result) { - if (err) { - return callback(err, result); + var error = new Error(err.message); + return callback(error, result); } result = self.format(result, override); From 444595d49d41efcc0b74517357b7048149727bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 25 Jul 2017 19:14:04 +0200 Subject: [PATCH 11/66] Rely on windshaft's repo branch --- package.json | 2 +- yarn.lock | 92 ++++++++++++++++++++++++++-------------------------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/package.json b/package.json index 19c00700..093a6948 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "step-profiler": "~0.3.0", "turbo-carto": "0.19.2", "underscore": "~1.6.0", - "windshaft": "3.2.2", + "windshaft": "cartodb/windshaft#response-timeout-limit", "yargs": "~5.0.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index c6a468fa..14962708 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -"abaculus@github:cartodb/abaculus#2.0.3-cdb1": +abaculus@cartodb/abaculus#2.0.3-cdb1: version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -205,7 +205,7 @@ camshaft@0.55.6: dot "^1.0.3" request "^2.69.0" -"canvas@github:cartodb/node-canvas#1.6.2-cdb2": +canvas@cartodb/node-canvas#1.6.2-cdb2: version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -223,15 +223,7 @@ carto@0.16.3: semver "^5.1.0" yargs "^4.2.0" -carto@CartoDB/carto#0.15.1-cdb1: - version "0.15.1-cdb1" - resolved "https://codeload.github.com/CartoDB/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" - dependencies: - mapnik-reference "~6.0.2" - optimist "~0.6.0" - underscore "~1.6.0" - -"carto@github:cartodb/carto#0.15.1-cdb3": +carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -239,6 +231,14 @@ carto@CartoDB/carto#0.15.1-cdb1: optimist "~0.6.0" underscore "1.8.3" +"carto@github:cartodb/carto#0.15.1-cdb1": + version "0.15.1-cdb1" + resolved "https://codeload.github.com/cartodb/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" + dependencies: + mapnik-reference "~6.0.2" + optimist "~0.6.0" + underscore "~1.6.0" + cartocolor@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/cartocolor/-/cartocolor-4.0.0.tgz#841a3222d8b5b22718d9d545b1e5b972cb26eb36" @@ -1741,32 +1741,7 @@ repeat-string@^1.5.2: version "1.6.1" resolved "https://registry.yarnpkg.com/repeat-string/-/repeat-string-1.6.1.tgz#8dcae470e1c88abc2d600fff4a776286da75e637" -request@2.x, request@^2.55.0, request@^2.69.0, request@~2.79.0: - version "2.79.0" - resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" - dependencies: - aws-sign2 "~0.6.0" - aws4 "^1.2.1" - caseless "~0.11.0" - combined-stream "~1.0.5" - extend "~3.0.0" - forever-agent "~0.6.1" - form-data "~2.1.1" - har-validator "~2.0.6" - hawk "~3.1.3" - http-signature "~1.1.0" - is-typedarray "~1.0.0" - isstream "~0.1.2" - json-stringify-safe "~5.0.1" - mime-types "~2.1.7" - oauth-sign "~0.8.1" - qs "~6.3.0" - stringstream "~0.0.4" - tough-cookie "~2.3.0" - tunnel-agent "~0.4.1" - uuid "^3.0.0" - -request@^2.81.0: +request@2.x, request@^2.81.0: version "2.81.0" resolved "https://registry.yarnpkg.com/request/-/request-2.81.0.tgz#c6928946a0e06c5f8d6f8a9333469ffda46298a0" dependencies: @@ -1793,6 +1768,31 @@ request@^2.81.0: tunnel-agent "^0.6.0" uuid "^3.0.0" +request@^2.55.0, request@^2.69.0, request@~2.79.0: + version "2.79.0" + resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" + dependencies: + aws-sign2 "~0.6.0" + aws4 "^1.2.1" + caseless "~0.11.0" + combined-stream "~1.0.5" + extend "~3.0.0" + forever-agent "~0.6.1" + form-data "~2.1.1" + har-validator "~2.0.6" + hawk "~3.1.3" + http-signature "~1.1.0" + is-typedarray "~1.0.0" + isstream "~0.1.2" + json-stringify-safe "~5.0.1" + mime-types "~2.1.7" + oauth-sign "~0.8.1" + qs "~6.3.0" + stringstream "~0.0.4" + tough-cookie "~2.3.0" + tunnel-agent "~0.4.1" + uuid "^3.0.0" + require-directory@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42" @@ -2103,17 +2103,17 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb3": - version "2.3.1-cdb3" - resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/bde83c8dcf4ada40c7c0eb1b477f212e75399d23" +tilelive-bridge@cartodb/tilelive-bridge#query-timeout: + version "2.3.1-cdb2" + resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/38d5d82355aa9eef68c29b7d7a33b60a3ae394f7" dependencies: mapnik "~3.5.0" mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb2": +tilelive-mapnik@cartodb/tilelive-mapnik#custom-timeout: version "0.6.18-cdb2" - resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/46f1adefee90f3f46c0ede5e0833f8522634a858" + resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/d9bfe27a4d5120be92e71337474135e2877e9cd4" dependencies: generic-pool "~2.4.0" mapnik "3.5.14" @@ -2274,9 +2274,9 @@ window-size@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/window-size/-/window-size-0.2.0.tgz#b4315bb4214a3d7058ebeee892e13fa24d98b075" -windshaft@3.2.2: +windshaft@cartodb/windshaft#response-timeout-limit: version "3.2.2" - resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-3.2.2.tgz#7afb9d8fd8bba1bf02d39c06e8bbe5a451aad953" + resolved "https://codeload.github.com/cartodb/windshaft/tar.gz/21ebe5f69b5b6215ec2f93088d8eee436f6b81d5" dependencies: abaculus cartodb/abaculus#2.0.3-cdb1 canvas cartodb/node-canvas#1.6.2-cdb2 @@ -2293,8 +2293,8 @@ windshaft@3.2.2: sphericalmercator "1.0.4" step "~0.0.6" tilelive "5.12.2" - tilelive-bridge cartodb/tilelive-bridge#2.3.1-cdb3 - tilelive-mapnik cartodb/tilelive-mapnik#0.6.18-cdb2 + tilelive-bridge cartodb/tilelive-bridge#query-timeout + tilelive-mapnik cartodb/tilelive-mapnik#custom-timeout torque.js "~2.11.0" underscore "~1.6.0" From 526e850f26b364be02a316401b05bc6a153b2b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 25 Jul 2017 19:15:43 +0200 Subject: [PATCH 12/66] Add method to set statements timeout for user's role and database --- test/support/test-client.js | 73 ++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 9d1d1d7e..411cdf3b 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -3,7 +3,8 @@ var qs = require('querystring'); var step = require('step'); var urlParser = require('url'); - +var PSQL = require('cartodb-psql'); +var _ = require('underscore'); var mapnik = require('windshaft').mapnik; var LayergroupToken = require('./layergroup-token'); @@ -72,6 +73,34 @@ module.exports.CARTOCSS = { ' line-width: 0.5;', ' line-opacity: 1;', '}' + ].join('\n'), + + TORQUE: [ + 'Map {', + ' -torque-frame-count: 256;', + ' -torque-animation-duration: 30;', + ' -torque-time-attribute: "cartodb_id";', + ' -torque-aggregation-function: "count(1)";', + ' -torque-resolution: 4;', + ' -torque-data-aggregation: linear;', + '}', + '#layer {', + ' marker-width: 7;', + ' marker-fill: #FFB927;', + ' marker-fill-opacity: 0.9;', + ' marker-line-width: 1;', + ' marker-line-color: #FFF;', + ' marker-line-opacity: 1;', + ' comp-op: lighter;', + '}', + '#layer[frame-offset=1] {', + ' marker-width: 9;', + ' marker-fill-opacity: 0.45;', + '}', + '#layer[frame-offset=2] {', + ' marker-width: 11;', + ' marker-fill-opacity: 0.225;', + '}' ].join('\n') }; @@ -800,4 +829,44 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi this.keysToDelete[userTimeoutLimitsKey] = 5; helper.configureMetadata('hmset', params, callback); -} +}; + +TestClient.prototype.setUserDatabaseTimeoutLimit = function (user, userTimeoutLimit, callback) { + const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; + const role = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + + const psql = new PSQL({ + user: 'postgres', + dbname: dbname, + host: global.environment.postgres.host, + port: global.environment.postgres.port + }); + + step( + function setTimeoutToUserRole (err, params) { + const next = this; + + const timeoutQuery = `ALTER ROLE \"${role}\" SET statement_timeout to ${userTimeoutLimit}`; + psql.query(timeoutQuery, function (err) { + if (err) { + return next(err); + } + next(null, params, psql); + }); + }, + function setTimeoutToDatabase (err, params, psql) { + assert.ifError(err); + + const timeoutQuery = `ALTER DATABASE \"${dbname}\" SET statement_timeout to ${userTimeoutLimit}`; + + psql.query(timeoutQuery, this); + }, + function finish (err) { + if (err) { + return callback(err); + } + + callback(); + } + ); +}; From 788b2f06830d093554edbed6d681bc4416ab9d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 25 Jul 2017 19:16:37 +0200 Subject: [PATCH 13/66] Implement test to validate response limits work as expected --- test/acceptance/user-timeout-limit.js | 324 ++++++++++++++++++++++---- 1 file changed, 278 insertions(+), 46 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index 1d588f74..6d30785a 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -5,87 +5,319 @@ const TestClient = require('../support/test-client'); const timeoutErrorTilePath = `${process.cwd()}/assets/render-timeout-fallback.png`; -var pointSleepSql = ` +const pointSleepSql = ` SELECT pg_sleep(0.5), 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, 1 cartodb_id `; -function createMapConfig (sql = pointSleepSql, cartocss = TestClient.CARTOCSS.POINTS) { - return { - version: '1.6.0', - layers: [{ - type: 'cartodb', - options: { - sql, - cartocss, - cartocss_version: '2.3.0', - interactivity: 'cartodb_id' +const validationPointSleepSql = ` + SELECT + pg_sleep(0.5), + 'SRID=3857;POINT(-180 90)'::geometry the_geom_webmercator, + 1 cartodb_id +`; + +const createMapConfig = ({ + version = '1.6.0', + type = 'cartodb', + sql = pointSleepSql, + cartocss = TestClient.CARTOCSS.POINTS, + cartocss_version = '2.3.0', + interactivity = 'cartodb_id', + countBy = 'cartodb_id' +} = {}) => ({ + version, + layers: [{ + type, + options: { + source: { + id: "a0" + }, + cartocss, + cartocss_version, + interactivity + } + }], + analyses: [ + { + id: 'a0', + type: 'source', + params: { + query: sql } - }] - }; -} + } + ], + dataviews: { + count: { + source: { + id: 'a0' + }, + type: 'formula', + options: { + column: countBy, + operation: 'count' + } + } + } +}); -describe('user timeout limits', function () { - describe('with onTileErrorStrategy ENABLED', function () { - let onTileErrorStrategy; - - before(function () { - onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; - global.environment.enabledFeatures.onTileErrorStrategy = true; +describe('user timeout limit', function () { + describe.skip('map instantiation', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ sql: validationPointSleepSql }); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); }); - after(function () { - global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + afterEach(function (done) { + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); - it('layergroup creation works if test tile is fast but tile request fails if they are slow', function (done) { - var testClient = new TestClient(createMapConfig(), 1234); + it('layergroup creation fails due to statement timeout', function (done) { + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; - testClient.setUserRenderTimeoutLimit('localhost', 50, function (err) { + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.ifError(err); - testClient.getTile(0, 0, 0, {}, function (err, res, tile) { - assert.ifError(err); - - assert.imageIsSimilarToFile(tile, timeoutErrorTilePath, 0.05, function (err) { - assert.ifError(err); - testClient.drain(done); - }); + assert.deepEqual(timeoutError, { + errors: ['Render timed out'], + errors_with_context: [{ type: 'unknown', message: 'Render timed out' }] }); + + done(); }); }); }); - describe('with onTileErrorStrategy DISABLED', function() { - var onTileErrorStrategy; - - beforeEach(function() { - onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; - global.environment.enabledFeatures.onTileErrorStrategy = false; + describe('user torque timeout limits', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ + type: 'torque', + cartocss: TestClient.CARTOCSS.TORQUE + }); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserDatabaseTimeoutLimit('localhost', 50, done); }); - afterEach(function() { - global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + afterEach(function (done) { + this.testClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); - it('layergroup creation works even if test tile is slow', function (done) { - var testClient = new TestClient(createMapConfig(), 1234); - testClient.setUserRenderTimeoutLimit('localhost', 50, function (err) { + it('layergroup creation fails due to statement timeout', function (done) { + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.ifError(err); + + assert.deepEqual(timeoutError, { + errors: ["TorqueRenderer: canceling statement due to statement timeout"], + errors_with_context: [{ + "type": "layer", + "message": "TorqueRenderer: canceling statement due to statement timeout", + "layer": { "id": "torque-layer0", "index": 0, "type": "torque" } + }] + }); + + done(); + }); + }); + }); + + describe('dataview', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserDatabaseTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + this.testClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('layergroup creation works but dataview request fails due to statement timeout', function (done) { + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getDataview('count', params, (err, dataview) => { + assert.ifError(err); + + assert.deepEqual(dataview, { + errors: ['canceling statement due to statement timeout'], + errors_with_context: [{ type: 'unknown', message: 'canceling statement due to statement timeout' }] + }); + + done(); + }); + }); + }); + + + describe('raster', function () { + describe('with onTileErrorStrategy ENABLED', function () { + let onTileErrorStrategy; + + beforeEach(function () { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = true; + }); + + afterEach(function () { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + }); + + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + + it('layergroup creation works but tile request fails due to render timeout', function (done) { + this.testClient.getTile(0, 0, 0, {}, (err, res, tile) => { + assert.ifError(err); + + assert.imageIsSimilarToFile(tile, timeoutErrorTilePath, 0.05, (err) => { + assert.ifError(err); + done(); + }); + }); + }); + }); + + describe('with onTileErrorStrategy DISABLED', function() { + var onTileErrorStrategy; + + beforeEach(function() { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = false; + }); + + afterEach(function() { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + }); + + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('layergroup creation works even if render tile is slow', function (done) { var params = { status: 400, contentType: 'application/json; charset=utf-8' }; - testClient.getTile(0, 0, 0, params, function (err, res, tile) { + this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { assert.ifError(err); assert.equal(tile.errors[0], 'Render timed out'); - testClient.drain(done); + done(); }); }); }); }); + + describe('user vector timeout limits', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('layergroup creation works but vector tile request fails due to render timeout', function (done) { + const params = { + format: 'mvt', + status: 400 + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { + assert.ifError(err); + + assert.deepEqual(tile, { + errors: ['Render timed out'], + errors_with_context: [{ type: 'unknown', message: 'Render timed out' }] + }); + + done(); + }); + }); + }); + + describe('user interativity timeout limits', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('layergroup creation works but "grid.json" tile request fails due to render timeout', function (done) { + const params = { + layers: 'mapnik', + format: 'grid.json', + status: 400 + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { + assert.ifError(err); + + assert.deepEqual(tile, { + errors: ['Render timed out'], + errors_with_context: [{ type: 'unknown', message: 'Render timed out' }] + }); + + done(); + }); + }); + }); }); + From 058f19ab3646b3dcd7c02f7996b159fc123b590e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 26 Jul 2017 13:25:37 +0200 Subject: [PATCH 14/66] Fix skipped test --- test/acceptance/user-timeout-limit.js | 33 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index 6d30785a..f4a3877e 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -9,14 +9,17 @@ const pointSleepSql = ` SELECT pg_sleep(0.5), 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, - 1 cartodb_id + 1 cartodb_id, + 2 value `; +// during instatiation we validate tile 30/0/0, creating a point in that tile `pg_sleep` will throw a timeout const validationPointSleepSql = ` SELECT - pg_sleep(0.5), - 'SRID=3857;POINT(-180 90)'::geometry the_geom_webmercator, - 1 cartodb_id + pg_sleep(1), + ST_Transform('SRID=4326;POINT(-180 85.05112877)'::geometry, 3857) the_geom_webmercator, + 1 cartodb_id, + 2 value `; const createMapConfig = ({ @@ -26,18 +29,20 @@ const createMapConfig = ({ cartocss = TestClient.CARTOCSS.POINTS, cartocss_version = '2.3.0', interactivity = 'cartodb_id', - countBy = 'cartodb_id' + countBy = 'cartodb_id', + attributes = { id: 'cartodb_id', columns: ['value'] }, } = {}) => ({ version, layers: [{ type, options: { source: { - id: "a0" + id: 'a0' }, cartocss, cartocss_version, - interactivity + interactivity, + attributes } }], analyses: [ @@ -64,7 +69,7 @@ const createMapConfig = ({ }); describe('user timeout limit', function () { - describe.skip('map instantiation', function () { + describe('map instantiation', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); this.testClient = new TestClient(mapconfig, 1234); @@ -92,8 +97,16 @@ describe('user timeout limit', function () { assert.ifError(err); assert.deepEqual(timeoutError, { - errors: ['Render timed out'], - errors_with_context: [{ type: 'unknown', message: 'Render timed out' }] + errors: ["Render timed out"], + errors_with_context: [{ + type: "layer", + message: "Render timed out", + layer: { + id: "layer0", + index: 0, + type: "mapnik" + } + }] }); done(); From 768d06c582098ecfd4b6de9430856c57dd6fa7a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 26 Jul 2017 18:35:40 +0200 Subject: [PATCH 15/66] Assert layer params is defined but mapnik layers --- test/support/test-client.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/support/test-client.js b/test/support/test-client.js index 411cdf3b..961914d7 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -16,6 +16,14 @@ var CartodbWindshaft = require('../../lib/cartodb/server'); var serverOptions = require('../../lib/cartodb/server_options'); serverOptions.analysis.batch.inlineExecution = true; +const MAPNIK_SUPPORTED_FORMATS = { + 'png': true, + 'png32': true, + 'grid.json': true, + 'geojson': true, + 'mvt': true +} + function TestClient(config, apiKey) { this.mapConfig = isMapConfig(config) ? config : null; this.template = isTemplate(config) ? config : null; @@ -543,6 +551,10 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { var format = params.format || 'png'; + if (layers === undefined && !MAPNIK_SUPPORTED_FORMATS[format]) { + throw new Error(`Missing layer filter while fetching ${format} tile, review params argument`); + } + url += [z,x,y].join('/'); url += '.' + format; From 522fc79d71dfa4f70baee5a9ca53713eb791ff3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 27 Jul 2017 12:46:38 +0200 Subject: [PATCH 16/66] Do not remove redis' keys if layergroup was not created --- test/support/test-client.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 961914d7..c78d6e04 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -441,9 +441,16 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { ); }, function finish(err, dataview) { - self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; - self.keysToDelete['user:localhost:mapviews:global'] = 5; - return callback(err, dataview); + if (err) { + return callback(err); + } + + if (layergroupId) { + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; + } + + return callback(null, dataview); } ); }; From 9e491e7e9a44d2134e2b896fc47b67062c37bb6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 27 Jul 2017 16:29:36 +0200 Subject: [PATCH 17/66] Fix tests names --- test/acceptance/user-timeout-limit.js | 4 ++-- test/support/test-client.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index f4a3877e..d3de7164 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -272,7 +272,7 @@ describe('user timeout limit', function () { }); }); - describe('user vector timeout limits', function () { + describe('vector', function () { beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); @@ -302,7 +302,7 @@ describe('user timeout limit', function () { }); }); - describe('user interativity timeout limits', function () { + describe('interativity', function () { beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); diff --git a/test/support/test-client.js b/test/support/test-client.js index c78d6e04..80869152 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -850,7 +850,7 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi helper.configureMetadata('hmset', params, callback); }; -TestClient.prototype.setUserDatabaseTimeoutLimit = function (user, userTimeoutLimit, callback) { +TestClient.setUserDatabaseTimeoutLimit = function (user, userTimeoutLimit, callback) { const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; const role = _.template(global.environment.postgres_auth_user, { user_id: 1 }) From eba97a41e5b912c4a87793583aa1d306a043456f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 27 Jul 2017 16:30:57 +0200 Subject: [PATCH 18/66] Going green, solves issue with role timeout in database --- test/acceptance/user-timeout-limit.js | 108 ++++++++++++-------------- 1 file changed, 51 insertions(+), 57 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index d3de7164..dca4ece7 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -7,10 +7,10 @@ const timeoutErrorTilePath = `${process.cwd()}/assets/render-timeout-fallback.pn const pointSleepSql = ` SELECT - pg_sleep(0.5), + pg_sleep(1), 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, 1 cartodb_id, - 2 value + 2 val `; // during instatiation we validate tile 30/0/0, creating a point in that tile `pg_sleep` will throw a timeout @@ -19,7 +19,7 @@ const validationPointSleepSql = ` pg_sleep(1), ST_Transform('SRID=4326;POINT(-180 85.05112877)'::geometry, 3857) the_geom_webmercator, 1 cartodb_id, - 2 value + 2 val `; const createMapConfig = ({ @@ -30,7 +30,7 @@ const createMapConfig = ({ cartocss_version = '2.3.0', interactivity = 'cartodb_id', countBy = 'cartodb_id', - attributes = { id: 'cartodb_id', columns: ['value'] }, + attributes } = {}) => ({ version, layers: [{ @@ -68,7 +68,15 @@ const createMapConfig = ({ } }); -describe('user timeout limit', function () { +describe.only('user timeout limit', function () { + before(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 900, done); + }); + + after(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + describe('map instantiation', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); @@ -114,64 +122,14 @@ describe('user timeout limit', function () { }); }); - describe('user torque timeout limits', function () { - beforeEach(function (done) { - const mapconfig = createMapConfig({ - type: 'torque', - cartocss: TestClient.CARTOCSS.TORQUE - }); - this.testClient = new TestClient(mapconfig, 1234); - this.testClient.setUserDatabaseTimeoutLimit('localhost', 50, done); - }); - - afterEach(function (done) { - this.testClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { - if (err) { - return done(err); - } - this.testClient.drain(done); - }); - }); - - it('layergroup creation fails due to statement timeout', function (done) { - const expectedResponse = { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; - - this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { - assert.ifError(err); - - assert.deepEqual(timeoutError, { - errors: ["TorqueRenderer: canceling statement due to statement timeout"], - errors_with_context: [{ - "type": "layer", - "message": "TorqueRenderer: canceling statement due to statement timeout", - "layer": { "id": "torque-layer0", "index": 0, "type": "torque" } - }] - }); - - done(); - }); - }); - }); - describe('dataview', function () { - beforeEach(function (done) { + beforeEach(function () { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); - this.testClient.setUserDatabaseTimeoutLimit('localhost', 50, done); }); afterEach(function (done) { - this.testClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { - if (err) { - return done(err); - } - this.testClient.drain(done); - }); + this.testClient.drain(done); }); it('layergroup creation works but dataview request fails due to statement timeout', function (done) { @@ -197,6 +155,42 @@ describe('user timeout limit', function () { }); }); + describe('torque', function () { + beforeEach(function () { + const mapconfig = createMapConfig({ + type: 'torque', + cartocss: TestClient.CARTOCSS.TORQUE + }); + this.testClient = new TestClient(mapconfig, 1234); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('layergroup creation fails due to statement timeout', function (done) { + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: ["TorqueRenderer: canceling statement due to statement timeout"], + errors_with_context: [{ + type: "layer", + message: "TorqueRenderer: canceling statement due to statement timeout", + layer: { id: 'torque-layer0', index: 0, type: "torque" } + }] + }); + + done(); + }); + }); + }); + describe('raster', function () { describe('with onTileErrorStrategy ENABLED', function () { From 664a4e673a1aec5d561d7a156a1c3794382afbd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 27 Jul 2017 17:08:29 +0200 Subject: [PATCH 19/66] Add missing after-each hooks and merged duplicates --- test/acceptance/user-timeout-limit.js | 50 +++++++++++++++++---------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-timeout-limit.js index dca4ece7..87c07b76 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-timeout-limit.js @@ -68,7 +68,7 @@ const createMapConfig = ({ } }); -describe.only('user timeout limit', function () { +describe('user timeout limit', function () { before(function (done) { TestClient.setUserDatabaseTimeoutLimit('localhost', 900, done); }); @@ -196,23 +196,24 @@ describe.only('user timeout limit', function () { describe('with onTileErrorStrategy ENABLED', function () { let onTileErrorStrategy; - beforeEach(function () { + beforeEach(function (done) { onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; global.environment.enabledFeatures.onTileErrorStrategy = true; - }); - afterEach(function () { - global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; - }); - - beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); }); afterEach(function (done) { - this.testClient.drain(done); + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); @@ -231,23 +232,24 @@ describe.only('user timeout limit', function () { describe('with onTileErrorStrategy DISABLED', function() { var onTileErrorStrategy; - beforeEach(function() { + beforeEach(function (done) { onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; global.environment.enabledFeatures.onTileErrorStrategy = false; - }); - afterEach(function() { - global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; - }); - - beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); }); afterEach(function (done) { - this.testClient.drain(done); + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); it('layergroup creation works even if render tile is slow', function (done) { @@ -274,7 +276,12 @@ describe.only('user timeout limit', function () { }); afterEach(function (done) { - this.testClient.drain(done); + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); it('layergroup creation works but vector tile request fails due to render timeout', function (done) { @@ -304,7 +311,12 @@ describe.only('user timeout limit', function () { }); afterEach(function (done) { - this.testClient.drain(done); + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); it('layergroup creation works but "grid.json" tile request fails due to render timeout', function (done) { From 33e77a42f21f3e5e3243a05f122ee91ea9c5a7a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 27 Jul 2017 18:50:27 +0200 Subject: [PATCH 20/66] Separate user timeout suites between render and database --- .../acceptance/user-database-timeout-limit.js | 133 ++++++++++++++++++ ...-limit.js => user-render-timeout-limit.js} | 82 +---------- test/support/test-client.js | 36 ++--- 3 files changed, 147 insertions(+), 104 deletions(-) create mode 100644 test/acceptance/user-database-timeout-limit.js rename test/acceptance/{user-timeout-limit.js => user-render-timeout-limit.js} (75%) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js new file mode 100644 index 00000000..32976b37 --- /dev/null +++ b/test/acceptance/user-database-timeout-limit.js @@ -0,0 +1,133 @@ +require('../support/test_helper'); + +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); + +const pointSleepSql = ` + SELECT + pg_sleep(1), + 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, + 1 cartodb_id, + 2 val +`; + +const createMapConfig = ({ + version = '1.6.0', + type = 'cartodb', + sql = pointSleepSql, + cartocss = TestClient.CARTOCSS.POINTS, + cartocss_version = '2.3.0', + countBy = 'cartodb_id' +} = {}) => ({ + version, + layers: [{ + type, + options: { + source: { + id: 'a0' + }, + cartocss, + cartocss_version + } + }], + analyses: [ + { + id: 'a0', + type: 'source', + params: { + query: sql + } + } + ], + dataviews: { + count: { + source: { + id: 'a0' + }, + type: 'formula', + options: { + column: countBy, + operation: 'count' + } + } + } +}); + +describe('user database timeout limit', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + + describe('dataview', function () { + beforeEach(function () { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('layergroup creation works but dataview request fails due to statement timeout', function (done) { + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getDataview('count', params, (err, dataview) => { + assert.ifError(err); + + assert.deepEqual(dataview, { + errors: ['canceling statement due to statement timeout'], + errors_with_context: [{ type: 'unknown', message: 'canceling statement due to statement timeout' }] + }); + + done(); + }); + }); + }); + + describe('torque', function () { + beforeEach(function () { + const mapconfig = createMapConfig({ + type: 'torque', + cartocss: TestClient.CARTOCSS.TORQUE + }); + this.testClient = new TestClient(mapconfig, 1234); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('layergroup creation fails due to statement timeout', function (done) { + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: ["TorqueRenderer: canceling statement due to statement timeout"], + errors_with_context: [{ + type: "layer", + message: "TorqueRenderer: canceling statement due to statement timeout", + layer: { id: 'torque-layer0', index: 0, type: "torque" } + }] + }); + + done(); + }); + }); + }); +}); diff --git a/test/acceptance/user-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js similarity index 75% rename from test/acceptance/user-timeout-limit.js rename to test/acceptance/user-render-timeout-limit.js index 87c07b76..8d7e9a6e 100644 --- a/test/acceptance/user-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -68,16 +68,8 @@ const createMapConfig = ({ } }); -describe('user timeout limit', function () { - before(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 900, done); - }); - - after(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); - }); - - describe('map instantiation', function () { +describe('user render timeout limit', function () { + describe('map instantiation => validation', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); this.testClient = new TestClient(mapconfig, 1234); @@ -122,76 +114,6 @@ describe('user timeout limit', function () { }); }); - describe('dataview', function () { - beforeEach(function () { - const mapconfig = createMapConfig(); - this.testClient = new TestClient(mapconfig, 1234); - }); - - afterEach(function (done) { - this.testClient.drain(done); - }); - - it('layergroup creation works but dataview request fails due to statement timeout', function (done) { - const params = { - response: { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - } - }; - - this.testClient.getDataview('count', params, (err, dataview) => { - assert.ifError(err); - - assert.deepEqual(dataview, { - errors: ['canceling statement due to statement timeout'], - errors_with_context: [{ type: 'unknown', message: 'canceling statement due to statement timeout' }] - }); - - done(); - }); - }); - }); - - describe('torque', function () { - beforeEach(function () { - const mapconfig = createMapConfig({ - type: 'torque', - cartocss: TestClient.CARTOCSS.TORQUE - }); - this.testClient = new TestClient(mapconfig, 1234); - }); - - afterEach(function (done) { - this.testClient.drain(done); - }); - - it('layergroup creation fails due to statement timeout', function (done) { - const expectedResponse = { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; - - this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { - assert.deepEqual(timeoutError, { - errors: ["TorqueRenderer: canceling statement due to statement timeout"], - errors_with_context: [{ - type: "layer", - message: "TorqueRenderer: canceling statement due to statement timeout", - layer: { id: 'torque-layer0', index: 0, type: "torque" } - }] - }); - - done(); - }); - }); - }); - - describe('raster', function () { describe('with onTileErrorStrategy ENABLED', function () { let onTileErrorStrategy; diff --git a/test/support/test-client.js b/test/support/test-client.js index 80869152..d03fef2e 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -850,9 +850,10 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi helper.configureMetadata('hmset', params, callback); }; -TestClient.setUserDatabaseTimeoutLimit = function (user, userTimeoutLimit, callback) { +TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) { const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; const role = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + const publicUser = global.environment.postgres.user; const psql = new PSQL({ user: 'postgres', @@ -862,30 +863,17 @@ TestClient.setUserDatabaseTimeoutLimit = function (user, userTimeoutLimit, callb }); step( - function setTimeoutToUserRole (err, params) { - const next = this; + function configureTimeouts () { + const timeoutSQLs = [ + `ALTER ROLE \"${publicUser}\" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, + `ALTER ROLE \"${role}\" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, + `ALTER DATABASE \"${dbname}\" SET STATEMENT_TIMEOUT TO ${timeoutLimit}` + ]; - const timeoutQuery = `ALTER ROLE \"${role}\" SET statement_timeout to ${userTimeoutLimit}`; - psql.query(timeoutQuery, function (err) { - if (err) { - return next(err); - } - next(null, params, psql); - }); + const group = this.group(); + + timeoutSQLs.forEach(sql => psql.query(sql, group())); }, - function setTimeoutToDatabase (err, params, psql) { - assert.ifError(err); - - const timeoutQuery = `ALTER DATABASE \"${dbname}\" SET statement_timeout to ${userTimeoutLimit}`; - - psql.query(timeoutQuery, this); - }, - function finish (err) { - if (err) { - return callback(err); - } - - callback(); - } + callback ); }; From b023a155b7782faa52aae6b430f88979f3d28b00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 28 Jul 2017 13:21:17 +0200 Subject: [PATCH 21/66] Be more accurate with timeouts and pg_sleep --- test/acceptance/user-database-timeout-limit.js | 4 ++-- test/acceptance/user-render-timeout-limit.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 32976b37..c59b26df 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -5,7 +5,7 @@ const TestClient = require('../support/test-client'); const pointSleepSql = ` SELECT - pg_sleep(1), + pg_sleep(0.5), 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, 1 cartodb_id, 2 val @@ -55,7 +55,7 @@ const createMapConfig = ({ describe('user database timeout limit', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 50, done); + TestClient.setUserDatabaseTimeoutLimit('localhost', 300, done); }); afterEach(function (done) { diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 8d7e9a6e..0119a1b9 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -16,7 +16,7 @@ const pointSleepSql = ` // during instatiation we validate tile 30/0/0, creating a point in that tile `pg_sleep` will throw a timeout const validationPointSleepSql = ` SELECT - pg_sleep(1), + pg_sleep(0.5), ST_Transform('SRID=4326;POINT(-180 85.05112877)'::geometry, 3857) the_geom_webmercator, 1 cartodb_id, 2 val From 815eac5a48987a521da7e2e215f54f3ab7727b15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 28 Jul 2017 13:22:16 +0200 Subject: [PATCH 22/66] Add hook to refresh all connections in the pool --- test/support/test_helper.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/support/test_helper.js b/test/support/test_helper.js index e8b057c9..fc9fcb34 100644 --- a/test/support/test_helper.js +++ b/test/support/test_helper.js @@ -14,6 +14,7 @@ var lzmaWorker = new LZMA(); var redis = require('redis'); var nock = require('nock'); var log4js = require('log4js'); +var pg = require('pg'); // set environment specific variables global.environment = require(__dirname + '/../../config/environments/test'); @@ -127,6 +128,12 @@ afterEach(function(done) { }); }); + +afterEach(function () { + // TODO: this method will be replaced by psql.end + pg.end(); +}); + function deleteRedisKeys(keysToDelete, callback) { if (Object.keys(keysToDelete).length === 0) { From cca570e832ea917eae7532467229f7fa0295c7ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 28 Jul 2017 13:23:13 +0200 Subject: [PATCH 23/66] Uncomment DB and role configuration --- test/support/test-client.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index d03fef2e..1d66d2b6 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -852,8 +852,9 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) { const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; - const role = _.template(global.environment.postgres_auth_user, { user_id: 1 }) - const publicUser = global.environment.postgres.user; + const dbuser = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + const pass = _.template(global.environment.postgres_auth_pass, { user_id: 1 }) + const publicuser = global.environment.postgres.user; const psql = new PSQL({ user: 'postgres', @@ -865,9 +866,9 @@ TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) step( function configureTimeouts () { const timeoutSQLs = [ - `ALTER ROLE \"${publicUser}\" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, - `ALTER ROLE \"${role}\" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, - `ALTER DATABASE \"${dbname}\" SET STATEMENT_TIMEOUT TO ${timeoutLimit}` + // `ALTER ROLE "${publicuser}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, + `ALTER ROLE "${dbuser}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, + // `ALTER DATABASE "${dbname}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}` ]; const group = this.group(); From c3e137bb0029538c8bd4913f010c2057477884d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 28 Jul 2017 16:02:54 +0200 Subject: [PATCH 24/66] Update dependency --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 14962708..d7c06ac0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2113,7 +2113,7 @@ tilelive-bridge@cartodb/tilelive-bridge#query-timeout: tilelive-mapnik@cartodb/tilelive-mapnik#custom-timeout: version "0.6.18-cdb2" - resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/d9bfe27a4d5120be92e71337474135e2877e9cd4" + resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/f9042e5eab92e26fb9e2a6ace65af92cc176a218" dependencies: generic-pool "~2.4.0" mapnik "3.5.14" From 6936107b683d7a9441a07870a0ca8c96f80bc66a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 28 Jul 2017 16:04:11 +0200 Subject: [PATCH 25/66] Adjust pg_sleep to timeout --- test/acceptance/user-render-timeout-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 0119a1b9..8c167d4f 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -7,7 +7,7 @@ const timeoutErrorTilePath = `${process.cwd()}/assets/render-timeout-fallback.pn const pointSleepSql = ` SELECT - pg_sleep(1), + pg_sleep(0.5), 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, 1 cartodb_id, 2 val From f2e043b0631d97d7bcbd65cc5a11ce8d095a2790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 17:56:58 +0200 Subject: [PATCH 26/66] Do not expose database error info --- lib/cartodb/controllers/base.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 7cdd0713..30883e04 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -229,6 +229,18 @@ function stripConnectionInfo(message) { .replace(/is the server.*encountered/im, 'encountered'); } +var ERROR_INFO_TO_EXOSE = { + message: true, + layer: true, + type: true, + analysis: true, + subtype: true +}; + +function shouldBeExposed (prop) { + return !!ERROR_INFO_TO_EXOSE[prop]; +} + function errorMessage(err) { // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 var message = (_.isString(err) ? err : err.message) || 'Unknown error'; @@ -247,7 +259,7 @@ function errorMessageWithContext(err) { for (var prop in err) { // type & message are properties from Error's prototype and will be skipped - if (err.hasOwnProperty(prop)) { + if (err.hasOwnProperty(prop) && shouldBeExposed(prop)) { error[prop] = err[prop]; } } From 398369a5c7ef301cb11975059391e7c9731cf90e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 17:58:33 +0200 Subject: [PATCH 27/66] Do not rely on expected defaults headers --- test/acceptance/mvt.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index b1ea4455..b5a59cd8 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -24,14 +24,24 @@ describe('mvt', function () { desc: 'should get empty mvt with code 204 (no content)', coords: { z: 0, x: 0, y: 0 }, format: 'mvt', - status: 204, + response: { + status: 204, + headers: { + 'Content-Type': undefined + } + }, mapConfig: createMapConfig(TestClient.SQL.EMPTY) }, { desc: 'should get mvt tile with code 200 (ok)', coords: { z: 0, x: 0, y: 0 }, format: 'mvt', - status: 200, + response: { + status: 200, + headers: { + 'Content-Type': 'application/x-protobuf' + } + }, mapConfig: createMapConfig() } ]; @@ -40,12 +50,12 @@ describe('mvt', function () { it(test.desc, done => { const testClient = new TestClient(test.mapConfig, 1234); const { z, x, y } = test.coords; - const { format, status } = test; + const { format, response } = test; - testClient.getTile(z, x, y, { format, status }, (err, res) => { + testClient.getTile(z, x, y, { format, response }, (err, res) => { assert.ifError(err); - assert.equal(res.statusCode, test.status); + assert.equal(res.statusCode, test.response.status); testClient.drain(done); }); }); From 64f19b65ec411b4d7f4e13c15513b85a17b8dbaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:01:19 +0200 Subject: [PATCH 28/66] Remove attributes param --- test/acceptance/user-render-timeout-limit.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 8c167d4f..a44c8793 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -29,8 +29,7 @@ const createMapConfig = ({ cartocss = TestClient.CARTOCSS.POINTS, cartocss_version = '2.3.0', interactivity = 'cartodb_id', - countBy = 'cartodb_id', - attributes + countBy = 'cartodb_id' } = {}) => ({ version, layers: [{ @@ -41,8 +40,7 @@ const createMapConfig = ({ }, cartocss, cartocss_version, - interactivity, - attributes + interactivity } }], analyses: [ From 664db4b5cfefbffc9886a7cfd16ac25e44284543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:07:44 +0200 Subject: [PATCH 29/66] Pass proper param to check content-type and status --- test/acceptance/user-render-timeout-limit.js | 25 +++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index a44c8793..cd5b03a7 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -136,7 +136,6 @@ describe('user render timeout limit', function () { }); }); - it('layergroup creation works but tile request fails due to render timeout', function (done) { this.testClient.getTile(0, 0, 0, {}, (err, res, tile) => { assert.ifError(err); @@ -172,10 +171,14 @@ describe('user render timeout limit', function () { }); }); - it('layergroup creation works even if render tile is slow', function (done) { + it('layergroup creation works and render tile fails', function (done) { var params = { - status: 400, - contentType: 'application/json; charset=utf-8' + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } }; this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { @@ -207,7 +210,12 @@ describe('user render timeout limit', function () { it('layergroup creation works but vector tile request fails due to render timeout', function (done) { const params = { format: 'mvt', - status: 400 + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } }; this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { @@ -243,7 +251,12 @@ describe('user render timeout limit', function () { const params = { layers: 'mapnik', format: 'grid.json', - status: 400 + response: { + status: 400, + headers: { + 'Content-Type': 'application/x-protobuf' + } + } }; this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { From 435d902e45bf3146fa13d4662eb7a230267dcff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:10:14 +0200 Subject: [PATCH 30/66] Expose function to clean all database connections in the pool --- test/support/test_helper.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/support/test_helper.js b/test/support/test_helper.js index fc9fcb34..49c6c1ee 100644 --- a/test/support/test_helper.js +++ b/test/support/test_helper.js @@ -128,11 +128,10 @@ afterEach(function(done) { }); }); - -afterEach(function () { +function cleanPGPoolConnections () { // TODO: this method will be replaced by psql.end pg.end(); -}); +} function deleteRedisKeys(keysToDelete, callback) { @@ -196,6 +195,7 @@ module.exports = { checkSurrogateKey: checkSurrogateKey, checkCache: checkCache, rmdirRecursiveSync: rmdirRecursiveSync, - configureMetadata + configureMetadata, + cleanPGPoolConnections }; From aa6d01f151a5fb614ce8ad3980035f10c6a1eed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:12:33 +0200 Subject: [PATCH 31/66] Add timeout test for attributes --- test/support/test-client.js | 104 ++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/test/support/test-client.js b/test/support/test-client.js index 1d66d2b6..8b6ab3e8 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -791,6 +791,110 @@ TestClient.prototype.getNodeStatus = function(nodeName, callback) { ); }; +TestClient.prototype.getAttributes = function(params, callback) { + var self = this; + + if (!Number.isFinite(params.featureId)) { + throw new Error('featureId param must be a number') + } + + if (!Number.isFinite(params.layer)) { + throw new Error('layer param must be a number') + } + + var url = '/api/v1/map'; + + if (this.apiKey) { + url += '?' + qs.stringify({ api_key: this.apiKey }); + } + + var layergroupid; + + if (params.layergroupid) { + layergroupid = params.layergroupid + } + + step( + function createLayergroup() { + var next = this; + + if (layergroupid) { + return next(null, layergroupid); + } + + assert.response(self.server, + { + url: url, + method: 'POST', + headers: { + host: 'localhost', + 'Content-Type': 'application/json' + }, + data: JSON.stringify(self.mapConfig) + }, + { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }, + function(res, err) { + if (err) { + return next(err); + } + var parsedBody = JSON.parse(res.body); + + return next(null, parsedBody.layergroupid); + } + ); + }, + function getAttributes(err, _layergroupid) { + assert.ifError(err); + + var next = this; + + layergroupid = _layergroupid; + + url = `/api/v1/map/${layergroupid}/${params.layer}/attributes/${params.featureId}`; + + if (self.apiKey) { + url += '?' + qs.stringify({api_key: self.apiKey}); + } + + var request = { + url: url, + method: 'GET', + headers: { + host: 'localhost' + } + }; + + var expectedResponse = params.response || { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + assert.response(self.server, request, expectedResponse, function (res, err) { + assert.ifError(err); + + var attributes = JSON.parse(res.body); + + next(null, res, attributes); + }); + }, + function finish(err, res, attributes) { + if (layergroupid) { + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupid).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; + } + + return callback(err, res, attributes); + } + ); +}; + TestClient.prototype.drain = function(callback) { helper.deleteRedisKeys(this.keysToDelete, callback); }; From 04da57fe0cfc7b566a0315edd1c514a9499751de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:14:32 +0200 Subject: [PATCH 32/66] Do not create layergroup if it is already provided --- test/support/test-client.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/support/test-client.js b/test/support/test-client.js index 8b6ab3e8..34d04200 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -471,6 +471,11 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { } var layergroupId; + + if (params.layergroupid) { + layergroupId = params.layergroupid + } + step( function createTemplate () { var next = this; @@ -512,6 +517,10 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { function createLayergroup(err, templateId) { var next = this; + if (layergroupId) { + return next(null, layergroupId); + } + var data = templateId ? params.placeholders : self.mapConfig var path = templateId ? urlNamed + '/' + templateId + '?' + qs.stringify({api_key: self.apiKey}) : From f079c24554bbfc8c5d10e525770d4c7d9867f944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:22:13 +0200 Subject: [PATCH 33/66] Use parsed body variable --- test/support/test-client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 34d04200..2f322770 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -707,7 +707,7 @@ TestClient.prototype.getLayergroup = function(expectedResponse, callback) { return callback(err); } - return callback(null, JSON.parse(res.body)); + return callback(null, parsedBody); } ); }; From 9b5482489e2e9261a6fe216d04b5aa1e21136b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:23:17 +0200 Subject: [PATCH 34/66] Fix content-type defaults --- test/support/test-client.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 2f322770..04a701b4 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -586,32 +586,30 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { } }; - var expectedResponse = { - status: params.status || 200, + var expectedResponse = Object.assign({}, { + status: 200, headers: { - 'Content-Type': 'application/json; charset=utf-8' + 'Content-Type': 'image/png' } - }; + }, params.response); + var isPng = format.match(/png$/); if (isPng) { request.encoding = 'binary'; - expectedResponse.headers['Content-Type'] = 'image/png'; } var isMvt = format.match(/mvt$/); if (isMvt) { request.encoding = 'binary'; - if (expectedResponse.status === 200) { expectedResponse.headers['Content-Type'] = 'application/x-protobuf'; - } else if (expectedResponse.status === 204) { - expectedResponse.headers['Content-Type'] = undefined; } } + var isGeojson = format.match(/geojson$/); if (isGeojson) { From ac7509b01a0029c3728871502fe90ec17bb8fdfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:24:42 +0200 Subject: [PATCH 35/66] Expose function to clean up database connections --- test/support/test-client.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/support/test-client.js b/test/support/test-client.js index 04a701b4..e35824d7 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -967,6 +967,9 @@ TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) const pass = _.template(global.environment.postgres_auth_pass, { user_id: 1 }) const publicuser = global.environment.postgres.user; + // we need to guarantee all new connections have the new settings + helper.cleanPGPoolConnections() + const psql = new PSQL({ user: 'postgres', dbname: dbname, @@ -989,3 +992,8 @@ TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) callback ); }; + +TestClient.cleanDatabaseConnections = function () { + helper.cleanPGPoolConnections() +} + From 227c2b336bcfc817f0cfb3ace69ac373ffe4e1c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:25:34 +0200 Subject: [PATCH 36/66] Uncomment database timeout configuration --- test/support/test-client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index e35824d7..750a9f69 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -980,9 +980,9 @@ TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) step( function configureTimeouts () { const timeoutSQLs = [ - // `ALTER ROLE "${publicuser}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, + `ALTER ROLE "${publicuser}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, `ALTER ROLE "${dbuser}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}`, - // `ALTER DATABASE "${dbname}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}` + `ALTER DATABASE "${dbname}" SET STATEMENT_TIMEOUT TO ${timeoutLimit}` ]; const group = this.group(); From b1114fc6063fdda92e3a554b5ac0f54f176d9439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:26:45 +0200 Subject: [PATCH 37/66] Add timeout test for toque.json tiles --- .../acceptance/user-database-timeout-limit.js | 272 +++++++++++++++--- 1 file changed, 236 insertions(+), 36 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index c59b26df..da9183c7 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -5,7 +5,7 @@ const TestClient = require('../support/test-client'); const pointSleepSql = ` SELECT - pg_sleep(0.5), + pg_sleep(0.3), 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, 1 cartodb_id, 2 val @@ -17,7 +17,8 @@ const createMapConfig = ({ sql = pointSleepSql, cartocss = TestClient.CARTOCSS.POINTS, cartocss_version = '2.3.0', - countBy = 'cartodb_id' + countBy = 'cartodb_id', + attributes } = {}) => ({ version, layers: [{ @@ -27,7 +28,8 @@ const createMapConfig = ({ id: 'a0' }, cartocss, - cartocss_version + cartocss_version, + attributes } }], analyses: [ @@ -54,15 +56,15 @@ const createMapConfig = ({ }); describe('user database timeout limit', function () { - beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 300, done); - }); - - afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); - }); - describe('dataview', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + beforeEach(function () { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); @@ -95,38 +97,236 @@ describe('user database timeout limit', function () { }); }); - describe('torque', function () { - beforeEach(function () { - const mapconfig = createMapConfig({ - type: 'torque', - cartocss: TestClient.CARTOCSS.TORQUE + describe('torque:', function () { + describe('while validating in layergroup creation', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ + type: 'torque', + cartocss: TestClient.CARTOCSS.TORQUE + }); + this.testClient = new TestClient(mapconfig, 1234); + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('fails due to statement timeout', function (done) { + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: [ 'TorqueRenderer: canceling statement due to statement timeout' ], + errors_with_context: [{ + type: 'layer', + message: 'TorqueRenderer: canceling statement due to statement timeout', + layer: { id: 'torque-layer0', index: 0, type: 'torque' } + }] + }); + + done(); + }); }); - this.testClient = new TestClient(mapconfig, 1234); }); - afterEach(function (done) { - this.testClient.drain(done); - }); + describe('fetching "torque.json" tile', function () { + before(function (done) { + const mapconfig = createMapConfig({ + type: 'torque', + cartocss: TestClient.CARTOCSS.TORQUE + }); + this.testClient = new TestClient(mapconfig, 1234); + const expectedResponse = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; - it('layergroup creation fails due to statement timeout', function (done) { - const expectedResponse = { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } - this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { - assert.deepEqual(timeoutError, { - errors: ["TorqueRenderer: canceling statement due to statement timeout"], - errors_with_context: [{ - type: "layer", - message: "TorqueRenderer: canceling statement due to statement timeout", - layer: { id: 'torque-layer0', index: 0, type: "torque" } - }] + this.layergroupid = res.layergroupid; + + done(); + }); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + describe('with user\'s timeout of 200 ms', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); }); - done(); + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + + it('fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'torque.json', + layers: [ 0 ], + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, attributes) => { + assert.ifError(err); + + assert.deepEqual(attributes, { + errors: [ 'TorqueRenderer: canceling statement due to statement timeout' ], + errors_with_context: [{ + type: 'unknown', + message: 'TorqueRenderer: canceling statement due to statement timeout', + }] + }); + + done(); + }); + }); + }); + }); + }); + + describe('attributes:', function () { + describe('while validating in map instatiation', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ + attributes: { + id: 'cartodb_id', + columns: [ 'val' ] + } + }); + this.testClient = new TestClient(mapconfig, 1234); + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('layergroup creation fails due to statement timeout', function (done) { + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: [ 'canceling statement due to statement timeout' ], + errors_with_context: [{ + type: 'layer', + message: 'canceling statement due to statement timeout', + layer: { + id: 'layer0', + index: 0, + type: 'mapnik' + } + }] + }); + + done(); + }); + }); + }); + + describe('fetching by feature id', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ + attributes: { + id: 'cartodb_id', + columns: [ 'val' ] + } + }); + + this.testClient = new TestClient(mapconfig, 1234); + + const expectedResponse = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } + + this.layergroupid = res.layergroupid; + + done(); + }); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + describe('with user\'s timeout of 200 ms', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + + it('fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + featureId: 1, + layer: 0, + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getAttributes(params, (err, res, attributes) => { + assert.ifError(err); + + assert.deepEqual(attributes, { + errors: ['canceling statement due to statement timeout'], + errors_with_context: [{ + type: 'unknown', + message: 'canceling statement due to statement timeout' + }] + }); + + done(); + }); + }); }); }); }); From 49d2f513c6c609e692c86383f7bfe1be5e5eb8ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:51:23 +0200 Subject: [PATCH 38/66] Fix typo --- lib/cartodb/controllers/base.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 30883e04..b8b73569 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -229,7 +229,7 @@ function stripConnectionInfo(message) { .replace(/is the server.*encountered/im, 'encountered'); } -var ERROR_INFO_TO_EXOSE = { +var ERROR_INFO_TO_EXPOSE = { message: true, layer: true, type: true, @@ -238,7 +238,7 @@ var ERROR_INFO_TO_EXOSE = { }; function shouldBeExposed (prop) { - return !!ERROR_INFO_TO_EXOSE[prop]; + return !!ERROR_INFO_TO_EXPOSE[prop]; } function errorMessage(err) { From bb599025354ebccc978af7bed78fce13742cdcfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 18:52:09 +0200 Subject: [PATCH 39/66] Refactoring tests hooks --- test/acceptance/user-database-timeout-limit.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index da9183c7..35add015 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -58,20 +58,18 @@ const createMapConfig = ({ describe('user database timeout limit', function () { describe('dataview', function () { beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); - }); - - beforeEach(function () { - const mapconfig = createMapConfig(); - this.testClient = new TestClient(mapconfig, 1234); - }); - - afterEach(function (done) { - this.testClient.drain(done); + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); it('layergroup creation works but dataview request fails due to statement timeout', function (done) { From 5974413d5cead17efc5cdca3d6d0b77e4500d1d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 19:08:29 +0200 Subject: [PATCH 40/66] Use 429 to indicate timeout errors --- lib/cartodb/controllers/base.js | 7 +++++++ test/acceptance/user-database-timeout-limit.js | 10 +++++----- test/acceptance/user-render-timeout-limit.js | 8 ++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index b8b73569..c5afdecb 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -301,5 +301,12 @@ function statusFromErrorMessage(errMsg) { statusCode = 404; } } + else if ( -1 !== errMsg.indexOf('Render timed out') ) { + statusCode = 429; // under discussion + } + else if ( -1 !== errMsg.indexOf('canceling statement due to statement timeout') ) { + statusCode = 429; // under discussion + } + return statusCode; } diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 35add015..53b69775 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -75,7 +75,7 @@ describe('user database timeout limit', function () { it('layergroup creation works but dataview request fails due to statement timeout', function (done) { const params = { response: { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -117,7 +117,7 @@ describe('user database timeout limit', function () { it('fails due to statement timeout', function (done) { const expectedResponse = { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -182,7 +182,7 @@ describe('user database timeout limit', function () { format: 'torque.json', layers: [ 0 ], response: { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -231,7 +231,7 @@ describe('user database timeout limit', function () { it('layergroup creation fails due to statement timeout', function (done) { const expectedResponse = { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -304,7 +304,7 @@ describe('user database timeout limit', function () { featureId: 1, layer: 0, response: { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index cd5b03a7..e579a132 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -85,7 +85,7 @@ describe('user render timeout limit', function () { it('layergroup creation fails due to statement timeout', function (done) { const expectedResponse = { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -174,7 +174,7 @@ describe('user render timeout limit', function () { it('layergroup creation works and render tile fails', function (done) { var params = { response: { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -211,7 +211,7 @@ describe('user render timeout limit', function () { const params = { format: 'mvt', response: { - status: 400, + status: 429, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -252,7 +252,7 @@ describe('user render timeout limit', function () { layers: 'mapnik', format: 'grid.json', response: { - status: 400, + status: 429, headers: { 'Content-Type': 'application/x-protobuf' } From 1fdc0621e780348c902e01a620eab3b24ed06a77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 19:36:07 +0200 Subject: [PATCH 41/66] Categorize timeout errors --- lib/cartodb/controllers/base.js | 34 +++++++++++++++++++ .../acceptance/user-database-timeout-limit.js | 18 +++++++--- test/acceptance/user-render-timeout-limit.js | 27 ++++++++++++--- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index c5afdecb..cdfc84ff 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -195,6 +195,9 @@ BaseController.prototype.send = function(req, res, body, status, headers) { BaseController.prototype.sendError = function(req, res, err, label) { var allErrors = Array.isArray(err) ? err : [err]; + + allErrors = populateTimeoutErrors(allErrors) + label = label || 'UNKNOWN'; err = allErrors[0] || new Error(label); allErrors[0] = err; @@ -310,3 +313,34 @@ function statusFromErrorMessage(errMsg) { return statusCode; } + +function isRenderTimeoutError (err) { + return err.message && (-1 !== err.message.indexOf('Render timed out')) +} + +function isDatasourceTimeoutError (err) { + return err.message && (-1 !== err.message.indexOf('canceling statement due to statement timeout')) +} + +function isTimeoutError (err) { + return isRenderTimeoutError(err) || isDatasourceTimeoutError(err); +} + +function populateTimeoutErrors (errors) { + return errors.map(function (error) { + if (isTimeoutError(error)) { + error.type = 'limit' + } + + if (isRenderTimeoutError(error)) { + error.subtype = 'render' + } + + if (isDatasourceTimeoutError(error)) { + + error.subtype = 'datasource' + } + + return error; + }) +} diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 53b69775..241b900c 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -87,7 +87,11 @@ describe('user database timeout limit', function () { assert.deepEqual(dataview, { errors: ['canceling statement due to statement timeout'], - errors_with_context: [{ type: 'unknown', message: 'canceling statement due to statement timeout' }] + errors_with_context: [{ + type: 'limit', + subtype: 'datasource', + message: 'canceling statement due to statement timeout' + }] }); done(); @@ -127,7 +131,8 @@ describe('user database timeout limit', function () { assert.deepEqual(timeoutError, { errors: [ 'TorqueRenderer: canceling statement due to statement timeout' ], errors_with_context: [{ - type: 'layer', + type: 'limit', + subtype: 'datasource', message: 'TorqueRenderer: canceling statement due to statement timeout', layer: { id: 'torque-layer0', index: 0, type: 'torque' } }] @@ -195,7 +200,8 @@ describe('user database timeout limit', function () { assert.deepEqual(attributes, { errors: [ 'TorqueRenderer: canceling statement due to statement timeout' ], errors_with_context: [{ - type: 'unknown', + type: 'limit', + subtype: 'datasource', message: 'TorqueRenderer: canceling statement due to statement timeout', }] }); @@ -241,7 +247,8 @@ describe('user database timeout limit', function () { assert.deepEqual(timeoutError, { errors: [ 'canceling statement due to statement timeout' ], errors_with_context: [{ - type: 'layer', + type: 'limit', + subtype: 'datasource', message: 'canceling statement due to statement timeout', layer: { id: 'layer0', @@ -317,7 +324,8 @@ describe('user database timeout limit', function () { assert.deepEqual(attributes, { errors: ['canceling statement due to statement timeout'], errors_with_context: [{ - type: 'unknown', + type: 'limit', + subtype: 'datasource', message: 'canceling statement due to statement timeout' }] }); diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index e579a132..c5751bed 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -97,7 +97,8 @@ describe('user render timeout limit', function () { assert.deepEqual(timeoutError, { errors: ["Render timed out"], errors_with_context: [{ - type: "layer", + type: 'limit', + subtype: 'render', message: "Render timed out", layer: { id: "layer0", @@ -181,10 +182,18 @@ describe('user render timeout limit', function () { } }; - this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { + this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { assert.ifError(err); - assert.equal(tile.errors[0], 'Render timed out'); + assert.deepEqual(timeoutError, { + errors: ["Render timed out"], + errors_with_context: [{ + type: 'limit', + subtype: 'render', + message: "Render timed out" + }] + }); + done(); }); }); @@ -223,7 +232,11 @@ describe('user render timeout limit', function () { assert.deepEqual(tile, { errors: ['Render timed out'], - errors_with_context: [{ type: 'unknown', message: 'Render timed out' }] + errors_with_context: [{ + type: 'limit', + subtype: 'render', + message: 'Render timed out' + }] }); done(); @@ -264,7 +277,11 @@ describe('user render timeout limit', function () { assert.deepEqual(tile, { errors: ['Render timed out'], - errors_with_context: [{ type: 'unknown', message: 'Render timed out' }] + errors_with_context: [{ + type: 'limit', + subtype: 'render', + message: 'Render timed out' + }] }); done(); From 44fc34b1ce5c4449fb11266dd2292f10e5cabfb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 31 Jul 2017 20:07:31 +0200 Subject: [PATCH 42/66] Improve timeout error message --- lib/cartodb/controllers/base.js | 27 ++++++++----------- test/acceptance/limits.js | 12 ++++++--- .../acceptance/user-database-timeout-limit.js | 20 +++++++------- test/acceptance/user-render-timeout-limit.js | 16 +++++------ 4 files changed, 37 insertions(+), 38 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index cdfc84ff..5072d899 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -196,7 +196,7 @@ BaseController.prototype.send = function(req, res, body, status, headers) { BaseController.prototype.sendError = function(req, res, err, label) { var allErrors = Array.isArray(err) ? err : [err]; - allErrors = populateTimeoutErrors(allErrors) + allErrors = populateTimeoutErrors(allErrors); label = label || 'UNKNOWN'; err = allErrors[0] || new Error(label); @@ -304,22 +304,16 @@ function statusFromErrorMessage(errMsg) { statusCode = 404; } } - else if ( -1 !== errMsg.indexOf('Render timed out') ) { - statusCode = 429; // under discussion - } - else if ( -1 !== errMsg.indexOf('canceling statement due to statement timeout') ) { - statusCode = 429; // under discussion - } return statusCode; } function isRenderTimeoutError (err) { - return err.message && (-1 !== err.message.indexOf('Render timed out')) + return err.message && (-1 !== err.message.indexOf('Render timed out')); } function isDatasourceTimeoutError (err) { - return err.message && (-1 !== err.message.indexOf('canceling statement due to statement timeout')) + return err.message && (-1 !== err.message.indexOf('canceling statement due to statement timeout')); } function isTimeoutError (err) { @@ -328,19 +322,20 @@ function isTimeoutError (err) { function populateTimeoutErrors (errors) { return errors.map(function (error) { - if (isTimeoutError(error)) { - error.type = 'limit' - } - if (isRenderTimeoutError(error)) { - error.subtype = 'render' + error.subtype = 'render'; } if (isDatasourceTimeoutError(error)) { + error.subtype = 'datasource'; + } - error.subtype = 'datasource' + if (isTimeoutError(error)) { + error.message = 'You are over platform limits. Please contact us to know more details'; + error.type = 'limit'; + error.http_status = 429; } return error; - }) + }); } diff --git a/test/acceptance/limits.js b/test/acceptance/limits.js index d0126623..f4156363 100644 --- a/test/acceptance/limits.js +++ b/test/acceptance/limits.js @@ -102,11 +102,13 @@ describe('render limits', function() { assert.response(server, createRequest(layergroup, user), { - status: 400 + status: 429 }, function(res) { var parsed = JSON.parse(res.body); - assert.deepEqual(parsed.errors, [ 'Render timed out' ]); + assert.deepEqual(parsed.errors, [ + 'You are over platform limits. Please contact us to know more details' + ]); done(); } ); @@ -167,11 +169,13 @@ describe('render limits', function() { encoding: 'binary' }, { - status: 400 + status: 429 }, function(res) { var parsed = JSON.parse(res.body); - assert.deepEqual(parsed.errors, ['Render timed out']); + assert.deepEqual(parsed.errors, [ + 'You are over platform limits. Please contact us to know more details' + ]); done(); } ); diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 241b900c..8662636f 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -86,11 +86,11 @@ describe('user database timeout limit', function () { assert.ifError(err); assert.deepEqual(dataview, { - errors: ['canceling statement due to statement timeout'], + errors: ['You are over platform limits. Please contact us to know more details'], errors_with_context: [{ type: 'limit', subtype: 'datasource', - message: 'canceling statement due to statement timeout' + message: 'You are over platform limits. Please contact us to know more details' }] }); @@ -129,11 +129,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'TorqueRenderer: canceling statement due to statement timeout' ], + errors: [ 'You are over platform limits. Please contact us to know more details' ], errors_with_context: [{ type: 'limit', subtype: 'datasource', - message: 'TorqueRenderer: canceling statement due to statement timeout', + message: 'You are over platform limits. Please contact us to know more details', layer: { id: 'torque-layer0', index: 0, type: 'torque' } }] }); @@ -198,11 +198,11 @@ describe('user database timeout limit', function () { assert.ifError(err); assert.deepEqual(attributes, { - errors: [ 'TorqueRenderer: canceling statement due to statement timeout' ], + errors: [ 'You are over platform limits. Please contact us to know more details' ], errors_with_context: [{ type: 'limit', subtype: 'datasource', - message: 'TorqueRenderer: canceling statement due to statement timeout', + message: 'You are over platform limits. Please contact us to know more details', }] }); @@ -245,11 +245,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'canceling statement due to statement timeout' ], + errors: [ 'You are over platform limits. Please contact us to know more details' ], errors_with_context: [{ type: 'limit', subtype: 'datasource', - message: 'canceling statement due to statement timeout', + message: 'You are over platform limits. Please contact us to know more details', layer: { id: 'layer0', index: 0, @@ -322,11 +322,11 @@ describe('user database timeout limit', function () { assert.ifError(err); assert.deepEqual(attributes, { - errors: ['canceling statement due to statement timeout'], + errors: ['You are over platform limits. Please contact us to know more details'], errors_with_context: [{ type: 'limit', subtype: 'datasource', - message: 'canceling statement due to statement timeout' + message: 'You are over platform limits. Please contact us to know more details' }] }); diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index c5751bed..a986dd34 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -95,11 +95,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(timeoutError, { - errors: ["Render timed out"], + errors: ["You are over platform limits. Please contact us to know more details"], errors_with_context: [{ type: 'limit', subtype: 'render', - message: "Render timed out", + message: "You are over platform limits. Please contact us to know more details", layer: { id: "layer0", index: 0, @@ -186,11 +186,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(timeoutError, { - errors: ["Render timed out"], + errors: ["You are over platform limits. Please contact us to know more details"], errors_with_context: [{ type: 'limit', subtype: 'render', - message: "Render timed out" + message: "You are over platform limits. Please contact us to know more details" }] }); @@ -231,11 +231,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(tile, { - errors: ['Render timed out'], + errors: ['You are over platform limits. Please contact us to know more details'], errors_with_context: [{ type: 'limit', subtype: 'render', - message: 'Render timed out' + message: 'You are over platform limits. Please contact us to know more details' }] }); @@ -276,11 +276,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(tile, { - errors: ['Render timed out'], + errors: ['You are over platform limits. Please contact us to know more details'], errors_with_context: [{ type: 'limit', subtype: 'render', - message: 'Render timed out' + message: 'You are over platform limits. Please contact us to know more details' }] }); From dd5209b9a724902142db0154502af62f320a7a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 09:39:37 +0200 Subject: [PATCH 43/66] Add torque.png timeout error test --- .../acceptance/user-database-timeout-limit.js | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 8662636f..6eb2f0e5 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -55,6 +55,15 @@ const createMapConfig = ({ } }); +const DATASOURCE_TIMEOUT_ERROR = { + errors: ['You are over platform limits. Please contact us to know more details'], + errors_with_context: [{ + type: 'limit', + subtype: 'datasource', + message: 'You are over platform limits. Please contact us to know more details' + }] +}; + describe('user database timeout limit', function () { describe('dataview', function () { beforeEach(function (done) { @@ -85,21 +94,14 @@ describe('user database timeout limit', function () { this.testClient.getDataview('count', params, (err, dataview) => { assert.ifError(err); - assert.deepEqual(dataview, { - errors: ['You are over platform limits. Please contact us to know more details'], - errors_with_context: [{ - type: 'limit', - subtype: 'datasource', - message: 'You are over platform limits. Please contact us to know more details' - }] - }); + assert.deepEqual(dataview, DATASOURCE_TIMEOUT_ERROR); done(); }); }); }); - describe('torque:', function () { + describe('torque', function () { describe('while validating in layergroup creation', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ @@ -143,8 +145,8 @@ describe('user database timeout limit', function () { }); }); - describe('fetching "torque.json" tile', function () { - before(function (done) { + describe('fetching torque tiles', function () { + beforeEach(function (done) { const mapconfig = createMapConfig({ type: 'torque', cartocss: TestClient.CARTOCSS.TORQUE @@ -181,7 +183,7 @@ describe('user database timeout limit', function () { TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); }); - it('fails due to statement timeout', function (done) { + it('"torque.json" fails due to statement timeout', function (done) { const params = { layergroupid: this.layergroupid, format: 'torque.json', @@ -197,14 +199,29 @@ describe('user database timeout limit', function () { this.testClient.getTile(0, 0, 0, params, (err, res, attributes) => { assert.ifError(err); - assert.deepEqual(attributes, { - errors: [ 'You are over platform limits. Please contact us to know more details' ], - errors_with_context: [{ - type: 'limit', - subtype: 'datasource', - message: 'You are over platform limits. Please contact us to know more details', - }] - }); + assert.deepEqual(attributes, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); + }); + + it('".png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'torque.png', + layers: [ 0 ], + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, attributes) => { + assert.ifError(err); + + assert.deepEqual(attributes, DATASOURCE_TIMEOUT_ERROR); done(); }); @@ -321,14 +338,7 @@ describe('user database timeout limit', function () { this.testClient.getAttributes(params, (err, res, attributes) => { assert.ifError(err); - assert.deepEqual(attributes, { - errors: ['You are over platform limits. Please contact us to know more details'], - errors_with_context: [{ - type: 'limit', - subtype: 'datasource', - message: 'You are over platform limits. Please contact us to know more details' - }] - }); + assert.deepEqual(attributes, DATASOURCE_TIMEOUT_ERROR); done(); }); From 0c4e67d6a8ea9f4aebdd9898cf5114e45064ecf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 10:21:39 +0200 Subject: [PATCH 44/66] Implemented database timeout test while requesting tiles --- .../acceptance/user-database-timeout-limit.js | 328 +++++++++++++++++- 1 file changed, 321 insertions(+), 7 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 6eb2f0e5..62aae5ce 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -11,12 +11,21 @@ const pointSleepSql = ` 2 val `; +const validationPointSleepSql = ` + SELECT + pg_sleep(0.3), + ST_Transform('SRID=4326;POINT(-180 85.05112877)'::geometry, 3857) the_geom_webmercator, + 1 cartodb_id, + 2 val +`; + const createMapConfig = ({ version = '1.6.0', type = 'cartodb', sql = pointSleepSql, cartocss = TestClient.CARTOCSS.POINTS, cartocss_version = '2.3.0', + interactivity = 'cartodb_id', countBy = 'cartodb_id', attributes } = {}) => ({ @@ -29,7 +38,8 @@ const createMapConfig = ({ }, cartocss, cartocss_version, - attributes + attributes, + interactivity } }], analyses: [ @@ -91,16 +101,320 @@ describe('user database timeout limit', function () { } }; - this.testClient.getDataview('count', params, (err, dataview) => { + this.testClient.getDataview('count', params, (err, timeoutError) => { assert.ifError(err); - assert.deepEqual(dataview, DATASOURCE_TIMEOUT_ERROR); + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); done(); }); }); }); + describe('raster', function () { + describe('while validating in layergroup creation', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ sql: validationPointSleepSql }); + this.testClient = new TestClient(mapconfig, 1234); + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('fails due to statement timeout', function (done) { + const expectedResponse = { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: [ 'You are over platform limits. Please contact us to know more details' ], + errors_with_context: [{ + type: 'limit', + subtype: 'datasource', + message: 'You are over platform limits. Please contact us to know more details', + layer: { id: 'layer0', index: 0, type: 'mapnik' } + }] + }); + + done(); + }); + }); + }); + + describe('fetching raster tiles', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + const expectedResponse = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } + + this.layergroupid = res.layergroupid; + + done(); + }); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + describe('with user\'s timeout of 200 ms', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + + it('"png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'png', + layers: [ 0 ], + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { + assert.ifError(err); + + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); + }); + }); + }); + }); + + describe('vector', function () { + describe('while validating in layergroup creation', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ sql: validationPointSleepSql }); + this.testClient = new TestClient(mapconfig, 1234); + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('fails due to statement timeout', function (done) { + const expectedResponse = { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: [ 'You are over platform limits. Please contact us to know more details' ], + errors_with_context: [{ + type: 'limit', + subtype: 'datasource', + message: 'You are over platform limits. Please contact us to know more details', + layer: { id: 'layer0', index: 0, type: 'mapnik' } + }] + }); + + done(); + }); + }); + }); + + describe('fetching vector tiles', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + const expectedResponse = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } + + this.layergroupid = res.layergroupid; + + done(); + }); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + describe('with user\'s timeout of 200 ms', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + + it('"mvt" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'mvt', + layers: [ 0 ], + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { + assert.ifError(err); + + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); + }); + }); + }); + }); + + + describe('interactivity', function () { + describe('while validating in layergroup creation', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ sql: validationPointSleepSql, interactivity: 'val' }); + this.testClient = new TestClient(mapconfig, 1234); + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('fails due to statement timeout', function (done) { + const expectedResponse = { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { + assert.deepEqual(timeoutError, { + errors: [ 'You are over platform limits. Please contact us to know more details' ], + errors_with_context: [{ + type: 'limit', + subtype: 'datasource', + message: 'You are over platform limits. Please contact us to know more details', + layer: { id: 'layer0', index: 0, type: 'mapnik' } + }] + }); + + done(); + }); + }); + }); + + describe('fetching interactivity tiles', function () { + beforeEach(function (done) { + const mapconfig = createMapConfig({ interactivity: 'val' }); + this.testClient = new TestClient(mapconfig, 1234); + const expectedResponse = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } + + this.layergroupid = res.layergroupid; + + done(); + }); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + describe('with user\'s timeout of 200 ms', function () { + beforeEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + }); + + afterEach(function (done) { + TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + }); + + it.skip('"grid.json" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'grid.json', + layers: [ 0 ], + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { + assert.ifError(err); + + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); + }); + }); + }); + }); + describe('torque', function () { describe('while validating in layergroup creation', function () { beforeEach(function (done) { @@ -196,10 +510,10 @@ describe('user database timeout limit', function () { } }; - this.testClient.getTile(0, 0, 0, params, (err, res, attributes) => { + this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { assert.ifError(err); - assert.deepEqual(attributes, DATASOURCE_TIMEOUT_ERROR); + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); done(); }); @@ -335,10 +649,10 @@ describe('user database timeout limit', function () { } }; - this.testClient.getAttributes(params, (err, res, attributes) => { + this.testClient.getAttributes(params, (err, res, timeoutError) => { assert.ifError(err); - assert.deepEqual(attributes, DATASOURCE_TIMEOUT_ERROR); + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); done(); }); From 9af372381c0cdc3ce0693f7c99724eb17ab205a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 10:29:29 +0200 Subject: [PATCH 45/66] Fix content-type assertion --- test/acceptance/user-render-timeout-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index a986dd34..382a8035 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -267,7 +267,7 @@ describe('user render timeout limit', function () { response: { status: 429, headers: { - 'Content-Type': 'application/x-protobuf' + 'Content-Type': 'application/json; charset=utf-8' } } }; From aa4bb62f38153bdd0f91c423a557e49951a2c430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 10:29:46 +0200 Subject: [PATCH 46/66] Fix test --- test/acceptance/user-database-timeout-limit.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 62aae5ce..1caeafec 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -390,11 +390,11 @@ describe('user database timeout limit', function () { TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); }); - it.skip('"grid.json" fails due to statement timeout', function (done) { + it('"grid.json" fails due to statement timeout', function (done) { const params = { layergroupid: this.layergroupid, format: 'grid.json', - layers: [ 0 ], + layers: 'mapnik', response: { status: 429, headers: { From 91b3e373b756856785cb4516881218de41b6148c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 11:46:48 +0200 Subject: [PATCH 47/66] Add helper method to fetch static images --- test/support/test-client.js | 104 ++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/test/support/test-client.js b/test/support/test-client.js index 750a9f69..6eeb260d 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -710,6 +710,110 @@ TestClient.prototype.getLayergroup = function(expectedResponse, callback) { ); }; +TestClient.prototype.getStaticCenter = function (params, callback) { + var self = this; + + let { layergroupid, z, lat, lng, width, height, format } = params + + var url = `/api/v1/map/`; + + if (this.apiKey) { + url += '?' + qs.stringify({api_key: this.apiKey}); + } + + step( + function createLayergroup() { + var next = this; + + if (layergroupid) { + return next(null, layergroupid); + } + + var data = self.mapConfig + var path = url; + + assert.response(self.server, + { + url: path, + method: 'POST', + headers: { + host: 'localhost', + 'Content-Type': 'application/json' + }, + data: JSON.stringify(data) + }, + { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }, + function(res, err) { + if (err) { + return next(err); + } + return next(null, JSON.parse(res.body).layergroupid); + } + ); + }, + function getStaticResult(err, _layergroupid) { + assert.ifError(err); + + var next = this; + + layergroupid = _layergroupid; + + url = `/api/v1/map/static/center/${layergroupid}/${z}/${lat}/${lng}/${width}/${height}.${format}` + + if (self.apiKey) { + url += '?' + qs.stringify({api_key: self.apiKey}); + } + + var request = { + url: url, + encoding: 'binary', + method: 'GET', + headers: { + host: 'localhost' + } + }; + + var expectedResponse = Object.assign({}, { + status: 200, + headers: { + 'Content-Type': 'image/png' + } + }, params.response); + + assert.response(self.server, request, expectedResponse, function(res, err) { + assert.ifError(err); + + var body; + switch (res.headers['content-type']) { + case 'image/png': + body = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); + break; + case 'application/json; charset=utf-8': + body = JSON.parse(res.body); + break; + default: + body = res.body + break; + } + + next(null, res, body); + }); + }, + function finish(err, res, image) { + if (layergroupid) { + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupid).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; + } + return callback(err, res, image); + } + ); +}; + TestClient.prototype.getNodeStatus = function(nodeName, callback) { var self = this; From 40af73d524da922bef3c5b34d8fa86d2df8ab30a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 11:47:50 +0200 Subject: [PATCH 48/66] Implement test to check render timeout error for static api --- test/acceptance/user-render-timeout-limit.js | 100 +++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 382a8035..23b3cc70 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -288,5 +288,105 @@ describe('user render timeout limit', function () { }); }); }); + + describe('static images', function () { + describe('with onTileErrorStrategy ENABLED', function () { + let onTileErrorStrategy; + + beforeEach(function (done) { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = true; + + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('layergroup creation works but static image fails due to render timeout', function (done) { + const params = { + zoom: 0, + lat: 0, + lng: 0, + width: 256, + height: 256, + format: 'png' + }; + + this.testClient.getStaticCenter(params, function (err, res, tile) { + assert.ifError(err); + + assert.imageIsSimilarToFile(tile, timeoutErrorTilePath, 0.05, (err) => { + assert.ifError(err); + done(); + }); + }); + }); + }); + + describe('with onTileErrorStrategy DISABLED', function() { + var onTileErrorStrategy; + + beforeEach(function (done) { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = false; + + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + this.testClient.setUserRenderTimeoutLimit('localhost', 50, done); + }); + + afterEach(function (done) { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); + }); + + it('layergroup creation works and render tile fails', function (done) { + const params = { + zoom: 0, + lat: 0, + lng: 0, + width: 256, + height: 256, + format: 'png', + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getStaticCenter(params, function (err, res, timeoutError) { + assert.ifError(err); + + assert.deepEqual(timeoutError, { + errors: ["You are over platform limits. Please contact us to know more details"], + errors_with_context: [{ + type: 'limit', + subtype: 'render', + message: "You are over platform limits. Please contact us to know more details" + }] + }); + + done(); + }); + }); + }); + }); }); From 7d136031638cd93c2ede04fff753f0d3fa743d4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 11:58:43 +0200 Subject: [PATCH 49/66] Implement test to validate database timeout error for static api --- .../acceptance/user-database-timeout-limit.js | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 1caeafec..a320d08f 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -208,6 +208,32 @@ describe('user database timeout limit', function () { done(); }); }); + + it('"static png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + zoom: 0, + lat: 0, + lng: 0, + width: 256, + height: 256, + format: 'png', + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getStaticCenter(params, function (err, res, timeoutError) { + assert.ifError(err); + + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); + }); }); }); }); From 4c3e3005aa3cc51117577b7846990db3600f7317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 12:52:34 +0200 Subject: [PATCH 50/66] Apply asset fallback to database timeout errors --- lib/cartodb/server.js | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 13d64804..7968dc5f 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -118,8 +118,27 @@ module.exports = function(serverOptions) { var onTileErrorStrategy; if (global.environment.enabledFeatures.onTileErrorStrategy !== false) { onTileErrorStrategy = function onTileErrorStrategy$TimeoutTile(err, tile, headers, stats, format, callback) { - if (err && err.message === 'Render timed out' && format === 'png') { - return callback(null, timeoutErrorTile, { 'Content-Type': 'image/png' }, {}); + + function isRenderTimeoutError (err) { + return err.message && (-1 !== err.message.indexOf('Render timed out')); + } + + function isDatasourceTimeoutError (err) { + return err.message && (-1 !== err.message.indexOf('canceling statement due to statement timeout')); + } + + function isTimeoutError (err) { + return isRenderTimeoutError(err) || isDatasourceTimeoutError(err); + } + + function isRasterFormat (format) { + return format === 'png' || format === 'jpg' + } + + if (isTimeoutError(err) && isRasterFormat(format)) { + return callback(null, timeoutErrorTile, { + 'Content-Type': 'image/png', + }, {}); } else { return callback(err, tile, headers, stats); } From 05ddf1d505b4ccee25bb3589be5fc84d9154ad96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 12:53:29 +0200 Subject: [PATCH 51/66] Add test to check if asset fallback is working when enabled and database timeout erro happens --- .../acceptance/user-database-timeout-limit.js | 193 +++++++++++++----- 1 file changed, 138 insertions(+), 55 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index a320d08f..4b9df8a4 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -3,6 +3,8 @@ require('../support/test_helper'); const assert = require('../support/assert'); const TestClient = require('../support/test-client'); +const timeoutErrorTilePath = `${process.cwd()}/assets/render-timeout-fallback.png`; + const pointSleepSql = ` SELECT pg_sleep(0.3), @@ -153,31 +155,6 @@ describe('user database timeout limit', function () { }); describe('fetching raster tiles', function () { - beforeEach(function (done) { - const mapconfig = createMapConfig(); - this.testClient = new TestClient(mapconfig, 1234); - const expectedResponse = { - status: 200, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; - - this.testClient.getLayergroup(expectedResponse, (err, res) => { - if (err) { - return done(err); - } - - this.layergroupid = res.layergroupid; - - done(); - }); - }); - - afterEach(function (done) { - this.testClient.drain(done); - }); - describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); @@ -187,51 +164,157 @@ describe('user database timeout limit', function () { TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); }); - it('"png" fails due to statement timeout', function (done) { - const params = { - layergroupid: this.layergroupid, - format: 'png', - layers: [ 0 ], - response: { - status: 429, + describe('with onTileErrorStrategy ENABLED', function () { + let onTileErrorStrategy; + + beforeEach(function (done) { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = true; + + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + const expectedResponse = { + status: 200, headers: { 'Content-Type': 'application/json; charset=utf-8' } - } - }; + }; - this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { - assert.ifError(err); + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } - assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + this.layergroupid = res.layergroupid; - done(); + done(); + }); + }); + + afterEach(function (done) { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + + this.testClient.drain(done); + }); + + it('"png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'png', + layers: [ 0 ] + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { + assert.ifError(err); + + assert.imageIsSimilarToFile(tile, timeoutErrorTilePath, 0.05, (err) => { + assert.ifError(err); + done(); + }); + }); + }); + + it('"static png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + zoom: 0, + lat: 0, + lng: 0, + width: 256, + height: 256, + format: 'png' + }; + + this.testClient.getStaticCenter(params, function (err, res, tile) { + assert.ifError(err); + + assert.imageIsSimilarToFile(tile, timeoutErrorTilePath, 0.05, (err) => { + assert.ifError(err); + done(); + }); + }); }); }); - it('"static png" fails due to statement timeout', function (done) { - const params = { - layergroupid: this.layergroupid, - zoom: 0, - lat: 0, - lng: 0, - width: 256, - height: 256, - format: 'png', - response: { - status: 429, + describe('with onTileErrorStrategy DISABLED', function () { + let onTileErrorStrategy; + + beforeEach(function (done) { + onTileErrorStrategy = global.environment.enabledFeatures.onTileErrorStrategy; + global.environment.enabledFeatures.onTileErrorStrategy = false; + + const mapconfig = createMapConfig(); + this.testClient = new TestClient(mapconfig, 1234); + const expectedResponse = { + status: 200, headers: { 'Content-Type': 'application/json; charset=utf-8' } - } - }; + }; - this.testClient.getStaticCenter(params, function (err, res, timeoutError) { - assert.ifError(err); + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } - assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + this.layergroupid = res.layergroupid; - done(); + done(); + }); + }); + + afterEach(function (done) { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + + this.testClient.drain(done); + }); + + it('"png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + format: 'png', + layers: [ 0 ], + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, params, (err, res, timeoutError) => { + assert.ifError(err); + + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); + }); + + it('"static png" fails due to statement timeout', function (done) { + const params = { + layergroupid: this.layergroupid, + zoom: 0, + lat: 0, + lng: 0, + width: 256, + height: 256, + format: 'png', + response: { + status: 429, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getStaticCenter(params, (err, res, timeoutError) => { + assert.ifError(err); + + assert.deepEqual(timeoutError, DATASOURCE_TIMEOUT_ERROR); + + done(); + }); }); }); }); From 46fee774bdba8b9c54323e3e0d17e530c0dcaf56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 12:54:41 +0200 Subject: [PATCH 52/66] Fix misconfiguration in test's hook --- test/acceptance/user-render-timeout-limit.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 23b3cc70..2fab657f 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -303,6 +303,8 @@ describe('user render timeout limit', function () { }); afterEach(function (done) { + global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; + this.testClient.setUserRenderTimeoutLimit('localhost', 0, (err) => { if (err) { return done(err); From 9258ad7eccb8663fbd58885e96536929b4de8258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 12:56:03 +0200 Subject: [PATCH 53/66] Fix style typo --- lib/cartodb/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 7968dc5f..7b6fc33a 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -132,7 +132,7 @@ module.exports = function(serverOptions) { } function isRasterFormat (format) { - return format === 'png' || format === 'jpg' + return format === 'png' || format === 'jpg'; } if (isTimeoutError(err) && isRasterFormat(format)) { From 262f854e68dfe787525a5962ddc49a557fd6bb21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 13:10:55 +0200 Subject: [PATCH 54/66] Remove error wrapping --- lib/cartodb/models/dataview/base.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index 7c3dbe6f..90d8a299 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -10,8 +10,7 @@ BaseDataview.prototype.getResult = function(psql, override, callback) { this.sql(psql, override, function(err, query) { psql.query(query, function(err, result) { if (err) { - var error = new Error(err.message); - return callback(error, result); + return callback(err, result); } result = self.format(result, override); From 0594407b3837b651944c69ebf5b703f8092363e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 15:03:09 +0200 Subject: [PATCH 55/66] Change error message --- lib/cartodb/controllers/base.js | 2 +- test/acceptance/limits.js | 4 ++-- .../acceptance/user-database-timeout-limit.js | 24 +++++++++---------- test/acceptance/user-render-timeout-limit.js | 20 ++++++++-------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 5072d899..e3d2a407 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -331,7 +331,7 @@ function populateTimeoutErrors (errors) { } if (isTimeoutError(error)) { - error.message = 'You are over platform limits. Please contact us to know more details'; + error.message = 'You are over platform\'s limits. Please contact us to know more details'; error.type = 'limit'; error.http_status = 429; } diff --git a/test/acceptance/limits.js b/test/acceptance/limits.js index f4156363..372fb189 100644 --- a/test/acceptance/limits.js +++ b/test/acceptance/limits.js @@ -107,7 +107,7 @@ describe('render limits', function() { function(res) { var parsed = JSON.parse(res.body); assert.deepEqual(parsed.errors, [ - 'You are over platform limits. Please contact us to know more details' + 'You are over platform\'s limits. Please contact us to know more details' ]); done(); } @@ -174,7 +174,7 @@ describe('render limits', function() { function(res) { var parsed = JSON.parse(res.body); assert.deepEqual(parsed.errors, [ - 'You are over platform limits. Please contact us to know more details' + 'You are over platform\'s limits. Please contact us to know more details' ]); done(); } diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 4b9df8a4..e1ff7bda 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -68,11 +68,11 @@ const createMapConfig = ({ }); const DATASOURCE_TIMEOUT_ERROR = { - errors: ['You are over platform limits. Please contact us to know more details'], + 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 limits. Please contact us to know more details' + message: 'You are over platform\'s limits. Please contact us to know more details' }] }; @@ -140,11 +140,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'You are over platform limits. Please contact us to know more details' ], + 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 limits. Please contact us to know more details', + message: 'You are over platform\'s limits. Please contact us to know more details', layer: { id: 'layer0', index: 0, type: 'mapnik' } }] }); @@ -348,11 +348,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'You are over platform limits. Please contact us to know more details' ], + 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 limits. Please contact us to know more details', + message: 'You are over platform\'s limits. Please contact us to know more details', layer: { id: 'layer0', index: 0, type: 'mapnik' } }] }); @@ -450,11 +450,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'You are over platform limits. Please contact us to know more details' ], + 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 limits. Please contact us to know more details', + message: 'You are over platform\'s limits. Please contact us to know more details', layer: { id: 'layer0', index: 0, type: 'mapnik' } }] }); @@ -554,11 +554,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'You are over platform limits. Please contact us to know more details' ], + 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 limits. Please contact us to know more details', + message: 'You are over platform\'s limits. Please contact us to know more details', layer: { id: 'torque-layer0', index: 0, type: 'torque' } }] }); @@ -685,11 +685,11 @@ describe('user database timeout limit', function () { this.testClient.getLayergroup(expectedResponse, (err, timeoutError) => { assert.deepEqual(timeoutError, { - errors: [ 'You are over platform limits. Please contact us to know more details' ], + 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 limits. Please contact us to know more details', + message: 'You are over platform\'s limits. Please contact us to know more details', layer: { id: 'layer0', index: 0, diff --git a/test/acceptance/user-render-timeout-limit.js b/test/acceptance/user-render-timeout-limit.js index 2fab657f..45e41f89 100644 --- a/test/acceptance/user-render-timeout-limit.js +++ b/test/acceptance/user-render-timeout-limit.js @@ -95,11 +95,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(timeoutError, { - errors: ["You are over platform limits. Please contact us to know more details"], + errors: ["You are over platform\'s limits. Please contact us to know more details"], errors_with_context: [{ type: 'limit', subtype: 'render', - message: "You are over platform limits. Please contact us to know more details", + message: "You are over platform\'s limits. Please contact us to know more details", layer: { id: "layer0", index: 0, @@ -186,11 +186,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(timeoutError, { - errors: ["You are over platform limits. Please contact us to know more details"], + errors: ["You are over platform\'s limits. Please contact us to know more details"], errors_with_context: [{ type: 'limit', subtype: 'render', - message: "You are over platform limits. Please contact us to know more details" + message: "You are over platform\'s limits. Please contact us to know more details" }] }); @@ -231,11 +231,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(tile, { - errors: ['You are over platform limits. Please contact us to know more details'], + errors: ['You are over platform\'s limits. Please contact us to know more details'], errors_with_context: [{ type: 'limit', subtype: 'render', - message: 'You are over platform limits. Please contact us to know more details' + message: 'You are over platform\'s limits. Please contact us to know more details' }] }); @@ -276,11 +276,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(tile, { - errors: ['You are over platform limits. Please contact us to know more details'], + errors: ['You are over platform\'s limits. Please contact us to know more details'], errors_with_context: [{ type: 'limit', subtype: 'render', - message: 'You are over platform limits. Please contact us to know more details' + message: 'You are over platform\'s limits. Please contact us to know more details' }] }); @@ -377,11 +377,11 @@ describe('user render timeout limit', function () { assert.ifError(err); assert.deepEqual(timeoutError, { - errors: ["You are over platform limits. Please contact us to know more details"], + errors: ["You are over platform\'s limits. Please contact us to know more details"], errors_with_context: [{ type: 'limit', subtype: 'render', - message: "You are over platform limits. Please contact us to know more details" + message: "You are over platform\'s limits. Please contact us to know more details" }] }); From ab879e263465e024af45b235952d2c2760a902b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 1 Aug 2017 19:13:55 +0200 Subject: [PATCH 56/66] Use new version of `getUserTimeoutRenderLimits` --- lib/cartodb/api/user_limits_api.js | 17 ++++++++++------- yarn.lock | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/api/user_limits_api.js b/lib/cartodb/api/user_limits_api.js index 7e004e31..8954c261 100644 --- a/lib/cartodb/api/user_limits_api.js +++ b/lib/cartodb/api/user_limits_api.js @@ -87,13 +87,16 @@ UserLimitsApi.prototype.getTimeoutRenderLimit = function (username, apiKey, call return next(err); } - self.metadataBackend.getUserTimeoutRenderLimits(username, authorized, next); + self.metadataBackend.getUserTimeoutRenderLimits(username, function (err, timeoutRenderLimit) { + if (err) { + return next(err) + } + + next(null, { + render: authorized ? timeoutRenderLimit.render : timeoutRenderLimit.renderPublic + }); + }); }, - function setTilerRenderLimit(err, timeoutRenderLimit) { - if (err) { - return callback(err); - } - return callback(null, timeoutRenderLimit); - } + callback ); }; diff --git a/yarn.lock b/yarn.lock index d7c06ac0..63d9f6fe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -260,7 +260,7 @@ cartodb-query-tables@0.2.0: cartodb-redis@cartodb/node-cartodb-redis#timeout-limits: version "0.13.3" - resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/0c0e5c410243bd4a16e9d94b52821e9d6533eb16" + resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/530ae2736a184a3b9c9e978efa7dcb4faae7abee" dependencies: dot "~1.0.2" redis-mpool "~0.4.1" From 3574700c2d4c13cecffc438105ecaa645833067b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 2 Aug 2017 11:07:44 +0200 Subject: [PATCH 57/66] Remove tiler render limit --- lib/cartodb/api/user_limits_api.js | 21 +- test/acceptance/limits.js | 315 ----------------------------- yarn.lock | 2 +- 3 files changed, 2 insertions(+), 336 deletions(-) delete mode 100644 test/acceptance/limits.js diff --git a/lib/cartodb/api/user_limits_api.js b/lib/cartodb/api/user_limits_api.js index 8954c261..74d0cf21 100644 --- a/lib/cartodb/api/user_limits_api.js +++ b/lib/cartodb/api/user_limits_api.js @@ -24,21 +24,6 @@ UserLimitsApi.prototype.getRenderLimits = function (username, apiKey, callback) }; step( - function getTilerLimit() { - var next = this; - self.getTilerRenderLimit(username, function (err, tilerRenderLimit) { - if (err) { - return callback(err); - } - - if (Number.isFinite(tilerRenderLimit)) { - limits.render = tilerRenderLimit; - return callback(null, limits); - } - - return next(); - }); - }, function getTimeoutLimit() { self.getTimeoutRenderLimit(username, apiKey, function (err, timeoutRenderLimit) { if (err) { @@ -57,10 +42,6 @@ UserLimitsApi.prototype.getRenderLimits = function (username, apiKey, callback) ); }; -UserLimitsApi.prototype.getTilerRenderLimit = function (username, callback) { - this.metadataBackend.getTilerRenderLimit(username, callback); -}; - UserLimitsApi.prototype.getTimeoutRenderLimit = function (username, apiKey, callback) { var self = this; @@ -89,7 +70,7 @@ UserLimitsApi.prototype.getTimeoutRenderLimit = function (username, apiKey, call self.metadataBackend.getUserTimeoutRenderLimits(username, function (err, timeoutRenderLimit) { if (err) { - return next(err) + return next(err); } next(null, { diff --git a/test/acceptance/limits.js b/test/acceptance/limits.js deleted file mode 100644 index 372fb189..00000000 --- a/test/acceptance/limits.js +++ /dev/null @@ -1,315 +0,0 @@ -var testHelper = require('../support/test_helper'); - -var assert = require('../support/assert'); -var _ = require('underscore'); -var redis = require('redis'); - -var CartodbWindshaft = require('../../lib/cartodb/server'); -var serverOptions = require('../../lib/cartodb/server_options'); - -var LayergroupToken = require('../support/layergroup-token'); - -describe('render limits', function() { - - var layergroupUrl = '/api/v1/map'; - - var redisClient = redis.createClient(global.environment.redis.port); - - var server; - var keysToDelete; - beforeEach(function() { - keysToDelete = {}; - server = new CartodbWindshaft(serverOptions); - server.setMaxListeners(0); - }); - - afterEach(function(done) { - testHelper.deleteRedisKeys(keysToDelete, done); - }); - - var user = 'localhost'; - - var pointSleepSql = "SELECT pg_sleep(0.5)," + - " 'SRID=3857;POINT(0 0)'::geometry the_geom_webmercator, 1 cartodb_id"; - var pointCartoCss = '#layer { marker-fill:red; }'; - var polygonSleepSql = "SELECT pg_sleep(0.5)," + - " ST_Buffer('SRID=3857;POINT(0 0)'::geometry, 100000000) the_geom_webmercator, 1 cartodb_id"; - var polygonCartoCss = '#layer { polygon-fill:red; }'; - - function singleLayergroupConfig(sql, cartocss) { - return { - version: '1.0.0', - layers: [ - { - type: 'mapnik', - options: { - sql: sql, - cartocss: cartocss, - cartocss_version: '2.0.1' - } - } - ] - }; - } - - function createRequest(layergroup, userHost) { - return { - url: layergroupUrl, - method: 'POST', - headers: { - host: userHost, - 'Content-Type': 'application/json' - }, - data: JSON.stringify(layergroup) - }; - } - - function withRenderLimit(user, renderLimit, callback) { - redisClient.SELECT(5, function(err) { - if (err) { - return callback(err); - } - var userLimitsKey = 'limits:tiler:' + user; - redisClient.HSET(userLimitsKey, 'render', renderLimit, function(err) { - if (err) { - return callback(err); - } - keysToDelete[userLimitsKey] = 5; - return callback(); - }); - }); - - } - - describe('with onTileErrorStrategy DISABLED', function() { - var onTileErrorStrategyEnabled; - before(function() { - onTileErrorStrategyEnabled = global.environment.enabledFeatures.onTileErrorStrategy; - global.environment.enabledFeatures.onTileErrorStrategy = false; - }); - - after(function() { - global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategyEnabled; - }); - - it("layergroup creation fails if test tile is slow", function(done) { - withRenderLimit(user, 50, function(err) { - if (err) { - return done(err); - } - - var layergroup = singleLayergroupConfig(polygonSleepSql, polygonCartoCss); - assert.response(server, - createRequest(layergroup, user), - { - status: 429 - }, - function(res) { - var parsed = JSON.parse(res.body); - assert.deepEqual(parsed.errors, [ - 'You are over platform\'s limits. Please contact us to know more details' - ]); - done(); - } - ); - }); - }); - - it("layergroup creation does not fail if user limit is high enough even if test tile is slow", function(done) { - withRenderLimit(user, 5000, function(err) { - if (err) { - return done(err); - } - - var layergroup = singleLayergroupConfig(polygonSleepSql, polygonCartoCss); - assert.response(server, - createRequest(layergroup, user), - { - status: 200 - }, - function(res) { - var parsed = JSON.parse(res.body); - assert.ok(parsed.layergroupid); - keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - done(); - } - ); - }); - }); - - - it("layergroup creation works if test tile is fast but tile request fails if they are slow", function(done) { - withRenderLimit(user, 50, function(err) { - if (err) { - return done(err); - } - - var layergroup = singleLayergroupConfig(pointSleepSql, pointCartoCss); - assert.response(server, - createRequest(layergroup, user), - { - status: 200 - }, - function(res) { - keysToDelete['map_cfg|' + LayergroupToken.parse(JSON.parse(res.body).layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - assert.response(server, - { - url: layergroupUrl + _.template('/<%= layergroupId %>/<%= z %>/<%= x %>/<%= y %>.png', { - layergroupId: JSON.parse(res.body).layergroupid, - z: 0, - x: 0, - y: 0 - }), - method: 'GET', - headers: { - host: 'localhost' - }, - encoding: 'binary' - }, - { - status: 429 - }, - function(res) { - var parsed = JSON.parse(res.body); - assert.deepEqual(parsed.errors, [ - 'You are over platform\'s limits. Please contact us to know more details' - ]); - done(); - } - ); - - } - ); - }); - }); - - it("tile request does not fail if user limit is high enough", function(done) { - withRenderLimit(user, 5000, function(err) { - if (err) { - return done(err); - } - - var layergroup = singleLayergroupConfig(pointSleepSql, pointCartoCss); - assert.response(server, - createRequest(layergroup, user), - { - status: 200 - }, - function(res) { - keysToDelete['map_cfg|' + LayergroupToken.parse(JSON.parse(res.body).layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - assert.response(server, - { - url: layergroupUrl + _.template('/<%= layergroupId %>/<%= z %>/<%= x %>/<%= y %>.png', { - layergroupId: JSON.parse(res.body).layergroupid, - z: 0, - x: 0, - y: 0 - }), - method: 'GET', - headers: { - host: 'localhost' - }, - encoding: 'binary' - }, - { - status: 200, - headers: { - 'Content-Type': 'image/png' - } - }, - function(res, err) { - done(err); - } - ); - - } - ); - }); - }); - - }); - - describe('with onTileErrorStrategy', function() { - - it("layergroup creation works even if test tile is slow", function(done) { - withRenderLimit(user, 50, function(err) { - if (err) { - return done(err); - } - - var layergroup = singleLayergroupConfig(polygonSleepSql, polygonCartoCss); - assert.response(server, - createRequest(layergroup, user), - { - status: 200 - }, - function(res) { - var parsed = JSON.parse(res.body); - assert.ok(parsed.layergroupid); - keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - done(); - } - ); - }); - }); - - it("layergroup creation and tile requests works even if they are slow but returns fallback", function(done) { - withRenderLimit(user, 50, function(err) { - if (err) { - return done(err); - } - - var layergroup = singleLayergroupConfig(pointSleepSql, pointCartoCss); - assert.response(server, - createRequest(layergroup, user), - { - status: 200 - }, - function(res) { - keysToDelete['map_cfg|' + LayergroupToken.parse(JSON.parse(res.body).layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - assert.response(server, - { - url: layergroupUrl + _.template('/<%= layergroupId %>/<%= z %>/<%= x %>/<%= y %>.png', { - layergroupId: JSON.parse(res.body).layergroupid, - z: 0, - x: 0, - y: 0 - }), - method: 'GET', - headers: { - host: 'localhost' - }, - encoding: 'binary' - }, - { - status: 200, - headers: { - 'Content-Type': 'image/png' - } - }, - function(res, err) { - if (err) { - done(err); - } - var referenceImagePath = './test/fixtures/render-timeout-fallback.png'; - assert.imageBufferIsSimilarToFile(res.body, referenceImagePath, 25, - function(imgErr/*, similarity*/) { - done(imgErr); - } - ); - } - ); - - } - ); - }); - }); - - }); - -}); diff --git a/yarn.lock b/yarn.lock index 63d9f6fe..ebc2c0d0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -260,7 +260,7 @@ cartodb-query-tables@0.2.0: cartodb-redis@cartodb/node-cartodb-redis#timeout-limits: version "0.13.3" - resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/530ae2736a184a3b9c9e978efa7dcb4faae7abee" + resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/4c0586009dbfd464eb9e742bed39170334f40453" dependencies: dot "~1.0.2" redis-mpool "~0.4.1" From 1bc5c044890cfe2ed169a93f7887275017cfa33b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 2 Aug 2017 13:15:40 +0200 Subject: [PATCH 58/66] Remove unused method --- test/support/test-client.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 6eeb260d..15448f68 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1097,7 +1097,4 @@ TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) ); }; -TestClient.cleanDatabaseConnections = function () { - helper.cleanPGPoolConnections() -} From d27a2810671f45ab0ac1b75ab9f3a17535ad8c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 9 Aug 2017 17:55:39 +0200 Subject: [PATCH 59/66] Upgrade cartodb-redis to 0.14.0 --- package.json | 2 +- yarn.lock | 74 ++++++++++++++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index 093a6948..212689cb 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "camshaft": "0.55.6", "cartodb-psql": "0.8.0", "cartodb-query-tables": "0.2.0", - "cartodb-redis": "cartodb/node-cartodb-redis#timeout-limits", + "cartodb-redis": "0.14.0", "debug": "~2.2.0", "dot": "~1.0.2", "express": "~4.13.3", diff --git a/yarn.lock b/yarn.lock index ebc2c0d0..74f6d935 100644 --- a/yarn.lock +++ b/yarn.lock @@ -223,6 +223,14 @@ carto@0.16.3: semver "^5.1.0" yargs "^4.2.0" +carto@CartoDB/carto#0.15.1-cdb1: + version "0.15.1-cdb1" + resolved "https://codeload.github.com/CartoDB/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" + dependencies: + mapnik-reference "~6.0.2" + optimist "~0.6.0" + underscore "~1.6.0" + carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" @@ -231,14 +239,6 @@ carto@cartodb/carto#0.15.1-cdb3: optimist "~0.6.0" underscore "1.8.3" -"carto@github:cartodb/carto#0.15.1-cdb1": - version "0.15.1-cdb1" - resolved "https://codeload.github.com/cartodb/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" - dependencies: - mapnik-reference "~6.0.2" - optimist "~0.6.0" - underscore "~1.6.0" - cartocolor@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/cartocolor/-/cartocolor-4.0.0.tgz#841a3222d8b5b22718d9d545b1e5b972cb26eb36" @@ -258,9 +258,9 @@ cartodb-query-tables@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/cartodb-query-tables/-/cartodb-query-tables-0.2.0.tgz#b4d672accde04da5b890a5d56a87b761fa7eec44" -cartodb-redis@cartodb/node-cartodb-redis#timeout-limits: - version "0.13.3" - resolved "https://codeload.github.com/cartodb/node-cartodb-redis/tar.gz/4c0586009dbfd464eb9e742bed39170334f40453" +cartodb-redis@0.14.0: + version "0.14.0" + resolved "https://registry.yarnpkg.com/cartodb-redis/-/cartodb-redis-0.14.0.tgz#6f82fdb3e5b7c8005dbaccd6172c1706c4378df2" dependencies: dot "~1.0.2" redis-mpool "~0.4.1" @@ -1741,7 +1741,32 @@ repeat-string@^1.5.2: version "1.6.1" resolved "https://registry.yarnpkg.com/repeat-string/-/repeat-string-1.6.1.tgz#8dcae470e1c88abc2d600fff4a776286da75e637" -request@2.x, request@^2.81.0: +request@2.x, request@^2.55.0, request@^2.69.0, request@~2.79.0: + version "2.79.0" + resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" + dependencies: + aws-sign2 "~0.6.0" + aws4 "^1.2.1" + caseless "~0.11.0" + combined-stream "~1.0.5" + extend "~3.0.0" + forever-agent "~0.6.1" + form-data "~2.1.1" + har-validator "~2.0.6" + hawk "~3.1.3" + http-signature "~1.1.0" + is-typedarray "~1.0.0" + isstream "~0.1.2" + json-stringify-safe "~5.0.1" + mime-types "~2.1.7" + oauth-sign "~0.8.1" + qs "~6.3.0" + stringstream "~0.0.4" + tough-cookie "~2.3.0" + tunnel-agent "~0.4.1" + uuid "^3.0.0" + +request@^2.81.0: version "2.81.0" resolved "https://registry.yarnpkg.com/request/-/request-2.81.0.tgz#c6928946a0e06c5f8d6f8a9333469ffda46298a0" dependencies: @@ -1768,31 +1793,6 @@ request@2.x, request@^2.81.0: tunnel-agent "^0.6.0" uuid "^3.0.0" -request@^2.55.0, request@^2.69.0, request@~2.79.0: - version "2.79.0" - resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" - dependencies: - aws-sign2 "~0.6.0" - aws4 "^1.2.1" - caseless "~0.11.0" - combined-stream "~1.0.5" - extend "~3.0.0" - forever-agent "~0.6.1" - form-data "~2.1.1" - har-validator "~2.0.6" - hawk "~3.1.3" - http-signature "~1.1.0" - is-typedarray "~1.0.0" - isstream "~0.1.2" - json-stringify-safe "~5.0.1" - mime-types "~2.1.7" - oauth-sign "~0.8.1" - qs "~6.3.0" - stringstream "~0.0.4" - tough-cookie "~2.3.0" - tunnel-agent "~0.4.1" - uuid "^3.0.0" - require-directory@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42" From ce97844f37e29816255e991548b65610946f3ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 9 Aug 2017 18:42:44 +0200 Subject: [PATCH 60/66] Upgrade windshaft version to 3.3.0 --- package.json | 2 +- yarn.lock | 108 +++++++++++++++++++++++++++------------------------ 2 files changed, 59 insertions(+), 51 deletions(-) diff --git a/package.json b/package.json index 212689cb..fe80d464 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "step-profiler": "~0.3.0", "turbo-carto": "0.19.2", "underscore": "~1.6.0", - "windshaft": "cartodb/windshaft#response-timeout-limit", + "windshaft": "3.3.0", "yargs": "~5.0.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index 74f6d935..e6aa6bce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -abaculus@cartodb/abaculus#2.0.3-cdb1: +"abaculus@github:cartodb/abaculus#2.0.3-cdb1": version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -205,7 +205,7 @@ camshaft@0.55.6: dot "^1.0.3" request "^2.69.0" -canvas@cartodb/node-canvas#1.6.2-cdb2: +"canvas@github:cartodb/node-canvas#1.6.2-cdb2": version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -223,15 +223,15 @@ carto@0.16.3: semver "^5.1.0" yargs "^4.2.0" -carto@CartoDB/carto#0.15.1-cdb1: +"carto@github:cartodb/carto#0.15.1-cdb1": version "0.15.1-cdb1" - resolved "https://codeload.github.com/CartoDB/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" + resolved "https://codeload.github.com/cartodb/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" dependencies: mapnik-reference "~6.0.2" optimist "~0.6.0" underscore "~1.6.0" -carto@cartodb/carto#0.15.1-cdb3: +"carto@github:cartodb/carto#0.15.1-cdb3": version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -254,6 +254,14 @@ cartodb-psql@0.8.0: step "~0.0.6" underscore "~1.6.0" +cartodb-psql@0.9.0: + version "0.9.0" + resolved "https://registry.yarnpkg.com/cartodb-psql/-/cartodb-psql-0.9.0.tgz#887ba006fb499ab789c15d10f34f6afbe1f2f68d" + dependencies: + debug "~2.2.0" + pg cartodb/node-postgres#6.1.2-cdb1 + underscore "~1.6.0" + cartodb-query-tables@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/cartodb-query-tables/-/cartodb-query-tables-0.2.0.tgz#b4d672accde04da5b890a5d56a87b761fa7eec44" @@ -1312,7 +1320,7 @@ mv@~2: ncp "~2.0.0" rimraf "~2.4.0" -nan@^2.0.8, nan@^2.3.4, nan@^2.4.0: +nan@^2.0.8, nan@^2.3.4, nan@^2.4.0, nan@~2.6.2: version "2.6.2" resolved "https://registry.yarnpkg.com/nan/-/nan-2.6.2.tgz#e4ff34e6c95fdfb5aecc08de6596f43605a7db45" @@ -1343,7 +1351,7 @@ nock@~2.11.0: mkdirp "^0.5.0" propagate "0.3.x" -node-pre-gyp@~0.6.27, node-pre-gyp@~0.6.30, node-pre-gyp@~0.6.31: +node-pre-gyp@~0.6.27, node-pre-gyp@~0.6.30, node-pre-gyp@~0.6.36: version "0.6.36" resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.6.36.tgz#db604112cb74e0d477554e9b505b17abddfab786" dependencies: @@ -1520,7 +1528,7 @@ pg-types@1.*: postgres-date "~1.0.0" postgres-interval "~1.0.0" -pg@cartodb/node-postgres#6.1.2-cdb1: +pg@cartodb/node-postgres#6.1.2-cdb1, "pg@github:cartodb/node-postgres#6.1.2-cdb1": version "6.1.2" resolved "https://codeload.github.com/cartodb/node-postgres/tar.gz/3c81aea432ce58d20a795786c58bbb14f68f9689" dependencies: @@ -1741,32 +1749,7 @@ repeat-string@^1.5.2: version "1.6.1" resolved "https://registry.yarnpkg.com/repeat-string/-/repeat-string-1.6.1.tgz#8dcae470e1c88abc2d600fff4a776286da75e637" -request@2.x, request@^2.55.0, request@^2.69.0, request@~2.79.0: - version "2.79.0" - resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" - dependencies: - aws-sign2 "~0.6.0" - aws4 "^1.2.1" - caseless "~0.11.0" - combined-stream "~1.0.5" - extend "~3.0.0" - forever-agent "~0.6.1" - form-data "~2.1.1" - har-validator "~2.0.6" - hawk "~3.1.3" - http-signature "~1.1.0" - is-typedarray "~1.0.0" - isstream "~0.1.2" - json-stringify-safe "~5.0.1" - mime-types "~2.1.7" - oauth-sign "~0.8.1" - qs "~6.3.0" - stringstream "~0.0.4" - tough-cookie "~2.3.0" - tunnel-agent "~0.4.1" - uuid "^3.0.0" - -request@^2.81.0: +request@2.x, request@^2.81.0: version "2.81.0" resolved "https://registry.yarnpkg.com/request/-/request-2.81.0.tgz#c6928946a0e06c5f8d6f8a9333469ffda46298a0" dependencies: @@ -1793,6 +1776,31 @@ request@^2.81.0: tunnel-agent "^0.6.0" uuid "^3.0.0" +request@^2.55.0, request@^2.69.0, request@~2.79.0: + version "2.79.0" + resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" + dependencies: + aws-sign2 "~0.6.0" + aws4 "^1.2.1" + caseless "~0.11.0" + combined-stream "~1.0.5" + extend "~3.0.0" + forever-agent "~0.6.1" + form-data "~2.1.1" + har-validator "~2.0.6" + hawk "~3.1.3" + http-signature "~1.1.0" + is-typedarray "~1.0.0" + isstream "~0.1.2" + json-stringify-safe "~5.0.1" + mime-types "~2.1.7" + oauth-sign "~0.8.1" + qs "~6.3.0" + stringstream "~0.0.4" + tough-cookie "~2.3.0" + tunnel-agent "~0.4.1" + uuid "^3.0.0" + require-directory@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42" @@ -1964,11 +1972,11 @@ sprintf-js@~1.0.2: resolved "https://registry.yarnpkg.com/sprintf-js/-/sprintf-js-1.0.3.tgz#04e6926f662895354f3dd015203633b857297e2c" "sqlite3@2.x || 3.x": - version "3.1.8" - resolved "https://registry.yarnpkg.com/sqlite3/-/sqlite3-3.1.8.tgz#4cbcf965d8b901d1b1015cbc7fc415aae157dfaa" + version "3.1.9" + resolved "https://registry.yarnpkg.com/sqlite3/-/sqlite3-3.1.9.tgz#b2e7fbaa348380318d3834323918c3c351b8bf18" dependencies: - nan "~2.4.0" - node-pre-gyp "~0.6.31" + nan "~2.6.2" + node-pre-gyp "~0.6.36" srs@1.x: version "1.2.0" @@ -2103,17 +2111,17 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -tilelive-bridge@cartodb/tilelive-bridge#query-timeout: - version "2.3.1-cdb2" - resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/38d5d82355aa9eef68c29b7d7a33b60a3ae394f7" +"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb4": + version "2.3.1-cdb4" + resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/faa2b638da2d119b78281575d40255cb523f6ca6" dependencies: mapnik "~3.5.0" mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -tilelive-mapnik@cartodb/tilelive-mapnik#custom-timeout: - version "0.6.18-cdb2" - resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/f9042e5eab92e26fb9e2a6ace65af92cc176a218" +"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb3": + version "0.6.18-cdb3" + resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/23bd1c31dd57d0b76c86b9f1eaf62462b3c17d01" dependencies: generic-pool "~2.4.0" mapnik "3.5.14" @@ -2274,14 +2282,14 @@ window-size@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/window-size/-/window-size-0.2.0.tgz#b4315bb4214a3d7058ebeee892e13fa24d98b075" -windshaft@cartodb/windshaft#response-timeout-limit: - version "3.2.2" - resolved "https://codeload.github.com/cartodb/windshaft/tar.gz/21ebe5f69b5b6215ec2f93088d8eee436f6b81d5" +windshaft@3.3.0: + version "3.3.0" + resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-3.3.0.tgz#bf8ef35f9a2bb126934531b0020f5e8f96b37dfa" dependencies: abaculus cartodb/abaculus#2.0.3-cdb1 canvas cartodb/node-canvas#1.6.2-cdb2 carto cartodb/carto#0.15.1-cdb3 - cartodb-psql "0.8.0" + cartodb-psql "0.9.0" debug "~2.2.0" dot "~1.0.2" grainstore "~1.6.0" @@ -2293,8 +2301,8 @@ windshaft@cartodb/windshaft#response-timeout-limit: sphericalmercator "1.0.4" step "~0.0.6" tilelive "5.12.2" - tilelive-bridge cartodb/tilelive-bridge#query-timeout - tilelive-mapnik cartodb/tilelive-mapnik#custom-timeout + tilelive-bridge cartodb/tilelive-bridge#2.3.1-cdb4 + tilelive-mapnik cartodb/tilelive-mapnik#0.6.18-cdb3 torque.js "~2.11.0" underscore "~1.6.0" From 23edf78a67e5c87753059bd3eeb989c5b063dbfd Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 10 Aug 2017 15:58:25 +0200 Subject: [PATCH 61/66] Remove unnecessary step --- lib/cartodb/api/user_limits_api.js | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/api/user_limits_api.js b/lib/cartodb/api/user_limits_api.js index 74d0cf21..0f4f0f4c 100644 --- a/lib/cartodb/api/user_limits_api.js +++ b/lib/cartodb/api/user_limits_api.js @@ -23,23 +23,19 @@ UserLimitsApi.prototype.getRenderLimits = function (username, apiKey, callback) render: self.options.limits.render || 0 }; - step( - function getTimeoutLimit() { - self.getTimeoutRenderLimit(username, apiKey, function (err, timeoutRenderLimit) { - if (err) { - return callback(err); - } - - if (timeoutRenderLimit && timeoutRenderLimit.render) { - if (Number.isFinite(timeoutRenderLimit.render)) { - limits.render = timeoutRenderLimit.render; - } - } - - return callback(null, limits); - }); + self.getTimeoutRenderLimit(username, apiKey, function (err, timeoutRenderLimit) { + if (err) { + return callback(err); } - ); + + if (timeoutRenderLimit && timeoutRenderLimit.render) { + if (Number.isFinite(timeoutRenderLimit.render)) { + limits.render = timeoutRenderLimit.render; + } + } + + return callback(null, limits); + }); }; UserLimitsApi.prototype.getTimeoutRenderLimit = function (username, apiKey, callback) { From 69eaa72819a0efd0d9c0a8a1e9c0115cb6aca2fc Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 10 Aug 2017 16:06:10 +0200 Subject: [PATCH 62/66] String comparison and regex to match errors instead of indexOf --- lib/cartodb/controllers/base.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 98c5ee2f..f74702e9 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -301,11 +301,11 @@ function statusFromErrorMessage(errMsg) { } function isRenderTimeoutError (err) { - return err.message && (-1 !== err.message.indexOf('Render timed out')); + return err.message === 'Render timed out'; } function isDatasourceTimeoutError (err) { - return err.message && (-1 !== err.message.indexOf('canceling statement due to statement timeout')); + return err.message && err.message.match(/canceling statement due to statement timeout/i); } function isTimeoutError (err) { From e7c206762dda88ea4e3b7322cad70ccca31da6e2 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 10 Aug 2017 16:09:26 +0200 Subject: [PATCH 63/66] String comparison and regex to match errors instead of indexOf --- lib/cartodb/server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 7aea23cd..d9302276 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -120,11 +120,11 @@ module.exports = function(serverOptions) { onTileErrorStrategy = function onTileErrorStrategy$TimeoutTile(err, tile, headers, stats, format, callback) { function isRenderTimeoutError (err) { - return err.message && (-1 !== err.message.indexOf('Render timed out')); + return err.message === 'Render timed out'; } function isDatasourceTimeoutError (err) { - return err.message && (-1 !== err.message.indexOf('canceling statement due to statement timeout')); + return err.message && err.message.match(/canceling statement due to statement timeout/i); } function isTimeoutError (err) { From 14d5ee417881206bb7b73e60cfcdf8aab040e67a Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 10 Aug 2017 16:22:31 +0200 Subject: [PATCH 64/66] Remove user param --- .../acceptance/user-database-timeout-limit.js | 44 +++++++++---------- test/support/test-client.js | 2 +- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index e1ff7bda..03a5e0c5 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -81,11 +81,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + TestClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -118,11 +118,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + TestClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -157,11 +157,11 @@ describe('user database timeout limit', function () { describe('fetching raster tiles', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + TestClient.setUserDatabaseTimeoutLimit(0, done); }); describe('with onTileErrorStrategy ENABLED', function () { @@ -326,11 +326,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + TestClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -390,11 +390,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + TestClient.setUserDatabaseTimeoutLimit(0, done); }); it('"mvt" fails due to statement timeout', function (done) { @@ -428,11 +428,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql, interactivity: 'val' }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + TestClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -492,11 +492,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + TestClient.setUserDatabaseTimeoutLimit(0, done); }); it('"grid.json" fails due to statement timeout', function (done) { @@ -532,11 +532,11 @@ describe('user database timeout limit', function () { cartocss: TestClient.CARTOCSS.TORQUE }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + TestClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -599,11 +599,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + TestClient.setUserDatabaseTimeoutLimit(0, done); }); it('"torque.json" fails due to statement timeout', function (done) { @@ -663,11 +663,11 @@ describe('user database timeout limit', function () { } }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, (err) => { + TestClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -738,11 +738,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 200, done); + TestClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit('localhost', 0, done); + TestClient.setUserDatabaseTimeoutLimit(0, done); }); it('fails due to statement timeout', function (done) { diff --git a/test/support/test-client.js b/test/support/test-client.js index 196e1a8e..3be8bf71 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1164,7 +1164,7 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi helper.configureMetadata('hmset', params, callback); }; -TestClient.setUserDatabaseTimeoutLimit = function (user, timeoutLimit, callback) { +TestClient.setUserDatabaseTimeoutLimit = function (timeoutLimit, callback) { const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; const dbuser = _.template(global.environment.postgres_auth_user, { user_id: 1 }) const pass = _.template(global.environment.postgres_auth_pass, { user_id: 1 }) From 95244334374d0fe31b7b2927d740294988926d1d Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 10 Aug 2017 16:24:40 +0200 Subject: [PATCH 65/66] Use instance server --- test/support/test-client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 3be8bf71..1f8344df 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -487,7 +487,7 @@ TestClient.prototype.getFeatureAttributes = function(featureId, layerId, params, step( function createLayergroup() { var next = this; - assert.response(server, + assert.response(self.server, { url: url, method: 'POST', @@ -526,7 +526,7 @@ TestClient.prototype.getFeatureAttributes = function(featureId, layerId, params, url = '/api/v1/map/' + layergroupId + '/' + layerId + '/attributes/' + featureId; - assert.response(server, + assert.response(self.server, { url: url, method: 'GET', From 01a22a45bb3973e86dba433773d2507286a958d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 10 Aug 2017 17:09:05 +0200 Subject: [PATCH 66/66] Move `setUserDatabaseTimeoutLimit` from class method to a instance method --- .../acceptance/user-database-timeout-limit.js | 88 +++++++++++-------- test/support/test-client.js | 2 +- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 03a5e0c5..42c468fd 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -81,11 +81,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, (err) => { + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -118,11 +118,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, (err) => { + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -156,14 +156,6 @@ describe('user database timeout limit', function () { describe('fetching raster tiles', function () { describe('with user\'s timeout of 200 ms', function () { - beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(200, done); - }); - - afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, done); - }); - describe('with onTileErrorStrategy ENABLED', function () { let onTileErrorStrategy; @@ -180,21 +172,31 @@ describe('user database timeout limit', function () { } }; - this.testClient.getLayergroup(expectedResponse, (err, res) => { + this.testClient.setUserDatabaseTimeoutLimit(200, (err) => { if (err) { return done(err); } - this.layergroupid = res.layergroupid; + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } - done(); + this.layergroupid = res.layergroupid; + + done(); + }); }); }); afterEach(function (done) { global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; - - this.testClient.drain(done); + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { + if (err) { + return done(err); + } + this.testClient.drain(done); + }); }); it('"png" fails due to statement timeout', function (done) { @@ -252,21 +254,33 @@ describe('user database timeout limit', function () { } }; - this.testClient.getLayergroup(expectedResponse, (err, res) => { + this.testClient.setUserDatabaseTimeoutLimit(200, (err) => { if (err) { return done(err); } - this.layergroupid = res.layergroupid; + this.testClient.getLayergroup(expectedResponse, (err, res) => { + if (err) { + return done(err); + } - done(); + this.layergroupid = res.layergroupid; + + done(); + }); }); }); afterEach(function (done) { global.environment.enabledFeatures.onTileErrorStrategy = onTileErrorStrategy; - this.testClient.drain(done); + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { + if (err) { + return done(err); + } + + this.testClient.drain(done); + }); }); it('"png" fails due to statement timeout', function (done) { @@ -326,11 +340,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, (err) => { + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -390,11 +404,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, done); + this.testClient.setUserDatabaseTimeoutLimit(0, done); }); it('"mvt" fails due to statement timeout', function (done) { @@ -428,11 +442,11 @@ describe('user database timeout limit', function () { beforeEach(function (done) { const mapconfig = createMapConfig({ sql: validationPointSleepSql, interactivity: 'val' }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, (err) => { + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -492,11 +506,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, done); + this.testClient.setUserDatabaseTimeoutLimit(0, done); }); it('"grid.json" fails due to statement timeout', function (done) { @@ -532,11 +546,11 @@ describe('user database timeout limit', function () { cartocss: TestClient.CARTOCSS.TORQUE }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, (err) => { + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -599,11 +613,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, done); + this.testClient.setUserDatabaseTimeoutLimit(0, done); }); it('"torque.json" fails due to statement timeout', function (done) { @@ -663,11 +677,11 @@ describe('user database timeout limit', function () { } }); this.testClient = new TestClient(mapconfig, 1234); - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, (err) => { + this.testClient.setUserDatabaseTimeoutLimit(0, (err) => { if (err) { return done(err); } @@ -738,11 +752,11 @@ describe('user database timeout limit', function () { describe('with user\'s timeout of 200 ms', function () { beforeEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(200, done); + this.testClient.setUserDatabaseTimeoutLimit(200, done); }); afterEach(function (done) { - TestClient.setUserDatabaseTimeoutLimit(0, done); + this.testClient.setUserDatabaseTimeoutLimit(0, done); }); it('fails due to statement timeout', function (done) { diff --git a/test/support/test-client.js b/test/support/test-client.js index 1f8344df..21918665 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1164,7 +1164,7 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi helper.configureMetadata('hmset', params, callback); }; -TestClient.setUserDatabaseTimeoutLimit = function (timeoutLimit, callback) { +TestClient.prototype.setUserDatabaseTimeoutLimit = function (timeoutLimit, callback) { const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; const dbuser = _.template(global.environment.postgres_auth_user, { user_id: 1 }) const pass = _.template(global.environment.postgres_auth_pass, { user_id: 1 })