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

removing retry after when no necessary

Browse Source
This commit is contained in:
Simon Martín 2018-03-23 10:23:57 +01:00
parent 771eaf97c8
commit fb784d6a91
2 changed files with 45 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/cartodb/middleware/rate-limit.js
View File

@ -39,11 +39,13 @@ function rateLimit(userLimitsApi, endpointGroup = null) {
res.set({ res.set({
'Carto-Rate-Limit-Limit': limit, 'Carto-Rate-Limit-Limit': limit,
'Carto-Rate-Limit-Remaining': remaining, 'Carto-Rate-Limit-Remaining': remaining,
'Retry-After': retry,
'Carto-Rate-Limit-Reset': reset 'Carto-Rate-Limit-Reset': reset
}); });
if (isBlocked) { if (isBlocked) {
// retry is floor rounded in seconds by redis-cell
res.set('Retry-After', retry + 1);
const rateLimitError = new Error('You are over the limits.'); const rateLimitError = new Error('You are over the limits.');
rateLimitError.http_status = 429; rateLimitError.http_status = 429;
rateLimitError.type = 'limit'; rateLimitError.type = 'limit';

70
test/acceptance/rate-limit.test.js
View File

@ -87,8 +87,12 @@ function getReqAndRes() {
req: {}, req: {},
res: { res: {
headers: {}, headers: {},
set(headers) { set(headers, value) {
if(typeof headers === 'object') {
this.headers = headers; this.headers = headers;
} else {
this.headers[headers] = value;
}
}, },
locals: { locals: {
user: 'localhost' user: 'localhost'
@ -97,18 +101,21 @@ function getReqAndRes() {
}; };
} }
function assertGetLayergroupRequest (status, limit, remaining, reset, retry, done = null) { function assertGetLayergroupRequest (status, limit, remaining, reset, retry, done) {
const response = { let response = {
status, status,
headers: { headers: {
'Content-Type': 'application/json; charset=utf-8', 'Content-Type': 'application/json; charset=utf-8',
'Carto-Rate-Limit-Limit': limit, 'Carto-Rate-Limit-Limit': limit,
'Carto-Rate-Limit-Remaining': remaining, 'Carto-Rate-Limit-Remaining': remaining,
'Carto-Rate-Limit-Reset': reset, 'Carto-Rate-Limit-Reset': reset
'Retry-After': retry
} }
}; };
if(retry) {
response.headers['Retry-After'] = retry;
}
testClient.getLayergroup({ response }, err => { testClient.getLayergroup({ response }, err => {
assert.ifError(err); assert.ifError(err);
if (done) { if (done) {
@ -117,15 +124,20 @@ function assertGetLayergroupRequest (status, limit, remaining, reset, retry, don
}); });
} }
function assertRateLimitRequest (status, limit, remaining, reset, retry, done = null) { function assertRateLimitRequest (status, limit, remaining, reset, retry, done) {
const { req, res } = getReqAndRes(); const { req, res } = getReqAndRes();
rateLimit(req, res, function (err) { rateLimit(req, res, function (err) {
assert.deepEqual(res.headers, { let expectedHeaders = {
"Carto-Rate-Limit-Limit": limit, "Carto-Rate-Limit-Limit": limit,
"Carto-Rate-Limit-Remaining": remaining, "Carto-Rate-Limit-Remaining": remaining,
"Carto-Rate-Limit-Reset": reset, "Carto-Rate-Limit-Reset": reset
"Retry-After": retry };
});
if(retry) {
expectedHeaders['Retry-After'] = retry;
}
assert.deepEqual(res.headers, expectedHeaders);
if(status === 200) { if(status === 200) {
assert.ifError(err); assert.ifError(err);
@ -178,7 +190,7 @@ describe('rate limit', function() {
const burst = 1; const burst = 1;
setLimit(count, period, burst); setLimit(count, period, burst);
assertGetLayergroupRequest(200, '2', '1', '1', '-1', done); assertGetLayergroupRequest(200, '2', '1', '1', null, done);
}); });
it("1 req/sec: 2 req/seg should be limited", function(done) { it("1 req/sec: 2 req/seg should be limited", function(done) {
@ -187,12 +199,12 @@ describe('rate limit', function() {
const burst = 1; const burst = 1;
setLimit(count, period, burst); setLimit(count, period, burst);
assertGetLayergroupRequest(200, '2', '1', '1', '-1'); assertGetLayergroupRequest(200, '2', '1', '1');
setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1', '-1'), 250); setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1'), 250);
setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '0'), 500); setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '1'), 500);
setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '0'), 750); setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '1'), 750);
setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '0'), 950); setTimeout( () => assertGetLayergroupRequest(429, '2', '0', '1', '1'), 950);
setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1', '-1', done), 1050); setTimeout( () => assertGetLayergroupRequest(200, '2', '0', '1', null, done), 1050);
}); });
}); });
@ -234,12 +246,12 @@ describe('rate limit middleware', function () {
}); });
it("1 req/sec: 2 req/seg should be limited", function (done) { it("1 req/sec: 2 req/seg should be limited", function (done) {
assertRateLimitRequest(200, 1, 0, 1, -1); assertRateLimitRequest(200, 1, 0, 1);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 250); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 250);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 500); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 500);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 750); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 750);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 950); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 950);
setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, -1, done), 1050); setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, null, done), 1050);
}); });
it("1 req/sec: 2 req/seg should be limited, removing SHA script from Redis", function (done) { it("1 req/sec: 2 req/seg should be limited, removing SHA script from Redis", function (done) {
@ -248,12 +260,12 @@ describe('rate limit middleware', function () {
'SCRIPT', 'SCRIPT',
['FLUSH'], ['FLUSH'],
function () { function () {
assertRateLimitRequest(200, 1, 0, 1, -1); assertRateLimitRequest(200, 1, 0, 1);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 500); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 500);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 500); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 500);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 750); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 750);
setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 0), 950); setTimeout( () => assertRateLimitRequest(429, 1, 0, 0, 1), 950);
setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, -1, done), 1050); setTimeout( () => assertRateLimitRequest(200, 1, 0, 1, null, done), 1050);
} }
); );
}); });