From 6d9c95720d473a745c74eaff00c7072b74f1c185 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Thu, 23 May 2019 16:24:55 -0700 Subject: [PATCH] Fix extensions to swallow exceptions when sending telemetry (#5600) * Fix extensions to swallow exceptions when sending telemetry * Remove reference to GDPR * Clean up now-unused code --- extensions/admin-tool-ext-win/src/utils.ts | 2 +- .../import/src/services/serviceUtils.ts | 48 ------------ extensions/import/src/services/telemetry.ts | 73 ++++--------------- extensions/mssql/src/telemetry.ts | 53 ++++---------- 4 files changed, 33 insertions(+), 143 deletions(-) diff --git a/extensions/admin-tool-ext-win/src/utils.ts b/extensions/admin-tool-ext-win/src/utils.ts index d03a9278f1..40a5268670 100644 --- a/extensions/admin-tool-ext-win/src/utils.ts +++ b/extensions/admin-tool-ext-win/src/utils.ts @@ -39,7 +39,7 @@ export function backEscapeDoubleQuotes(value: string): string { } /** - * Map an error message into a GDPR-Compliant short name for the type of error. + * Map an error message into a friendly short name for the type of error. * @param msg The error message to map */ export function getTelemetryErrorType(msg: string): string { diff --git a/extensions/import/src/services/serviceUtils.ts b/extensions/import/src/services/serviceUtils.ts index cd49aede50..4ad647faa5 100644 --- a/extensions/import/src/services/serviceUtils.ts +++ b/extensions/import/src/services/serviceUtils.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import * as path from 'path'; -import * as crypto from 'crypto'; import * as os from 'os'; const baseConfig = require('./config.json'); @@ -48,53 +47,6 @@ export function getPackageInfo(packageJson: any): IPackageInfo { } } -export function generateUserId(): Promise { - return new Promise(resolve => { - try { - let interfaces = os.networkInterfaces(); - let mac; - for (let key of Object.keys(interfaces)) { - let item = interfaces[key][0]; - if (!item.internal) { - mac = item.mac; - break; - } - } - if (mac) { - resolve(crypto.createHash('sha256').update(mac + os.homedir(), 'utf8').digest('hex')); - } else { - resolve(generateGuid()); - } - } catch (err) { - resolve(generateGuid()); // fallback - } - }); -} - -export function generateGuid(): string { - let hexValues: string[] = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F']; - // c.f. rfc4122 (UUID version 4 = xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx) - let oct: string = ''; - let tmp: number; - /* tslint:disable:no-bitwise */ - for (let a: number = 0; a < 4; a++) { - tmp = (4294967296 * Math.random()) | 0; - oct += hexValues[tmp & 0xF] + - hexValues[tmp >> 4 & 0xF] + - hexValues[tmp >> 8 & 0xF] + - hexValues[tmp >> 12 & 0xF] + - hexValues[tmp >> 16 & 0xF] + - hexValues[tmp >> 20 & 0xF] + - hexValues[tmp >> 24 & 0xF] + - hexValues[tmp >> 28 & 0xF]; - } - - // 'Set the two most significant bits (bits 6 and 7) of the clock_seq_hi_and_reserved to zero and one, respectively' - let clockSequenceHi: string = hexValues[8 + (Math.random() * 4) | 0]; - return oct.substr(0, 8) + '-' + oct.substr(9, 4) + '-4' + oct.substr(13, 3) + '-' + clockSequenceHi + oct.substr(16, 3) + '-' + oct.substr(19, 12); - /* tslint:enable:no-bitwise */ -} - export function verifyPlatform(): Thenable { if (os.platform() === 'darwin' && parseFloat(os.release()) < 16) { return Promise.resolve(false); diff --git a/extensions/import/src/services/telemetry.ts b/extensions/import/src/services/telemetry.ts index 6ec193c87a..13f9564430 100644 --- a/extensions/import/src/services/telemetry.ts +++ b/extensions/import/src/services/telemetry.ts @@ -5,13 +5,11 @@ import { ErrorAction, CloseAction } from 'vscode-languageclient'; import TelemetryReporter from 'vscode-extension-telemetry'; -import { PlatformInformation } from 'service-downloader/out/platform'; import * as vscode from 'vscode'; import * as nls from 'vscode-nls'; const localize = nls.loadMessageBundle(); import * as constants from '../constants'; -import * as serviceUtils from './serviceUtils'; import { IMessage, ITelemetryEventProperties, ITelemetryEventMeasures } from './contracts'; @@ -90,39 +88,8 @@ export function FilterErrorPath(line: string): string { export class Telemetry { private static reporter: TelemetryReporter; - private static userId: string; - private static platformInformation: PlatformInformation; private static disabled: boolean; - // Get the unique ID for the current user of the extension - public static getUserId(): Promise { - return new Promise(resolve => { - // Generate the user id if it has not been created already - if (typeof this.userId === 'undefined') { - let id = serviceUtils.generateUserId(); - id.then(newId => { - this.userId = newId; - resolve(this.userId); - }); - } else { - resolve(this.userId); - } - }); - } - - public static getPlatformInformation(): Promise { - if (this.platformInformation) { - return Promise.resolve(this.platformInformation); - } else { - return new Promise(resolve => { - PlatformInformation.getCurrent().then(info => { - this.platformInformation = info; - resolve(this.platformInformation); - }); - }); - } - } - /** * Disable telemetry reporting */ @@ -149,25 +116,19 @@ export class Telemetry { * Send a telemetry event for an exception */ public static sendTelemetryEventForException( - err: any, methodName: string, extensionConfigName: string): void { - try { - let stackArray: string[]; - let firstLine: string = ''; - if (err !== undefined && err.stack !== undefined) { - stackArray = err.stack.split('\n'); - if (stackArray !== undefined && stackArray.length >= 2) { - firstLine = stackArray[1]; // The fist line is the error message and we don't want to send that telemetry event - firstLine = FilterErrorPath(firstLine); - } + err: any, methodName: string): void { + let stackArray: string[]; + let firstLine: string = ''; + if (err !== undefined && err.stack !== undefined) { + stackArray = err.stack.split('\n'); + if (stackArray !== undefined && stackArray.length >= 2) { + firstLine = stackArray[1]; // The fist line is the error message and we don't want to send that telemetry event + firstLine = FilterErrorPath(firstLine); } - - // Only adding the method name and the fist line of the stack trace. We don't add the error message because it might have PII - this.sendTelemetryEvent('Exception', { methodName: methodName, errorLine: firstLine }); - // Utils.logDebug('Unhandled Exception occurred. error: ' + err + ' method: ' + methodName, extensionConfigName); - } catch (telemetryErr) { - // If sending telemetry event fails ignore it so it won't break the extension - // Utils.logDebug('Failed to send telemetry event. error: ' + telemetryErr, extensionConfigName); } + + // Only adding the method name and the fist line of the stack trace. We don't add the error message because it might have PII + this.sendTelemetryEvent('Exception', { methodName: methodName, errorLine: firstLine }); } /** @@ -191,14 +152,12 @@ export class Telemetry { properties = {}; } - // Augment the properties structure with additional common properties before sending - Promise.all([this.getUserId(), this.getPlatformInformation()]).then(() => { - properties['userId'] = this.userId; - properties['distribution'] = (this.platformInformation && this.platformInformation.distribution) ? - `${this.platformInformation.distribution.name}, ${this.platformInformation.distribution.version}` : ''; - + try { this.reporter.sendTelemetryEvent(eventName, properties, measures); - }); + } catch (telemetryErr) { + // If sending telemetry event fails ignore it so it won't break the extension + console.error('Failed to send telemetry event. error: ' + telemetryErr); + } } } diff --git a/extensions/mssql/src/telemetry.ts b/extensions/mssql/src/telemetry.ts index a62436736b..7c11848608 100644 --- a/extensions/mssql/src/telemetry.ts +++ b/extensions/mssql/src/telemetry.ts @@ -5,7 +5,6 @@ import * as vscode from 'vscode'; import TelemetryReporter from 'vscode-extension-telemetry'; -import { PlatformInformation } from 'service-downloader/out/platform'; import { ErrorAction, ErrorHandler, Message, CloseAction } from 'vscode-languageclient'; import * as Utils from './utils'; @@ -39,22 +38,8 @@ export function FilterErrorPath(line: string): string { export class Telemetry { private static reporter: TelemetryReporter; - private static platformInformation: PlatformInformation; private static disabled: boolean; - public static getPlatformInformation(): Promise { - if (this.platformInformation) { - return Promise.resolve(this.platformInformation); - } else { - return new Promise(resolve => { - PlatformInformation.getCurrent().then(info => { - this.platformInformation = info; - resolve(this.platformInformation); - }); - }); - } - } - /** * Disable telemetry reporting */ @@ -83,24 +68,18 @@ export class Telemetry { */ public static sendTelemetryEventForException( err: any, methodName: string, extensionConfigName: string): void { - try { - let stackArray: string[]; - let firstLine: string = ''; - if (err !== undefined && err.stack !== undefined) { - stackArray = err.stack.split('\n'); - if (stackArray !== undefined && stackArray.length >= 2) { - firstLine = stackArray[1]; // The fist line is the error message and we don't want to send that telemetry event - firstLine = FilterErrorPath(firstLine); - } + let stackArray: string[]; + let firstLine: string = ''; + if (err !== undefined && err.stack !== undefined) { + stackArray = err.stack.split('\n'); + if (stackArray !== undefined && stackArray.length >= 2) { + firstLine = stackArray[1]; // The fist line is the error message and we don't want to send that telemetry event + firstLine = FilterErrorPath(firstLine); } - - // Only adding the method name and the fist line of the stack trace. We don't add the error message because it might have PII - this.sendTelemetryEvent('Exception', { methodName: methodName, errorLine: firstLine }); - // Utils.logDebug('Unhandled Exception occurred. error: ' + err + ' method: ' + methodName, extensionConfigName); - } catch (telemetryErr) { - // If sending telemetry event fails ignore it so it won't break the extension - // Utils.logDebug('Failed to send telemetry event. error: ' + telemetryErr, extensionConfigName); } + + // Only adding the method name and the fist line of the stack trace. We don't add the error message because it might have PII + this.sendTelemetryEvent('Exception', { methodName: methodName, errorLine: firstLine }); } /** @@ -124,13 +103,13 @@ export class Telemetry { properties = {}; } - // Augment the properties structure with additional common properties before sending - Promise.all([this.getPlatformInformation()]).then(() => { - properties['distribution'] = (this.platformInformation && this.platformInformation.distribution) ? - `${this.platformInformation.distribution.name}, ${this.platformInformation.distribution.version}` : ''; - + try { this.reporter.sendTelemetryEvent(eventName, properties, measures); - }); + } catch (telemetryErr) { + // If sending telemetry event fails ignore it so it won't break the extension + console.error('Failed to send telemetry event. error: ' + telemetryErr); + } + } }