fix: replace p-throttle with bottleneck

Author: pvdlg

lib/get-client.js

@@ -2,7 +2,7 @@ const url = require('url');
 const {memoize} = require('lodash');
 const Octokit = require('@octokit/rest');
 const pRetry = require('p-retry');
-const pThrottle = require('p-throttle');
+const Bottleneck = require('bottleneck');
 
 /**
  * Default exponential backoff configuration for retries.
@@ -16,44 +16,33 @@ const DEFAULT_RETRY = {retries: 3, factor: 2, minTimeout: 1000};
  * See {@link https://developer.github.com/v3/#rate-limiting|Rate limiting}.
  */
 const RATE_LIMITS = {
-  search: [30, 60 * 1000],
-  core: [5000, 60 * 60 * 1000],
+  search: 60 * 1000 / 30, // 30 calls per minutes => 1 call per 2s
+  core: 60 * 60 * 1000 / 5000, // 5000 calls per hour => 1 call per 720ms
 };
 
 /**
  * Global rate limit to prevent abuse.
  *
  * See {@link https://developer.github.com/v3/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits|Dealing with abuse rate limits}
  */
-const GLOBAL_RATE_LIMIT = [1, 1000];
+const GLOBAL_RATE_LIMIT = 1000;
 
 /**
  * Http error codes for which to not retry.
  */
 const SKIP_RETRY_CODES = [400, 401, 403];
 
-/**
- * @typedef {Function} Throttler
- * @param {Function} func The function to throttle.
- * @param {Arguments} args The arguments to pass to the function to throttle.
- */
-
 /**
  * Create or retrieve the throttler function for a given rate limit group.
  *
  * @param {Array} rate The rate limit group.
  * @param {String} limit The rate limits per API endpoints.
- * @return {Throttler} The throller function for the given rate limit group.
- */
-const getThrottler = memoize((rate, limit) => pThrottle((func, ...args) => func(...args), ...limit[rate]));
-
-/**
- * Create the global throttler function to comply with GitHub abuse prevention recommandations.
- *
- * @param {Array} globalLimit The global rate limit.
- * @return {Throttler} The throller function for the global rate limit.
+ * @param {Bottleneck} globalThrottler The global throttler.
+ * @return {Bottleneck} The throller function for the given rate limit group.
  */
