terminal: cancel unnecessary telemetry timeout on disposal

src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalTelemetry.ts

@@ -6,6 +6,8 @@
 import { getWindowById } from '../../../../../base/browser/dom.js';
 import { isAuxiliaryWindow } from '../../../../../base/browser/window.js';
 import { timeout } from '../../../../../base/common/async.js';
+import { cancelOnDispose } from '../../../../../base/common/cancellation.js';
+import { isCancellationError } from '../../../../../base/common/errors.js';
 import { Event } from '../../../../../base/common/event.js';
 import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js';
 import { basename } from '../../../../../base/common/path.js';
@@ -32,18 +34,26 @@ export class TerminalTelemetryContribution extends Disposable implements IWorkbe
 		this._register(terminalService.onDidCreateInstance(async instance => {
 			const store = new DisposableStore();
 			this._store.add(store);
+			const cancellationToken = cancelOnDispose(store);
 
-			await Promise.race([
-				// Wait for process ready so the shell launch config is fully resolved, then
-				// allow another 10 seconds for the shell integration to be fully initialized
-				instance.processReady.then(() => {
-					return timeout(10000);
-				}),
-				// If the terminal is disposed, it's ready to report on immediately
-				Event.toPromise(instance.onDisposed, store),
-				// If the app is shutting down, flush
-				Event.toPromise(lifecycleService.onWillShutdown, store),
-			]);
+			try {
+				await Promise.race([
+					// Wait for process ready so the shell launch config is fully resolved, then
+					// allow another 10 seconds for the shell integration to be fully initialized
+					instance.processReady.then(() => {
+						return timeout(10000, cancellationToken);
+					}),
+					// If the terminal is disposed, it's ready to report on immediately
+					Event.toPromise(instance.onDisposed, store),
+					// If the app is shutting down, flush
+					Event.toPromise(lifecycleService.onWillShutdown, store),
+				]);
+			} catch (error) {
+				if (cancellationToken.isCancellationRequested && isCancellationError(error)) {
+					return;
+				}
+				throw error;
+			}
 
 			// Determine window status, this is done some time after the process is ready and could
 			// reflect the terminal being moved.

src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalTelemetry.test.ts

@@ -0,0 +1,94 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { Emitter } from '../../../../../../base/common/event.js';
+import { URI } from '../../../../../../base/common/uri.js';
+import { upcastPartial } from '../../../../../../base/test/common/mock.js';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
+import { captureGlobalTimeApi, type TimeoutId } from '../../../../../../base/test/common/virtualScheduling/timeApi.js';
+import { pushGlobalTimeApi } from '../../../../../../base/test/common/virtualScheduling/globalTimeApi.js';
+import { TerminalCapabilityStore } from '../../../../../../platform/terminal/common/capabilities/terminalCapabilityStore.js';
+import { TerminalLocation } from '../../../../../../platform/terminal/common/terminal.js';
+import { ITelemetryService } from '../../../../../../platform/telemetry/common/telemetry.js';
+import { ILifecycleService, type WillShutdownEvent } from '../../../../../services/lifecycle/common/lifecycle.js';
+import { ITerminalEditorService, type ITerminalInstance, ITerminalService } from '../../../../terminal/browser/terminal.js';
+import { TerminalTelemetryContribution } from '../../browser/terminalTelemetry.js';
+
+suite('TerminalTelemetryContribution', () => {
+	const store = ensureNoDisposablesAreLeakedInTestSuite();
+
+	test('cancels the shell integration timeout when the terminal or contribution is disposed', async () => {
+		const onDidCreateInstance = store.add(new Emitter<ITerminalInstance>());
+		const onAnyInstanceShellTypeChanged = store.add(new Emitter<ITerminalInstance>());
+		const onDisposed = store.add(new Emitter<ITerminalInstance>());
+		const onWillShutdown = store.add(new Emitter<WillShutdownEvent>());
+		const timeoutHandle = 1 as unknown as TimeoutId;
+		let scheduledTimeout: number | undefined;
+		let clearedTimeout: TimeoutId | undefined;
+		let telemetryEvents = 0;
+
+		const timeApi = captureGlobalTimeApi();
+		store.add(pushGlobalTimeApi({
+			...timeApi,
+			setTimeout: (_handler, timeout) => {
+				scheduledTimeout = timeout;
+				return timeoutHandle;
+			},
+			clearTimeout: handle => clearedTimeout = handle,
+		}));
+
+		const terminalService = upcastPartial<ITerminalService>({
+			onDidCreateInstance: onDidCreateInstance.event,
+			onAnyInstanceShellTypeChanged: onAnyInstanceShellTypeChanged.event,
+		});
+		const lifecycleService = upcastPartial<ILifecycleService>({ onWillShutdown: onWillShutdown.event });
+		const terminalEditorService = upcastPartial<ITerminalEditorService>({
+			getInputFromResource: () => { throw new Error('Not an editor terminal'); },
+		});
+		const telemetryService = upcastPartial<ITelemetryService>({
+			publicLog2: () => { telemetryEvents++; },
+		});
+		const contribution = store.add(new TerminalTelemetryContribution(lifecycleService, terminalService, terminalEditorService, telemetryService));
+
+		const instance = upcastPartial<ITerminalInstance>({
+			resource: URI.parse('terminal:test'),
+			target: TerminalLocation.Panel,
+			processReady: Promise.resolve(),
+			onDisposed: onDisposed.event,
+			shellLaunchConfig: {},
+			capabilities: store.add(new TerminalCapabilityStore()),
+			hasRemoteAuthority: false,
+			usedShellIntegrationInjection: false,
+			shellIntegrationInjectionFailureReason: undefined,
+			sessionId: 'test',
+		});
+
+		onDidCreateInstance.fire(instance);
+		onDisposed.fire(instance);
+		await Promise.resolve();
+		await Promise.resolve();
+
+		assert.deepStrictEqual({ scheduledTimeout, clearedTimeout, telemetryEvents }, {
+			scheduledTimeout: 10_000,
+			clearedTimeout: timeoutHandle,
+			telemetryEvents: 1,
+		});
+
+		scheduledTimeout = undefined;
+		clearedTimeout = undefined;
+		onDidCreateInstance.fire(instance);
+		await Promise.resolve();
+		contribution.dispose();
+		await Promise.resolve();
+		await Promise.resolve();
+
+		assert.deepStrictEqual({ scheduledTimeout, clearedTimeout, telemetryEvents }, {
+			scheduledTimeout: 10_000,
+			clearedTimeout: timeoutHandle,
+			telemetryEvents: 1,
+		});
+	});
+});