Skip to content

Commit b52a2ee

Browse files
alexr00Copilot
alexr00
and
Copilot
authored
Track inaccessible repos, and don't try to use them again (#8691)
* Track inaccessible repos, and don't try to use them again Co-authored-by: Copilot <copilot@github.com> * use get metadata Co-authored-by: Copilot <copilot@github.com> --------- Co-authored-by: Copilot <copilot@github.com>
1 parent b70db01 commit b52a2ee

3 files changed

Lines changed: 52 additions & 15 deletions

File tree

‎src/github/createPRViewProvider.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,11 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
129129

130130
protected abstract getTitleAndDescription(compareBranch: Branch, baseBranch: string): Promise<{ title: string, description: string }>;
131131

132-
protected async getMergeConfiguration(owner: string, name: string, refetch: boolean = false): Promise<RepoAccessAndMergeMethods> {
132+
protected async getMergeConfiguration(owner: string, name: string, refetch: boolean = false): Promise<RepoAccessAndMergeMethods | undefined> {
133133
const repo = await this._folderRepositoryManager.createGitHubRepositoryFromOwnerName(owner, name);
134+
if (!repo) {
135+
return undefined;
136+
}
134137
return repo.getRepoAccessAndMergeMethods(refetch);
135138
}
136139

@@ -212,7 +215,7 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
212215
const defaultBaseBranch = detectedBaseMetadata?.branch ?? this._pullRequestDefaults.base;
213216

214217
let defaultTitleAndDescription: { title: string; description: string };
215-
let mergeConfiguration: RepoAccessAndMergeMethods;
218+
let mergeConfiguration: RepoAccessAndMergeMethods | undefined;
216219
let viewerPermission: ViewerPermission;
217220
let mergeQueueMethodForBranch: MergeMethod | undefined;
218221
let labels: ILabel[];
@@ -255,7 +258,7 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
255258

256259
const defaultCreateOption = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<'lastUsed' | 'create' | 'createDraft' | 'createAutoMerge'>(DEFAULT_CREATE_OPTION, 'lastUsed');
257260
const lastCreateMethod: { autoMerge: boolean, mergeMethod: MergeMethod | undefined, isDraft: boolean } | undefined = this._folderRepositoryManager.context.workspaceState.get<{ autoMerge: boolean, mergeMethod: MergeMethod, isDraft } | undefined>(PREVIOUS_CREATE_METHOD, undefined);
258-
const repoMergeMethod = getDefaultMergeMethod(mergeConfiguration.mergeMethodsAvailability);
261+
const repoMergeMethod = mergeConfiguration ? getDefaultMergeMethod(mergeConfiguration.mergeMethodsAvailability) : 'merge' as MergeMethod;
259262

260263
// default values are for 'create'
261264
let defaultMergeMethod: MergeMethod = repoMergeMethod;
@@ -266,11 +269,11 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
266269
if (defaultCreateOption === 'lastUsed') {
267270
defaultMergeMethod = lastCreateMethod?.mergeMethod ?? repoMergeMethod;
268271
isDraftDefault = !!lastCreateMethod?.isDraft;
269-
autoMergeDefault = mergeConfiguration.viewerCanAutoMerge && !!lastCreateMethod?.autoMerge;
272+
autoMergeDefault = !!mergeConfiguration?.viewerCanAutoMerge && !!lastCreateMethod?.autoMerge;
270273
} else if (defaultCreateOption === 'createDraft') {
271274
isDraftDefault = true;
272275
} else if (defaultCreateOption === 'createAutoMerge') {
273-
autoMergeDefault = mergeConfiguration.viewerCanAutoMerge;
276+
autoMergeDefault = !!mergeConfiguration?.viewerCanAutoMerge;
274277
}
275278
commands.setContext(contexts.CREATE_PR_PERMISSIONS, viewerPermission);
276279

@@ -301,8 +304,8 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
301304
defaultMergeMethod,
302305
baseHasMergeQueue: !!mergeQueueMethodForBranch,
303306
remoteCount: remotes.length,
304-
allowAutoMerge: mergeConfiguration.viewerCanAutoMerge,
305-
mergeMethodsAvailability: mergeConfiguration.mergeMethodsAvailability,
307+
allowAutoMerge: mergeConfiguration?.viewerCanAutoMerge,
308+
mergeMethodsAvailability: mergeConfiguration?.mergeMethodsAvailability,
306309
autoMergeDefault,
307310
createError: '',
308311
labels: this.labels,
@@ -1073,7 +1076,7 @@ Don't forget to commit your template file to the repository so that it can be us
10731076
this.getTitleAndDescription(compareBranch, this.model.baseBranch),
10741077
this._folderRepositoryManager.mergeQueueMethodForBranch(this.model.baseBranch, this.model.baseOwner, this.model.repositoryName)]);
10751078
let autoMergeDefault = false;
1076-
if (mergeConfiguration.viewerCanAutoMerge) {
1079+
if (mergeConfiguration?.viewerCanAutoMerge) {
10771080
const defaultCreateOption = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<'lastUsed' | 'create' | 'createDraft' | 'createAutoMerge'>(DEFAULT_CREATE_OPTION, 'lastUsed');
10781081
const lastCreateMethod: { autoMerge: boolean, mergeMethod: MergeMethod | undefined, isDraft: boolean } | undefined = this._folderRepositoryManager.context.workspaceState.get<{ autoMerge: boolean, mergeMethod: MergeMethod, isDraft } | undefined>(PREVIOUS_CREATE_METHOD, undefined);
10791082
autoMergeDefault = (defaultCreateOption === 'lastUsed' && lastCreateMethod?.autoMerge) || (defaultCreateOption === 'createAutoMerge');
@@ -1083,10 +1086,10 @@ Don't forget to commit your template file to the repository so that it can be us
10831086
baseRemote: result.remote,
10841087
baseBranch: result.branch,
10851088
defaultBaseBranch: defaultBranch,
1086-
defaultMergeMethod: getDefaultMergeMethod(mergeConfiguration.mergeMethodsAvailability),
1087-
allowAutoMerge: mergeConfiguration.viewerCanAutoMerge,
1089+
defaultMergeMethod: mergeConfiguration ? getDefaultMergeMethod(mergeConfiguration.mergeMethodsAvailability) : 'merge' as MergeMethod,
1090+
allowAutoMerge: mergeConfiguration?.viewerCanAutoMerge ?? false,
10881091
baseHasMergeQueue: !!mergeQueueMethodForBranch,
1089-
mergeMethodsAvailability: mergeConfiguration.mergeMethodsAvailability,
1092+
mergeMethodsAvailability: mergeConfiguration?.mergeMethodsAvailability ?? { merge: true, squash: true, rebase: true },
10901093
autoMergeDefault,
10911094
defaultTitle: titleAndDescription.title,
10921095
defaultDescription: titleAndDescription.description

‎src/github/folderRepositoryManager.ts‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ export class FolderRepositoryManager extends Disposable {
227227
readonly onDidDispose: vscode.Event<void> = this._onDidDispose.event;
228228

229229
private _sessionIgnoredRemoteNames: Set<string> = new Set();
230+
private _inaccessibleRepos: Set<string> = new Set();
230231

231232
constructor(
232233
private readonly _id: number,
@@ -1326,6 +1327,9 @@ export class FolderRepositoryManager extends Disposable {
13261327
const protocol = new Protocol(item.repository_url);
13271328

13281329
const prRepo = await this.createGitHubRepositoryFromOwnerName(protocol.owner, protocol.repositoryName);
1330+
if (!prRepo) {
1331+
return undefined;
1332+
}
13291333
const data = await prRepo.getPullRequest(item.number, 'FolderRepositoryManager.getPullRequestsForCategory', false, true);
13301334
return { data, repo: prRepo };
13311335
});
@@ -2379,7 +2383,7 @@ export class FolderRepositoryManager extends Disposable {
23792383
async resolveUser(owner: string, repositoryName: string, login: string): Promise<User | undefined> {
23802384
Logger.debug(`Fetch user ${login}`, this.id);
23812385
const githubRepository = await this.createGitHubRepositoryFromOwnerName(owner, repositoryName);
2382-
return githubRepository.resolveUser(login);
2386+
return githubRepository?.resolveUser(login);
23832387
}
23842388

23852389
async getMatchingPullRequestMetadataForBranch() {
@@ -2558,7 +2562,8 @@ export class FolderRepositoryManager extends Disposable {
25582562
}
25592563
let continueWithMerge = true;
25602564
if (pullRequest.item.mergeable === PullRequestMergeability.Conflict) {
2561-
const githubRepos = await Promise.all([this.createGitHubRepositoryFromOwnerName(pullRequest.head!.owner, pullRequest.head!.repositoryCloneUrl.repositoryName), this.createGitHubRepositoryFromOwnerName(pullRequest.base.owner, pullRequest.base.repositoryCloneUrl.repositoryName)]);
2565+
const githubRepoResults = await Promise.all([this.createGitHubRepositoryFromOwnerName(pullRequest.head!.owner, pullRequest.head!.repositoryCloneUrl.repositoryName), this.createGitHubRepositoryFromOwnerName(pullRequest.base.owner, pullRequest.base.repositoryCloneUrl.repositoryName)]);
2566+
const githubRepos = githubRepoResults.filter((r): r is GitHubRepository => !!r);
25622567
const coordinator = new ConflictResolutionCoordinator(this.telemetry, conflictModel, githubRepos);
25632568
continueWithMerge = await coordinator.enterConflictResolutionAndWaitForExit();
25642569
coordinator.dispose();
@@ -2900,15 +2905,39 @@ export class FolderRepositoryManager extends Disposable {
29002905
});
29012906
}
29022907

2903-
async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string): Promise<GitHubRepository> {
2908+
async createGitHubRepositoryFromOwnerName(owner: string, repositoryName: string): Promise<GitHubRepository | undefined> {
29042909
const existing = this.findExistingGitHubRepository({ owner, repositoryName });
29052910
if (existing) {
29062911
return existing;
29072912
}
2913+
const repoKey = `${owner.toLowerCase()}/${repositoryName.toLowerCase()}`;
2914+
if (this._inaccessibleRepos.has(repoKey)) {
2915+
Logger.debug(`Skipping inaccessible repository: ${owner}/${repositoryName}`, this.id);
2916+
return undefined;
2917+
}
29082918
const gitRemotes = parseRepositoryRemotes(this.repository);
29092919
const gitRemote = gitRemotes.find(r => r.owner === owner && r.repositoryName === repositoryName);
29102920
const uri = gitRemote?.url ?? `https://github.com/${owner}/${repositoryName}`;
2911-
return this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore);
2921+
const repo = await this.createAndAddGitHubRepository(new Remote(gitRemote?.remoteName ?? repositoryName, uri, new Protocol(uri)), this._credentialStore);
2922+
let reason: string;
2923+
try {
2924+
await repo.getMetadata();
2925+
return repo;
2926+
} catch (e) {
2927+
reason = 'error';
2928+
Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible: ${e}`, this.id);
2929+
}
2930+
Logger.appendLine(`Repository ${owner}/${repositoryName} is not accessible.`, this.id);
2931+
this._inaccessibleRepos.add(repoKey);
2932+
this.removeGitHubRepository(repo.remote);
2933+
/* __GDPR__
2934+
"repository.inaccessible" : {
2935+
"hasLocalRemote" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" },
2936+
"reason" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
2937+
}
2938+
*/
2939+
this.telemetry.sendTelemetryEvent('repository.inaccessible', { hasLocalRemote: (!!gitRemote).toString(), reason });
2940+
return undefined;
29122941
}
29132942

29142943
async findUpstreamForItem(item: {

‎src/github/overviewRestorer.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ export class OverviewRestorer extends Disposable implements vscode.WebviewPanelS
5353
repo = await folderManager.createGitHubRepositoryFromOwnerName(state.owner, state.repo);
5454
}
5555

56+
if (!repo || !folderManager) {
57+
webviewPanel.dispose();
58+
return;
59+
}
60+
5661
const identity = { owner: state.owner, repo: state.repo, number: state.number };
5762
if (state.isIssue) {
5863
const issueModel = await repo.getIssue(state.number, true);

0 commit comments

Comments
 (0)