Skip to content

Commit

Permalink
Fix Issue Reporter API (#180971)
Browse files Browse the repository at this point in the history 
* 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 #180890
Fixes #180920
Fixes #180887

* remove import

* remove another import
  • Loading branch information
@TylerLeonhardt
TylerLeonhardt authored Apr 26, 2023
1 parent 407cb5a commit 326dc5a
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 42 deletions.
40 changes: 18 additions & 22 deletions src/vs/code/electron-sandbox/issue/IssueReporterService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>);
});
}
Expand Down Expand Up @@ -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;
Expand All @@ -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>);
});
}
Expand Down Expand Up @@ -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) => {
Expand All @@ -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();
}
});

Expand All @@ -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();
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
15 changes: 7 additions & 8 deletions src/vs/platform/issue/common/issue.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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>;
}
23 changes: 13 additions & 10 deletions src/vs/platform/issue/electron-main/issueMainService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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 });
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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');
}
Expand All @@ -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();
}

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/services/issue/electron-sandbox/issueService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
};
Expand Down
14 changes: 14 additions & 0 deletions src/vscode-dts/vscode.proposed.handleIssueUri.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

0 comments on commit 326dc5a

Please sign in to comment.