Fix Issue Reporter API (microsoft#180971)

* Fix Issue Reporter API

CancellationTokens aren't hydrated over ProxyChannels so I've removed cancellation logic.

Additionally, there was a bad string compare that needed toLower when checking if an extension has a handler.

Additionally, added docs.

Fixes microsoft#180890
Fixes microsoft#180920
Fixes microsoft#180887

* remove import

* remove another import

Author: TylerLeonhardt

src/vs/code/electron-sandbox/issue/IssueReporterService.ts

@@ -20,7 +20,6 @@ import { normalizeGitHubUrl } from 'vs/platform/issue/common/issueReporterUtil';
 import { INativeHostService } from 'vs/platform/native/common/native';
 import { applyZoom, zoomIn, zoomOut } from 'vs/platform/window/electron-sandbox/window';
 import { CancellationError } from 'vs/base/common/errors';
-import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
 
 // GitHub has let us know that we could up our limit here to 8k. We chose 7500 to play it safe.
 // ref https://github.com/microsoft/vscode/issues/159191
@@ -91,15 +90,15 @@ export class IssueReporter extends Disposable {
 			}
 		}
 
-		this.issueMainService.getSystemInfo().then(info => {
+		this.issueMainService.$getSystemInfo().then(info => {
 			this.issueReporterModel.update({ systemInfo: info });
 			this.receivedSystemInfo = true;
 
 			this.updateSystemInfo(this.issueReporterModel.getData());
 			this.updatePreviewButtonState();
 		});
 		if (configuration.data.issueType === IssueType.PerformanceIssue) {
-			this.issueMainService.getPerformanceInfo().then(info => {
+			this.issueMainService.$getPerformanceInfo().then(info => {
 				this.updatePerformanceInfo(info as Partial<IssueReporterData>);
 			});
 		}
@@ -225,9 +224,9 @@ export class IssueReporter extends Disposable {
 		this.updateExtensionSelector(installedExtensions);
 	}
 
-	private async updateIssueReporterUri(extension: IssueReporterExtensionData, token: CancellationToken): Promise<void> {
+	private async updateIssueReporterUri(extension: IssueReporterExtensionData): Promise<void> {
 		try {
-			const uri = await this.issueMainService.getIssueReporterUri(extension.id, token);
+			const uri = await this.issueMainService.$getIssueReporterUri(extension.id);
 			extension.bugsUrl = uri.toString(true);
 		} catch (e) {
 			extension.hasIssueUriRequestHandler = false;
@@ -241,7 +240,7 @@ export class IssueReporter extends Disposable {
 			const issueType = parseInt((<HTMLInputElement>event.target).value);
 			this.issueReporterModel.update({ issueType: issueType });
 			if (issueType === IssueType.PerformanceIssue && !this.receivedPerformanceInfo) {
-				this.issueMainService.getPerformanceInfo().then(info => {
+				this.issueMainService.$getPerformanceInfo().then(info => {
 					this.updatePerformanceInfo(info as Partial<IssueReporterData>);
 				});
 			}
@@ -340,7 +339,7 @@ export class IssueReporter extends Disposable {
 		});
 
 		this.addEventListener('disableExtensions', 'click', () => {
-			this.issueMainService.reloadWithExtensionsDisabled();
+			this.issueMainService.$reloadWithExtensionsDisabled();
 		});
 
 		this.addEventListener('extensionBugsLink', 'click', (e: Event) => {
@@ -351,7 +350,7 @@ export class IssueReporter extends Disposable {
 		this.addEventListener('disableExtensions', 'keydown', (e: Event) => {
 			e.stopPropagation();
 			if ((e as KeyboardEvent).keyCode === 13 || (e as KeyboardEvent).keyCode === 32) {
-				this.issueMainService.reloadWithExtensionsDisabled();
+				this.issueMainService.$reloadWithExtensionsDisabled();
 			}
 		});
 
@@ -375,7 +374,7 @@ export class IssueReporter extends Disposable {
 				const { issueDescription } = this.issueReporterModel.getData();
 				if (!this.hasBeenSubmitted && (issueTitle || issueDescription)) {
 					// fire and forget
-					this.issueMainService.showConfirmCloseDialog();
+					this.issueMainService.$showConfirmCloseDialog();
 				} else {
 					this.close();
 				}
@@ -505,7 +504,7 @@ export class IssueReporter extends Disposable {
 	}
 
 	private async close(): Promise<void> {
-		await this.issueMainService.closeReporter();
+		await this.issueMainService.$closeReporter();
 	}
 
 	private clearSearchResults(): void {
@@ -882,7 +881,7 @@ export class IssueReporter extends Disposable {
 	}
 
 	private async writeToClipboard(baseUrl: string, issueBody: string): Promise<string> {
-		const shouldWrite = await this.issueMainService.showClipboardDialog();
+		const shouldWrite = await this.issueMainService.$showClipboardDialog();
 		if (!shouldWrite) {
 			throw new CancellationError();
 		}
@@ -1058,23 +1057,19 @@ export class IssueReporter extends Disposable {
 			const { selectedExtension } = this.issueReporterModel.getData();
 			reset(extensionsSelector, $<HTMLOptionElement>('option'), ...extensionOptions.map(extension => makeOption(extension, selectedExtension)));
 
-			let tokenSource: CancellationTokenSource | undefined;
 			this.addEventListener('extension-selector', 'change', (e: Event) => {
-				tokenSource?.cancel();
 				const selectedExtensionId = (<HTMLInputElement>e.target).value;
 				const extensions = this.issueReporterModel.getData().allExtensions;
 				const matches = extensions.filter(extension => extension.id === selectedExtensionId);
 				if (matches.length) {
 					this.issueReporterModel.update({ selectedExtension: matches[0] });
-					this.validateSelectedExtension();
-
 					if (matches[0].hasIssueUriRequestHandler) {
-						tokenSource = new CancellationTokenSource();
-						this.updateIssueReporterUri(matches[0], tokenSource?.token);
+						this.updateIssueReporterUri(matches[0]);
+					} else {
+						this.validateSelectedExtension();
+						const title = (<HTMLInputElement>this.getElementById('issue-title')).value;
+						this.searchExtensionIssues(title);
 					}
-
-					const title = (<HTMLInputElement>this.getElementById('issue-title')).value;
-					this.searchExtensionIssues(title);
 				} else {
 					this.issueReporterModel.update({ selectedExtension: undefined });
 					this.clearSearchResults();
@@ -1096,13 +1091,14 @@ export class IssueReporter extends Disposable {
 		hide(extensionValidationMessage);
 		hide(extensionValidationNoUrlsMessage);
 
-		if (!this.issueReporterModel.getData().selectedExtension) {
+		const extension = this.issueReporterModel.getData().selectedExtension;
+		if (!extension) {
 			this.previewButton.enabled = true;
 			return;
 		}
 
 		const hasValidGitHubUrl = this.getExtensionGitHubUrl();
-		if (hasValidGitHubUrl) {
+		if (hasValidGitHubUrl || extension.hasIssueUriRequestHandler) {
 			this.previewButton.enabled = true;
 		} else {
 			this.setExtensionValidationMessage();

src/vs/platform/issue/common/issue.ts

@@ -3,7 +3,6 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { CancellationToken } from 'vs/base/common/cancellation';
 import { URI } from 'vs/base/common/uri';
 import { ISandboxConfiguration } from 'vs/base/parts/sandbox/common/sandboxTypes';
 import { PerformanceInfo, SystemInfo } from 'vs/platform/diagnostics/common/diagnostics';
@@ -124,11 +123,11 @@ export interface IIssueMainService {
 
 	// Used by the issue reporter
 
-	getSystemInfo(): Promise<SystemInfo>;
-	getPerformanceInfo(): Promise<PerformanceInfo>;
-	reloadWithExtensionsDisabled(): Promise<void>;
-	showConfirmCloseDialog(): Promise<void>;
-	showClipboardDialog(): Promise<boolean>;
-	getIssueReporterUri(extensionId: string, token: CancellationToken): Promise<URI>;
-	closeReporter(): Promise<void>;
+	$getSystemInfo(): Promise<SystemInfo>;
+	$getPerformanceInfo(): Promise<PerformanceInfo>;
+	$reloadWithExtensionsDisabled(): Promise<void>;
+	$showConfirmCloseDialog(): Promise<void>;
+	$showClipboardDialog(): Promise<boolean>;
+	$getIssueReporterUri(extensionId: string): Promise<URI>;
+	$closeReporter(): Promise<void>;
 }

src/vs/platform/issue/electron-main/issueMainService.ts

@@ -27,7 +27,7 @@ import { randomPath } from 'vs/base/common/extpath';
 import { withNullAsUndefined } from 'vs/base/common/types';
 import { IStateService } from 'vs/platform/state/node/state';
 import { UtilityProcess } from 'vs/platform/utilityProcess/electron-main/utilityProcess';
-import { CancellationToken } from 'vs/base/common/cancellation';
+import { CancellationTokenSource } from 'vs/base/common/cancellation';
 import { URI } from 'vs/base/common/uri';
 import { IWindowsMainService } from 'vs/platform/windows/electron-main/windows';
 import { Promises, timeout } from 'vs/base/common/async';
@@ -302,13 +302,13 @@ export class IssueMainService implements IIssueMainService {
 
 	//#region used by issue reporter window
 
-	async getSystemInfo(): Promise<SystemInfo> {
+	async $getSystemInfo(): Promise<SystemInfo> {
 		const [info, remoteData] = await Promise.all([this.diagnosticsMainService.getMainDiagnostics(), this.diagnosticsMainService.getRemoteDiagnostics({ includeProcesses: false, includeWorkspaceMetadata: false })]);
 		const msg = await this.diagnosticsService.getSystemInfo(info, remoteData);
 		return msg;
 	}
 
-	async getPerformanceInfo(): Promise<PerformanceInfo> {
+	async $getPerformanceInfo(): Promise<PerformanceInfo> {
 		try {
 			const [info, remoteData] = await Promise.all([this.diagnosticsMainService.getMainDiagnostics(), this.diagnosticsMainService.getRemoteDiagnostics({ includeProcesses: true, includeWorkspaceMetadata: true })]);
 			return await this.diagnosticsService.getPerformanceInfo(info, remoteData);
@@ -319,7 +319,7 @@ export class IssueMainService implements IIssueMainService {
 		}
 	}
 
-	async reloadWithExtensionsDisabled(): Promise<void> {
+	async $reloadWithExtensionsDisabled(): Promise<void> {
 		if (this.issueReporterParentWindow) {
 			try {
 				await this.nativeHostMainService.reload(this.issueReporterParentWindow.id, { disableExtensions: true });
@@ -329,7 +329,7 @@ export class IssueMainService implements IIssueMainService {
 		}
 	}
 
-	async showConfirmCloseDialog(): Promise<void> {
+	async $showConfirmCloseDialog(): Promise<void> {
 		if (this.issueReporterWindow) {
 			const { response } = await this.dialogMainService.showMessageBox({
 				type: 'warning',
@@ -349,7 +349,7 @@ export class IssueMainService implements IIssueMainService {
 		}
 	}
 
-	async showClipboardDialog(): Promise<boolean> {
+	async $showClipboardDialog(): Promise<boolean> {
 		if (this.issueReporterWindow) {
 			const { response } = await this.dialogMainService.showMessageBox({
 				type: 'warning',
@@ -366,7 +366,7 @@ export class IssueMainService implements IIssueMainService {
 		return false;
 	}
 
-	async getIssueReporterUri(extensionId: string, token: CancellationToken): Promise<URI> {
+	async $getIssueReporterUri(extensionId: string): Promise<URI> {
 		if (!this.issueReporterParentWindow) {
 			throw new Error('Issue reporter window not available');
 		}
@@ -376,22 +376,25 @@ export class IssueMainService implements IIssueMainService {
 		}
 		const replyChannel = `vscode:triggerIssueUriRequestHandlerResponse${window.id}`;
 		return Promises.withAsyncBody<URI>(async (resolve, reject) => {
-			window.sendWhenReady('vscode:triggerIssueUriRequestHandler', token, { replyChannel, extensionId });
+
+			const cts = new CancellationTokenSource();
+			window.sendWhenReady('vscode:triggerIssueUriRequestHandler', cts.token, { replyChannel, extensionId });
 
 			validatedIpcMain.once(replyChannel, (_: unknown, data: string) => {
 				resolve(URI.parse(data));
 			});
 
 			try {
-				await timeout(5000, token);
+				await timeout(5000);
+				cts.cancel();
 				reject(new Error('Timed out waiting for issue reporter URI'));
 			} finally {
 				validatedIpcMain.removeHandler(replyChannel);
 			}
 		});
 	}
 
-	async closeReporter(): Promise<void> {
+	async $closeReporter(): Promise<void> {
 		this.issueReporterWindow?.close();
 	}
 

src/vs/workbench/services/issue/electron-sandbox/issueService.ts

@@ -64,7 +64,7 @@ export class NativeIssueService implements IWorkbenchIssueService {
 					version: manifest.version,
 					repositoryUrl: manifest.repository && manifest.repository.url,
 					bugsUrl: manifest.bugs && manifest.bugs.url,
-					hasIssueUriRequestHandler: this._handlers.has(extension.identifier.id),
+					hasIssueUriRequestHandler: this._handlers.has(extension.identifier.id.toLowerCase()),
 					displayName: manifest.displayName,
 					id: extension.identifier.id,
 					isTheme,
@@ -145,7 +145,7 @@ export class NativeIssueService implements IWorkbenchIssueService {
 	}
 
 	registerIssueUriRequestHandler(extensionId: string, handler: IIssueUriRequestHandler): IDisposable {
-		this._handlers.set(extensionId, handler);
+		this._handlers.set(extensionId.toLowerCase(), handler);
 		return {
 			dispose: () => this._handlers.delete(extensionId)
 		};

src/vscode-dts/vscode.proposed.handleIssueUri.d.ts

@@ -8,10 +8,24 @@ declare module 'vscode' {
 	// https://github.com/microsoft/vscode/issues/46726
 
 	export interface IssueUriRequestHandler {
+		/**
+		 *Handle the request by the issue reporter for the Uri you want to direct the user to.
+		 */
 		handleIssueUrlRequest(): ProviderResult<Uri>;
 	}
 
 	export namespace env {
+		/**
+		 * Register an {@link IssueUriRequestHandler}. By registering an issue uri request handler,
+		 * you can direct the built-in issue reporter to your issue reporting web experience of choice.
+		 * The Uri that the handler returns will be opened in the user's browser.
+		 *
+		 * Examples of this include:
+		 * - Using GitHub Issue Forms or GitHub Discussions you can pre-fill the issue creation with relevant information from the current workspace using query parameters
+		 * - Directing to a different web form that isn't on GitHub for reporting issues
+		 *
+		 * @param handler the issue uri request handler to register for this extension.
+		 */
 		export function registerIssueUriRequestHandler(handler: IssueUriRequestHandler): Disposable;
 	}
 }