feat: Throttle search routes, allow parallel GET/HEAD

Author: Simon Grondin

lib/index.js

@@ -14,7 +14,13 @@ const groups = {}
 const createGroups = function (Bottleneck, common) {
   groups.global = new Bottleneck.Group({
     id: 'octokit-global',
+    maxConcurrent: 10,
+    ...common
+  })
+  groups.search = new Bottleneck.Group({
+    id: 'octokit-search',
     maxConcurrent: 1,
+    minTime: 2000,
     ...common
   })
   groups.write = new Bottleneck.Group({

lib/wrap-request.js

@@ -8,6 +8,8 @@ function wrapRequest (state, request, options) {
 
 async function doRequest (state, request, options) {
   const isWrite = options.method !== 'GET' && options.method !== 'HEAD'
+  const isSearch = options.method === 'GET' && options.url.startsWith('/search/')
+
   const retryCount = ~~options.request.retryCount
   const jobOptions = retryCount > 0 ? { priority: 0, weight: 0 } : {}
   if (state.clustering) {
@@ -26,5 +28,9 @@ async function doRequest (state, request, options) {
     await state.notifications.key(state.id).schedule(jobOptions, noop)
   }
 
+  if (isSearch) {
+    await state.search.key(state.id).schedule(jobOptions, noop)
+  }
+
   return state.global.key(state.id).schedule(jobOptions, request, options)
 }

test/integration/index.js

@@ -49,7 +49,7 @@ describe('General', function () {
 })
 
 describe('Github API best practices', function () {
-  it('Should not allow more than 1 request concurrently', async function () {
+  it('Should linearize requests', async function () {
     const octokit = new Octokit({ throttle: { onAbuseLimit: () => 1, onRateLimit: () => 1 } })
     const req1 = octokit.request('GET /route1', {
       request: {
@@ -172,6 +172,43 @@ describe('Github API best practices', function () {
     expect(plugin.triggersNotification('/repos/:foo/:bar/issues')).to.equal(true)
   })
 
+  it('Should maintain 2000ms between search requests', async function () {
+    const octokit = new Octokit({
+      throttle: {
+        search: new Bottleneck.Group({ minTime: 50 }),
+        onAbuseLimit: () => 1,
+        onRateLimit: () => 1
+      }
+    })
+
+    const req1 = octokit.request('GET /search/route1', {
+      request: {
+        responses: [{ status: 201, headers: {}, data: {} }]
+      }
+    })
+    const req2 = octokit.request('GET /route2', {
+      request: {
+        responses: [{ status: 202, headers: {}, data: {} }]
+      }
+    })
+    const req3 = octokit.request('GET /search/route3', {
+      request: {
+        responses: [{ status: 203, headers: {}, data: {} }]
+      }
+    })
+
+    await Promise.all([req1, req2, req3])
+    expect(octokit.__requestLog).to.deep.equal([
+      'START GET /route2',
+      'END GET /route2',
+      'START GET /search/route1',
+      'END GET /search/route1',
+      'START GET /search/route3',
+      'END GET /search/route3'
+    ])
+    expect(octokit.__requestTimings[4] - octokit.__requestTimings[2]).to.be.closeTo(50, 20)
+  })
+
   it('Should optimize throughput rather than maintain ordering', async function () {
     const octokit = new Octokit({
       throttle: {