From 493e8110e6c8a8e27ee3ded514279ef1e1e2ef89 Mon Sep 17 00:00:00 2001
From: Ilia Babanov <ilia.babanov@databricks.com>
Date: Mon, 13 Nov 2023 09:45:15 +0100
Subject: [PATCH] Improve properties capture for an error event

`sender.sendErrorData` doesn't always receive data object with a properties
field. In the case of unhandled errors vscode itself calls this with all the
properties being in the top level of the `data` argument. This PR accounts for
that and fallbacks to using full data object if a properties field doesn't exist
---
 src/common/baseTelemetrySender.ts |  7 ++---
 test/baseTelemetrySender.test.ts  | 43 ++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/src/common/baseTelemetrySender.ts b/src/common/baseTelemetrySender.ts
index 2369608..f810981 100644
--- a/src/common/baseTelemetrySender.ts
+++ b/src/common/baseTelemetrySender.ts
@@ -65,7 +65,7 @@ export class BaseTelemetrySender implements ILazyTelemetrySender {
 	 * @param exception The exception to collect
 	 * @param data Data associated with the exception
 	 */
-	sendErrorData(exception: Error, data?: SenderData): void {
+	sendErrorData(exception: Error, data?: SenderData | Record<string, any>): void {
 		if (!this._telemetryClient) {
 			if (this._instantiationStatus !== InstantiationStatus.INSTANTIATED) {
 				this._exceptionQueue.push({ exception, data });
@@ -74,7 +74,8 @@ export class BaseTelemetrySender implements ILazyTelemetrySender {
 		}
 		const errorData = { stack: exception.stack, message: exception.message, name: exception.name };
 		if (data) {
-			data.properties = { ...data.properties, ...errorData };
+			const errorProperties = data.properties || data;
+			data.properties = { ...errorProperties, ...errorData };
 		} else {
 			data = { properties: errorData };
 		}
@@ -126,4 +127,4 @@ export class BaseTelemetrySender implements ILazyTelemetrySender {
 			this._instantiationStatus = InstantiationStatus.INSTANTIATED;
 		});
 	}
-}
\ No newline at end of file
+}
diff --git a/test/baseTelemetrySender.test.ts b/test/baseTelemetrySender.test.ts
index b04f1a8..33b83d6 100644
--- a/test/baseTelemetrySender.test.ts
+++ b/test/baseTelemetrySender.test.ts
@@ -58,4 +58,45 @@ describe("Base telemetry sender test suite", () => {
 		assert.strictEqual(sender._exceptionQueue.length, 0);
 		assert.strictEqual((telemetryClient.logEvent as sinon.SinonSpy).callCount, 1);
 	});
-});
\ No newline at end of file
+
+	describe("Send error data logic", () => {
+		let sender: BaseTelemetrySender;
+
+		beforeEach(async () => {
+			sender = new BaseTelemetrySender("key", telemetryClientFactory);
+			sender.instantiateSender();
+			// Wait 10ms to ensure that the sender has instantiated the client
+			await new Promise((resolve) => setTimeout(resolve, 10));
+		});
+
+		it("Error properties are correctly created for an empty data argument", () => {
+			const error = new Error("test");
+			sender.sendErrorData(error);
+			assert.strictEqual((telemetryClient.logEvent as sinon.SinonSpy).callCount, 1);
+			sinon.assert.calledWithMatch(
+				telemetryClient.logEvent as sinon.SinonSpy, "unhandlederror",
+				{properties: {name: error.name, message: error.message, stack: error.stack}}
+			);
+		})
+
+		it("Error properties are correctly created for a data without properties field", () => {
+			const error = new Error("test");
+			sender.sendErrorData(error, {prop1: 1, prop2: "two"});
+			assert.strictEqual((telemetryClient.logEvent as sinon.SinonSpy).callCount, 1);
+			sinon.assert.calledWithMatch(
+				telemetryClient.logEvent as sinon.SinonSpy, "unhandlederror",
+				{properties: {prop1: 1, prop2: "two", name: error.name, message: error.message, stack: error.stack}}
+			);
+		});
+
+		it("Error properties are correctly created for a data with properties field", () => {
+			const error = new Error("uh oh");
+			sender.sendErrorData(error, {properties: {prop1: 1, prop2: "two"}});
+			assert.strictEqual((telemetryClient.logEvent as sinon.SinonSpy).callCount, 1);
+			sinon.assert.calledWithMatch(
+				telemetryClient.logEvent as sinon.SinonSpy, "unhandlederror",
+				{properties: {prop1: 1, prop2: "two", name: error.name, message: error.message, stack: error.stack}}
+			);
+		});
+	})
+});