diff --git a/src/github/folderRepositoryManager.ts b/src/github/folderRepositoryManager.ts index abe1c4cc1f..5ce2da6d8e 100644 --- a/src/github/folderRepositoryManager.ts +++ b/src/github/folderRepositoryManager.ts @@ -12,7 +12,7 @@ import { ConflictResolutionCoordinator } from './conflictResolutionCoordinator'; import { Conflict, ConflictResolutionModel } from './conflictResolutionModel'; import { CredentialStore } from './credentials'; import { CopilotWorkingStatus, GitHubRepository, isRateLimitError, ItemsData, PULL_REQUEST_PAGE_SIZE, PullRequestChangeEvent, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository'; -import { PullRequestResponse, PullRequestState } from './graphql'; +import { PullRequestState } from './graphql'; import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, PullRequestMergeability, RepoAccessAndMergeMethods, User } from './interface'; import { IssueModel } from './issueModel'; import { PullRequestGitHelper, PullRequestMetadata } from './pullRequestGitHelper'; @@ -24,7 +24,6 @@ import { getPRFetchQuery, getStateFromQuery, loginComparator, - parseGraphQLPullRequest, teamComparator, variableSubstitution, } from './utils'; @@ -952,7 +951,7 @@ export class FolderRepositoryManager extends Disposable { ); if (githubRepo) { - const pullRequest: PullRequestModel | undefined = await githubRepo.getPullRequest(prNumber); + const pullRequest: PullRequestModel | undefined = await githubRepo.getPullRequest(prNumber, 'FolderRepositoryManager.getLocalPullRequests'); if (pullRequest) { pullRequest.localBranchName = localBranchName; @@ -1307,7 +1306,7 @@ export class FolderRepositoryManager extends Disposable { async getPullRequestsForCategory(githubRepository: GitHubRepository, categoryQuery: string, page?: number): Promise { try { Logger.debug(`Fetch pull request category ${categoryQuery} - enter`, this.id); - const { octokit, query, schema } = await githubRepository.ensure(); + const { octokit } = await githubRepository.ensure(); /* __GDPR__ "pr.search.category" : { @@ -1323,18 +1322,11 @@ export class FolderRepositoryManager extends Disposable { page: page || 1, }); - const promises: Promise<{ data: PullRequestResponse, repo: GitHubRepository } | undefined>[] = data.items.map(async (item) => { + const promises: Promise<{ data: PullRequestModel | undefined, repo: GitHubRepository } | undefined>[] = data.items.map(async (item) => { const protocol = new Protocol(item.repository_url); const prRepo = await this.createGitHubRepositoryFromOwnerName(protocol.owner, protocol.repositoryName); - const { data } = await query({ - query: schema.PullRequest, - variables: { - owner: prRepo.remote.owner, - name: prRepo.remote.repositoryName, - number: item.number - } - }); + const data = await prRepo.getPullRequest(item.number, 'FolderRepositoryManager.getPullRequestsForCategory', false, true); return { data, repo: prRepo }; }); @@ -1343,16 +1335,12 @@ export class FolderRepositoryManager extends Disposable { const pullRequests = (await Promise.all(pullRequestResponses .map(async response => { - if (!response?.data.repository) { + if (!response?.data) { Logger.appendLine('Pull request doesn\'t appear to exist.', this.id); return null; } - // Pull requests fetched with a query can be from any repo. - // We need to use the correct GitHubRepository for this PR. - return response.repo.createOrUpdatePullRequestModel( - await parseGraphQLPullRequest(response.data.repository.pullRequest, response.repo), true - ); + return response.data; }))) .filter(item => item !== null) as PullRequestModel[]; @@ -2364,7 +2352,7 @@ export class FolderRepositoryManager extends Disposable { const githubRepo = await this.resolveItem(owner, repositoryName); Logger.trace(`Found GitHub repo for pr #${pullRequestNumber}: ${githubRepo ? 'yes' : 'no'}`, this.id); if (githubRepo) { - const pr = await githubRepo.getPullRequest(pullRequestNumber, useCache); + const pr = await githubRepo.getPullRequest(pullRequestNumber, 'FolderRepositoryManager.resolvePullRequest', useCache); Logger.trace(`Found GitHub pr repo for pr #${pullRequestNumber}: ${pr ? 'yes' : 'no'}`, this.id); return pr; } @@ -2644,7 +2632,7 @@ export class FolderRepositoryManager extends Disposable { } async fetchById(githubRepo: GitHubRepository, id: number): Promise { - const pullRequest = await githubRepo.getPullRequest(id); + const pullRequest = await githubRepo.getPullRequest(id, 'FolderRepositoryManager.fetchById'); if (pullRequest) { return pullRequest; } else { diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index dbe093e402..6eb66c01aa 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -188,6 +188,8 @@ export interface PullRequestChangeEvent { export class GitHubRepository extends Disposable { static ID = 'GitHubRepository'; + private static _allRepoIds: Set = new Set(); + private static _succeededPullRequests: Map> = new Map(); protected _initialized: boolean = false; protected _hub: GitHub | undefined; protected _metadata: Promise | undefined; @@ -270,6 +272,10 @@ export class GitHubRepository extends Disposable { override dispose() { super.dispose(); + GitHubRepository._allRepoIds.delete(this._id); + for (const repoIds of GitHubRepository._succeededPullRequests.values()) { + repoIds.delete(this._id); + } this.commentsController = undefined; this.commentsHandler = undefined; } @@ -291,6 +297,7 @@ export class GitHubRepository extends Disposable { silent: boolean = false ) { super(); + GitHubRepository._allRepoIds.add(this._id); this._queriesSchema = mergeQuerySchemaWithShared(sharedSchema.default, defaultSchema); // kick off the comments controller early so that the Comments view is visible and doesn't pop up later in an way that's jarring if (!silent) { @@ -1207,7 +1214,7 @@ export class GitHubRepository extends Disposable { } } - async getPullRequest(id: number, useCache: boolean = false): Promise { + async getPullRequest(id: number, callerName: string, useCache: boolean = false, silent: boolean = false): Promise { if (useCache && this._pullRequestModelsByNumber.has(id)) { Logger.debug(`Using cached pull request model for ${id}`, this.id); return this._pullRequestModelsByNumber.get(id)!.model; @@ -1231,11 +1238,41 @@ export class GitHubRepository extends Disposable { } Logger.debug(`Fetch pull request ${id} - done`, this.id); - const pr = this.createOrUpdatePullRequestModel(await parseGraphQLPullRequest(data.repository.pullRequest, this)); + const pr = this.createOrUpdatePullRequestModel(await parseGraphQLPullRequest(data.repository.pullRequest, this), silent); await pr.getLastUpdateTime(new Date(pr.item.updatedAt)); + let repoIds = GitHubRepository._succeededPullRequests.get(id); + if (!repoIds) { + repoIds = new Set(); + GitHubRepository._succeededPullRequests.set(id, repoIds); + } + repoIds.add(this._id); return pr; } catch (e) { Logger.error(`Unable to fetch PR: ${e}`, this.id); + const succeededRepos = GitHubRepository._succeededPullRequests.get(id); + const succeededInOtherRepo = succeededRepos ? succeededRepos.size > 0 && !succeededRepos.has(this._id) : false; + const properties: { succeededInOtherRepo: string; callerName: string; errorCode?: string } = { + succeededInOtherRepo: String(succeededInOtherRepo), + callerName + }; + if (e.status !== undefined) { + properties.errorCode = String(e.status); + } else if (e.graphQLErrors?.[0]?.extensions?.code) { + properties.errorCode = String(e.graphQLErrors[0].extensions.code); + } + /* __GDPR__ + "pr.getPullRequestFailed" : { + "prNumber": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, + "gitHubRepoCount": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, + "succeededInOtherRepo": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, + "errorCode": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }, + "callerName": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" } + } + */ + this.telemetry.sendTelemetryErrorEvent('pr.getPullRequestFailed', properties, { + prNumber: id, + gitHubRepoCount: GitHubRepository._allRepoIds.size + }); return; } } diff --git a/src/github/issueModel.ts b/src/github/issueModel.ts index 7fb5c830b1..ff6b910ac7 100644 --- a/src/github/issueModel.ts +++ b/src/github/issueModel.ts @@ -503,7 +503,7 @@ export class IssueModel extends Disposable { for (const unseenPrs of crossRefs) { // Kick off getting the new PRs so that the system knows about them (and refreshes the tree when they're found) - this.githubRepository.getPullRequest(unseenPrs.source.number); + this.githubRepository.getPullRequest(unseenPrs.source.number, 'IssueModel.getTimelineEvents'); } this.timelineEvents = events; diff --git a/src/github/overviewRestorer.ts b/src/github/overviewRestorer.ts index fdb59e64b7..a6c0f753af 100644 --- a/src/github/overviewRestorer.ts +++ b/src/github/overviewRestorer.ts @@ -62,7 +62,7 @@ export class OverviewRestorer extends Disposable implements vscode.WebviewPanelS } return IssueOverviewPanel.createOrShow(this._telemetry, this._extensionUri, folderManager, identity, issueModel, undefined, true, webviewPanel); } else { - const pullRequestModel = await repo.getPullRequest(state.number, true); + const pullRequestModel = await repo.getPullRequest(state.number, 'OverviewRestorer.deserializeWebviewPanel', true); if (!pullRequestModel) { webviewPanel.dispose(); return; diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index 2a88e65034..74b7be8c69 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -344,11 +344,11 @@ export class PullRequestModel extends IssueModel implements IPullRe let rejectMessage: string | undefined; if (this.isActive) { localHead = repository.state.HEAD?.commit; - remoteHead = (await this.githubRepository.getPullRequest(this.number))?.head?.sha; + remoteHead = (await this.githubRepository.getPullRequest(this.number, 'PullRequestModel.approve'))?.head?.sha; rejectMessage = vscode.l10n.t('The remote head of the pull request branch has changed. Please pull the latest changes from the remote branch before approving.'); } else { localHead = this.head?.sha; - remoteHead = (await this.githubRepository.getPullRequest(this.number))?.head?.sha; + remoteHead = (await this.githubRepository.getPullRequest(this.number, 'PullRequestModel.approve'))?.head?.sha; rejectMessage = vscode.l10n.t('The remote head of the pull request branch has changed. Please refresh the pull request before approving.'); } diff --git a/src/issues/userCompletionProvider.ts b/src/issues/userCompletionProvider.ts index fbd5413cfd..ffc96c9bba 100644 --- a/src/issues/userCompletionProvider.ts +++ b/src/issues/userCompletionProvider.ts @@ -238,7 +238,7 @@ export class UserCompletionProvider implements vscode.CompletionItemProvider { ); if (githubRepo) { - const pr = await githubRepo.getPullRequest(prNumber); + const pr = await githubRepo.getPullRequest(prNumber, 'UserCompletionProvider.provideCompletionItems'); this.cachedForPrNumber = prNumber; this.cachedPrTimelineEvents = await pr!.getTimelineEvents(); } diff --git a/src/lm/tools/activePullRequestTool.ts b/src/lm/tools/activePullRequestTool.ts index 2163755694..78f4911941 100644 --- a/src/lm/tools/activePullRequestTool.ts +++ b/src/lm/tools/activePullRequestTool.ts @@ -55,7 +55,7 @@ export abstract class PullRequestTool implements vscode.LanguageModelTool { if (result === 'Refresh') { - this.pullRequestModel.githubRepository.getPullRequest(this.pullRequestModel.number); + this.pullRequestModel.githubRepository.getPullRequest(this.pullRequestModel.number, 'PullRequestCommentController.replyThread'); } }); } else {