From 1215d438a9cf4e208f4543aa8caa93a7cb765630 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 6 Nov 2017 00:10:23 +0200 Subject: [PATCH] Do not break CLI process in case analytics fail In case we are unable to start the Analytics Broker process, CLI will fail. But analytics errors should not break user's workflow, so catch the error and ensure the actions will continue. Add tests to verify the behavior. --- lib/services/analytics/analytics-service.ts | 11 +- test/services/analytics/analytics-service.ts | 295 +++++++++++++++++++ test/stubs.ts | 5 +- 3 files changed, 308 insertions(+), 3 deletions(-) create mode 100644 test/services/analytics/analytics-service.ts diff --git a/lib/services/analytics/analytics-service.ts b/lib/services/analytics/analytics-service.ts index 298de76d42..3c28cd138f 100644 --- a/lib/services/analytics/analytics-service.ts +++ b/lib/services/analytics/analytics-service.ts @@ -6,7 +6,7 @@ import { isInteractive } from '../../common/helpers'; import { DeviceTypes, AnalyticsClients } from "../../common/constants"; export class AnalyticsService extends AnalyticsServiceBase { - private static ANALYTICS_BROKER_START_TIMEOUT = 30 * 1000; + private static ANALYTICS_BROKER_START_TIMEOUT = 10 * 1000; private brokerProcess: ChildProcess; constructor(protected $logger: ILogger, @@ -182,7 +182,14 @@ export class AnalyticsService extends AnalyticsServiceBase { } private async sendMessageToBroker(message: ITrackingInformation): Promise { - const broker = await this.getAnalyticsBroker(); + let broker: ChildProcess; + try { + broker = await this.getAnalyticsBroker(); + } catch (err) { + this.$logger.trace("Unable to get broker instance due to error: ", err); + return; + } + return new Promise((resolve, reject) => { if (broker && broker.connected) { try { diff --git a/test/services/analytics/analytics-service.ts b/test/services/analytics/analytics-service.ts new file mode 100644 index 0000000000..eb3ae50f08 --- /dev/null +++ b/test/services/analytics/analytics-service.ts @@ -0,0 +1,295 @@ +import { AnalyticsService } from "../../../lib/services/analytics/analytics-service"; +import { Yok } from "../../../lib/common/yok"; +import * as stubs from "../../stubs"; +import { assert } from "chai"; +import { EventEmitter } from "events"; +import { AnalyticsClients } from "../../../lib/common/constants"; + +const helpers = require("../../../lib/common/helpers"); +const originalIsInteractive = helpers.isInteractive; + +const trackFeatureUsage = "TrackFeatureUsage"; +const createTestInjector = (): IInjector => { + const testInjector = new Yok(); + testInjector.register("options", {}); + testInjector.register("logger", stubs.LoggerStub); + + testInjector.register("staticConfig", { + disableAnalytics: false, + TRACK_FEATURE_USAGE_SETTING_NAME: trackFeatureUsage, + PATH_TO_BOOTSTRAP: "pathToBootstrap.js" + }); + + testInjector.register("prompter", { + + }); + + testInjector.register("userSettingsService", { + getSettingValue: async (settingName: string): Promise => { + return "true"; + } + }); + testInjector.register("analyticsSettingsService", { + canDoRequest: (): Promise => Promise.resolve(true) + }); + testInjector.register("osInfo", {}); + testInjector.register("childProcess", {}); + testInjector.register("processService", { + attachToProcessExitSignals: (context: any, callback: () => void): void => undefined + }); + testInjector.register("projectDataService", {}); + testInjector.register("mobileHelper", {}); + + return testInjector; +}; + +describe("analyticsService", () => { + afterEach(() => { + helpers.isInteractive = originalIsInteractive; + }); + + describe("trackInGoogleAnalytics", () => { + describe("does not track", () => { + const testScenario = async (configuration: { + disableAnalytics: boolean, + assertMessage: string, + userSettingsServiceOpts?: { trackFeatureUsageValue: string, defaultValue: string } + }) => { + const testInjector = createTestInjector(); + const staticConfig = testInjector.resolve("staticConfig"); + staticConfig.disableAnalytics = configuration.disableAnalytics; + + configuration.userSettingsServiceOpts = configuration.userSettingsServiceOpts || { trackFeatureUsageValue: "false", defaultValue: "true" }; + const userSettingsService = testInjector.resolve("userSettingsService"); + userSettingsService.getSettingValue = async (settingName: string): Promise => { + if (settingName === trackFeatureUsage) { + return configuration.userSettingsServiceOpts.trackFeatureUsageValue; + } + + return configuration.userSettingsServiceOpts.defaultValue; + }; + + let isChildProcessSpawned = false; + const childProcess = testInjector.resolve("childProcess"); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + isChildProcessSpawned = true; + }; + + const analyticsService = testInjector.resolve(AnalyticsService); + await analyticsService.trackInGoogleAnalytics({ + googleAnalyticsDataType: GoogleAnalyticsDataType.Page, + customDimensions: { + customDimension1: "value1" + } + }); + + assert.isFalse(isChildProcessSpawned, configuration.assertMessage); + }; + + it("does not track when staticConfig's disableAnalytics is true", () => { + return testScenario({ + disableAnalytics: true, + assertMessage: "When staticConfig.disableAnalytics is true, no child process should be started, i.e. we should not track anything." + }); + }); + + it(`does not track when ${trackFeatureUsage} is not true`, async () => { + await testScenario({ + disableAnalytics: false, + assertMessage: `When ${trackFeatureUsage} is false, no child process should be started, i.e. we should not track anything.`, + userSettingsServiceOpts: { + trackFeatureUsageValue: "false", defaultValue: "true" + } + }); + + await testScenario({ + disableAnalytics: false, + assertMessage: `When ${trackFeatureUsage} is undefined, no child process should be started, i.e. we should not track anything.`, + userSettingsServiceOpts: { + trackFeatureUsageValue: undefined, defaultValue: "true" + } + }); + }); + + }); + + const getSpawnedProcess = (): any => { + const spawnedProcess: any = new EventEmitter(); + spawnedProcess.stdout = new EventEmitter(); + spawnedProcess.stderr = new EventEmitter(); + spawnedProcess.unref = (): void => undefined; + return spawnedProcess; + }; + + describe("does not fail", () => { + const assertExpectedError = async (testInjector: IInjector, opts: { isChildProcessSpawned: boolean, expectedErrorMessage: string }) => { + const analyticsService = testInjector.resolve(AnalyticsService); + await analyticsService.trackInGoogleAnalytics({ + googleAnalyticsDataType: GoogleAnalyticsDataType.Page, + customDimensions: { + customDimension1: "value1" + } + }); + + assert.isTrue(opts.isChildProcessSpawned); + const logger = testInjector.resolve("logger"); + assert.isTrue(logger.traceOutput.indexOf(opts.expectedErrorMessage) !== -1); + }; + + const setupTest = (expectedErrorMessage: string): any => { + const testInjector = createTestInjector(); + const opts = { + isChildProcessSpawned: false, + expectedErrorMessage + }; + + const childProcess = testInjector.resolve("childProcess"); + return { + testInjector, + opts, + childProcess + }; + }; + + it("when unable to start broker process", async () => { + const { testInjector, childProcess, opts } = setupTest("Unable to get broker instance due to error: Error: custom error"); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + throw new Error("custom error"); + }; + + await assertExpectedError(testInjector, opts); + }); + + it("when broker cannot start for required timeout", async () => { + const { testInjector, childProcess, opts } = setupTest("Unable to get broker instance due to error: Error: Unable to start Analytics Broker process."); + const originalSetTimeout = setTimeout; + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + global.setTimeout = (callback: (...args: any[]) => void, ms: number, ...otherArgs: any[]) => originalSetTimeout(callback, 1); + return getSpawnedProcess(); + }; + + await assertExpectedError(testInjector, opts); + + global.setTimeout = originalSetTimeout; + }); + + it("when broker is not connected", async () => { + const { testInjector, childProcess, opts } = setupTest("Broker not found or not connected."); + + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = false; + spawnedProcess.send = (): void => undefined; + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + return spawnedProcess; + }; + + await assertExpectedError(testInjector, opts); + }); + + it("when sending message fails", async () => { + const { testInjector, childProcess, opts } = setupTest("Error while trying to send message to broker: Error: Failed to sent data."); + + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = true; + spawnedProcess.send = (): void => { + throw new Error("Failed to sent data."); + }; + + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + return spawnedProcess; + }; + + await assertExpectedError(testInjector, opts); + }); + }); + + describe("sends correct message to broker", () => { + const setupTest = (expectedResult: any, dataToSend: any, terminalOpts?: { isInteractive: boolean }): { testInjector: IInjector, opts: any } => { + helpers.isInteractive = () => terminalOpts ? terminalOpts.isInteractive : true; + + const testInjector = createTestInjector(); + const opts = { + isChildProcessSpawned: false, + expectedResult, + dataToSend, + messageSent: null + }; + + const childProcess = testInjector.resolve("childProcess"); + childProcess.spawn = (command: string, args?: string[], options?: any): any => { + opts.isChildProcessSpawned = true; + const spawnedProcess: any = getSpawnedProcess(); + + spawnedProcess.connected = true; + spawnedProcess.send = (msg: any, action: () => void): void => { + opts.messageSent = msg; + action(); + }; + + setTimeout(() => spawnedProcess.emit("message", AnalyticsMessages.BrokerReadyToReceive), 1); + + return spawnedProcess; + }; + + return { + testInjector, + opts + }; + }; + + const assertExpectedResult = async (testInjector: IInjector, opts: { isChildProcessSpawned: boolean, expectedResult: any, messageSent: any, dataToSend: any }) => { + const analyticsService = testInjector.resolve(AnalyticsService); + await analyticsService.trackInGoogleAnalytics(opts.dataToSend); + + assert.isTrue(opts.isChildProcessSpawned); + assert.deepEqual(opts.messageSent, opts.expectedResult); + }; + + const getDataToSend = (gaDataType: string): any => ({ + googleAnalyticsDataType: gaDataType, + customDimensions: { + customDimension1: "value1" + } + }); + + const getExpectedResult = (gaDataType: string, analyticsClient?: string): any => ({ + type: "googleAnalyticsData", + category: "CLI", + googleAnalyticsDataType: gaDataType, + customDimensions: { customDimension1: "value1", cd5: analyticsClient || "CLI" } + }); + + _.each([GoogleAnalyticsDataType.Page, GoogleAnalyticsDataType.Event], (googleAnalyticsDataType: string) => { + it(`when data is ${googleAnalyticsDataType}`, async () => { + const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType), getDataToSend(googleAnalyticsDataType)); + await assertExpectedResult(testInjector, opts); + }); + + it(`when data is ${googleAnalyticsDataType} and terminal is not interactive`, async () => { + const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType, AnalyticsClients.Unknown), getDataToSend(googleAnalyticsDataType), { isInteractive: false }); + await assertExpectedResult(testInjector, opts); + }); + + _.each([true, false], (isInteractive) => { + it(`when data is ${googleAnalyticsDataType} terminal is ${isInteractive ? "" : "not "}interactive and --analyticsClient is passed`, async () => { + const analyticsClient = "AnalyticsClient"; + + const { testInjector, opts } = setupTest(getExpectedResult(googleAnalyticsDataType, analyticsClient), getDataToSend(googleAnalyticsDataType), { isInteractive }); + const options = testInjector.resolve("options"); + options.analyticsClient = analyticsClient; + + await assertExpectedResult(testInjector, opts); + }); + }); + }); + }); + }); +}); diff --git a/test/stubs.ts b/test/stubs.ts index f92ea63a1c..30f9613946 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -13,9 +13,12 @@ export class LoggerStub implements ILogger { warnWithLabel(...args: string[]): void { } info(...args: string[]): void { } debug(...args: string[]): void { } - trace(...args: string[]): void { } + trace(...args: string[]): void { + this.traceOutput += util.format.apply(null, args) + "\n"; + } public output = ""; + public traceOutput = ""; out(...args: string[]): void { this.output += util.format.apply(null, args) + "\n";