Skip to content

Commit 1d536f3

Browse files
Merge pull request #472 from ryanseys/fix-retry
Fix rate limit requests
2 parents 058c3c1 + 2eb7ec4 commit 1d536f3

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed

lib/common/util.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -507,13 +507,15 @@ function makeAuthorizedRequest(config) {
507507
}
508508

509509
function handleRateLimitResp(err, res, body) {
510-
if (shouldRetry(err) && autoRetry && MAX_RETRIES > attemptedRetries) {
511-
setTimeout(function() {
512-
request(authorizedReqOpts, handleRateLimitResp);
513-
}, getNextRetryWait(attemptedRetries++));
514-
} else {
515-
handleResp(err, res, body, callback);
516-
}
510+
handleResp(err, res, body, function(err, body, resp) {
511+
if (shouldRetry(err) && autoRetry && MAX_RETRIES > attemptedRetries) {
512+
setTimeout(function() {
513+
request(authorizedReqOpts, handleRateLimitResp);
514+
}, getNextRetryWait(attemptedRetries++));
515+
} else {
516+
callback(err, body, resp);
517+
}
518+
});
517519
}
518520

519521
if (callback.onAuthorized) {

package.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@
7777
"docs": "./scripts/docs.sh",
7878
"lint": "jshint lib/ regression/ test/",
7979
"test": "mocha test/*",
80-
"regression-test": "mocha regression/* --timeout 20000",
81-
"cover": "istanbul cover -x 'regression/*' _mocha -- --timeout 20000 test/* regression/*",
82-
"coveralls": "istanbul cover -x 'regression/*' _mocha --report lcovonly -- --timeout 20000 test/* regression/* -R spec && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage"
80+
"regression-test": "mocha regression/* --timeout 30000",
81+
"cover": "istanbul cover -x 'regression/*' _mocha -- --timeout 30000 test/* regression/*",
82+
"coveralls": "istanbul cover -x 'regression/*' _mocha --report lcovonly -- --timeout 30000 test/* regression/* -R spec && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage"
8383
},
8484
"license": "Apache 2"
8585
}

test/common/util.js

+37
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,43 @@ describe('common/util', function() {
536536
});
537537
});
538538

539+
it('should retry rate limits on API errors', function(done) {
540+
var attemptedRetries = 0;
541+
var codes = [429, 503, 500, 'done'];
542+
var error = new Error('Rate Limit Error.');
543+
error.code = codes[0]; // Rate limit error
544+
545+
var authorizedReqOpts = { a: 'b', c: 'd' };
546+
547+
var old_setTimeout = setTimeout;
548+
setTimeout = function(callback, time) {
549+
var MIN_TIME = (Math.pow(2, attemptedRetries) * 1000);
550+
var MAX_TIME = (Math.pow(2, attemptedRetries) * 1000) + 1000;
551+
assert(time >= MIN_TIME && time <= MAX_TIME);
552+
attemptedRetries++;
553+
error.code = codes[attemptedRetries]; // test a new code
554+
callback(); // make the request again
555+
};
556+
557+
gsa_Override = function() {
558+
return function authorize(reqOpts, callback) {
559+
callback(null, authorizedReqOpts);
560+
};
561+
};
562+
563+
request_Override = function(reqOpts, callback) {
564+
callback(null, null, { error: error });
565+
};
566+
567+
var makeRequest = util.makeAuthorizedRequest({});
568+
makeRequest({}, function(err) {
569+
setTimeout = old_setTimeout;
570+
assert.equal(err.message, 'Rate Limit Error.');
571+
assert.equal(err.code, 'done');
572+
done();
573+
});
574+
});
575+
539576
it('should retry rate limits 3x by default', function(done) {
540577
var attemptedRetries = 0;
541578
var error = new Error('Rate Limit Error.');

0 commit comments

Comments
 (0)