fix: throw underlying API error when the manifest fetch commits when …

…determining the latest released version (#1571)

* test: add failing test for Manifest.fromConfig when graphQL fails

* fix: allow callers of graphQL to requireResponse

If set, we will throw the underlying API error if we have received
too many 502 errors.

* fix: retry graphql 5 times with max 20 seconds sleep, always throw

* fix: fix log levels

src/github.ts

@@ -27,6 +27,7 @@ import {
 } from './errors';
 
 const MAX_ISSUE_BODY_SIZE = 65536;
+const MAX_SLEEP_SECONDS = 20;
 export const GH_API_URL = 'https://api.github.com';
 export const GH_GRAPHQL_URL = 'https://api.github.com';
 type OctokitType = InstanceType<typeof Octokit>;
@@ -506,8 +507,11 @@ export class GitHub {
       opts: {
         [key: string]: string | number | null | undefined;
       },
-      maxRetries = 1
+      options?: {
+        maxRetries?: number;
+      }
     ) => {
+      let maxRetries = options?.maxRetries ?? 5;
       let seconds = 1;
       while (maxRetries >= 0) {
         try {
@@ -520,13 +524,17 @@ export class GitHub {
           if ((err as GitHubAPIError).status !== 502) {
             throw err;
           }
-          logger.trace('received 502 error, retrying');
+          if (maxRetries === 0) {
+            logger.warn('ran out of retries and response is required');
+            throw err;
+          }
+          logger.info(`received 502 error, ${maxRetries} attempts remaining`);
         }
         maxRetries -= 1;
         if (maxRetries >= 0) {
           logger.trace(`sleeping ${seconds} seconds`);
           await sleepInMs(1000 * seconds);
-          seconds *= 2;
+          seconds = Math.min(seconds * 2, MAX_SLEEP_SECONDS);
         }
       }
       logger.trace('ran out of retries');
@@ -586,9 +594,8 @@ export class GitHub {
     logger.debug(
       `Fetching ${states} pull requests on branch ${targetBranch} with cursor ${cursor}`
     );
-    const response = await this.graphqlRequest(
-      {
-        query: `query mergedPullRequests($owner: String!, $repo: String!, $num: Int!, $maxFilesChanged: Int, $targetBranch: String!, $states: [PullRequestState!], $cursor: String) {
+    const response = await this.graphqlRequest({
+      query: `query mergedPullRequests($owner: String!, $repo: String!, $num: Int!, $maxFilesChanged: Int, $targetBranch: String!, $states: [PullRequestState!], $cursor: String) {
         repository(owner: $owner, name: $repo) {
           pullRequests(first: $num, after: $cursor, baseRefName: $targetBranch, states: $states, orderBy: {field: CREATED_AT, direction: DESC}) {
             nodes {
@@ -622,16 +629,14 @@ export class GitHub {
           }
         }
       }`,
-        cursor,
-        owner: this.repository.owner,
-        repo: this.repository.repo,
-        num: 25,
-        targetBranch,
-        states,
-        maxFilesChanged: 64,
-      },
-      3
-    );
+      cursor,
+      owner: this.repository.owner,
+      repo: this.repository.repo,
+      num: 25,
+      targetBranch,
+      states,
+      maxFilesChanged: 64,
+    });
     if (!response?.repository?.pullRequests) {
       logger.warn(
         `Could not find merged pull requests for branch ${targetBranch} - it likely does not exist.`
@@ -1355,5 +1360,5 @@ const wrapAsync = <T extends Array<any>, V>(
   };
 };
 
-const sleepInMs = (ms: number) =>
+export const sleepInMs = (ms: number) =>
   new Promise(resolve => setTimeout(resolve, ms));

src/manifest.ts

@@ -1373,7 +1373,9 @@ async function latestReleaseVersion(
   // only look at the last 250 or so commits to find the latest tag - we
   // don't want to scan the entire repository history if this repo has never
   // been released
-  const generator = github.mergeCommitIterator(targetBranch, {maxResults: 250});
+  const generator = github.mergeCommitIterator(targetBranch, {
+    maxResults: 250,
+  });
   for await (const commitWithPullRequest of generator) {
     commitShas.add(commitWithPullRequest.sha);
     const mergedPullRequest = commitWithPullRequest.pullRequest;

test/manifest.ts

@@ -15,6 +15,7 @@
 import {describe, it, beforeEach, afterEach} from 'mocha';
 import {Manifest} from '../src/manifest';
 import {GitHub, ReleaseOptions} from '../src/github';
+import * as githubModule from '../src/github';
 import * as sinon from 'sinon';
 import {
   buildGitHubFileContent,
@@ -46,8 +47,12 @@ import {
   DuplicateReleaseError,
   FileNotFoundError,
   ConfigurationError,
+  GitHubAPIError,
 } from '../src/errors';
 import {RequestError} from '@octokit/request-error';
+import * as nock from 'nock';
+
+nock.disableNetConnect();
 
 const sandbox = sinon.createSandbox();
 const fixturesPath = './test/fixtures';
@@ -1303,6 +1308,36 @@ describe('Manifest', () => {
         '3.3.1'
       );
     });
+
+    it('should fail if graphQL commits API is too slow', async () => {
+      // In this scenario, graphQL fails with a 502 when pulling the list of
+      // recent commits. We are unable to determine the latest release and thus
+      // we should abort with the underlying API error.
+      const scope = nock('https://api.github.com/')
+        .post('/graphql')
+        .times(6) // original + 5 retries
+        .reply(502);
+      const sleepStub = sandbox.stub(githubModule, 'sleepInMs').resolves();
+      await assert.rejects(
+        async () => {
+          await Manifest.fromConfig(github, 'target-branch', {
+            releaseType: 'simple',
+            bumpMinorPreMajor: true,
+            bumpPatchForMinorPreMajor: true,
+            component: 'foobar',
+            includeComponentInTag: false,
+          });
+        },
+        error => {
+          return (
+            error instanceof GitHubAPIError &&
+            (error as GitHubAPIError).status === 502
+          );
+        }
+      );
+      scope.done();
+      sinon.assert.callCount(sleepStub, 5);
+    });
   });
 
   describe('buildPullRequests', () => {