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

Merge branch 'master' into fix-rate-limit-endpoint-several-limits

Browse Source
This commit is contained in:
Simon Martín
2018-07-06 11:06:23 +02:00
parent df71d93dd9 0829d5bc7d
commit 9530f17194
9 changed files with 87 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
NEWS.md
+4
View File
@@ -45,8 +45,12 @@ Bug Fixes:
- Static maps fails for unsupported formats
- Handling errors extracting the column type on dataviews
- Fix `meta.stats.estimatedFeatureCount` for aggregations and queries with tokens
- Fix numeric histogram bounds when `start` and `end` are specified (#991)
- Static maps filters correctly if `layer` option is passed in the url.
Announcements:
* Improve error message when the DB query is over the user's limits
## 6.1.0
Released 2018-04-16
lib/cartodb/api/middlewares/error-middleware.js
+23 -10
View File
@@ -46,26 +46,39 @@ function isDatasourceTimeoutError (err) {
return err.message && err.message.match(/canceling statement due to statement timeout/i);
}
function isTimeoutError (err) {
return isRenderTimeoutError(err) || isDatasourceTimeoutError(err);
function isTimeoutError (errorTypes) {
return errorTypes.renderTimeoutError || errorTypes.datasourceTimeoutError;
}
function getErrorTypes(error) {
return {
renderTimeoutError: isRenderTimeoutError(error),
datasourceTimeoutError: isDatasourceTimeoutError(error),
};
}
function populateTimeoutErrors (errors) {
return errors.map(function (error) {
if (isRenderTimeoutError(error)) {
error.subtype = 'render';
}
const errorTypes = getErrorTypes(error);
if (isDatasourceTimeoutError(error)) {
error.subtype = 'datasource';
}
if (isTimeoutError(error)) {
if (isTimeoutError(errorTypes)) {
error.message = 'You are over platform\'s limits. Please contact us to know more details';
error.type = 'limit';
error.http_status = 429;
}
if (errorTypes.datasourceTimeoutError) {
error.subtype = 'datasource';
error.message = 'You are over platform\'s limits: SQL query timeout error.' +
' Refactor your query before running again or contact CARTO support for more details.';
}
if (errorTypes.renderTimeoutError) {
error.subtype = 'render';
error.message = 'You are over platform\'s limits: Render timeout error.' +
' Contact CARTO support for more details.';
}
return error;
});
}
lib/cartodb/api/middlewares/rate-limit.js
+2 -1
View File
@@ -47,7 +47,8 @@ function rateLimit(userLimitsBackend, endpointGroup = null) {
res.set('Retry-After', retry + 1);
let rateLimitError = new Error(
'You are over platform\'s limits. Please contact us to know more details'
'You are over platform\'s limits: too many requests.' +
' Please contact us to know more details'
);
rateLimitError.http_status = 429;
rateLimitError.type = 'limit';
lib/cartodb/models/dataview/histograms/numeric-histogram.js
+8 -2
View File
@@ -99,8 +99,14 @@ module.exports = class NumericHistogram extends BaseHistogram {
var extra_tables = ``;
var extra_queries = ``;
var extra_groupby = ``;
var extra_filter = ``;
if (ctx.start >= ctx.end) {
if (ctx.start < ctx.end) {
extra_filter = `
WHERE __ctx_query.${ctx.column} >= ${ctx.start}
AND __ctx_query.${ctx.column} <= ${ctx.end}
`;
} else {
ctx.end = `__cdb_basics.__cdb_max_val`;
ctx.start = `__cdb_basics.__cdb_min_val`;
extra_groupby = `, __cdb_basics.__cdb_max_val, __cdb_basics.__cdb_min_val`;
@@ -135,7 +141,7 @@ SELECT
END AS bin
FROM
(
${ctx.query}
SELECT * FROM (${ctx.query}) __ctx_query${extra_tables} ${extra_filter}
) __cdb_filtered_source_query${extra_tables}
GROUP BY 10${extra_groupby}
ORDER BY 10;`;
run_tests.sh
+1 -1
View File
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash
OPT_CREATE_REDIS=yes # create the redis test environment
OPT_CREATE_PGSQL=yes # create the PostgreSQL test environment
test/acceptance/dataviews/histogram.js
+21
View File
@@ -67,6 +67,27 @@ describe('histogram-dataview', function() {
]
);
it('should get bins with min >= start and max <= end', function(done) {
var params = {
bins: 3,
start: 50,
end: 500
};
this.testClient = new TestClient(mapConfig, 1234);
this.testClient.getDataview('pop_max_histogram', params, function(err, dataview) {
assert.ok(!err, err);
assert.ok(3 === dataview.bins_count, 'Unexpected bin count: ' + dataview.bins_count);
assert.ok(3 === dataview.bins.length, 'Unexpected number of bins: ' + dataview.bins.length);
dataview.bins.forEach(function(bin) {
assert.ok(bin.min >= params.start, 'bin min < start: ' + JSON.stringify(bin));
assert.ok(bin.max <= params.end, 'bin max > end: ' + JSON.stringify(bin));
});
done();
});
});
it('should get bin_width right when max > min in filter', function(done) {
var params = {
bins: 10,
test/acceptance/rate-limit.test.js
+2 -1
View File
@@ -144,7 +144,8 @@ function assertRateLimitRequest (status, limit, remaining, reset, retry, done) {
assert.ifError(err);
} else {
assert.ok(err);
assert.equal(err.message, 'You are over platform\'s limits. Please contact us to know more details');
assert.equal(err.message, 'You are over platform\'s limits: too many requests.' +
' Please contact us to know more details');
assert.equal(err.http_status, 429);
assert.equal(err.type, 'limit');
assert.equal(err.subtype, 'rate-limit');
test/acceptance/user-database-timeout-limit.js
+15 -12
View File
@@ -68,12 +68,15 @@ const createMapConfig = ({
}
});
const db_limit_error_message = 'You are over platform\'s limits: SQL query timeout error.' +
' Refactor your query before running again or contact CARTO support for more details.';
const DATASOURCE_TIMEOUT_ERROR = {
errors: ['You are over platform\'s limits. Please contact us to know more details'],
errors: [db_limit_error_message],
errors_with_context: [{
type: 'limit',
subtype: 'datasource',
message: 'You are over platform\'s limits. Please contact us to know more details'
message: db_limit_error_message
}]
};
@@ -141,11 +144,11 @@ describe('user database timeout limit', function () {
this.testClient.getLayergroup({ response: expectedResponse }, (err, timeoutError) => {
assert.deepEqual(timeoutError, {
errors: [ 'You are over platform\'s limits. Please contact us to know more details' ],
errors: [db_limit_error_message ],
errors_with_context: [{
type: 'limit',
subtype: 'datasource',
message: 'You are over platform\'s limits. Please contact us to know more details',
message: db_limit_error_message,
layer: { id: 'layer0', index: 0, type: 'mapnik' }
}]
});
@@ -363,11 +366,11 @@ describe('user database timeout limit', function () {
this.testClient.getLayergroup({ response: expectedResponse }, (err, timeoutError) => {
assert.deepEqual(timeoutError, {
errors: [ 'You are over platform\'s limits. Please contact us to know more details' ],
errors: [ db_limit_error_message ],
errors_with_context: [{
type: 'limit',
subtype: 'datasource',
message: 'You are over platform\'s limits. Please contact us to know more details',
message: db_limit_error_message,
layer: { id: 'layer0', index: 0, type: 'mapnik' }
}]
});
@@ -541,11 +544,11 @@ describe('user database timeout limit', function () {
this.testClient.getLayergroup({ response: expectedResponse }, (err, timeoutError) => {
assert.deepEqual(timeoutError, {
errors: [ 'You are over platform\'s limits. Please contact us to know more details' ],
errors: [ db_limit_error_message ],
errors_with_context: [{
type: 'limit',
subtype: 'datasource',
message: 'You are over platform\'s limits. Please contact us to know more details',
message: db_limit_error_message,
layer: { id: 'layer0', index: 0, type: 'mapnik' }
}]
});
@@ -645,11 +648,11 @@ describe('user database timeout limit', function () {
this.testClient.getLayergroup({ response: expectedResponse }, (err, timeoutError) => {
assert.deepEqual(timeoutError, {
errors: [ 'You are over platform\'s limits. Please contact us to know more details' ],
errors: [db_limit_error_message],
errors_with_context: [{
type: 'limit',
subtype: 'datasource',
message: 'You are over platform\'s limits. Please contact us to know more details',
message: db_limit_error_message,
layer: { id: 'torque-layer0', index: 0, type: 'torque' }
}]
});
@@ -776,11 +779,11 @@ describe('user database timeout limit', function () {
this.testClient.getLayergroup({ response: expectedResponse }, (err, timeoutError) => {
assert.deepEqual(timeoutError, {
errors: [ 'You are over platform\'s limits. Please contact us to know more details' ],
errors: [db_limit_error_message],
errors_with_context: [{
type: 'limit',
subtype: 'datasource',
message: 'You are over platform\'s limits. Please contact us to know more details',
message: db_limit_error_message,
layer: {
id: 'layer0',
index: 0,
test/acceptance/user-render-timeout-limit.js
+11 -8
View File
@@ -23,6 +23,9 @@ const validationPointSleepSql = `
2 val
`;
const renderTimeoutErrorMessage = 'You are over platform\'s limits: Render timeout error.' +
' Contact CARTO support for more details.';
const createMapConfig = ({
version = '1.6.0',
type = 'cartodb',
@@ -96,11 +99,11 @@ describe('user render timeout limit', function () {
assert.ifError(err);
assert.deepEqual(timeoutError, {
errors: ["You are over platform\'s limits. Please contact us to know more details"],
errors: [renderTimeoutErrorMessage],
errors_with_context: [{
type: 'limit',
subtype: 'render',
message: "You are over platform\'s limits. Please contact us to know more details",
message: renderTimeoutErrorMessage,
layer: {
id: "layer0",
index: 0,
@@ -188,11 +191,11 @@ describe('user render timeout limit', function () {
assert.ifError(err);
assert.deepEqual(timeoutError, {
errors: ["You are over platform\'s limits. Please contact us to know more details"],
errors: [renderTimeoutErrorMessage],
errors_with_context: [{
type: 'limit',
subtype: 'render',
message: "You are over platform\'s limits. Please contact us to know more details"
message: renderTimeoutErrorMessage
}]
});
@@ -282,11 +285,11 @@ describe('user render timeout limit', function () {
assert.ifError(err);
assert.deepEqual(tile, {
errors: ['You are over platform\'s limits. Please contact us to know more details'],
errors: [renderTimeoutErrorMessage],
errors_with_context: [{
type: 'limit',
subtype: 'render',
message: 'You are over platform\'s limits. Please contact us to know more details'
message: renderTimeoutErrorMessage
}]
});
@@ -384,11 +387,11 @@ describe('user render timeout limit', function () {
assert.ifError(err);
assert.deepEqual(timeoutError, {
errors: ["You are over platform\'s limits. Please contact us to know more details"],
errors: [renderTimeoutErrorMessage],
errors_with_context: [{
type: 'limit',
subtype: 'render',
message: "You are over platform\'s limits. Please contact us to know more details"
message: renderTimeoutErrorMessage
}]
});