diff --git a/packages/telemetry/src/backends/application-insights.ts b/packages/telemetry/src/backends/application-insights.ts index 8b40090e7c..dcb6485153 100644 --- a/packages/telemetry/src/backends/application-insights.ts +++ b/packages/telemetry/src/backends/application-insights.ts @@ -1,3 +1,4 @@ +import type { ClientRequest } from 'node:http'; import * as https from 'node:https'; import os from 'node:os'; @@ -8,6 +9,8 @@ import type { TelemetryData, } from '../types'; +const MaxFlushTime = 5000; // 5 seconds + // This key is meant to be publically shared. However, I'm adding a simple // obfuscation to mitigate key scraping bots on GitHub. The key is split on // hypens and base64 encoded without padding. @@ -26,6 +29,7 @@ export class ApplicationInsightsBackend implements TelemetryBackend { private readonly productName: string; private readonly hostname: string; private readonly uname: string; + private pendingRequests = new Set(); constructor( userId: string, @@ -61,9 +65,30 @@ export class ApplicationInsightsBackend implements TelemetryBackend { this._post([envelope]); } + // Resolve once all in-flight requests have settled (their response/error + // handlers, including any warning, have run), so callers — and tests — can + // await the full request lifecycle rather than just the request being sent. flush(callback?: FlushCallback): void { - // No batching, so nothing to flush. Just call callback. - callback?.(); + const startTime = process.hrtime.bigint(); + + const checkPending = () => { + if (this.pendingRequests.size === 0) { + callback?.(); + return; + } + + const elapsedMs = Number((process.hrtime.bigint() - startTime) / BigInt(1_000_000)); + if (elapsedMs > MaxFlushTime) { + console.warn(`ApplicationInsightsBackend: Flush timed out after ${MaxFlushTime}ms`); + this.pendingRequests.forEach((req) => req.destroy()); + callback?.(); + return; + } + + setTimeout(checkPending, 10); + }; + + checkPending(); } private _post(envelopes: unknown[]): void { @@ -85,10 +110,13 @@ export class ApplicationInsightsBackend implements TelemetryBackend { if (res.statusCode && (res.statusCode < 200 || res.statusCode >= 300)) { console.warn(`ApplicationInsightsBackend: Failed to send telemetry event. Status: ${res.statusCode}`); } + this.pendingRequests.delete(req); }); }); + this.pendingRequests.add(req); req.on('error', (e) => { console.warn('Error sending telemetry data to Application Insights', e); + this.pendingRequests.delete(req); }); req.write(body); req.end(); diff --git a/packages/telemetry/tests/backends/application-insights.spec.ts b/packages/telemetry/tests/backends/application-insights.spec.ts index 1c64db0157..a130b52aa8 100644 --- a/packages/telemetry/tests/backends/application-insights.spec.ts +++ b/packages/telemetry/tests/backends/application-insights.spec.ts @@ -34,6 +34,7 @@ describe('ApplicationInsightsBackend', () => { afterEach(() => { nock.cleanAll(); nock.enableNetConnect(); + jest.restoreAllMocks(); }); const waitForRequest = async (scope: nock.Scope) => { @@ -118,7 +119,7 @@ describe('ApplicationInsightsBackend', () => { it('handles https errors', async () => { nock.cleanAll(); // Don't use the default scope - const errorScope = nock(AI_ENDPOINT).post(AI_PATH).replyWithError('test error'); + nock(AI_ENDPOINT).post(AI_PATH).replyWithError('test error'); const warnSpy = jest.spyOn(console, 'warn').mockImplementation(); const backend = new ApplicationInsightsBackend('user-id', 'session-id', 'product-name', { type: 'application-insights', @@ -130,7 +131,9 @@ describe('ApplicationInsightsBackend', () => { backend.sendEvent(event); - await waitForRequest(errorScope); + // Wait for the request's error handler (and its warning) to run, not just + // for the request to be sent. + await new Promise((resolve) => backend.flush(resolve)); expect(warnSpy).toHaveBeenCalledWith( 'Error sending telemetry data to Application Insights', @@ -140,7 +143,7 @@ describe('ApplicationInsightsBackend', () => { it('logs a warning for non-2xx HTTP responses', async () => { nock.cleanAll(); - const errorScope = nock(AI_ENDPOINT).post(AI_PATH).reply(500, { message: 'Internal Server Error' }); + nock(AI_ENDPOINT).post(AI_PATH).reply(500, { message: 'Internal Server Error' }); const warnSpy = jest.spyOn(console, 'warn').mockImplementation(); const backend = new ApplicationInsightsBackend('user-id', 'session-id', 'product-name', { type: 'application-insights', @@ -152,7 +155,9 @@ describe('ApplicationInsightsBackend', () => { backend.sendEvent(event); - await waitForRequest(errorScope); + // Wait for the response handler (and its warning) to run, not just for the + // request to be sent. + await new Promise((resolve) => backend.flush(resolve)); expect(warnSpy).toHaveBeenCalledWith( 'ApplicationInsightsBackend: Failed to send telemetry event. Status: 500'