move ErrorTelemetry into its own type, #5219

src/vs/base/common/timer.ts

@@ -7,6 +7,7 @@
 import Platform = require('vs/base/common/platform');
 import errors = require('vs/base/common/errors');
 import precision = require('vs/base/common/stopwatch');
+import {IDisposable} from 'vs/base/common/lifecycle';
 
 export var ENABLE_TIMER = false;
 var msWriteProfilerMark = Platform.globals['msWriteProfilerMark'];
@@ -205,17 +206,18 @@ export class TimeKeeper /*extends EventEmitter.EventEmitter*/ {
 		}
 	}
 
-	public addListener(listener: IEventsListener): void {
+	public addListener(listener: IEventsListener): IDisposable {
 		this.listeners.push(listener);
-	}
-
-	public removeListener(listener: IEventsListener): void {
-		for (var i = 0; i < this.listeners.length; i++) {
-			if (this.listeners[i] === listener) {
-				this.listeners.splice(i, 1);
-				return;
+		return {
+			dispose: () => {
+				for (var i = 0; i < this.listeners.length; i++) {
+					if (this.listeners[i] === listener) {
+						this.listeners.splice(i, 1);
+						return;
+					}
+				}
 			}
-		}
+		};
 	}
 
 	private addEvent(event: ITimerEvent): void {

src/vs/platform/telemetry/browser/mainTelemetryService.ts

@@ -8,13 +8,12 @@
 import * as Platform from 'vs/base/common/platform';
 import * as uuid from 'vs/base/common/uuid';
 import {ITelemetryService, ITelemetryServiceConfig, ITelemetryAppender, ITelemetryInfo} from 'vs/platform/telemetry/common/telemetry';
+import ErrorTelemetry from 'vs/platform/telemetry/common/errorTelemetry';
 import {IdleMonitor, UserStatus} from 'vs/base/browser/idleMonitor';
 import {TPromise} from 'vs/base/common/winjs.base';
-import {IDisposable} from 'vs/base/common/lifecycle';
-import Errors = require('vs/base/common/errors');
-import {TimeKeeper, IEventsListener, ITimerEvent} from 'vs/base/common/timer';
-import {safeStringify, withDefaults, cloneAndChange} from 'vs/base/common/objects';
-
+import {IDisposable, dispose} from 'vs/base/common/lifecycle';
+import {TimeKeeper, ITimerEvent} from 'vs/base/common/timer';
+import {withDefaults, cloneAndChange} from 'vs/base/common/objects';
 
 const DefaultTelemetryServiceConfig: ITelemetryServiceConfig = {
 	enableHardIdle: true,
@@ -33,30 +32,23 @@ export abstract class AbstractTelemetryService implements ITelemetryService {
 	public serviceId = ITelemetryService;
 
 	private _timeKeeper: TimeKeeper;
-	private _oldOnError: any;
-	private _timeKeeperListener: IEventsListener;
-	private _errorBuffer: { [stack: string]: any };
-	private _errorFlushTimeout: number;
-
 	protected _config: ITelemetryServiceConfig;
 	protected _sessionId: string;
 	protected _instanceId: string;
 	protected _machineId: string;
 	protected _appenders: ITelemetryAppender[] = [];
-	protected _toUnbind: any[] = [];
+	protected _disposables: IDisposable[] = [];
 
 	constructor(config?: ITelemetryServiceConfig) {
 		this._config = withDefaults(config, DefaultTelemetryServiceConfig);
 		this._sessionId = 'SESSION_ID_NOT_SET';
 		this._timeKeeper = new TimeKeeper();
 
-		this._timeKeeperListener = (events: ITimerEvent[]) => this._onTelemetryTimerEventStop(events);
-		this._timeKeeper.addListener(this._timeKeeperListener);
-		this._toUnbind.push(Errors.errorHandler.addListener(this._onErrorEvent.bind(this)));
+		this._disposables.push(this._timeKeeper);
+		this._disposables.push(this._timeKeeper.addListener(events => this._onTelemetryTimerEventStop(events)));
 
-		this._errorBuffer = Object.create(null);
-		this._enableGlobalErrorHandler();
-		this._errorFlushTimeout = -1;
+		const errorTelemetry = new ErrorTelemetry(this, AbstractTelemetryService.ERROR_FLUSH_TIMEOUT);
+		this._disposables.push(errorTelemetry);
 	}
 
 	private _onTelemetryTimerEventStop(events: ITimerEvent[]): void {
@@ -68,110 +60,6 @@ export abstract class AbstractTelemetryService implements ITelemetryService {
 		}
 	}
 
-	private _onErrorEvent(e: any): void {
-
-		if(!e) {
-			return;
-		}
-
-		let error = Object.create(null);
-
-		// unwrap nested errors from loader
-		if (e.detail && e.detail.stack) {
-			e = e.detail;
-		}
-
-		// work around behavior in workerServer.ts that breaks up Error.stack
-		let stack = Array.isArray(e.stack) ? e.stack.join('\n') : e.stack;
-		let message = e.message ? e.message : safeStringify(e);
-
-		// errors without a stack are not useful telemetry
-		if (!stack) {
-			return;
-		}
-
-		error['message'] = this._cleanupInfo(message);
-		error['stack'] = this._cleanupInfo(stack);
-
-		this._addErrorToBuffer(error);
-	}
-
-	private _addErrorToBuffer(e: any): void {
-		if (this._errorBuffer[e.stack]) {
-			this._errorBuffer[e.stack].count++;
-		} else {
-			e.count = 1;
-			this._errorBuffer[e.stack] = e;
-		}
-		this._tryScheduleErrorFlush();
-	}
-
-	private _tryScheduleErrorFlush(): void {
-		if (this._errorFlushTimeout === -1) {
-			this._errorFlushTimeout = setTimeout(() => this._flushErrorBuffer(), AbstractTelemetryService.ERROR_FLUSH_TIMEOUT);
-		}
-	}
-
-	private _flushErrorBuffer(): void {
-		if (this._errorBuffer) {
-			for (let stack in this._errorBuffer) {
-				this.publicLog('UnhandledError', this._errorBuffer[stack]);
-			}
-		}
-
-		this._errorBuffer = Object.create(null);
-		this._errorFlushTimeout = -1;
-	}
-
-	private _enableGlobalErrorHandler(): void {
-		if (typeof Platform.globals.onerror === 'function') {
-			this._oldOnError = Platform.globals.onerror;
-		}
-
-		let that = this;
-		let newHandler: any = function(message: string, filename: string, line: number, column?: number, e?: any) {
-			that._onUncaughtError(message, filename, line, column, e);
-			if (that._oldOnError) {
-				that._oldOnError.apply(this, arguments);
-			}
-		};
-
-		Platform.globals.onerror = newHandler;
-	}
-
-	private _onUncaughtError(message: string, filename: string, line: number, column?: number, e?: any): void {
-		filename = this._cleanupInfo(filename);
-		message = this._cleanupInfo(message);
-		let data: any = {
-			message: message,
-			filename: filename,
-			line: line,
-			column: column
-		};
-
-		if (e) {
-			data.error = {
-				name: e.name,
-				message: e.message
-			};
-
-			if (e.stack) {
-
-				if (Array.isArray(e.stack)) {
-					e.stack = e.stack.join('\n');
-				}
-
-				data.stack = this._cleanupInfo(e.stack);
-			}
-		}
-
-		if (!data.stack) {
-			data.stack = data.message;
-		}
-
-		this._addErrorToBuffer(data);
-	}
-
 	public getTelemetryInfo(): TPromise<ITelemetryInfo> {
 		return TPromise.as({
 			instanceId: this._instanceId,
@@ -181,19 +69,9 @@ export abstract class AbstractTelemetryService implements ITelemetryService {
 	}
 
 	public dispose(): void {
-		if (this._errorFlushTimeout !== -1) {
-			clearTimeout(this._errorFlushTimeout);
-			this._flushErrorBuffer();
-		}
-
-		while (this._toUnbind.length) {
-			this._toUnbind.pop()();
-		}
-		this._timeKeeper.removeListener(this._timeKeeperListener);
-		this._timeKeeper.dispose();
-
-		for (let i = 0; i < this._appenders.length; i++) {
-			this._appenders[i].dispose();
+		this._disposables = dispose(this._disposables);
+		for (let appender of this._appenders) {
+			appender.dispose();
 		}
 	}
 
@@ -311,7 +189,7 @@ export class MainTelemetryService extends AbstractTelemetryService implements IT
 		}
 
 		// don't send events when the user is optout unless the event is flaged as optin friendly
-		if(!this._config.userOptIn && this._optInFriendly.indexOf(eventName) === -1) {
+		if (!this._config.userOptIn && this._optInFriendly.indexOf(eventName) === -1) {
 			return;
 		}
 

src/vs/platform/telemetry/common/errorTelemetry.ts

+/*---------------------------------------------------------------------------------------------
+ *  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 {globals} from 'vs/base/common/platform';
+import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
+import {IDisposable, toDisposable, dispose} from 'vs/base/common/lifecycle';
+import Errors = require('vs/base/common/errors');
+import {safeStringify} from 'vs/base/common/objects';
+
+interface ErrorEvent {
+	stack: string;
+	message?: string;
+	filename?: string;
+	line?: number;
+	column?: number;
+	error?: { name: string; message: string; };
+}
+
+export default class ErrorTelemetry {
+
+	private _telemetryService: ITelemetryService;
+	private _flushDelay: number;
+	private _flushHandle = -1;
+	private _buffer: { [stack: string]: any } = Object.create(null);
+	private _disposables: IDisposable[] = [];
+
+	constructor(telemetryService: ITelemetryService, flushDelay) {
+		this._telemetryService = telemetryService;
+		this._flushDelay = flushDelay;
+
+		// (1) check for unexpected but handled errors
+		const unbind = Errors.errorHandler.addListener((err) => this._onErrorEvent(err));
+		this._disposables.push(toDisposable(unbind));
+
+		// (2) check for uncaught global errors
+		let oldOnError: Function;
+		let that = this;
+		if (typeof globals.onerror === 'function') {
+			oldOnError = globals.onerror;
+		}
+		globals.onerror = function (message: string, filename: string, line: number, column?: number, e?: any) {
+			that._onUncaughtError(message, filename, line, column, e);
+			if (oldOnError) {
+				oldOnError.apply(this, arguments);
+			}
+		};
+		this._disposables.push(toDisposable(function () {
+			if (oldOnError) {
+				globals.onerror = oldOnError;
+			}
+		}));
+	}
+
+	dispose() {
+		clearTimeout(this._flushHandle);
+		this._flushBuffer();
+		this._disposables = dispose(this._disposables);
+	}
+
+	private _onErrorEvent(err: any): void {
+
+		if (!err) {
+			return;
+		}
+
+		// unwrap nested errors from loader
+		if (err.detail && err.detail.stack) {
+			err = err.detail;
+		}
+
+		// work around behavior in workerServer.ts that breaks up Error.stack
+		let stack = Array.isArray(err.stack) ? err.stack.join('\n') : err.stack;
+		let message = err.message ? err.message : safeStringify(err);
+
+		// errors without a stack are not useful telemetry
+		if (!stack) {
+			return;
+		}
+
+		this._enqueue({ message, stack });
+	}
+
+	private _onUncaughtError(message: string, filename: string, line: number, column?: number, err?: any): void {
+
+		let data: ErrorEvent = {
+			stack: message,
+			message,
+			filename,
+			line,
+			column
+		};
+
+		if (err) {
+			let {name, message, stack} = err;
+			data.error = { name, message };
+			if (stack) {
+				data.stack = Array.isArray(err.stack)
+					? err.stack = err.stack.join('\n')
+					: err.stack;
+			}
+		}
+
+		this._enqueue(data);
+	}
+
+	private _enqueue(e: any): void {
+		if (this._buffer[e.stack]) {
+			this._buffer[e.stack].count++;
+		} else {
+			e.count = 1;
+			this._buffer[e.stack] = e;
+		}
+		if (this._flushHandle === -1) {
+			this._flushHandle = setTimeout(() => {
+				this._flushBuffer();
+				this._flushHandle = -1;
+			}, this._flushDelay);
+		}
+	}
+
+	private _flushBuffer(): void {
+		for (let stack in this._buffer) {
+			this._telemetryService.publicLog('UnhandledError', this._buffer[stack]);
+		}
+		this._buffer = Object.create(null);
+	}
+}

src/vs/platform/telemetry/electron-browser/electronTelemetryService.ts

@@ -14,7 +14,7 @@ import {MainTelemetryService} from 'vs/platform/telemetry/browser/mainTelemetryS
 import {ITelemetryService, ITelemetryInfo, ITelemetryServiceConfig} from 'vs/platform/telemetry/common/telemetry';
 import {IStorageService} from 'vs/platform/storage/common/storage';
 import {IConfigurationRegistry, Extensions} from 'vs/platform/configuration/common/configurationRegistry';
-import {IConfigurationService, IConfigurationServiceEvent, ConfigurationServiceEventTypes} from 'vs/platform/configuration/common/configuration';
+import {IConfigurationService} from 'vs/platform/configuration/common/configuration';
 import {Registry} from 'vs/platform/platform';
 
 
@@ -83,9 +83,9 @@ export class ElectronTelemetryService extends MainTelemetryService implements IT
 		this.publicLog('optInStatus', {optIn: this._config.userOptIn});
 		this.flushBuffer();
 
-		this._toUnbind.push(this.configurationService.addListener(ConfigurationServiceEventTypes.UPDATED, (e: IConfigurationServiceEvent) => {
+		this.configurationService.onDidUpdateConfiguration(e => {
 			this._config.userOptIn = e.config && e.config[TELEMETRY_SECTION_ID] ? e.config[TELEMETRY_SECTION_ID].enableTelemetry : this._config.userOptIn;
-		}));
+		}, undefined, this._disposables);
 	}
 
 	private setupIds(): TPromise<ITelemetryInfo> {