-const getGlobalThrottler = globalLimit => pThrottle((func, ...args) => func(...args), ...globalLimit);
+const getThrottler = memoize((rate, limit, globalThrottler) =>
+  new Bottleneck({minTime: limit[rate]}).chain(globalThrottler)
+);
 
 /**
  * Create a`handler` for a `Proxy` wrapping an Octokit instance to:
@@ -89,11 +78,11 @@ const handler = (retry, limit, globalThrottler, endpoint) => ({
    * @return {Promise<Any>} The result of the function called.
    */
   apply: (func, that, args) => {
-    const throttler = getThrottler(limit[endpoint] ? endpoint : 'core', limit);
+    const throttler = getThrottler(limit[endpoint] ? endpoint : 'core', limit, globalThrottler);
 
     return pRetry(async () => {
       try {
-        return await globalThrottler((func, ...args) => throttler(func, ...args), func, ...args);
+        return await throttler.wrap(func)(...args);
       } catch (err) {
         if (SKIP_RETRY_CODES.includes(err.code)) {
           throw new pRetry.AbortError(err);
@@ -120,5 +109,5 @@ module.exports = ({
     pathPrefix: githubApiPathPrefix,
   });
   github.authenticate({type: 'token', token: githubToken});
-  return new Proxy(github, handler(retry, limit, getGlobalThrottler(globalLimit)));
+  return new Proxy(github, handler(retry, limit, new Bottleneck({minTime: globalLimit})));
 };

package.json

@@ -19,14 +19,14 @@
     "@octokit/rest": "^14.0.9",
     "@semantic-release/error": "^2.2.0",
     "aggregate-error": "^1.0.0",
+    "bottleneck": "^2.0.1",
     "debug": "^3.1.0",
     "fs-extra": "^5.0.0",
     "globby": "^8.0.0",
     "issue-parser": "^1.0.1",
     "lodash": "^4.17.4",
     "mime": "^2.0.3",
     "p-retry": "^1.0.0",
-    "p-throttle": "^1.1.0",
     "parse-github-url": "^1.0.1",
     "url-join": "^4.0.0"
   },

test/fail.test.js

@@ -12,7 +12,10 @@ import {authenticate} from './helpers/mock-github';
 
 const fail = proxyquire('../lib/fail', {
   './get-client': conf =>
-    getClient({...conf, ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, globalLimit: [99, 1]}}),
+    getClient({
+      ...conf,
+      ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, limit: {search: 1, core: 1}, globalLimit: 1},
+    }),
 });
 
 // Save the current process.env

test/find-sr-issue.test.js

@@ -15,7 +15,8 @@ const githubToken = 'github_token';
 const client = getClient({
   githubToken,
   retry: {retries: 3, factor: 2, minTimeout: 1, maxTimeout: 1},
-  globalLimit: [99, 1],
+  globalLimit: 1,
+  limit: {search: 1, core: 1},
 });
 
 test.beforeEach(t => {

test/get-client.test.js

@@ -27,8 +27,8 @@ test('Use the global throttler for all endpoints', async t => {
   const octokit = {repos: {createRelease}, issues: {createComment}, search: {issues}, authenticate: stub()};
   const rate = 150;
   const github = proxyquire('../lib/get-client', {'@octokit/rest': stub().returns(octokit)})({
-    limit: {search: [99, 1], core: [99, 1]},
-    globalLimit: [1, rate],
+    limit: {search: 1, core: 1},
+    globalLimit: rate,
   });
 
   const a = await github.repos.createRelease();
@@ -58,8 +58,8 @@ test('Use the same throttler for endpoints in the same rate limit group', async
   const searchRate = 300;
   const coreRate = 150;
   const github = proxyquire('../lib/get-client', {'@octokit/rest': stub().returns(octokit)})({
-    limit: {search: [1, searchRate], core: [1, coreRate]},
-    globalLimit: [99, 1],
+    limit: {search: searchRate, core: coreRate},
+    globalLimit: 1,
   });
 
   const a = await github.repos.createRelease();
@@ -93,9 +93,9 @@ test('Use the same throttler when retrying', async t => {
   const octokit = {repos: {createRelease}, authenticate: stub()};
   const coreRate = 200;
   const github = proxyquire('../lib/get-client', {'@octokit/rest': stub().returns(octokit)})({
-    limit: {core: [1, coreRate]},
+    limit: {core: coreRate},
     retry: {retries: 3, factor: 1, minTimeout: 1},
-    globalLimit: [6, 1],
+    globalLimit: 1,
   });
 
   await t.throws(github.repos.createRelease());

test/publish.test.js

@@ -12,7 +12,10 @@ import {authenticate, upload} from './helpers/mock-github';
 
 const publish = proxyquire('../lib/publish', {
   './get-client': conf =>
-    getClient({...conf, ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, globalLimit: [99, 1]}}),
+    getClient({
+      ...conf,
+      ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, limit: {search: 1, core: 1}, globalLimit: 1},
+    }),
 });
 
 // Save the current process.env

test/success.test.js

@@ -12,7 +12,10 @@ import {authenticate} from './helpers/mock-github';
 
 const success = proxyquire('../lib/success', {
   './get-client': conf =>
-    getClient({...conf, ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, globalLimit: [99, 1]}}),
+    getClient({
+      ...conf,
+      ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, limit: {search: 1, core: 1}, globalLimit: 1},
+    }),
 });
 
 // Save the current process.env

test/verify.test.js

@@ -10,7 +10,10 @@ const envBackup = Object.assign({}, process.env);
 
 const verify = proxyquire('../lib/verify', {
   './get-client': conf =>
-    getClient({...conf, ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, globalLimit: [99, 1]}}),
+    getClient({
+      ...conf,
+      ...{retry: {retries: 3, factor: 1, minTimeout: 1, maxTimeout: 1}, limit: {search: 1, core: 1}, globalLimit: 1},
+    }),
 });
 
 test.beforeEach(t => {