Repository: microsoft/ApplicationInsights-node.js
Pull request #1282 (2024-02-17 02:25:40)

[Beta] Fix Auto Exception Collection
Exceptions were not being captured before process exit. This PR unifies how we handle `uncaughtException` and `unhandledRejection` events between all Node.js versions, as the `uncaughtExceptionMonitor` event listener was behaving unpredictably and not respecting the fulfillment of the `.forceFlush()` function before exiting program execution. 


Patch:
diff --git a/src/logs/exceptions.ts b/src/logs/exceptions.ts
index 91ba17d..d98c804 100644
--- a/src/logs/exceptions.ts
+++ b/src/logs/exceptions.ts
@@ -6,86 +6,57 @@ import { LogApi } from "../shim/logsApi";
 import { LoggerProvider } from "@opentelemetry/sdk-logs";
 
 type ExceptionHandle = "uncaughtExceptionMonitor" | "uncaughtException" | "unhandledRejection";
-const UNCAUGHT_EXCEPTION_MONITOR_HANDLER_NAME: ExceptionHandle = "uncaughtExceptionMonitor";
 const UNCAUGHT_EXCEPTION_HANDLER_NAME: ExceptionHandle = "uncaughtException";
 const UNHANDLED_REJECTION_HANDLER_NAME: ExceptionHandle = "unhandledRejection";
 const FALLBACK_ERROR_MESSAGE =
     "A promise was rejected without providing an error. Application Insights generated this error stack for you.";
 
 export class AutoCollectExceptions {
-    private _canUseUncaughtExceptionMonitor = false;
     private _exceptionListenerHandle?: (error: Error | undefined) => void;
     private _rejectionListenerHandle?: (error: Error | undefined) => void;
     private _client: LogApi;
 
     constructor(client: LogApi) {
         this._client = client;
-        // Only use for 13.7.0+
-        const nodeVer = process.versions.node.split(".");
-        this._canUseUncaughtExceptionMonitor =
-            parseInt(nodeVer[0]) > 13 || (parseInt(nodeVer[0]) === 13 && parseInt(nodeVer[1]) >= 7);
-
-        // For scenarios like Promise.reject(), an error won't be passed to the handle. Create a placeholder
-        // error for these scenarios.
-        if (this._canUseUncaughtExceptionMonitor) {
-            // Node.js >= 13.7.0, use uncaughtExceptionMonitor. It handles both promises and exceptions
-            this._exceptionListenerHandle = this._handleException.bind(
-                this,
-                false,
-                UNCAUGHT_EXCEPTION_MONITOR_HANDLER_NAME
-            ); // never rethrows
-            (<any>process).on(
-                UNCAUGHT_EXCEPTION_MONITOR_HANDLER_NAME,
-                this._exceptionListenerHandle
-            );
-        } else {
-            this._exceptionListenerHandle = this._handleException.bind(
-                this,
-                true,
-                UNCAUGHT_EXCEPTION_HANDLER_NAME
-            );
-            this._rejectionListenerHandle = this._handleException.bind(
-                this,
-                false,
-                UNHANDLED_REJECTION_HANDLER_NAME
-            ); // never rethrows
-            (<any>process).on(
-                UNCAUGHT_EXCEPTION_HANDLER_NAME,
-                this._exceptionListenerHandle
-            );
-            (<any>process).on(
-                UNHANDLED_REJECTION_HANDLER_NAME,
-                this._rejectionListenerHandle
-            );
-        }
+        this._exceptionListenerHandle = this._handleException.bind(
+            this,
+            true,
+            UNCAUGHT_EXCEPTION_HANDLER_NAME
+        );
+        this._rejectionListenerHandle = this._handleException.bind(
+            this,
+            false,
+            UNHANDLED_REJECTION_HANDLER_NAME
+        ); // never rethrows
+        (<any>process).on(
+            UNCAUGHT_EXCEPTION_HANDLER_NAME,
+            this._exceptionListenerHandle
+        );
+        (<any>process).on(
+            UNHANDLED_REJECTION_HANDLER_NAME,
+            this._rejectionListenerHandle
+        );
     }
 
     public shutdown() {
         if (this._exceptionListenerHandle) {
-            if (this._canUseUncaughtExceptionMonitor) {
+            if (this._exceptionListenerHandle) {
                 process.removeListener(
-                    UNCAUGHT_EXCEPTION_MONITOR_HANDLER_NAME,
+                    UNCAUGHT_EXCEPTION_HANDLER_NAME,
                     this._exceptionListenerHandle
                 );
-            } else {
-                if (this._exceptionListenerHandle) {
-                    process.removeListener(
-                        UNCAUGHT_EXCEPTION_HANDLER_NAME,
-                        this._exceptionListenerHandle
-                    );
-                }
-                if (this._rejectionListenerHandle) {
-                    process.removeListener(
-                        UNHANDLED_REJECTION_HANDLER_NAME,
-                        this._rejectionListenerHandle
-                    );
-                }
             }
-            this._exceptionListenerHandle = undefined;
-            this._rejectionListenerHandle = undefined;
-            delete this._exceptionListenerHandle;
-            delete this._rejectionListenerHandle;
+            if (this._rejectionListenerHandle) {
+                process.removeListener(
+                    UNHANDLED_REJECTION_HANDLER_NAME,
+                    this._rejectionListenerHandle
+                );
+            }
         }
+        this._exceptionListenerHandle = undefined;
+        this._rejectionListenerHandle = undefined;
+        delete this._exceptionListenerHandle;
+        delete this._rejectionListenerHandle;
     }
 
     private _handleException(
@@ -95,13 +66,18 @@ export class AutoCollectExceptions {
     ) {
         if (this._client) {
             this._client.trackException({ exception: error });
-            (logs.getLoggerProvider() as LoggerProvider).forceFlush();
-            // only rethrow when we are the only listener
-            if (reThrow && name && process.listeners(name as any).length === 1) {
-                // eslint-disable-next-line no-console
-                console.error(error);
-                // eslint-disable-next-line no-process-exit
-                process.exit(1);
+            try {
+                (logs.getLoggerProvider() as LoggerProvider).forceFlush().then(() => {
+                    // only rethrow when we are the only listener
+                    if (reThrow && name && process.listeners(name as any).length === 1) {
+                        // eslint-disable-next-line no-console
+                        console.error(error);
+                        // eslint-disable-next-line no-process-exit
+                        process.exit(1);
+                    }
+                });
+            } catch (error) {
+                console.error(`Could not get the loggerProvider upon handling a tracked exception: ${error}`);
             }
         } else {
             // eslint-disable-next-line no-console
diff --git a/src/main.ts b/src/main.ts
index 0fabbfd..faf326e 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -36,10 +36,14 @@ export function useAzureMonitor(options?: AzureMonitorOpenTelemetryOptions) {
         exceptions = new AutoCollectExceptions(logApi);
     }
     if (internalConfig.enableAutoCollectPerformance) {
-        perfCounters = new PerformanceCounterMetrics(internalConfig);
-        // Add SpanProcessor to calculate Request Metrics
-        if (typeof (trace.getTracerProvider() as BasicTracerProvider).addSpanProcessor === "function") {
-            (trace.getTracerProvider() as BasicTracerProvider).addSpanProcessor(new AzureMonitorSpanProcessor(perfCounters));
+        try {
+            perfCounters = new PerformanceCounterMetrics(internalConfig);
+            // Add SpanProcessor to calculate Request Metrics
+            if (typeof (trace.getTracerProvider() as BasicTracerProvider).addSpanProcessor === "function") {
+                (trace.getTracerProvider() as BasicTracerProvider).addSpanProcessor(new AzureMonitorSpanProcessor(perfCounters));
+            }
+        } catch (err) {
+            diag.error("Failed to initialize PerformanceCounterMetrics: ", err);
         }
     }
     autoCollectLogs.enable(internalConfig.instrumentationOptions);
diff --git a/src/shim/telemetryClient.ts b/src/shim/telemetryClient.ts
index 0be98ee..ef0c174 100644
--- a/src/shim/telemetryClient.ts
+++ b/src/shim/telemetryClient.ts
@@ -48,14 +48,18 @@ export class TelemetryClient {
         // Parse shim config to Azure Monitor options
         this._options = this.config.parseConfig();
         useAzureMonitor(this._options);
-        // LoggerProvider would be initialized when client is instantiated
-        // Get Logger from global provider
-        this._logApi = new LogApi(logs.getLogger("ApplicationInsightsLogger"));
-        this._attributeSpanProcessor = new AttributeSpanProcessor({ ...this.context.tags, ...this.commonProperties });
-        ((trace.getTracerProvider() as ProxyTracerProvider).getDelegate() as NodeTracerProvider).addSpanProcessor(this._attributeSpanProcessor);
+        try {
+            // LoggerProvider would be initialized when client is instantiated
+            // Get Logger from global provider
+            this._logApi = new LogApi(logs.getLogger("ApplicationInsightsLogger"));
+            this._attributeSpanProcessor = new AttributeSpanProcessor({ ...this.context.tags, ...this.commonProperties });
+            ((trace.getTracerProvider() as ProxyTracerProvider).getDelegate() as NodeTracerProvider).addSpanProcessor(this._attributeSpanProcessor);
 
-        this._attributeLogProcessor = new AttributeLogProcessor({ ...this.context.tags, ...this.commonProperties });
-        (logs.getLoggerProvider() as LoggerProvider).addLogRecordProcessor(this._attributeLogProcessor);
+            this._attributeLogProcessor = new AttributeLogProcessor({ ...this.context.tags, ...this.commonProperties });
+            (logs.getLoggerProvider() as LoggerProvider).addLogRecordProcessor(this._attributeLogProcessor);
+        } catch (error) {
+            diag.error(`Failed to initialize TelemetryClient ${error}`);
+        }
     }
 
     /**
@@ -124,9 +128,13 @@ export class TelemetryClient {
             this.initialize();
         }
         // Create custom metric
-        const meter = metrics.getMeterProvider().getMeter("ApplicationInsightsMetrics");
-        const histogram = meter.createHistogram(telemetry.name);
-        histogram.record(telemetry.value, { ...telemetry.properties, ...this.commonProperties, ...this.context.tags });
+        try {
+            const meter = metrics.getMeterProvider().getMeter("ApplicationInsightsMetrics");
+            const histogram = meter.createHistogram(telemetry.name);
+            histogram.record(telemetry.value, { ...telemetry.properties, ...this.commonProperties, ...this.context.tags });
+        } catch (error) {
+            diag.error(`Failed to record metric: ${error}`);
+        }
     }
 
     /**


Test patch:
diff --git a/test/unitTests/logs/exceptions.tests.ts b/test/unitTests/logs/exceptions.tests.ts
index 366690a..3973fe4 100644
--- a/test/unitTests/logs/exceptions.tests.ts
+++ b/test/unitTests/logs/exceptions.tests.ts
@@ -14,54 +14,28 @@ describe("AutoCollection/Exceptions", () => {
         sandbox.restore();
     });
 
-    it("should use uncaughtExceptionMonitor for node 13.7.0+", () => {
-        const nodeVer = process.versions.node.split(".");
-        const expectation =
-            parseInt(nodeVer[0]) > 13 || (parseInt(nodeVer[0]) === 13 && parseInt(nodeVer[1]) >= 7);
-        const exceptions = new AutoCollectExceptions(null);
-        assert.equal(exceptions["_canUseUncaughtExceptionMonitor"], expectation);
-    });
-
     it("enable auto collection", () => {
         const processOnSpy = sandbox.spy(global.process, "on");
-        const exceptions = new AutoCollectExceptions(null);
-        if (exceptions["_canUseUncaughtExceptionMonitor"]) {
-            assert.equal(
-                processOnSpy.callCount,
-                1,
-                "After enabling exception auto collection, there should be 1 call to processOnSpy"
-            );
-            assert.equal(processOnSpy.getCall(0).args[0], "uncaughtExceptionMonitor");
-        } else {
-            assert.equal(
-                processOnSpy.callCount,
-                2,
-                "After enabling exception auto collection, there should be 2 calls to processOnSpy"
-            );
-            assert.equal(processOnSpy.getCall(0).args[0], "uncaughtException");
-            assert.equal(processOnSpy.getCall(1).args[0], "unhandledRejection");
-        }
+        new AutoCollectExceptions(null);
+        assert.equal(
+            processOnSpy.callCount,
+            2,
+            "After enabling exception auto collection, there should be 2 calls to processOnSpy"
+        );
+        assert.equal(processOnSpy.getCall(0).args[0], "uncaughtException");
+        assert.equal(processOnSpy.getCall(1).args[0], "unhandledRejection");
     });
 
     it("disables auto collection", () => {
         const processRemoveListenerSpy = sandbox.spy(global.process, "removeListener");
         const exceptions = new AutoCollectExceptions(null);
         exceptions.shutdown();
-        if (exceptions["_canUseUncaughtExceptionMonitor"]) {
-            assert.equal(
-                processRemoveListenerSpy.callCount,
-                1,
-                "After enabling exception auto collection, there should be 1 call to processOnSpy"
-            );
-            assert.equal(processRemoveListenerSpy.getCall(0).args[0], "uncaughtExceptionMonitor");
-        } else {
-            assert.equal(
-                processRemoveListenerSpy.callCount,
-                2,
-                "After enabling exception auto collection, there should be 2 calls to processOnSpy"
-            );
-            assert.equal(processRemoveListenerSpy.getCall(0).args[0], "uncaughtException");
-            assert.equal(processRemoveListenerSpy.getCall(1).args[0], "unhandledRejection");
-        }
+        assert.equal(
+            processRemoveListenerSpy.callCount,
+            2,
+            "After enabling exception auto collection, there should be 2 calls to processOnSpy"
+        );
+        assert.equal(processRemoveListenerSpy.getCall(0).args[0], "uncaughtException");
+        assert.equal(processRemoveListenerSpy.getCall(1).args[0], "unhandledRejection");
     });
 });