From c9dbcdb3f61df90bdf0db3df7e7cb2c09cc5eca6 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Thu, 16 Jan 2025 15:28:59 +0100 Subject: [PATCH] Require component name in error logs Part of #6513 --- src/commands.ts | 11 ++++++----- src/common/logger.ts | 2 +- src/common/protocol.ts | 3 ++- src/common/uri.ts | 2 +- src/github/createPRViewProvider.ts | 4 ++-- src/github/credentials.ts | 7 ++++--- src/github/folderRepositoryManager.ts | 22 +++++++++++----------- src/github/pullRequestOverview.ts | 2 +- src/issues/issueFeatureRegistrar.ts | 3 ++- src/view/compareChangesTreeDataProvider.ts | 3 ++- src/view/fileChangeModel.ts | 3 ++- src/view/gitContentProvider.ts | 9 +++++---- src/view/reviewCommentController.ts | 6 +++--- src/view/treeNodes/pullRequestNode.ts | 2 +- 14 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 2eece72893..ea0846dd7b 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -149,6 +149,7 @@ export function registerCommands( telemetry: ITelemetry, tree: PullRequestsTreeDataProvider, ) { + const logId = 'RegisterCommands'; context.subscriptions.push( vscode.commands.registerCommand( 'pr.openPullRequestOnGitHub', @@ -290,7 +291,7 @@ export function registerCommands( await vscode.workspace.fs.delete(tempUri); } catch (err) { const moreError = `${err}${err.stderr ? `\n${err.stderr}` : ''}`; - Logger.error(`Applying patch failed: ${moreError}`); + Logger.error(`Applying patch failed: ${moreError}`, logId); vscode.window.showErrorMessage(vscode.l10n.t('Applying patch failed: {0}', formatError(err))); } }), @@ -511,7 +512,7 @@ export function registerCommands( vscode.commands.registerCommand('pr.pick', async (pr: PRNode | DescriptionNode | PullRequestModel) => { if (pr === undefined) { // This is unexpected, but has happened a few times. - Logger.error('Unexpectedly received undefined when picking a PR.'); + Logger.error('Unexpectedly received undefined when picking a PR.', logId); return vscode.window.showErrorMessage(vscode.l10n.t('No pull request was selected to checkout, please try again.')); } @@ -552,7 +553,7 @@ export function registerCommands( vscode.commands.registerCommand('pr.openChanges', async (pr: PRNode | DescriptionNode | PullRequestModel) => { if (pr === undefined) { // This is unexpected, but has happened a few times. - Logger.error('Unexpectedly received undefined when picking a PR.'); + Logger.error('Unexpectedly received undefined when picking a PR.', logId); return vscode.window.showErrorMessage(vscode.l10n.t('No pull request was selected to checkout, please try again.')); } @@ -606,7 +607,7 @@ export function registerCommands( vscode.commands.registerCommand('pr.pickOnVscodeDev', async (pr: PRNode | DescriptionNode | PullRequestModel) => { if (pr === undefined) { // This is unexpected, but has happened a few times. - Logger.error('Unexpectedly received undefined when picking a PR.'); + Logger.error('Unexpectedly received undefined when picking a PR.', logId); return vscode.window.showErrorMessage(vscode.l10n.t('No pull request was selected to checkout, please try again.')); } @@ -1090,7 +1091,7 @@ export function registerCommands( const commentEditor = vscode.window.activeTextEditor?.document.uri.scheme === Schemes.Comment ? vscode.window.activeTextEditor : vscode.window.visibleTextEditors.find(visible => (visible.document.uri.scheme === Schemes.Comment) && (visible.document.uri.query === '')); if (!commentEditor) { - Logger.error('No comment editor visible for making a suggestion.'); + Logger.error('No comment editor visible for making a suggestion.', logId); vscode.window.showErrorMessage(vscode.l10n.t('No available comment editor to make a suggestion in.')); return; } diff --git a/src/common/logger.ts b/src/common/logger.ts index fd00691978..f43ede65e6 100644 --- a/src/common/logger.ts +++ b/src/common/logger.ts @@ -57,7 +57,7 @@ class Log extends Disposable { this._outputChannel.warn(this.logString(message, component)); } - public error(message: any, component?: string) { + public error(message: any, component: string) { this._outputChannel.error(this.logString(message, component)); } } diff --git a/src/common/protocol.ts b/src/common/protocol.ts index a3b074f91f..9af6576d51 100644 --- a/src/common/protocol.ts +++ b/src/common/protocol.ts @@ -17,6 +17,7 @@ export enum ProtocolType { } export class Protocol { + private static readonly ID = 'Protocol'; public type: ProtocolType = ProtocolType.OTHER; public host: string = ''; @@ -44,7 +45,7 @@ export class Protocol { this.owner = this.getOwnerName(this.url.path) || ''; } } catch (e) { - Logger.error(`Failed to parse '${uriString}'`); + Logger.error(`Failed to parse '${uriString}'`, Protocol.ID); vscode.window.showWarningMessage( vscode.l10n.t('Unable to parse remote \'{0}\'. Please check that it is correctly formatted.', uriString) ); diff --git a/src/common/uri.ts b/src/common/uri.ts index 9ab25b3e53..8eef14d65e 100644 --- a/src/common/uri.ts +++ b/src/common/uri.ts @@ -283,7 +283,7 @@ export namespace DataUri { try { await vscode.workspace.fs.delete(vscode.Uri.joinPath(cacheLocation(context), id)); } catch (e) { - Logger.error(`Failed to delete avatar from cache: ${e}`); + Logger.error(`Failed to delete avatar from cache: ${e}`, 'avatarCirclesAsImageDataUris'); } })); } diff --git a/src/github/createPRViewProvider.ts b/src/github/createPRViewProvider.ts index 4e9ef7c3c3..f99ece2e93 100644 --- a/src/github/createPRViewProvider.ts +++ b/src/github/createPRViewProvider.ts @@ -280,7 +280,7 @@ export abstract class BaseCreatePullRequestViewProvider this._githubManager.isGitHub(remote.gitProtocol.normalizeUri()!)), ).catch(e => { - Logger.error(`Resolving GitHub remotes failed: ${e}`); + Logger.error(`Resolving GitHub remotes failed: ${e}`, this.id); vscode.window.showErrorMessage(vscode.l10n.t('Resolving GitHub remotes failed: {0}', formatError(e))); return []; }); @@ -297,7 +297,7 @@ export class FolderRepositoryManager extends Disposable { const serverTypes = await Promise.all( potentialRemotes.map(remote => this._githubManager.isGitHub(remote.gitProtocol.normalizeUri()!)), ).catch(e => { - Logger.error(`Resolving GitHub remotes failed: ${e}`); + Logger.error(`Resolving GitHub remotes failed: ${e}`, this.id); vscode.window.showErrorMessage(vscode.l10n.t('Resolving GitHub remotes failed: {0}', formatError(e))); return []; }); @@ -316,7 +316,7 @@ export class FolderRepositoryManager extends Disposable { const remotesSetting = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(REMOTES); if (!remotesSetting) { - Logger.error(`Unable to read remotes setting`); + Logger.error(`Unable to read remotes setting`, this.id); return Promise.resolve([]); } @@ -621,7 +621,7 @@ export class FolderRepositoryManager extends Disposable { } private async getCachedFromGlobalState(userKind: 'assignableUsers' | 'teamReviewers' | 'mentionableUsers' | 'orgProjects'): Promise<{ [key: string]: T[] } | undefined> { - Logger.appendLine(`Trying to use globalState for ${userKind}.`); + Logger.appendLine(`Trying to use globalState for ${userKind}.`, this.id); const usersCacheLocation = vscode.Uri.joinPath(this.context.globalStorageUri, userKind); let usersCacheExists; @@ -631,7 +631,7 @@ export class FolderRepositoryManager extends Disposable { // file doesn't exit } if (!usersCacheExists) { - Logger.appendLine(`GlobalState does not exist for ${userKind}.`); + Logger.appendLine(`GlobalState does not exist for ${userKind}.`, this.id); return undefined; } @@ -646,7 +646,7 @@ export class FolderRepositoryManager extends Disposable { cacheAsJson = JSON.parse(repoSpecificCache.toString()); } catch (e) { if (e instanceof Error && e.message.includes('Unexpected non-whitespace character after JSON')) { - Logger.error(`Error parsing ${userKind} cache for ${repo.remote.remoteName}.`); + Logger.error(`Error parsing ${userKind} cache for ${repo.remote.remoteName}.`, this.id); } // file doesn't exist } @@ -656,11 +656,11 @@ export class FolderRepositoryManager extends Disposable { } }))).every(value => value); if (hasAllRepos) { - Logger.appendLine(`Using globalState ${userKind} for ${Object.keys(cache).length}.`); + Logger.appendLine(`Using globalState ${userKind} for ${Object.keys(cache).length}.`, this.id); return cache; } - Logger.appendLine(`No globalState for ${userKind}.`); + Logger.appendLine(`No globalState for ${userKind}.`, this.id); return undefined; } @@ -861,7 +861,7 @@ export class FolderRepositoryManager extends Disposable { const serverTypes = await Promise.all( remotes.map(remote => this._githubManager.isGitHub(remote.gitProtocol.normalizeUri()!)), ).catch(e => { - Logger.error(`Resolving GitHub remotes failed: ${e}`); + Logger.error(`Resolving GitHub remotes failed: ${e}`, this.id); vscode.window.showErrorMessage(vscode.l10n.t('Resolving GitHub remotes failed: {0}', formatError(e))); return []; }); @@ -1458,7 +1458,7 @@ export class FolderRepositoryManager extends Disposable { return this.createAndAddGitHubRepository(remote, this._credentialStore); } - Logger.error(`The remote '${upstreamRef.remote}' is not a GitHub repository.`); + Logger.error(`The remote '${upstreamRef.remote}' is not a GitHub repository.`, this.id); // No GitHubRepository? We currently won't try pushing elsewhere, // so fail. @@ -2461,7 +2461,7 @@ export class FolderRepositoryManager extends Disposable { return; } } - Logger.error(`Exiting failed: ${e}. Target branch ${branch} used to find branch ${branchObj?.name ?? 'unknown'} with upstream ${branchObj?.upstream?.name ?? 'unknown'}.`); + Logger.error(`Exiting failed: ${e}. Target branch ${branch} used to find branch ${branchObj?.name ?? 'unknown'} with upstream ${branchObj?.upstream?.name ?? 'unknown'}.`, this.id); vscode.window.showErrorMessage(`Exiting failed: ${e}`); } } diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index c317553fe7..0f6b80d7a3 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -465,7 +465,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel, BaseTreeNode { + private static readonly ID = 'CompareChangesTreeProvider'; private _view: vscode.TreeView; private _children: TreeNode[] | undefined; private _onDidChangeTreeData = new vscode.EventEmitter(); @@ -188,7 +189,7 @@ abstract class CompareChangesTreeProvider extends Disposable implements vscode.T this._children = await this.getGitChildren(element); } } catch (e) { - Logger.error(`Comparing changes failed: ${e}`); + Logger.error(`Comparing changes failed: ${e}`, CompareChangesTreeProvider.ID); return []; } return this._children; diff --git a/src/view/fileChangeModel.ts b/src/view/fileChangeModel.ts index cee4923308..3c4c81dbcf 100644 --- a/src/view/fileChangeModel.ts +++ b/src/view/fileChangeModel.ts @@ -13,6 +13,7 @@ import { FolderRepositoryManager } from '../github/folderRepositoryManager'; import { IResolvedPullRequestModel, PullRequestModel } from '../github/pullRequestModel'; export abstract class FileChangeModel { + private static readonly ID = 'FileChangeModel'; protected _filePath: vscode.Uri; get filePath(): vscode.Uri { return this._filePath; @@ -59,7 +60,7 @@ export abstract class FileChangeModel { const patch = await this.folderRepoManager.repository.diffBetween(this.pullRequest.base.sha, commit, this.fileName); diffHunks = parsePatch(patch); } catch (e) { - Logger.error(`Failed to parse patch for outdated comments: ${e}`); + Logger.error(`Failed to parse patch for outdated comments: ${e}`, FileChangeModel.ID); } } return diffHunks; diff --git a/src/view/gitContentProvider.ts b/src/view/gitContentProvider.ts index 046d716959..bcfd5dac71 100644 --- a/src/view/gitContentProvider.ts +++ b/src/view/gitContentProvider.ts @@ -18,6 +18,7 @@ import { ReviewManager } from './reviewManager'; import { GitFileChangeNode, RemoteFileChangeNode } from './treeNodes/fileChangeNode'; export class GitContentFileSystemProvider extends RepositoryFileSystemProvider { + private static readonly ID = 'GitContentFileSystemProvider'; private _fallback?: (uri: vscode.Uri) => Promise; constructor(gitAPI: GitApiImpl, credentialStore: CredentialStore, private readonly getReviewManagers: () => ReviewManager[]) { @@ -72,17 +73,17 @@ export class GitContentFileSystemProvider extends RepositoryFileSystemProvider { const absolutePath = pathLib.join(repository.rootUri.fsPath, path).replace(/\\/g, '/'); let content: string | undefined; try { - Logger.appendLine(`Getting change model (${repository.rootUri}) content for commit ${commit} and path ${absolutePath}`, 'GitContentFileSystemProvider'); + Logger.appendLine(`Getting change model (${repository.rootUri}) content for commit ${commit} and path ${absolutePath}`, GitContentFileSystemProvider.ID); content = await this.getChangeModelForFile(uri)?.showBase(); if (!content) { - Logger.appendLine(`Getting repository (${repository.rootUri}) content for commit ${commit} and path ${absolutePath}`, 'GitContentFileSystemProvider'); + Logger.appendLine(`Getting repository (${repository.rootUri}) content for commit ${commit} and path ${absolutePath}`, GitContentFileSystemProvider.ID); content = await repository.show(commit, absolutePath); } if (!content) { throw new Error(); } } catch (_) { - Logger.appendLine('Using fallback content provider.', 'GitContentFileSystemProvider'); + Logger.appendLine('Using fallback content provider.', GitContentFileSystemProvider.ID); content = await this._fallback(uri); if (!content) { // Content does not exist for the base or modified file for a file deletion or addition. @@ -91,7 +92,7 @@ export class GitContentFileSystemProvider extends RepositoryFileSystemProvider { try { await repository.getCommit(commit); } catch (err) { - Logger.error(err); + Logger.error(err, GitContentFileSystemProvider.ID); // Only show the error if we know it's not an outdated commit if (!this.getOutdatedChangeModelForFile(uri)) { vscode.window.showErrorMessage( diff --git a/src/view/reviewCommentController.ts b/src/view/reviewCommentController.ts index e44c1bd891..cafdaf4732 100644 --- a/src/view/reviewCommentController.ts +++ b/src/view/reviewCommentController.ts @@ -142,7 +142,7 @@ export class ReviewCommentController extends CommentControllerBase implements Co const adjustedStartLine = startLine - 1; const adjustedEndLine = endLine - 1; if (adjustedStartLine < 0 || adjustedEndLine < 0) { - Logger.error(`Mapped new position for workspace comment thread is invalid. Original: (${thread.startLine}, ${thread.endLine}) New: (${adjustedStartLine}, ${adjustedEndLine})`); + Logger.error(`Mapped new position for workspace comment thread is invalid. Original: (${thread.startLine}, ${thread.endLine}) New: (${adjustedStartLine}, ${adjustedEndLine})`, ReviewCommentController.ID); } range = threadRange(adjustedStartLine, adjustedEndLine); } @@ -541,7 +541,7 @@ export class ReviewCommentController extends CommentControllerBase implements Co editor => editor.document.uri.toString() === uri.toString(), ); if (!this._folderRepoManager.activePullRequest?.head) { - Logger.error('Failed to get content diff. Cannot get content diff without an active pull request head.'); + Logger.error('Failed to get content diff. Cannot get content diff without an active pull request head.', ReviewCommentController.ID); throw new Error('Cannot get content diff without an active pull request head.'); } @@ -561,7 +561,7 @@ export class ReviewCommentController extends CommentControllerBase implements Co return await this._repository.diffWith(this._folderRepoManager.activePullRequest.head.sha, fileName); } } catch (e) { - Logger.error(`Failed to get content diff. ${formatError(e)}`); + Logger.error(`Failed to get content diff. ${formatError(e)}`, ReviewCommentController.ID); if ((e.stderr as string | undefined)?.includes('bad object')) { if (this._repository.state.HEAD?.upstream && retry) { const pullBeforeCheckoutSetting = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, 'pull'); diff --git a/src/view/treeNodes/pullRequestNode.ts b/src/view/treeNodes/pullRequestNode.ts index 475f02fb17..53f538eea1 100644 --- a/src/view/treeNodes/pullRequestNode.ts +++ b/src/view/treeNodes/pullRequestNode.ts @@ -126,7 +126,7 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2 return result; } catch (e) { - Logger.error(e); + Logger.error(`Error getting children ${e}: ${e.message}`, PRNode.ID); return []; } }