fix: do not overwrite options.throttle passed to `{Octokit: ProbotO…

…ctokit.defaults(options)} (#1373)
probot · Oct 4, 2020 · 9483546 · 9483546
1 parent f537204
commit 9483546
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 30 deletions.
diff --git a/src/octokit/get-probot-octokit-with-defaults.ts b/src/octokit/get-probot-octokit-with-defaults.ts
@@ -39,37 +39,41 @@ export function getProbotOctokitWithDefaults(options: Options) {
         },
         authStrategy: createAppAuth,
       };
-  const defaultOptions = {
+
+  const octokitThrottleOptions = getOctokitThrottleOptions({
+    log: options.log,
+    redisConfig: options.redisConfig,
+  });
+
+  const defaultOptions: any = {
     baseUrl:
       process.env.GHE_HOST &&
       `${process.env.GHE_PROTOCOL || "https"}://${process.env.GHE_HOST}/api/v3`,
-    throttle: Object.assign(
-      {},
-      options.throttleOptions,
-      getOctokitThrottleOptions({
-        log: options.log,
-        redisConfig: options.redisConfig,
-      })
-    ),
     ...authOptions,
   };
 
-  return options.Octokit.defaults((instanceOptions: any) => {
-    const options = Object.assign(
+  if (options.throttleOptions || octokitThrottleOptions) {
+    defaultOptions.throttle = Object.assign(
       {},
-      defaultOptions,
-      instanceOptions,
-      {
-        auth: instanceOptions.auth
-          ? Object.assign({}, defaultOptions.auth, instanceOptions.auth)
-          : defaultOptions.auth,
-      },
-      {
-        throttle: instanceOptions.throttle
-          ? Object.assign({}, defaultOptions.throttle, instanceOptions.throttle)
-          : defaultOptions.throttle,
-      }
+      options.throttleOptions,
+      octokitThrottleOptions
     );
+  }
+
+  return options.Octokit.defaults((instanceOptions: any) => {
+    const options = Object.assign({}, defaultOptions, instanceOptions, {
+      auth: instanceOptions.auth
+        ? Object.assign({}, defaultOptions.auth, instanceOptions.auth)
+        : defaultOptions.auth,
+    });
+
+    if (instanceOptions.throttle) {
+      options.throttle = Object.assign(
+        {},
+        defaultOptions.throttle,
+        instanceOptions.throttle
+      );
+    }
 
     return options;
   });

diff --git a/test/index.test.ts b/test/index.test.ts
@@ -4,8 +4,7 @@ import { NextFunction, Request, Response } from "express";
 import request = require("supertest");
 import nock from "nock";
 
-import { Application, Probot } from "../src";
-import { ProbotOctokit } from "../src/octokit/probot-octokit";
+import { Application, Probot, ProbotOctokit } from "../src";
 
 import path = require("path");
 
@@ -40,12 +39,29 @@ describe("Probot", () => {
     };
   });
 
-  it("constructor", () => {
-    // probot with token. Should not throw
-    new Probot({ githubToken: "faketoken" });
+  describe("constructor", () => {
+    it('{ githubToken: "faketoken" }', () => {
+      // probot with token. Should not throw
+      new Probot({ githubToken: "faketoken" });
+    });
+    it('{ id, privateKey" }', () => {
+      // probot with id/privateKey
+      new Probot({ id, privateKey });
+    });
+
+    it.only("shouldn't overwrite `options.throttle` passed to `{Octokit: ProbotOctokit.defaults(optiosn)}`", () => {
+      expect.assertions(1);
 
-    // probot with id/privateKey
-    new Probot({ id, privateKey });
+      const MyOctokit = ProbotOctokit.plugin((octokit, options) => {
+        expect(options.throttle.enabled).toEqual(false);
+      }).defaults({
+        throttle: {
+          enabled: false,
+        },
+      });
+
+      new Probot({ Octokit: MyOctokit });
+    });
   });
 
   describe("run", () => {