diff --git a/gulpfile.js b/gulpfile.js index 771d4c8df3..403c6fa570 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -110,6 +110,7 @@ gulp.task('test-coverage', function() { ,'!out/src/extension.js' ,'!out/src/extensionmanager.js' ,'!out/src/team-extension.js' + ,'!out/src/clients/baseclient.js' ,'!out/src/clients/buildclient.js' ,'!out/src/clients/coreapiclient.js' ,'!out/src/clients/feedbackclient.js' diff --git a/src/clients/baseclient.ts b/src/clients/baseclient.ts new file mode 100644 index 0000000000..58d88f71cc --- /dev/null +++ b/src/clients/baseclient.ts @@ -0,0 +1,58 @@ +/*--------------------------------------------------------------------------------------------- +* Copyright (c) Microsoft Corporation. All rights reserved. +* Licensed under the MIT License. See License.txt in the project root for license information. +*--------------------------------------------------------------------------------------------*/ +"use strict"; + +import { StatusBarItem } from "vscode"; +import { Logger } from "../helpers/logger"; +import { Telemetry } from "../services/telemetry"; +import { TeamServerContext} from "../contexts/servercontext"; +import { CommandNames } from "../helpers/constants"; +import { Strings } from "../helpers/strings"; +import { Utils } from "../helpers/utils"; +import { VsCodeUtils } from "../helpers/vscodeutils"; + +export abstract class BaseClient { + protected _serverContext: TeamServerContext; + protected _statusBarItem: StatusBarItem; + + constructor(context: TeamServerContext, statusBarItem: StatusBarItem) { + this._serverContext = context; + this._statusBarItem = statusBarItem; + } + + protected handleError(err: Error, offlineText: string, polling: boolean, infoMessage?: string) : void { + let offline: boolean = Utils.IsOffline(err); + let msg: string = Utils.GetMessageForStatusCode(err, err.message); + let logPrefix: string = (infoMessage === undefined) ? "" : infoMessage + " "; + + //When polling, we never display an error, we only log it (no telemetry either) + if (polling === true) { + Logger.LogError(logPrefix + msg); + if (offline === true) { + if (this._statusBarItem !== undefined) { + this._statusBarItem.text = offlineText; + this._statusBarItem.tooltip = Strings.StatusCodeOffline + " " + Strings.ClickToRetryConnection; + this._statusBarItem.command = CommandNames.RefreshPollingStatus; + } + } else { + //Could happen if PAT doesn't have proper permissions + if (this._statusBarItem !== undefined) { + this._statusBarItem.text = offlineText; + this._statusBarItem.tooltip = msg; + } + } + //If we aren't polling, we always log an error and, optionally, send telemetry + } else { + let logMessage: string = logPrefix + msg; + if (offline === true) { + Logger.LogError(logMessage); + } else { + Logger.LogError(logMessage); + Telemetry.SendException(err); + } + VsCodeUtils.ShowErrorMessage(msg); + } + } +} diff --git a/src/clients/buildclient.ts b/src/clients/buildclient.ts index 1cb97d65ca..a6b5d577ec 100644 --- a/src/clients/buildclient.ts +++ b/src/clients/buildclient.ts @@ -13,17 +13,14 @@ import { TeamServerContext} from "../contexts/servercontext"; import { CommandNames, TelemetryEvents, WellKnownRepositoryTypes } from "../helpers/constants"; import { Strings } from "../helpers/strings"; import { Utils } from "../helpers/utils"; -import { VsCodeUtils } from "../helpers/vscodeutils"; import { IRepositoryContext, RepositoryType } from "../contexts/repositorycontext"; +import { BaseClient } from "./baseclient"; -export class BuildClient { - private _serverContext: TeamServerContext; - private _statusBarItem: StatusBarItem; +export class BuildClient extends BaseClient { private _buildSummaryUrl: string; constructor(context: TeamServerContext, statusBarItem: StatusBarItem) { - this._serverContext = context; - this._statusBarItem = statusBarItem; + super(context, statusBarItem); } //Gets any available build status information and adds it to the status bar @@ -69,7 +66,7 @@ export class BuildClient { } } } catch (err) { - this.handleError(err, polling, "Failed to get current build status"); + this.handleError(err, BuildClient.GetOfflineBuildStatusText(), polling, "Failed to get current build status"); } } @@ -114,40 +111,6 @@ export class BuildClient { Utils.OpenUrl(url); } - private handleError(reason: any, polling: boolean, infoMessage?: string) : void { - let offline: boolean = Utils.IsOffline(reason); - let msg: string = Utils.GetMessageForStatusCode(reason, reason.message); - let logPrefix: string = (infoMessage === undefined) ? "" : infoMessage + " "; - - //When polling, we never display an error, we only log it (no telemetry either) - if (polling === true) { - Logger.LogError(logPrefix + msg); - if (offline === true) { - if (this._statusBarItem !== undefined) { - this._statusBarItem.text = BuildClient.GetOfflineBuildStatusText(); - this._statusBarItem.tooltip = Strings.StatusCodeOffline + " " + Strings.ClickToRetryConnection; - this._statusBarItem.command = CommandNames.RefreshPollingStatus; - } - } else { - //Could happen if PAT doesn't have proper permissions - if (this._statusBarItem !== undefined) { - this._statusBarItem.text = BuildClient.GetOfflineBuildStatusText(); - this._statusBarItem.tooltip = msg; - } - } - //If we aren't polling, we always log an error and, optionally, send telemetry - } else { - let logMessage: string = logPrefix + msg; - if (offline === true) { - Logger.LogError(logMessage); - } else { - Logger.LogError(logMessage); - Telemetry.SendException(logMessage); - } - VsCodeUtils.ShowErrorMessage(msg); - } - } - public static GetOfflineBuildStatusText() : string { return `$(icon octicon-package) ` + `???`; } diff --git a/src/clients/feedbackclient.ts b/src/clients/feedbackclient.ts index 93cbf755e7..2b2582e4f3 100644 --- a/src/clients/feedbackclient.ts +++ b/src/clients/feedbackclient.ts @@ -54,7 +54,7 @@ export class FeedbackClient { } catch (err) { let message: string = Utils.GetMessageForStatusCode(0, err.message, "Failed getting SendFeedback selection"); Logger.LogError(message); - Telemetry.SendException(message); + Telemetry.SendException(err); } } } diff --git a/src/clients/gitclient.ts b/src/clients/gitclient.ts index ae6b2fc012..6eeff65bc4 100644 --- a/src/clients/gitclient.ts +++ b/src/clients/gitclient.ts @@ -15,16 +15,14 @@ import { IRepositoryContext, RepositoryType } from "../contexts/repositorycontex import { TeamServerContext} from "../contexts/servercontext"; import { GitVcService, PullRequestScore } from "../services/gitvc"; import { Telemetry } from "../services/telemetry"; +import { BaseClient } from "./baseclient"; -var path = require("path"); +import * as path from "path"; -export class GitClient { - private _serverContext: TeamServerContext; - private _statusBarItem: StatusBarItem; +export class GitClient extends BaseClient { constructor(context: TeamServerContext, statusBarItem: StatusBarItem) { - this._serverContext = context; - this._statusBarItem = statusBarItem; + super(context, statusBarItem); } //Initial method to display, select and navigate to my pull requests @@ -45,7 +43,7 @@ export class GitClient { Utils.OpenUrl(discUrl); } } catch (err) { - this.handleError(err, "Error selecting pull request from QuickPick"); + this.handleError(err, GitClient.GetOfflinePullRequestStatusText(), false, "Error selecting pull request from QuickPick"); } } @@ -122,7 +120,7 @@ export class GitClient { //Remove the default Strings.BrowseYourPullRequests item from the calculation this._statusBarItem.text = GitClient.GetPullRequestStatusText(requests.length - 1); } catch (err) { - this.handleError(err, "Attempting to poll my pull requests", true); + this.handleError(err, GitClient.GetOfflinePullRequestStatusText(), true, "Attempting to poll my pull requests"); } } @@ -179,40 +177,6 @@ export class GitClient { return { label: scoreLabel + " (" + displayName + ") " + title, description: description, id: id }; } - private handleError(reason: any, infoMessage?: string, polling?: boolean) : void { - let offline: boolean = Utils.IsOffline(reason); - let msg: string = Utils.GetMessageForStatusCode(reason, reason.message); - let logPrefix: string = (infoMessage === undefined) ? "" : infoMessage + " "; - - //When polling, we never display an error, we only log it (no telemetry either) - if (polling === true) { - Logger.LogError(logPrefix + msg); - if (offline === true) { - if (this._statusBarItem !== undefined) { - this._statusBarItem.text = GitClient.GetOfflinePullRequestStatusText(); - this._statusBarItem.tooltip = Strings.StatusCodeOffline + " " + Strings.ClickToRetryConnection; - this._statusBarItem.command = CommandNames.RefreshPollingStatus; - } - } else { - //Could happen if PAT doesn't have proper permissions - if (this._statusBarItem !== undefined) { - this._statusBarItem.text = GitClient.GetOfflinePullRequestStatusText(); - this._statusBarItem.tooltip = msg; - } - } - //If we aren't polling, we always log an error and, optionally, send telemetry - } else { - let logMessage: string = logPrefix + msg; - if (offline === true) { - Logger.LogError(logMessage); - } else { - Logger.LogError(logMessage); - Telemetry.SendException(logMessage); - } - VsCodeUtils.ShowErrorMessage(msg); - } - } - public static GetOfflinePullRequestStatusText() : string { return `$(icon octicon-git-pull-request) ` + `???`; } diff --git a/src/clients/witclient.ts b/src/clients/witclient.ts index 12ced520db..244d11ec26 100644 --- a/src/clients/witclient.ts +++ b/src/clients/witclient.ts @@ -10,21 +10,19 @@ import { Logger } from "../helpers/logger"; import { SimpleWorkItem, WorkItemTrackingService } from "../services/workitemtracking"; import { Telemetry } from "../services/telemetry"; import { TeamServerContext} from "../contexts/servercontext"; -import { BaseQuickPickItem, VsCodeUtils, WorkItemQueryQuickPickItem } from "../helpers/vscodeutils"; -import { CommandNames, TelemetryEvents, WitQueries, WitTypes } from "../helpers/constants"; +import { BaseQuickPickItem, WorkItemQueryQuickPickItem } from "../helpers/vscodeutils"; +import { TelemetryEvents, WitQueries, WitTypes } from "../helpers/constants"; import { Strings } from "../helpers/strings"; import { Utils } from "../helpers/utils"; import { IPinnedQuery } from "../helpers/settings"; +import { BaseClient } from "./baseclient"; -export class WitClient { - private _serverContext: TeamServerContext; - private _statusBarItem: StatusBarItem; +export class WitClient extends BaseClient { private _pinnedQuery: IPinnedQuery; private _myQueriesFolder: string; constructor(context: TeamServerContext, pinnedQuery: IPinnedQuery, statusBarItem: StatusBarItem) { - this._serverContext = context; - this._statusBarItem = statusBarItem; + super(context, statusBarItem); this._pinnedQuery = pinnedQuery; } @@ -51,7 +49,7 @@ export class WitClient { Utils.OpenUrl(newItemUrl); } } catch (err) { - this.handleError(err, "Error creating new work item"); + this.handleError(err, WitClient.GetOfflinePinnedQueryStatusText(), false, "Error creating new work item"); } } @@ -82,7 +80,7 @@ export class WitClient { } } } catch (err) { - this.handleError(err, "Error showing work item queries"); + this.handleError(err, WitClient.GetOfflinePinnedQueryStatusText(), false, "Error showing work item queries"); } } @@ -93,7 +91,7 @@ export class WitClient { let queryText: string = await this.getPinnedQueryText(); await this.showWorkItems(queryText); } catch (err) { - this.handleError(err, "Error showing pinned query work items"); + this.handleError(err, WitClient.GetOfflinePinnedQueryStatusText(), false, "Error showing pinned query work items"); } } @@ -103,7 +101,7 @@ export class WitClient { try { await this.showWorkItems(WitQueries.MyWorkItems); } catch (err) { - this.handleError(err, "Error showing my work items"); + this.handleError(err, WitClient.GetOfflinePinnedQueryStatusText(), false, "Error showing my work items"); } } @@ -144,7 +142,7 @@ export class WitClient { let svc: WorkItemTrackingService = new WorkItemTrackingService(this._serverContext); return svc.GetQueryResultCount(this._serverContext.RepoInfo.TeamProject, queryText); } catch (err) { - this.handleError(err, "Error getting pinned query result count"); + this.handleError(err, WitClient.GetOfflinePinnedQueryStatusText(), false, "Error getting pinned query result count"); } } @@ -240,40 +238,6 @@ export class WitClient { return workItemTypes; } - private handleError(reason: any, infoMessage?: string, polling?: boolean) : void { - let offline: boolean = Utils.IsOffline(reason); - let msg: string = Utils.GetMessageForStatusCode(reason, reason.message); - let logPrefix: string = (infoMessage === undefined) ? "" : infoMessage + " "; - - //When polling, we never display an error, we only log it (no telemetry either) - if (polling === true) { - Logger.LogError(logPrefix + msg); - if (offline === true) { - if (this._statusBarItem !== undefined) { - this._statusBarItem.text = WitClient.GetOfflinePinnedQueryStatusText(); - this._statusBarItem.tooltip = Strings.StatusCodeOffline + " " + Strings.ClickToRetryConnection; - this._statusBarItem.command = CommandNames.RefreshPollingStatus; - } - } else { - //Could happen if PAT doesn't have proper permissions - if (this._statusBarItem !== undefined) { - this._statusBarItem.text = WitClient.GetOfflinePinnedQueryStatusText(); - this._statusBarItem.tooltip = msg; - } - } - //If we aren't polling, we always log an error and, optionally, send telemetry - } else { - let logMessage: string = logPrefix + msg; - if (offline === true) { - Logger.LogError(logMessage); - } else { - Logger.LogError(logMessage); - Telemetry.SendException(logMessage); - } - VsCodeUtils.ShowErrorMessage(msg); - } - } - private logTelemetryForWorkItem(wit: string): void { switch (wit) { case WitTypes.Bug: @@ -291,8 +255,8 @@ export class WitClient { this.GetPinnedQueryResultCount().then((items) => { this._statusBarItem.tooltip = Strings.ViewYourPinnedQuery; this._statusBarItem.text = WitClient.GetPinnedQueryStatusText(items); - }).catch((reason) => { - this.handleError(reason, "Failed to get pinned query count during polling", true); + }).catch((err) => { + this.handleError(err, WitClient.GetOfflinePinnedQueryStatusText(), true, "Failed to get pinned query count during polling"); }); } diff --git a/src/extensionmanager.ts b/src/extensionmanager.ts index 52731da474..cec2f7bb91 100644 --- a/src/extensionmanager.ts +++ b/src/extensionmanager.ts @@ -129,14 +129,14 @@ export class ExtensionManager implements Disposable { } //Logs an error to the logger and sends an exception to telemetry service - public ReportError(message: string, reason?: any, showToUser: boolean = false): void { - let fullMessage = reason ? message + " " + reason : message; + public ReportError(err: Error, message: string, showToUser: boolean = false): void { + let fullMessage = err ? message + " " + err : message; // Log the message Logger.LogError(fullMessage); - if (reason && reason.message) { + if (err && err.message) { // Log additional information for debugging purposes - Logger.LogDebug(reason.message); + Logger.LogDebug(err.message); } // Show just the message to the user if needed @@ -145,11 +145,11 @@ export class ExtensionManager implements Disposable { } // Send it to telemetry - if (reason !== undefined && (Utils.IsUnauthorized(reason) || Utils.IsOffline(reason) || Utils.IsProxyIssue(reason))) { + if (err !== undefined && (Utils.IsUnauthorized(err) || Utils.IsOffline(err) || Utils.IsProxyIssue(err))) { //Don't log exceptions for Unauthorized, Offline or Proxy scenarios return; } - Telemetry.SendException(fullMessage); + Telemetry.SendException(err); } private displayNoCredentialsMessage(): void { @@ -259,7 +259,8 @@ export class ExtensionManager implements Disposable { this.logDebugInformation(); } catch (err) { this.setErrorStatus(Utils.GetMessageForStatusCode(err, err.message), (err.statusCode === 401 ? CommandNames.Signin : undefined), false); - this.ReportError(Utils.GetMessageForStatusCode(err, err.message, "Failed to get results with accountClient: "), err); + //Wrap err here to get a useful call stack + this.ReportError(new Error(err), Utils.GetMessageForStatusCode(err, err.message, "Failed to get results with accountClient: ")); } } catch (err) { //TODO: With TFVC, creating a RepositoryInfo can throw (can't get project collection, can't get team project, etc.) @@ -267,18 +268,20 @@ export class ExtensionManager implements Disposable { if (this._serverContext.RepoInfo.IsTeamFoundationServer === true && err.statusCode === 404) { this.setErrorStatus(Strings.UnsupportedServerVersion, undefined, false); Logger.LogError(Strings.UnsupportedServerVersion); + Telemetry.SendEvent(TelemetryEvents.UnsupportedServerVersion); } else { this.setErrorStatus(Utils.GetMessageForStatusCode(err, err.message), (err.statusCode === 401 ? CommandNames.Signin : undefined), false); - this.ReportError(Utils.GetMessageForStatusCode(err, err.message, "Failed call with repositoryClient: "), err); + //Wrap err here to get a useful call stack + this.ReportError(new Error(err), Utils.GetMessageForStatusCode(err, err.message, "Failed call with repositoryClient: ")); } } } - }).fail((reason) => { - this.setErrorStatus(Utils.GetMessageForStatusCode(reason, reason.message), (reason.statusCode === 401 ? CommandNames.Signin : undefined), false); + }).fail((err) => { + this.setErrorStatus(Utils.GetMessageForStatusCode(err, err.message), (err.statusCode === 401 ? CommandNames.Signin : undefined), false); //If we can't get a requestHandler, report the error via the feedbackclient - let message: string = Utils.GetMessageForStatusCode(reason, reason.message, "Failed to get a credential handler"); + let message: string = Utils.GetMessageForStatusCode(err, err.message, "Failed to get a credential handler"); Logger.LogError(message); - Telemetry.SendException(message); + Telemetry.SendException(err); }); } diff --git a/src/helpers/constants.ts b/src/helpers/constants.ts index f8ff5964d1..3fdd1040ea 100644 --- a/src/helpers/constants.ts +++ b/src/helpers/constants.ts @@ -93,6 +93,7 @@ export class TelemetryEvents { static StartUp: string = TelemetryEvents.TelemetryPrefix + "startup"; static TokenLearnMoreClick: string = TelemetryEvents.TelemetryPrefix + "tokenlearnmoreclick"; static TokenInSettings: string = TelemetryEvents.TelemetryPrefix + "tokeninsettings"; + static UnsupportedServerVersion: string = TelemetryEvents.TelemetryPrefix + "unsupportedversion"; static ViewPullRequest: string = TelemetryEvents.TelemetryPrefix + "viewpullrequest"; static ViewPullRequests: string = TelemetryEvents.TelemetryPrefix + "viewpullrequests"; static ViewMyWorkItems: string = TelemetryEvents.TelemetryPrefix + "viewmyworkitems"; diff --git a/src/services/telemetry.ts b/src/services/telemetry.ts index a4929b0fed..96fcf5fdbd 100644 --- a/src/services/telemetry.ts +++ b/src/services/telemetry.ts @@ -66,11 +66,11 @@ export class Telemetry { Telemetry._appInsightsClient.trackEvent(event, properties); } - public static SendException(message: string, properties?: any): void { + public static SendException(err: Error, properties?: any): void { Telemetry.ensureInitialized(); if (Telemetry._telemetryEnabled === true) { - Telemetry._appInsightsClient.trackException(new Error(message), properties); + Telemetry._appInsightsClient.trackException(err, properties); } } diff --git a/src/team-extension.ts b/src/team-extension.ts index e2580eb45a..c2eda26d26 100644 --- a/src/team-extension.ts +++ b/src/team-extension.ts @@ -73,10 +73,10 @@ export class TeamExtension { this._manager.CredentialManager.StoreCredentials(this._manager.ServerContext.RepoInfo.Host, username, password).then(() => { // We don't test the credentials to make sure they're good here. Do so on the next command that's run. this._manager.Reinitialize(); - }).catch((reason) => { + }).catch((err) => { // TODO: Should the message direct the user to open an issue? send feedback? let msg: string = Strings.UnableToStoreCredentials + this._manager.ServerContext.RepoInfo.Host; - this._manager.ReportError(msg, reason, true); + this._manager.ReportError(err, msg, true); }); } } @@ -87,10 +87,10 @@ export class TeamExtension { Logger.LogInfo("Signin: Personal Access Token provided as authentication."); this._manager.CredentialManager.StoreCredentials(this._manager.ServerContext.RepoInfo.Host, Constants.OAuth, token).then(() => { this._manager.Reinitialize(); - }).catch((reason) => { + }).catch((err) => { // TODO: Should the message direct the user to open an issue? send feedback? let msg: string = Strings.UnableToStoreCredentials + this._manager.ServerContext.RepoInfo.Host; - this._manager.ReportError(msg, reason, true); + this._manager.ReportError(err, msg, true); }); } } @@ -113,9 +113,9 @@ export class TeamExtension { this._manager.CredentialManager.RemoveCredentials(this._manager.ServerContext.RepoInfo.Host).then(() => { Logger.LogInfo("Signout: Removed credentials for host '" + this._manager.ServerContext.RepoInfo.Host + "'"); this._manager.Reinitialize(true); - }).catch((reason) => { + }).catch((err) => { let msg: string = Strings.UnableToRemoveCredentials + this._manager.ServerContext.RepoInfo.Host; - this._manager.ReportError(msg, reason, true); + this._manager.ReportError(err, msg, true); }); } else { this._manager.DisplayErrorMessage(Strings.NoRepoInformation);