Skip to content

Commit 9c60049

Browse files
authored
Try to recover from compareCommits timeout from GitHub (#6654)
Part of #6513
1 parent bb5bddc commit 9c60049

File tree

3 files changed

+53
-28
lines changed

3 files changed

+53
-28
lines changed

src/github/loggingOctokit.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@ import { Octokit } from '@octokit/rest';
77
import { ApolloClient, ApolloQueryResult, FetchResult, MutationOptions, NormalizedCacheObject, OperationVariables, QueryOptions } from 'apollo-boost';
88
import { bulkhead, BulkheadPolicy } from 'cockatiel';
99
import * as vscode from 'vscode';
10+
import { GitHubRef } from '../common/githubRef';
1011
import Logger from '../common/logger';
12+
import { GitHubRemote } from '../common/remote';
1113
import { ITelemetry } from '../common/telemetry';
1214
import { RateLimit } from './graphql';
15+
import { IRawFileChange } from './interface';
16+
import { restPaginate } from './utils';
1317

1418
interface RestResponse {
1519
headers: {
@@ -139,3 +143,47 @@ export class LoggingOctokit {
139143
return result;
140144
}
141145
}
146+
147+
export async function compareCommits(remote: GitHubRemote, octokit: LoggingOctokit, base: GitHubRef, head: GitHubRef, compareWithBaseRef: string, prNumber: number, logId: string): Promise<{ mergeBaseSha: string; files: IRawFileChange[] }> {
148+
Logger.debug(`Comparing commits for ${remote.owner}/${remote.repositoryName} with base ${base.repositoryCloneUrl.owner}:${compareWithBaseRef} and head ${head.repositoryCloneUrl.owner}:${head.sha}`, logId);
149+
let files: IRawFileChange[] | undefined;
150+
let mergeBaseSha: string | undefined;
151+
152+
const listFiles = async (perPage?: number) => {
153+
return restPaginate<typeof octokit.api.pulls.listFiles, IRawFileChange>(octokit.api.pulls.listFiles, {
154+
owner: base.repositoryCloneUrl.owner,
155+
pull_number: prNumber,
156+
repo: remote.repositoryName,
157+
}, perPage);
158+
};
159+
160+
try {
161+
const { data } = await octokit.call(octokit.api.repos.compareCommits, {
162+
repo: remote.repositoryName,
163+
owner: remote.owner,
164+
base: `${base.repositoryCloneUrl.owner}:${compareWithBaseRef}`,
165+
head: `${head.repositoryCloneUrl.owner}:${head.sha}`,
166+
});
167+
const MAX_FILE_CHANGES_IN_COMPARE_COMMITS = 100;
168+
169+
if (data.files && data.files.length >= MAX_FILE_CHANGES_IN_COMPARE_COMMITS) {
170+
// compareCommits will return a maximum of 100 changed files
171+
// If we have (maybe) more than that, we'll need to fetch them with listFiles API call
172+
Logger.appendLine(`More than ${MAX_FILE_CHANGES_IN_COMPARE_COMMITS} files changed in #${prNumber}`, logId);
173+
files = await listFiles();
174+
} else {
175+
// if we're under the limit, just use the result from compareCommits, don't make additional API calls.
176+
files = data.files ? data.files as IRawFileChange[] : [];
177+
}
178+
mergeBaseSha = data.merge_base_commit.sha;
179+
} catch (e) {
180+
if (e.message === 'Server Error') {
181+
// Happens when github times out. Let's try to get a few at a time.
182+
files = await listFiles(3);
183+
mergeBaseSha = base.sha;
184+
} else {
185+
throw e;
186+
}
187+
}
188+
return { mergeBaseSha, files };
189+
}

src/github/pullRequestModel.ts

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import {
7070
ReviewEvent,
7171
} from './interface';
7272
import { IssueModel } from './issueModel';
73+
import { compareCommits } from './loggingOctokit';
7374
import {
7475
convertRESTPullRequestToRawPullRequest,
7576
convertRESTReviewEvent,
@@ -1451,32 +1452,8 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
14511452
}
14521453

14531454
Logger.debug(`Comparing commits for ${remote.owner}/${remote.repositoryName} with base ${this.base.repositoryCloneUrl.owner}:${compareWithBaseRef} and head ${this.head!.repositoryCloneUrl.owner}:${this.head!.sha}`, PullRequestModel.ID);
1454-
const { data } = await octokit.call(octokit.api.repos.compareCommits, {
1455-
repo: remote.repositoryName,
1456-
owner: remote.owner,
1457-
base: `${this.base.repositoryCloneUrl.owner}:${compareWithBaseRef}`,
1458-
head: `${this.head!.repositoryCloneUrl.owner}:${this.head!.sha}`,
1459-
});
1460-
1461-
this.mergeBase = data.merge_base_commit.sha;
1462-
1463-
const MAX_FILE_CHANGES_IN_COMPARE_COMMITS = 100;
1464-
let files: IRawFileChange[] = [];
1465-
1466-
if (data.files && data.files.length >= MAX_FILE_CHANGES_IN_COMPARE_COMMITS) {
1467-
// compareCommits will return a maximum of 100 changed files
1468-
// If we have (maybe) more than that, we'll need to fetch them with listFiles API call
1469-
Logger.appendLine(
1470-
`More than ${MAX_FILE_CHANGES_IN_COMPARE_COMMITS} files changed in #${this.number}`, PullRequestModel.ID);
1471-
files = await restPaginate<typeof octokit.api.pulls.listFiles, IRawFileChange>(octokit.api.pulls.listFiles, {
1472-
owner: this.base.repositoryCloneUrl.owner,
1473-
pull_number: this.number,
1474-
repo: remote.repositoryName,
1475-
});
1476-
} else {
1477-
// if we're under the limit, just use the result from compareCommits, don't make additional API calls.
1478-
files = data.files ? data.files as IRawFileChange[] : [];
1479-
}
1455+
const { files, mergeBaseSha } = await compareCommits(remote, octokit, this.base, this.head!, compareWithBaseRef, this.number, PullRequestModel.ID);
1456+
this.mergeBase = mergeBaseSha;
14801457

14811458
if (oldHasChangesSinceReview !== undefined && oldHasChangesSinceReview !== this.hasChangesSinceLastReview && this.hasChangesSinceLastReview && this._showChangesSinceReview) {
14821459
this._onDidChangeChangesSinceReview.fire();

src/github/utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,7 +1135,7 @@ export function getReactionGroup(): { title: string; label: string; icon?: vscod
11351135
return ret;
11361136
}
11371137

1138-
export async function restPaginate<R extends OctokitTypes.RequestInterface, T>(request: R, variables: Parameters<R>[0]): Promise<T[]> {
1138+
export async function restPaginate<R extends OctokitTypes.RequestInterface, T>(request: R, variables: Parameters<R>[0], per_page: number = 100): Promise<T[]> {
11391139
let page = 1;
11401140
let results: T[] = [];
11411141
let hasNextPage = false;
@@ -1144,7 +1144,7 @@ export async function restPaginate<R extends OctokitTypes.RequestInterface, T>(r
11441144
const result = await request(
11451145
{
11461146
...(variables as any),
1147-
per_page: 100,
1147+
per_page,
11481148
page
11491149
}
11501150
);

0 commit comments

Comments
 (0)