Skip to content

Commit

Permalink
Require component name in error logs
Browse files Browse the repository at this point in the history
Part of #6513
  • Loading branch information
alexr00 committed Jan 16, 2025
1 parent d5de56b commit c9dbcdb
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 36 deletions.
11 changes: 6 additions & 5 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export function registerCommands(
telemetry: ITelemetry,
tree: PullRequestsTreeDataProvider,
) {
const logId = 'RegisterCommands';
context.subscriptions.push(
vscode.commands.registerCommand(
'pr.openPullRequestOnGitHub',
Expand Down Expand Up @@ -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)));
}
}),
Expand Down Expand Up @@ -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.'));
}

Expand Down Expand Up @@ -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.'));
}

Expand Down Expand Up @@ -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.'));
}

Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/common/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export enum ProtocolType {
}

export class Protocol {
private static readonly ID = 'Protocol';
public type: ProtocolType = ProtocolType.OTHER;
public host: string = '';

Expand Down Expand Up @@ -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)
);
Expand Down
2 changes: 1 addition & 1 deletion src/common/uri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}));
}
Expand Down
4 changes: 2 additions & 2 deletions src/github/createPRViewProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
try {
await pr.addAssignees([resolved]);
} catch (e) {
Logger.error(`Unable to assign pull request to user ${resolved}.`);
Logger.error(`Unable to assign pull request to user ${resolved}.`, BaseCreatePullRequestViewProvider.ID);
}
}

Expand Down Expand Up @@ -379,7 +379,7 @@ export abstract class BaseCreatePullRequestViewProvider<T extends BasePullReques
});
}
} catch (e) {
Logger.error(formatError(e));
Logger.error(`Failed to add reviewers: ${formatError(e)}`, BaseCreatePullRequestViewProvider.ID);
vscode.window.showErrorMessage(formatError(e));
} finally {
quickPick?.hide();
Expand Down
7 changes: 4 additions & 3 deletions src/github/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ interface AuthResult {
}

export class CredentialStore extends Disposable {
private static readonly ID = 'Authentication';
private _githubAPI: GitHub | undefined;
private _sessionId: string | undefined;
private _githubEnterpriseAPI: GitHub | undefined;
Expand Down Expand Up @@ -204,7 +205,7 @@ export class CredentialStore extends Disposable {
}
return authResult;
} else {
Logger.debug(`No GitHub${getGitHubSuffix(authProviderId)} token found.`, 'Authentication');
Logger.debug(`No GitHub${getGitHubSuffix(authProviderId)} token found.`, CredentialStore.ID);
return authResult;
}
}
Expand Down Expand Up @@ -328,9 +329,9 @@ export class CredentialStore extends Disposable {
try {
await this.initialize(authProviderId, sessionOptions);
} catch (e) {
Logger.error(`${errorPrefix}: ${e}`);
Logger.error(`Login error: ${errorPrefix}: ${e}`, CredentialStore.ID);
if (e instanceof Error && e.stack) {
Logger.error(e.stack);
Logger.error(e.stack, CredentialStore.ID);
}
if (e.message === 'Cancelled') {
isCanceled = true;
Expand Down
22 changes: 11 additions & 11 deletions src/github/folderRepositoryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,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 [];
});
Expand All @@ -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 [];
});
Expand All @@ -316,7 +316,7 @@ export class FolderRepositoryManager extends Disposable {
const remotesSetting = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<string[]>(REMOTES);

if (!remotesSetting) {
Logger.error(`Unable to read remotes setting`);
Logger.error(`Unable to read remotes setting`, this.id);
return Promise.resolve([]);
}

Expand Down Expand Up @@ -621,7 +621,7 @@ export class FolderRepositoryManager extends Disposable {
}

private async getCachedFromGlobalState<T>(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;
Expand All @@ -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;
}

Expand All @@ -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
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 [];
});
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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}`);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/github/pullRequestOverview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
});
}
} catch (e) {
Logger.error(formatError(e));
Logger.error(formatError(e), PullRequestOverviewPanel.ID);
vscode.window.showErrorMessage(formatError(e));
} finally {
quickPick?.hide();
Expand Down
3 changes: 2 additions & 1 deletion src/issues/issueFeatureRegistrar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import {
const CREATING_ISSUE_FROM_FILE_CONTEXT = 'issues.creatingFromFile';

export class IssueFeatureRegistrar extends Disposable {
private static readonly ID = 'IssueFeatureRegistrar';
private _stateManager: StateManager;
private _newIssueCache: NewIssueCache;

Expand Down Expand Up @@ -1260,7 +1261,7 @@ ${options?.body ?? ''}\n
}
if (!folderManager) {
vscode.window.showErrorMessage(vscode.l10n.t(`Could not find the correct repository for the issue; see logs for more details.`));
Logger.error(`Could not find the folder manager for the issue originUri: ${originUri.toString()}`);
Logger.error(`Could not find the folder manager for the issue originUri: ${originUri.toString()}`, IssueFeatureRegistrar.ID);
return false;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/view/compareChangesTreeDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class GitCommitNode extends TreeNode {
}

abstract class CompareChangesTreeProvider extends Disposable implements vscode.TreeDataProvider<TreeNode>, BaseTreeNode {
private static readonly ID = 'CompareChangesTreeProvider';
private _view: vscode.TreeView<TreeNode>;
private _children: TreeNode[] | undefined;
private _onDidChangeTreeData = new vscode.EventEmitter<TreeNode | void>();
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/view/fileChangeModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions src/view/gitContentProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>;

constructor(gitAPI: GitApiImpl, credentialStore: CredentialStore, private readonly getReviewManagers: () => ReviewManager[]) {
Expand Down Expand Up @@ -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.
Expand All @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions src/view/reviewCommentController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.');
}

Expand All @@ -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<PullPRBranchVariants>(PULL_PR_BRANCH_BEFORE_CHECKOUT, 'pull');
Expand Down
2 changes: 1 addition & 1 deletion src/view/treeNodes/pullRequestNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}
}
Expand Down

0 comments on commit c9dbcdb

Please sign in to comment.