Bug fixing: #147 and #56

AutoCollection/ClientRequests.ts

@@ -143,6 +143,7 @@ class AutoCollectClientRequests {
 
     public dispose() {
         AutoCollectClientRequests.INSTANCE = null;
+        this.enable(false);
         this._isInitialized = false;
     }
 }

AutoCollection/Console.ts

@@ -35,6 +35,7 @@ class AutoCollectConsole {
 
     public dispose() {
         AutoCollectConsole.INSTANCE = null;
+        this.enable(false);
     }
 }
 

AutoCollection/Exceptions.ts

@@ -9,14 +9,16 @@ import Util = require("../Library/Util");
 class AutoCollectExceptions {
 
     public static INSTANCE: AutoCollectExceptions = null;
+    public static get UNCAUGHT_EXCEPTION_HANDLER_NAME(): string { return "uncaughtException"; }
+    public static get UNHANDLED_REJECTION_HANDLER_NAME(): string { return "unhandledRejection"; }
 
     private _exceptionListenerHandle: (reThrow: boolean, error: Error) => void;
     private _rejectionListenerHandle: (reThrow: boolean, error: Error) => void;
     private _client: Client;
     private _isInitialized: boolean;
 
     constructor(client: Client) {
-        if(!!AutoCollectExceptions.INSTANCE) {
+        if (!!AutoCollectExceptions.INSTANCE) {
             throw new Error("Exception tracking should be configured from the applicationInsights object");
         }
 
@@ -29,7 +31,7 @@ class AutoCollectExceptions {
     }
 
     public enable(isEnabled: boolean) {
-        if(isEnabled) {
+        if (isEnabled) {
             this._isInitialized = true;
             var self = this;
             if (!this._exceptionListenerHandle) {
@@ -44,14 +46,14 @@ class AutoCollectExceptions {
                 this._exceptionListenerHandle = handle.bind(this, true);
                 this._rejectionListenerHandle = handle.bind(this, false);
 
-                process.on("uncaughtException", this._exceptionListenerHandle);
-                process.on("unhandledRejection", this._rejectionListenerHandle);
+                process.on(AutoCollectExceptions.UNCAUGHT_EXCEPTION_HANDLER_NAME, this._exceptionListenerHandle);
+                process.on(AutoCollectExceptions.UNHANDLED_REJECTION_HANDLER_NAME, this._rejectionListenerHandle);
             }
 
         } else {
             if (this._exceptionListenerHandle) {
-                process.removeListener("uncaughtException", this._exceptionListenerHandle);
-                process.removeListener("unhandledRejection", this._rejectionListenerHandle);
+                process.removeListener(AutoCollectExceptions.UNCAUGHT_EXCEPTION_HANDLER_NAME, this._exceptionListenerHandle);
+                process.removeListener(AutoCollectExceptions.UNHANDLED_REJECTION_HANDLER_NAME, this._rejectionListenerHandle);
                 this._exceptionListenerHandle = undefined;
                 this._rejectionListenerHandle = undefined;
                 delete this._exceptionListenerHandle;
@@ -67,7 +69,7 @@ class AutoCollectExceptions {
      * @param properties additional properties
      * @param measurements metrics associated with this event, displayed in Metrics Explorer on the portal. Defaults to empty.
      */
-    public static getExceptionData(error: Error, isHandled: boolean, properties?:{ [key: string]: string; }, measurements?:{ [key: string]: number; }): Contracts.Data<Contracts.ExceptionData> {
+    public static getExceptionData(error: Error, isHandled: boolean, properties?: { [key: string]: string; }, measurements?: { [key: string]: number; }): Contracts.Data<Contracts.ExceptionData> {
         var exception = new Contracts.ExceptionData();
         exception.properties = properties;
         exception.severityLevel = Contracts.SeverityLevel.Error;
@@ -144,6 +146,7 @@ class AutoCollectExceptions {
 
     public dispose() {
         AutoCollectExceptions.INSTANCE = null;
+        this.enable(false);
         this._isInitialized = false;
     }
 }

AutoCollection/Performance.ts

@@ -212,6 +212,7 @@ class AutoCollectPerformance {
 
     public dispose() {
         AutoCollectPerformance.INSTANCE = null;
+        this.enable(false);
         this._isInitialized = false;
     }
 }

AutoCollection/ServerRequests.ts

@@ -214,6 +214,7 @@ class AutoCollectServerRequests {
 
     public dispose() {
          AutoCollectServerRequests.INSTANCE = null;
+         this.enable(false);
          this._isInitialized = false;
     }
 }

Library/Client.ts

@@ -343,8 +343,8 @@ class Client {
                 }
 
             } catch (error) {
-                accepted = false;
-                Logging.warn("One of telemetry processors failed, telemetry item will not be sent.", error, envelope);
+                accepted = true;
+                Logging.warn("One of telemetry processors failed, telemetry item will be sent.", error, envelope);
             }
         }
 

Tests/AutoCollection/ClientRequests.tests.ts

@@ -0,0 +1,25 @@
+import assert = require("assert");
+import sinon = require("sinon");
+import ClientRequests = require("../../AutoCollection/ClientRequests")
+
+import AppInsights = require("../../applicationinsights");
+
+describe("AutoCollection/ClientRequests", () => {
+    afterEach(() => {
+        AppInsights.dispose();
+    });
+    describe("#init and #dispose()", () => {
+        it("init should enable and dispose should stop dependencies autocollection", () => {
+
+            var appInsights = AppInsights.setup("key").setAutoCollectDependencies(true);
+            var enableClientRequestsSpy = sinon.spy(ClientRequests.INSTANCE, "enable");
+            appInsights.start();
+
+            assert.equal(enableClientRequestsSpy.callCount, 1, "enable should be called once as part of dependencies autocollection initialization");
+            assert.equal(enableClientRequestsSpy.getCall(0).args[0], true);
+            AppInsights.dispose();
+            assert.equal(enableClientRequestsSpy.callCount, 2, "enable(false) should be called once as part of dependencies autocollection shutdown");
+            assert.equal(enableClientRequestsSpy.getCall(1).args[0], false);
+        });
+    });
+});

Tests/AutoCollection/Console.tests.ts

@@ -0,0 +1,25 @@
+import assert = require("assert");
+import sinon = require("sinon");
+import Console = require("../../AutoCollection/Console")
+
+import AppInsights = require("../../applicationinsights");
+
+describe("AutoCollection/Console", () => {
+    afterEach(() => {
+        AppInsights.dispose();
+    });
+    describe("#init and #dispose()", () => {
+        it("init should enable and dispose should stop console autocollection", () => {
+
+            var appInsights = AppInsights.setup("key").setAutoCollectConsole(true);
+            var enableConsoleRequestsSpy = sinon.spy(Console.INSTANCE, "enable");
+            appInsights.start();
+
+            assert.equal(enableConsoleRequestsSpy.callCount, 1, "enable should be called once as part of console autocollection initialization");
+            assert.equal(enableConsoleRequestsSpy.getCall(0).args[0], true);
+            AppInsights.dispose();
+            assert.equal(enableConsoleRequestsSpy.callCount, 2, "enable(false) should be called once as part of console autocollection shutdown");
+            assert.equal(enableConsoleRequestsSpy.getCall(1).args[0], false);
+        });
+    });
+});

Tests/AutoCollection/Exceptions.tests.ts

@@ -2,39 +2,57 @@ import assert = require("assert");
 import sinon = require("sinon");
 
 import AutoCollectionExceptions = require("../../AutoCollection/Exceptions");
+import Client = require("../../Library/Client");
+import AppInsights = require("../../applicationinsights");
 
 describe("AutoCollection/Exceptions", () => {
     describe("#getExceptionData()", () => {
         var simpleError: Error;
-        
+
         beforeEach(() => {
             try {
                 throw Error("simple error");
-            } catch(e) {
+            } catch (e) {
                 simpleError = e;
             }
         });
-            
+
         it("fills empty 'method' with '<no_method>'", () => {
             simpleError.stack = "  at \t (/path/file.js:12:34)\n" + simpleError.stack;
-            
+
             var exceptionData = AutoCollectionExceptions.getExceptionData(simpleError, false);
-            
+
             var actual = exceptionData.baseData.exceptions[0].parsedStack[0].method;
             var expected = "<no_method>";
-            
+
             assert.deepEqual(actual, expected);
         });
-        
+
         it("fills empty 'method' with '<no_filename>'", () => {
             simpleError.stack = "  at Context.<anonymous> (\t:12:34)\n" + simpleError.stack;
-            
+
             var exceptionData = AutoCollectionExceptions.getExceptionData(simpleError, false);
-            
+
             var actual = exceptionData.baseData.exceptions[0].parsedStack[0].fileName;
             var expected = "<no_filename>";
-            
+
             assert.deepEqual(actual, expected);
         });
-	});
+    });
+
+    describe("#init and dispose()", () => {
+        afterEach(() => {
+            AppInsights.dispose();
+        });
+
+        it("disables autocollection", () => {
+            var processOnSpy = sinon.spy(global.process, "on");
+            var processRemoveListenerSpy = sinon.spy(global.process, "removeListener");
+
+            AppInsights.setup("key").setAutoCollectExceptions(true).start();
+            assert.equal(processOnSpy.callCount, 2, "After enabling exception autocollection, there should be 2 calls to processOnSpy");
+            assert.equal(processOnSpy.getCall(0).args[0], AutoCollectionExceptions.UNCAUGHT_EXCEPTION_HANDLER_NAME);
+            assert.equal(processOnSpy.getCall(1).args[0], AutoCollectionExceptions.UNHANDLED_REJECTION_HANDLER_NAME);
+        });
+    });
 });

Tests/AutoCollection/Performance.tests.ts

@@ -0,0 +1,20 @@
+import assert = require("assert");
+import sinon = require("sinon");
+
+import AppInsights = require("../../applicationinsights");
+
+describe("AutoCollection/Performance", () => {
+    afterEach(() => {
+        AppInsights.dispose();
+    });
+    describe("#init and #dispose()", () => {
+        it("init should enable and dispose should stop autocollection interval", () => {
+            var setIntervalSpy = sinon.spy(global, "setInterval");
+            var clearIntervalSpy = sinon.spy(global, "clearInterval");
+            AppInsights.setup("key").setAutoCollectPerformance(true).start();
+            assert.equal(setIntervalSpy.callCount, 1, "setInteval should be called once as part of performance initialization");
+            AppInsights.dispose();
+            assert.equal(clearIntervalSpy.callCount, 1, "clearInterval should be called once as part of performance shutdown");
+        });
+    });
+});

Tests/AutoCollection/ServerRequests.tests.ts

@@ -0,0 +1,23 @@
+import assert = require("assert");
+import sinon = require("sinon");
+import ServerRequests = require("../../AutoCollection/ServerRequests")
+import AppInsights = require("../../applicationinsights");
+
+describe("AutoCollection/ServerRequests", () => {
+    afterEach(() => {
+        AppInsights.dispose();
+    });
+    describe("#init and #dispose()", () => {
+        it("init should enable and dispose should stop server requests autocollection", () => {
+        var appInsights = AppInsights.setup("key").setAutoCollectRequests(true);
+            var enableServerRequestsSpy = sinon.spy(ServerRequests.INSTANCE, "enable");
+            appInsights.start();
+
+            assert.equal(enableServerRequestsSpy.callCount, 1, "enable should be called once as part of requests autocollection initialization");
+            assert.equal(enableServerRequestsSpy.getCall(0).args[0], true);
+            AppInsights.dispose();
+            assert.equal(enableServerRequestsSpy.callCount, 2, "enable(false) should be called once as part of requests autocollection shutdown");
+            assert.equal(enableServerRequestsSpy.getCall(1).args[0], false);
+        });
+    });
+});

Tests/Library/Client.tests.ts

@@ -223,7 +223,7 @@ describe("Library/Client", () => {
                 return new eventEmitter.EventEmitter();
             },
             statusCode: 200,
-            headers: <{[id: string]: string}>{},
+            headers: <{ [id: string]: string }>{},
             getHeader: function (name: string) { return this.headers[name]; },
             setHeader: function (name: string, value: string) { this.headers[name] = value; },
         };
@@ -257,7 +257,7 @@ describe("Library/Client", () => {
             agent: {
                 protocol: 'http'
             },
-            headers: <{[id: string]: string}>{
+            headers: <{ [id: string]: string }>{
                 host: "bing.com"
             },
             getHeader: function (name: string) { return this.headers[name]; },
@@ -445,9 +445,9 @@ describe("Library/Client", () => {
                 trackStub.reset();
                 clock.reset();
                 client.trackDependencyRequest({
-                        host: 'bing.com',
-                        path: '/search?q=test'
-                    },
+                    host: 'bing.com',
+                    path: '/search?q=test'
+                },
                     <any>request, properties);
 
                 // response event was not emitted yet
@@ -523,9 +523,9 @@ describe("Library/Client", () => {
                 trackStub.reset();
                 clock.reset();
                 client.trackDependencyRequest({
-                        host: 'bing.com',
-                        path: '/search?q=test'
-                    },
+                    host: 'bing.com',
+                    path: '/search?q=test'
+                },
                     <any>request, properties);
 
                 // The client's correlationId should have been added as the request source correlationId header.
@@ -557,9 +557,9 @@ describe("Library/Client", () => {
 
                 client.config.correlationHeaderExcludedDomains = ["*.domain.com"]
                 client.trackDependencyRequest({
-                        host: 'excluded.domain.com',
-                        path: '/search?q=test'
-                    },
+                    host: 'excluded.domain.com',
+                    path: '/search?q=test'
+                },
                     <any>request, properties);
 
                 // The client's correlationId should NOT have been added for excluded domains
@@ -635,9 +635,9 @@ describe("Library/Client", () => {
             mockData.properties = {};
             var env = client.getEnvelope(mockData);
             assert.deepEqual(env.tags, client.context.tags, "tags are set by default");
-            var customTag = <{[id: string]: string}>{ "ai.cloud.roleInstance": "override" };
-            var expected: {[id: string]: string} = {};
-            for(var tag in client.context.tags) {
+            var customTag = <{ [id: string]: string }>{ "ai.cloud.roleInstance": "override" };
+            var expected: { [id: string]: string } = {};
+            for (var tag in client.context.tags) {
                 expected[tag] = customTag[tag] || client.context.tags[tag];
             }
             env = client.getEnvelope(mockData, <any>customTag);
@@ -703,7 +703,7 @@ describe("Library/Client", () => {
                 return true;
             });
 
-            client.track(mockData, null, {"name": expectedName});
+            client.track(mockData, null, { "name": expectedName });
 
             assert.equal(sendStub.callCount, 1, "send called once");
 
@@ -748,16 +748,23 @@ describe("Library/Client", () => {
             assert.ok(sendStub.notCalled, "send should not be called");
         });
 
-        it("envelope is rejected when processor throws exception", () => {
+        it("envelope is sent when processor throws exception", () => {
             trackStub.restore();
 
             client.addTelemetryProcessor((env): boolean => {
                 throw "telemetry processor failed";
             });
 
+            client.addTelemetryProcessor((env): boolean => {
+                env.name = "more data";
+                return true;
+            });
+
             client.track(mockData);
 
-            assert.ok(sendStub.notCalled, "send should not be called");
+            assert.ok(sendStub.called, "send should be called despite telemetry processor failure");
+            var actualData = sendStub.firstCall.args[0] as Contracts.Envelope;
+            assert.equal(actualData.name, "more data", "more data is added as part of telemetry processor");
         });
     });