|
protected override async updateItem(pullRequestModel: PullRequestModel): Promise<void> { |
|
if (this._updatingPromise) { |
|
Logger.error('Already updating pull request webview', PullRequestOverviewPanel.ID); |
|
return; |
|
} |
|
this._item = pullRequestModel; |
|
|
|
try { |
|
const updatingPromise = Promise.all([ |
|
this._folderRepositoryManager.resolvePullRequest( |
|
pullRequestModel.remote.owner, |
|
pullRequestModel.remote.repositoryName, |
|
pullRequestModel.number, |
|
), |
|
pullRequestModel.getTimelineEvents(), |
|
this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel), |
|
pullRequestModel.getStatusChecks(), |
|
pullRequestModel.getReviewRequests(), |
|
this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel), |
|
this._folderRepositoryManager.getBranchNameForPullRequest(pullRequestModel), |
|
this._folderRepositoryManager.getCurrentUser(pullRequestModel.githubRepository), |
|
pullRequestModel.canEdit(), |
|
this._folderRepositoryManager.getOrgTeamsCount(pullRequestModel.githubRepository), |
|
this._folderRepositoryManager.mergeQueueMethodForBranch(pullRequestModel.base.ref, pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName), |
|
this._folderRepositoryManager.isHeadUpToDateWithBase(pullRequestModel), |
|
pullRequestModel.getMergeability(), |
|
this._folderRepositoryManager.getPreferredEmail(pullRequestModel), |
|
pullRequestModel.getCoAuthors(), |
|
pullRequestModel.validateDraftMode(), |
|
this._folderRepositoryManager.getAssignableUsers() |
|
]); |
|
const clearingPromise = updatingPromise.finally(() => { |
|
if (this._updatingPromise === clearingPromise) { |
|
this._updatingPromise = undefined; |
|
} |
|
}); |
|
this._updatingPromise = clearingPromise; |
|
|
|
const [ |
|
pullRequest, |
|
timelineEvents, |
|
defaultBranch, |
|
status, |
|
requestedReviewers, |
|
repositoryAccess, |
|
branchInfo, |
|
currentUser, |
|
viewerCanEdit, |
|
orgTeamsCount, |
|
mergeQueueMethod, |
|
isBranchUpToDateWithBase, |
|
mergeability, |
|
emailForCommit, |
|
coAuthors, |
|
hasReviewDraft, |
|
assignableUsers |
|
] = await updatingPromise; |
|
|
|
if (!pullRequest) { |
|
throw new Error( |
|
`Fail to resolve Pull Request #${pullRequestModel.number} in ${pullRequestModel.remote.owner}/${pullRequestModel.remote.repositoryName}`, |
|
); |
|
} |
|
|
|
this._item = pullRequest; |
|
this.registerPrListeners(); |
|
this._repositoryDefaultBranch = defaultBranch!; |
|
this._teamsCount = orgTeamsCount; |
|
this._assignableUsers = assignableUsers; |
|
this.setPanelTitle(this.buildPanelTitle(pullRequestModel.number, pullRequestModel.title)); |
|
|
|
const isCurrentlyCheckedOut = pullRequestModel.equals(this._folderRepositoryManager.activePullRequest); |
|
const mergeMethodsAvailability = repositoryAccess!.mergeMethodsAvailability; |
|
|
|
const defaultMergeMethod = getDefaultMergeMethod(mergeMethodsAvailability); |
|
this._existingReviewers = parseReviewers(requestedReviewers!, timelineEvents, pullRequest.author); |
|
|
|
const isUpdateBranchWithGitHubEnabled: boolean = this.isUpdateBranchWithGitHubEnabled(); |
|
const reviewState = this.getCurrentUserReviewState(this._existingReviewers, currentUser); |
|
|
|
Logger.debug('pr.initialize', PullRequestOverviewPanel.ID); |
|
const users = this._assignableUsers[pullRequestModel.remote.remoteName] ?? []; |
|
const copilotUser = users.find(user => COPILOT_ACCOUNTS[user.login]); |
|
const isCopilotAlreadyReviewer = this._existingReviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); |
|
const baseContext = await this.getInitializeContext(currentUser, pullRequest, timelineEvents ?? [], repositoryAccess, viewerCanEdit, users); |
|
|
|
this.preLoadInfoNotRequiredForOverview(pullRequest); |
|
|
|
const postDoneAction = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<string>(POST_DONE, CHECKOUT_DEFAULT_BRANCH); |
|
const doneCheckoutBranch = postDoneAction.startsWith(CHECKOUT_PULL_REQUEST_BASE_BRANCH) |
|
? pullRequest.base.ref |
|
: defaultBranch; |
|
|
|
const context: Partial<PullRequest> = { |
|
...baseContext, |
|
canRequestCopilotReview: copilotUser !== undefined && !isCopilotAlreadyReviewer, |
|
isCurrentlyCheckedOut: isCurrentlyCheckedOut, |
|
isRemoteBaseDeleted: pullRequest.isRemoteBaseDeleted, |
|
base: `${pullRequest.base.owner}/${pullRequest.remote.repositoryName}:${pullRequest.base.ref}`, |
|
isRemoteHeadDeleted: pullRequest.isRemoteHeadDeleted, |
|
isLocalHeadDeleted: !branchInfo, |
|
head: pullRequest.head ? `${pullRequest.head.owner}/${pullRequest.remote.repositoryName}:${pullRequest.head.ref}` : '', |
|
repositoryDefaultBranch: defaultBranch, |
|
doneCheckoutBranch: doneCheckoutBranch, |
|
status: status[0], |
|
reviewRequirement: status[1], |
|
canUpdateBranch: pullRequest.item.viewerCanUpdate && !isBranchUpToDateWithBase && isUpdateBranchWithGitHubEnabled, |
|
mergeable: mergeability.mergeability, |
|
reviewers: this._existingReviewers, |
|
isDraft: pullRequest.isDraft, |
|
mergeMethodsAvailability, |
|
defaultMergeMethod, |
|
hasReviewDraft, |
|
autoMerge: pullRequest.autoMerge, |
|
allowAutoMerge: pullRequest.allowAutoMerge, |
|
autoMergeMethod: pullRequest.autoMergeMethod, |
|
mergeQueueMethod, |
|
mergeQueueEntry: pullRequest.mergeQueueEntry, |
|
mergeCommitMeta: pullRequest.mergeCommitMeta, |
|
squashCommitMeta: pullRequest.squashCommitMeta, |
|
isIssue: false, |
|
emailForCommit, |
|
currentUserReviewState: reviewState, |
|
revertable: pullRequest.state === GithubItemStateEnum.Merged, |
|
isCopilotOnMyBehalf: await isCopilotOnMyBehalf(pullRequest, currentUser, coAuthors), |
|
generateDescriptionTitle: this.getGenerateDescriptionTitle(), |
|
attestationCommitsEnabled: isAttestationCommitsEnabled(), |
|
closingIssues: await (async () => { |
|
const enterpriseUri = pullRequest.remote.isEnterprise ? getEnterpriseUri() : undefined; |
|
const issueOrUrlExpression = getIssueOrURLExpression(enterpriseUri); |
|
return Promise.all((pullRequest.closingIssues ?? []).map(async issue => { |
|
const parsed = parseIssueExpressionOutput(issue.url.match(issueOrUrlExpression)); |
|
const owner = parsed?.owner ?? pullRequest.remote.owner; |
|
const repo = parsed?.name ?? pullRequest.remote.repositoryName; |
|
const webviewUri = await toOpenIssueWebviewUri({ owner, repo, issueNumber: issue.number }); |
|
return { ...issue, url: webviewUri.toString() }; |
|
})); |
|
})(), |
|
}; |
|
this._postMessage({ |
|
command: 'pr.initialize', |
|
pullrequest: context |
|
}); |
|
if (pullRequest.isResolved()) { |
|
this._folderRepositoryManager.checkBranchUpToDate(pullRequest, true); |
|
} |
|
} catch (e) { |
|
vscode.window.showErrorMessage(`Error updating pull request description: ${formatError(e)}`); |
|
} |
|
} |
Summary
A transient network error (e.g. an Undici/Octokit
UND_ERR_CONNECT_TIMEOUTonGET /user) that occurs once during authentication/current-user or repository-metadata initialization becomes a permanent failure until the extension host (or window) is reloaded, even after connectivity is restored. This is because several code paths cache the rejected promise itself as the "result", and every later caller reuses that already-rejected promise instead of retrying.This was observed while opening a PR overview: the underlying failing request was
GET https://api.github.com/user(fromusers.getAuthenticated), which failed once with a nestedUND_ERR_CONNECT_TIMEOUT. After that single failure, connectivity was fine, but every subsequent attempt to open a PR overview kept failing with the generic messageError updating pull request description: ...until the window was reloaded. Manually replacing the cachedcurrentUser/isEmupromises and retrying (without reload) fixed it immediately, confirming the promises — not the network — were the persistent problem.I'm not claiming to know what originally caused the one-off connect timeout (that's environment/network dependent); the bug being reported is that the extension has no recovery path once such a transient rejection is cached.
Root cause 1 —
CredentialStore.setCurrentUsercaches a rejectedgetUserpromise forevervscode-pull-request-github/src/github/credentials.ts
Lines 569 to 582 in 849821a
setCurrentUseris called exactly once, fromcreateHub, when aGitHubhub is created:vscode-pull-request-github/src/github/credentials.ts
Lines 653 to 658 in 849821a
github.currentUserandgithub.isEmuare derived (.then(...)) from the singlegetUserpromise, so if the one underlyingGET /userrequest rejects (any reason — here a transientUND_ERR_CONNECT_TIMEOUT), bothgithub.currentUserandgithub.isEmubecome permanently rejected promises for the lifetime of thatGitHubhub instance.CredentialStore.getCurrentUser/getIsEmusimply return the same cached (possibly rejected) promises every time, with no re-fetch or retry:vscode-pull-request-github/src/github/credentials.ts
Lines 558 to 567 in 849821a
FolderRepositoryManager.getCurrentUserdelegates straight toCredentialStore.getCurrentUser, so every later consumer (PR overview, issue overview, comment controllers, assignment quick-picks, etc.) keeps awaiting and re-rejecting on the same stale promise:https://github.com/microsoft/vscode-pull-request-github/blob/849821a34981608ca9705439e458ffe5527fe480/src/github/folderRepositoryManager.ts (see
getCurrentUser)Root cause 2 —
GitHubRepository.getMetadatahas the same "cache the rejection" patternvscode-pull-request-github/src/github/githubRepository.ts
Lines 431 to 449 in 849821a
this._metadata(declared atvscode-pull-request-github/src/github/githubRepository.ts
Line 184 in 849821a
if (this._metadata)guard is a truthiness check on the field, not on whether it resolved. If the underlyingrepos.getcall rejects once (same class of transient network error),this._metadatapermanently holds a rejected promise and every futuregetMetadata()call re-awaits (and rethrows from) that same rejection — there is no retry and no way to clear the field.This directly breaks the two main consumers, which each call
getMetadata()and fail every time afterwards:getDefaultBranch()—vscode-pull-request-github/src/github/githubRepository.ts
Lines 508 to 523 in 849821a
getRepoAccessAndMergeMethods()—vscode-pull-request-github/src/github/githubRepository.ts
Lines 556 to 580 in 849821a
Both catch the error locally (falling back to
'master', or logging a warning), but neither resetsthis._metadata, so the cached rejection keeps being reused on every subsequent call for the lifetime of theGitHubRepositoryinstance.Compounding issue —
PullRequestOverview.updateItemmasks which dependency failedvscode-pull-request-github/src/github/pullRequestOverview.ts
Lines 338 to 487 in 849821a
updateItemawaits a singlePromise.all([...17 entries...])(lines 346–368), includingthis._folderRepositoryManager.getCurrentUser(pullRequestModel.githubRepository)(line 358) andthis._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel)/getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel)(lines 353, 356, which internally callgetMetadata()), all wrapped in onetry { ... } catch (e) { vscode.window.showErrorMessage(...) }block:This generic message (line 485) is shown regardless of which of the 17 concurrent operations actually failed, so the surfaced error looks like a PR-description-specific problem when the true cause is the unrelated, permanently-cached
currentUser(or_metadata) rejection from root causes 1/2.Verified repro
GET https://api.github.com/userduring hub creation (this call happens once per authenticatedGitHubhub, viaCredentialStore.createHub→setCurrentUser), or onrepos.getduringGitHubRepository.getMetadata(). In our case, the caught error's underlying cause wasUND_ERR_CONNECT_TIMEOUTnested under the Octokit request error forGET /user.updateItemfails every time withError updating pull request description: ..., becausegetCurrentUser/getMetadatakeep returning the same cached rejected promise from step 1.CredentialStore/GitHubRepositoryinstances and thus the cached promises) fixes it — confirming the cache, not the network, is the persistent problem.GitHubhub'scurrentUser/isEmufields with freshly retried promises (without reloading) also immediately fixed subsequent PR overview loads.Suggested fixes
CredentialStore/setCurrentUser: make the current-user fetch single-flight but rejection-safe — e.g. store the in-flight promise, and on rejection clear the stored reference only if it still points at the failing promise (to avoid a race with a newer, concurrently-started fetch), so the next caller triggers a freshgetAuthenticatedrequest instead of re-awaiting the stale rejection. CentralizecurrentUserandisEmuso both are derived from one retryable/refreshable "get current user" operation rather than two independent.then()chains hanging off a single one-shot promise.GitHubRepository.getMetadata/_metadata: same pattern — only treat_metadataas a valid cache once it has resolved; on rejection, clear_metadata(guarding against clearing a newer in-flight fetch) so the nextgetDefaultBranch()/getRepoAccessAndMergeMethods()(or any other caller) triggers a real retry instead of reusing the cached failure.PullRequestOverview.updateItem: narrow or tag the error surfaced from the combinedPromise.all(e.g. wrap each entry with context, or reporte?.message/the failing operation name) so failures unrelated to the PR description (e.g. current-user or repo-metadata fetch failures) aren't reported asError updating pull request description, which currently makes the actual root cause hard to diagnose from the visible error alone.Environment
mainbranch source, commit849821a34981608ca9705439e458ffe5527fe480.GET https://api.github.com/usercall made byusers.getAuthenticated.