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

Author: ilia-db

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;
 		});
 	}
-}
+}

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);
 	});
-});
+
+	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}}
+			);
+		});
+	})
+});