From 5b0e5f919a387d9575502a76c2d9f4f4c9c94d9c Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 26 Apr 2023 12:50:38 -0700 Subject: [PATCH 1/3] 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 https://github.com/microsoft/vscode/issues/180890 Fixes https://github.com/microsoft/vscode/issues/180920 Fixes https://github.com/microsoft/vscode/issues/180887 --- .../issue/IssueReporterService.ts | 39 +++++++++---------- src/vs/platform/issue/common/issue.ts | 14 +++---- .../issue/electron-main/issueMainService.ts | 23 ++++++----- .../issue/electron-sandbox/issueService.ts | 4 +- .../vscode.proposed.handleIssueUri.d.ts | 14 +++++++ 5 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/vs/code/electron-sandbox/issue/IssueReporterService.ts b/src/vs/code/electron-sandbox/issue/IssueReporterService.ts index bd741a3fdf7ab..83d7c3b651dc6 100644 --- a/src/vs/code/electron-sandbox/issue/IssueReporterService.ts +++ b/src/vs/code/electron-sandbox/issue/IssueReporterService.ts @@ -91,7 +91,7 @@ export class IssueReporter extends Disposable { } } - this.issueMainService.getSystemInfo().then(info => { + this.issueMainService.$getSystemInfo().then(info => { this.issueReporterModel.update({ systemInfo: info }); this.receivedSystemInfo = true; @@ -99,7 +99,7 @@ export class IssueReporter extends Disposable { this.updatePreviewButtonState(); }); if (configuration.data.issueType === IssueType.PerformanceIssue) { - this.issueMainService.getPerformanceInfo().then(info => { + this.issueMainService.$getPerformanceInfo().then(info => { this.updatePerformanceInfo(info as Partial); }); } @@ -225,9 +225,9 @@ export class IssueReporter extends Disposable { this.updateExtensionSelector(installedExtensions); } - private async updateIssueReporterUri(extension: IssueReporterExtensionData, token: CancellationToken): Promise { + private async updateIssueReporterUri(extension: IssueReporterExtensionData): Promise { 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 +241,7 @@ export class IssueReporter extends Disposable { const issueType = parseInt((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); }); } @@ -340,7 +340,7 @@ export class IssueReporter extends Disposable { }); this.addEventListener('disableExtensions', 'click', () => { - this.issueMainService.reloadWithExtensionsDisabled(); + this.issueMainService.$reloadWithExtensionsDisabled(); }); this.addEventListener('extensionBugsLink', 'click', (e: Event) => { @@ -351,7 +351,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 +375,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 +505,7 @@ export class IssueReporter extends Disposable { } private async close(): Promise { - await this.issueMainService.closeReporter(); + await this.issueMainService.$closeReporter(); } private clearSearchResults(): void { @@ -882,7 +882,7 @@ export class IssueReporter extends Disposable { } private async writeToClipboard(baseUrl: string, issueBody: string): Promise { - const shouldWrite = await this.issueMainService.showClipboardDialog(); + const shouldWrite = await this.issueMainService.$showClipboardDialog(); if (!shouldWrite) { throw new CancellationError(); } @@ -1058,23 +1058,19 @@ export class IssueReporter extends Disposable { const { selectedExtension } = this.issueReporterModel.getData(); reset(extensionsSelector, $('option'), ...extensionOptions.map(extension => makeOption(extension, selectedExtension))); - let tokenSource: CancellationTokenSource | undefined; this.addEventListener('extension-selector', 'change', (e: Event) => { - tokenSource?.cancel(); const selectedExtensionId = (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 = (this.getElementById('issue-title')).value; + this.searchExtensionIssues(title); } - - const title = (this.getElementById('issue-title')).value; - this.searchExtensionIssues(title); } else { this.issueReporterModel.update({ selectedExtension: undefined }); this.clearSearchResults(); @@ -1096,13 +1092,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(); diff --git a/src/vs/platform/issue/common/issue.ts b/src/vs/platform/issue/common/issue.ts index bb2dfd0479180..ed8357069d70c 100644 --- a/src/vs/platform/issue/common/issue.ts +++ b/src/vs/platform/issue/common/issue.ts @@ -124,11 +124,11 @@ export interface IIssueMainService { // Used by the issue reporter - getSystemInfo(): Promise; - getPerformanceInfo(): Promise; - reloadWithExtensionsDisabled(): Promise; - showConfirmCloseDialog(): Promise; - showClipboardDialog(): Promise; - getIssueReporterUri(extensionId: string, token: CancellationToken): Promise; - closeReporter(): Promise; + $getSystemInfo(): Promise; + $getPerformanceInfo(): Promise; + $reloadWithExtensionsDisabled(): Promise; + $showConfirmCloseDialog(): Promise; + $showClipboardDialog(): Promise; + $getIssueReporterUri(extensionId: string): Promise; + $closeReporter(): Promise; } diff --git a/src/vs/platform/issue/electron-main/issueMainService.ts b/src/vs/platform/issue/electron-main/issueMainService.ts index 658d50b33be56..92e294c03f6be 100644 --- a/src/vs/platform/issue/electron-main/issueMainService.ts +++ b/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 { + async $getSystemInfo(): Promise { 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 { + async $getPerformanceInfo(): Promise { 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 { + async $reloadWithExtensionsDisabled(): Promise { 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 { + async $showConfirmCloseDialog(): Promise { if (this.issueReporterWindow) { const { response } = await this.dialogMainService.showMessageBox({ type: 'warning', @@ -349,7 +349,7 @@ export class IssueMainService implements IIssueMainService { } } - async showClipboardDialog(): Promise { + async $showClipboardDialog(): Promise { 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 { + async $getIssueReporterUri(extensionId: string): Promise { if (!this.issueReporterParentWindow) { throw new Error('Issue reporter window not available'); } @@ -376,14 +376,17 @@ export class IssueMainService implements IIssueMainService { } const replyChannel = `vscode:triggerIssueUriRequestHandlerResponse${window.id}`; return Promises.withAsyncBody(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); @@ -391,7 +394,7 @@ export class IssueMainService implements IIssueMainService { }); } - async closeReporter(): Promise { + async $closeReporter(): Promise { this.issueReporterWindow?.close(); } diff --git a/src/vs/workbench/services/issue/electron-sandbox/issueService.ts b/src/vs/workbench/services/issue/electron-sandbox/issueService.ts index 948d9a4902e7d..7c6d856a877bc 100644 --- a/src/vs/workbench/services/issue/electron-sandbox/issueService.ts +++ b/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) }; diff --git a/src/vscode-dts/vscode.proposed.handleIssueUri.d.ts b/src/vscode-dts/vscode.proposed.handleIssueUri.d.ts index 60c8ad08609db..069720276e9ec 100644 --- a/src/vscode-dts/vscode.proposed.handleIssueUri.d.ts +++ b/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; } 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; } } From e4fcadce4db7bf40f2fdb3246f122eefaacee080 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 26 Apr 2023 12:52:34 -0700 Subject: [PATCH 2/3] remove import --- src/vs/platform/issue/common/issue.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/platform/issue/common/issue.ts b/src/vs/platform/issue/common/issue.ts index ed8357069d70c..edc7d5b07f633 100644 --- a/src/vs/platform/issue/common/issue.ts +++ b/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'; From 35ec7ad426115632e45a002d2143e41dfbf01262 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 26 Apr 2023 12:53:05 -0700 Subject: [PATCH 3/3] remove another import --- src/vs/code/electron-sandbox/issue/IssueReporterService.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/code/electron-sandbox/issue/IssueReporterService.ts b/src/vs/code/electron-sandbox/issue/IssueReporterService.ts index 83d7c3b651dc6..d0c47890e4bec 100644 --- a/src/vs/code/electron-sandbox/issue/IssueReporterService.ts +++ b/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