Analytics fixes (#3110)

Move lockfile service to common and lock operation for writing user settings file

Add new lockfile service (use lockfile npm package) and lock all operations for reading/writing user-settings.json.
These operations may be executed in different processes, so they must be synced between them.

Clean obsolete code for lockfile in services that do not longer use it.

Ensure failures in eqatec analytics will not prevent tracking in google analytics by try-catching execution in broker service.

Author: rosen-vladimirov

lib/bootstrap.ts

@@ -71,7 +71,6 @@ $injector.require("itmsTransporterService", "./services/itmstransporter-service"
 
 $injector.requirePublic("npm", "./node-package-manager");
 $injector.require("npmInstallationManager", "./npm-installation-manager");
-$injector.require("lockfile", "./lockfile");
 $injector.require("dynamicHelpProvider", "./dynamic-help-provider");
 $injector.require("mobilePlatformsCapabilities", "./mobile-platforms-capabilities");
 $injector.require("commandsServiceProvider", "./providers/commands-service-provider");

lib/common

@@ -318,12 +318,6 @@ interface IApplicationPackage {
 	time: Date;
 }
 
-interface ILockFile {
-	lock(): void;
-	unlock(): void;
-	check(): boolean;
-}
-
 interface IOpener {
 	open(target: string, appname: string): void;
 }

lib/declarations.d.ts

@@ -1,3 +1,5 @@
+// NOTE: This file is used to track data in a separate process.
+// The instances here are not shared with the ones in main CLI process.
 import * as fs from "fs";
 import { AnalyticsBroker } from "./analytics-broker";
 

lib/definitions/lockfile.d.ts

@@ -17,29 +17,32 @@ export class AnalyticsBroker implements IAnalyticsBroker {
 		private $injector: IInjector) { }
 
 	public async sendDataForTracking(trackInfo: ITrackingInformation): Promise<void> {
-		const eqatecProvider = await this.getEqatecAnalyticsProvider();
-		const googleProvider = await this.getGoogleAnalyticsProvider();
-
-		switch (trackInfo.type) {
-			case TrackingTypes.Exception:
-				await eqatecProvider.trackError(<IExceptionsTrackingInformation>trackInfo);
-				break;
-			case TrackingTypes.Feature:
-				await eqatecProvider.trackInformation(<IFeatureTrackingInformation>trackInfo);
-				break;
-			case TrackingTypes.AcceptTrackFeatureUsage:
-				await eqatecProvider.acceptFeatureUsageTracking(<IAcceptUsageReportingInformation>trackInfo);
-				break;
-			case TrackingTypes.GoogleAnalyticsData:
-				await googleProvider.trackHit(<IGoogleAnalyticsTrackingInformation>trackInfo);
-				break;
-			case TrackingTypes.Finish:
-				await eqatecProvider.finishTracking();
-				break;
-			default:
-				throw new Error(`Invalid tracking type: ${trackInfo.type}`);
+		try {
+			const eqatecProvider = await this.getEqatecAnalyticsProvider();
+			const googleProvider = await this.getGoogleAnalyticsProvider();
+
+			switch (trackInfo.type) {
+				case TrackingTypes.Exception:
+					await eqatecProvider.trackError(<IExceptionsTrackingInformation>trackInfo);
+					break;
+				case TrackingTypes.Feature:
+					await eqatecProvider.trackInformation(<IFeatureTrackingInformation>trackInfo);
+					break;
+				case TrackingTypes.AcceptTrackFeatureUsage:
+					await eqatecProvider.acceptFeatureUsageTracking(<IAcceptUsageReportingInformation>trackInfo);
+					break;
+				case TrackingTypes.GoogleAnalyticsData:
+					await googleProvider.trackHit(<IGoogleAnalyticsTrackingInformation>trackInfo);
+					break;
+				case TrackingTypes.Finish:
+					await eqatecProvider.finishTracking();
+					break;
+				default:
+					throw new Error(`Invalid tracking type: ${trackInfo.type}`);
+			}
+		} catch (err) {
+			// So, lets ignore the error for now until we find out what to do with it.
 		}
-
 	}
 
 }

lib/lockfile.ts

@@ -65,10 +65,11 @@ export class AnalyticsService extends AnalyticsServiceBase {
 	public async trackEventActionInGoogleAnalytics(data: IEventActionData): Promise<void> {
 		const device = data.device;
 		const platform = device ? device.deviceInfo.platform : data.platform;
+		const normalizedPlatform = platform ? this.$mobileHelper.normalizePlatformName(platform) : platform;
 		const isForDevice = device ? !device.isEmulator : data.isForDevice;
 
 		let label: string = "";
-		label = this.addDataToLabel(label, platform);
+		label = this.addDataToLabel(label, normalizedPlatform);
 
 		// In some cases (like in case action is Build and platform is Android), we do not know if the deviceType is emulator or device.
 		// Just exclude the device_type in this case.
@@ -182,7 +183,15 @@ export class AnalyticsService extends AnalyticsServiceBase {
 
 	private async sendMessageToBroker(message: ITrackingInformation): Promise<void> {
 		const broker = await this.getAnalyticsBroker();
-		return new Promise<void>((resolve, reject) => broker.send(message, resolve));
+		return new Promise<void>((resolve, reject) => {
+			if (broker && broker.connected) {
+				try {
+					broker.send(message, resolve);
+				} catch (err) {
+					this.$logger.trace("Error while trying to send message to broker:", err);
+				}
+			}
+		});
 	}
 }
 

lib/services/analytics/analytics-broker-process.ts

@@ -3,9 +3,14 @@ import * as userSettingsServiceBaseLib from "../common/services/user-settings-se
 
 class UserSettingsService extends userSettingsServiceBaseLib.UserSettingsServiceBase {
 	constructor($fs: IFileSystem,
-		$options: IOptions) {
+		$options: IOptions,
+		$lockfile: ILockFile) {
 		const userSettingsFilePath = path.join($options.profileDir, "user-settings.json");
-		super(userSettingsFilePath, $fs);
+		super(userSettingsFilePath, $fs, $lockfile);
+	}
+
+	public async loadUserSettingsFile(): Promise<void> {
+		await this.loadUserSettingsData();
 	}
 }
 $injector.register("userSettingsService", UserSettingsService);

lib/services/analytics/analytics-broker.ts

@@ -47,7 +47,7 @@
     "ios-device-lib": "0.4.9",
     "ios-mobileprovision-finder": "1.0.10",
     "ios-sim-portable": "3.1.1",
-    "lockfile": "1.0.1",
+    "lockfile": "1.0.3",
     "lodash": "4.13.1",
     "log4js": "1.0.1",
     "marked": "0.3.6",
@@ -87,6 +87,7 @@
     "@types/chai": "4.0.1",
     "@types/chai-as-promised": "0.0.31",
     "@types/chokidar": "1.6.0",
+    "@types/lockfile": "1.0.0",
     "@types/node": "6.0.61",
     "@types/qr-image": "3.2.0",
     "@types/request": "0.0.45",

lib/services/analytics/analytics-service.ts

@@ -15,7 +15,6 @@ function createTestInjector(): IInjector {
 
 	testInjector.register("config", ConfigLib.Configuration);
 	testInjector.register("logger", LoggerLib.Logger);
-	testInjector.register("lockfile", {});
 	testInjector.register("errors", ErrorsLib.Errors);
 	testInjector.register("options", OptionsLib.Options);
 	testInjector.register("fs", FsLib.FileSystem);

lib/services/user-settings-service.ts

@@ -23,7 +23,6 @@ import { ProjectFilesProvider } from "../lib/providers/project-files-provider";
 import { MobilePlatformsCapabilities } from "../lib/mobile-platforms-capabilities";
 import { DevicePlatformsConstants } from "../lib/common/mobile/device-platforms-constants";
 import { XmlValidator } from "../lib/xml-validator";
-import { LockFile } from "../lib/lockfile";
 import ProjectChangesLib = require("../lib/services/project-changes-service");
 import { Messages } from "../lib/common/messages/messages";
 import { NodeModulesDependenciesBuilder } from "../lib/tools/node-modules/node-modules-dependencies-builder";
@@ -48,7 +47,6 @@ function createTestInjector(): IInjector {
 	testInjector.register("platformService", PlatformServiceLib.PlatformService);
 	testInjector.register("logger", stubs.LoggerStub);
 	testInjector.register("npmInstallationManager", {});
-	testInjector.register("lockfile", LockFile);
 	testInjector.register("prompter", {});
 	testInjector.register("sysInfo", {});
 	testInjector.register("androidProjectService", {});

npm-shrinkwrap.json

@@ -112,7 +112,6 @@ function createTestInjector() {
 	testInjector.registerCommand("platform|remove", PlatformRemoveCommandLib.RemovePlatformCommand);
 	testInjector.registerCommand("platform|update", PlatformUpdateCommandLib.UpdatePlatformCommand);
 	testInjector.registerCommand("platform|clean", PlatformCleanCommandLib.CleanCommand);
-	testInjector.register("lockfile", {});
 	testInjector.register("resources", {});
 	testInjector.register("commandsServiceProvider", {
 		registerDynamicSubCommands: () => { /* intentionally left blank */ }

package.json

@@ -39,7 +39,6 @@ function createTestInjector() {
 	testInjector.register('projectDataService', stubs.ProjectDataService);
 	testInjector.register('prompter', {});
 	testInjector.register('sysInfo', {});
-	testInjector.register('lockfile', stubs.LockFile);
 	testInjector.register("commandsService", {
 		tryExecuteCommand: () => { /* intentionally left blank */ }
 	});

test/npm-installation-manager.ts

@@ -69,7 +69,6 @@ function createTestInjector() {
 		registerDynamicSubCommands: () => { /* intentionally empty body */ }
 	});
 	testInjector.register("hostInfo", HostInfo);
-	testInjector.register("lockfile", {});
 	testInjector.register("projectHelper", ProjectHelper);
 
 	testInjector.register("pluginsService", PluginsService);

test/npm-support.ts

@@ -145,7 +145,6 @@ class ProjectIntegrationTest {
 		this.testInjector.register("npmInstallationManager", NpmInstallationManager);
 		this.testInjector.register("npm", NpmLib.NodePackageManager);
 		this.testInjector.register("httpClient", HttpClientLib.HttpClient);
-		this.testInjector.register("lockfile", stubs.LockFile);
 
 		this.testInjector.register("options", Options);
 		this.testInjector.register("hostInfo", HostInfo);