Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: throw underlying API error when the manifest fetch commits when determining the latest released version #1571

Merged
merged 4 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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 {
Expand All @@ -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');
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.`
Expand Down Expand Up @@ -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));
4 changes: 3 additions & 1 deletion src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
35 changes: 35 additions & 0 deletions test/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down