Skip to content

Commit

Permalink
Improve properties capture for an error event
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
ilia-db committed Nov 13, 2023
1 parent cd3176a commit 493e811
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 4 deletions.
7 changes: 4 additions & 3 deletions src/common/baseTelemetrySender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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 };
}
Expand Down Expand Up @@ -126,4 +127,4 @@ export class BaseTelemetrySender implements ILazyTelemetrySender {
this._instantiationStatus = InstantiationStatus.INSTANTIATED;
});
}
}
}
43 changes: 42 additions & 1 deletion test/baseTelemetrySender.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
);
});
})
});

0 comments on commit 493e811

Please sign in to comment.