Skip to content

Commit 4c51288

Browse files
author
Rachel Macfarlane
committed
Improve error handling when fetching diagnostics
1 parent ed60eb6 commit 4c51288

6 files changed

Lines changed: 118 additions & 63 deletions

File tree

‎src/vs/code/electron-browser/processExplorer/media/processExplorer.css‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ td {
8585
height: 22px;
8686
}
8787

88+
.error {
89+
padding-left: 20px;
90+
white-space: nowrap;
91+
}
92+
8893
tbody > tr:hover {
8994
background-color: #2A2D2E;
9095
}

‎src/vs/code/electron-browser/processExplorer/processExplorerMain.ts‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { popup } from 'vs/base/parts/contextmenu/electron-browser/contextmenu';
1717
import { ProcessItem } from 'vs/base/common/processes';
1818
import { addDisposableListener } from 'vs/base/browser/dom';
1919
import { IDisposable } from 'vs/base/common/lifecycle';
20+
import { isRemoteDiagnosticError, IRemoteDiagnosticError } from 'vs/platform/diagnostics/common/diagnosticsService';
2021

2122

2223
let mapPidToWindowTitle = new Map<number, string>();
@@ -138,6 +139,46 @@ function updateSectionCollapsedState(shouldExpand: boolean, body: HTMLElement, t
138139
}
139140
}
140141

142+
function renderProcessFetchError(sectionName: string, errorMessage: string) {
143+
const container = document.getElementById('process-list');
144+
if (!container) {
145+
return;
146+
}
147+
148+
const body = document.createElement('tbody');
149+
150+
renderProcessGroupHeader(sectionName, body, container);
151+
152+
const errorRow = document.createElement('tr');
153+
const data = document.createElement('td');
154+
data.textContent = errorMessage;
155+
data.className = 'error';
156+
data.colSpan = 4;
157+
errorRow.appendChild(data);
158+
159+
body.appendChild(errorRow);
160+
container.appendChild(body);
161+
}
162+
163+
function renderProcessGroupHeader(sectionName: string, body: HTMLElement, container: HTMLElement) {
164+
const headerRow = document.createElement('tr');
165+
const data = document.createElement('td');
166+
data.textContent = sectionName;
167+
data.colSpan = 4;
168+
headerRow.appendChild(data);
169+
170+
const twistie = document.createElement('img');
171+
updateSectionCollapsedState(!collapsedStateCache.get(sectionName), body, twistie, sectionName);
172+
data.prepend(twistie);
173+
174+
listeners.push(addDisposableListener(data, 'click', (e) => {
175+
const isHidden = body.classList.contains('hidden');
176+
updateSectionCollapsedState(isHidden, body, twistie, sectionName);
177+
}));
178+
179+
container.appendChild(headerRow);
180+
}
181+
141182
function renderTableSection(sectionName: string, processList: FormattedProcessItem[], renderManySections: boolean, sectionIsLocal: boolean): void {
142183
const container = document.getElementById('process-list');
143184
if (!container) {
@@ -150,22 +191,7 @@ function renderTableSection(sectionName: string, processList: FormattedProcessIt
150191
const body = document.createElement('tbody');
151192

152193
if (renderManySections) {
153-
const headerRow = document.createElement('tr');
154-
const data = document.createElement('td');
155-
data.textContent = sectionName;
156-
data.colSpan = 4;
157-
headerRow.appendChild(data);
158-
159-
const twistie = document.createElement('img');
160-
updateSectionCollapsedState(!collapsedStateCache.get(sectionName), body, twistie, sectionName);
161-
data.prepend(twistie);
162-
163-
listeners.push(addDisposableListener(data, 'click', (e) => {
164-
const isHidden = body.classList.contains('hidden');
165-
updateSectionCollapsedState(isHidden, body, twistie, sectionName);
166-
}));
167-
168-
container.appendChild(headerRow);
194+
renderProcessGroupHeader(sectionName, body, container);
169195
}
170196

171197
processList.forEach(p => {
@@ -206,7 +232,7 @@ function renderTableSection(sectionName: string, processList: FormattedProcessIt
206232
container.appendChild(body);
207233
}
208234

209-
function updateProcessInfo(processLists: { [key: string]: ProcessItem }): void {
235+
function updateProcessInfo(processLists: { [key: string]: ProcessItem | IRemoteDiagnosticError }): void {
210236
const container = document.getElementById('process-list');
211237
if (!container) {
212238
return;
@@ -228,7 +254,12 @@ function updateProcessInfo(processLists: { [key: string]: ProcessItem }): void {
228254
const hasMultipleMachines = Object.keys(processLists).length > 1;
229255
Object.keys(processLists).forEach((key, i) => {
230256
const isLocal = i === 0;
231-
renderTableSection(key, getProcessList(processLists[key], isLocal), hasMultipleMachines, isLocal);
257+
const processItem = processLists[key];
258+
if (isRemoteDiagnosticError(processItem)) {
259+
renderProcessFetchError(key, processItem.errorMessage);
260+
} else {
261+
renderTableSection(key, getProcessList(processItem, isLocal), hasMultipleMachines, isLocal);
262+
}
232263
});
233264
}
234265

@@ -354,7 +385,7 @@ export function startup(data: ProcessExplorerData): void {
354385
windows.forEach(window => mapPidToWindowTitle.set(window.pid, window.title));
355386
});
356387

357-
ipcRenderer.on('vscode:listProcessesResponse', (_event: Event, processRoots: { [key: string]: ProcessItem }) => {
388+
ipcRenderer.on('vscode:listProcessesResponse', (_event: Event, processRoots: { [key: string]: ProcessItem | IRemoteDiagnosticError }) => {
358389
updateProcessInfo(processRoots);
359390
requestProcessList(0);
360391
});

‎src/vs/platform/diagnostics/common/diagnosticsService.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ export interface IRemoteDiagnosticInfo extends IDiagnosticInfo {
3030
hostName: string;
3131
}
3232

33+
export interface IRemoteDiagnosticError {
34+
hostName: string;
35+
errorMessage: string;
36+
}
37+
3338
export interface IDiagnosticInfoOptions {
3439
includeProcesses?: boolean;
3540
folders?: UriComponents[];
@@ -46,4 +51,8 @@ export interface WorkspaceStats {
4651
configFiles: WorkspaceStatItem[];
4752
fileCount: number;
4853
maxFilesReached: boolean;
54+
}
55+
56+
export function isRemoteDiagnosticError(x: any): x is IRemoteDiagnosticError {
57+
return !!x.hostName && !!x.errorMessage;
4958
}

‎src/vs/platform/diagnostics/electron-main/diagnosticsService.ts‎

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { app } from 'electron';
1515
import { basename } from 'vs/base/common/path';
1616
import { URI } from 'vs/base/common/uri';
1717
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
18-
import { IMachineInfo, WorkspaceStats, SystemInfo } from 'vs/platform/diagnostics/common/diagnosticsService';
18+
import { IMachineInfo, WorkspaceStats, SystemInfo, IRemoteDiagnosticInfo, isRemoteDiagnosticError } from 'vs/platform/diagnostics/common/diagnosticsService';
1919
import { collectWorkspaceStats, getMachineInfo } from 'vs/platform/diagnostics/node/diagnosticsService';
2020
import { ProcessItem } from 'vs/base/common/processes';
2121

@@ -92,23 +92,28 @@ export class DiagnosticsService implements IDiagnosticsService {
9292
try {
9393
const remoteData = await launchService.getRemoteDiagnostics({ includeProcesses: true, includeWorkspaceMetadata: true });
9494
remoteData.forEach(diagnostics => {
95-
processInfo += `\n\nRemote: ${diagnostics.hostName}`;
96-
if (diagnostics.processes) {
97-
processInfo += `\n${this.formatProcessList(info, diagnostics.processes)}`;
98-
}
95+
if (isRemoteDiagnosticError(diagnostics)) {
96+
processInfo += `\n${diagnostics.errorMessage}`;
97+
workspaceInfo += `\n${diagnostics.errorMessage}`;
98+
} else {
99+
processInfo += `\n\nRemote: ${diagnostics.hostName}`;
100+
if (diagnostics.processes) {
101+
processInfo += `\n${this.formatProcessList(info, diagnostics.processes)}`;
102+
}
99103

100-
if (diagnostics.workspaceMetadata) {
101-
workspaceInfo += `\n| Remote: ${diagnostics.hostName}`;
102-
for (const folder of Object.keys(diagnostics.workspaceMetadata)) {
103-
const metadata = diagnostics.workspaceMetadata[folder];
104+
if (diagnostics.workspaceMetadata) {
105+
workspaceInfo += `\n| Remote: ${diagnostics.hostName}`;
106+
for (const folder of Object.keys(diagnostics.workspaceMetadata)) {
107+
const metadata = diagnostics.workspaceMetadata[folder];
104108

105-
let countMessage = `${metadata.fileCount} files`;
106-
if (metadata.maxFilesReached) {
107-
countMessage = `more than ${countMessage}`;
108-
}
109+
let countMessage = `${metadata.fileCount} files`;
110+
if (metadata.maxFilesReached) {
111+
countMessage = `more than ${countMessage}`;
112+
}
109113

110-
workspaceInfo += `| Folder (${folder}): ${countMessage}`;
111-
workspaceInfo += this.formatWorkspaceStats(metadata);
114+
workspaceInfo += `| Folder (${folder}): ${countMessage}`;
115+
workspaceInfo += this.formatWorkspaceStats(metadata);
116+
}
112117
}
113118
}
114119
});
@@ -135,7 +140,7 @@ export class DiagnosticsService implements IDiagnosticsService {
135140
processArgs: `${info.mainArguments.join(' ')}`,
136141
gpuStatus: app.getGPUFeatureStatus(),
137142
screenReader: `${app.isAccessibilitySupportEnabled() ? 'yes' : 'no'}`,
138-
remoteData: await launchService.getRemoteDiagnostics({ includeProcesses: false, includeWorkspaceMetadata: false })
143+
remoteData: (await launchService.getRemoteDiagnostics({ includeProcesses: false, includeWorkspaceMetadata: false })).filter((x): x is IRemoteDiagnosticInfo => !(x instanceof Error))
139144
};
140145

141146

@@ -169,25 +174,29 @@ export class DiagnosticsService implements IDiagnosticsService {
169174
try {
170175
const data = await launchService.getRemoteDiagnostics({ includeProcesses: true, includeWorkspaceMetadata: true });
171176
data.forEach(diagnostics => {
172-
output.push('\n\n');
173-
output.push(`Remote: ${diagnostics.hostName}`);
174-
output.push(this.formatMachineInfo(diagnostics.machineInfo));
177+
if (isRemoteDiagnosticError(diagnostics)) {
178+
output.push(`\n${diagnostics.errorMessage}`);
179+
} else {
180+
output.push('\n\n');
181+
output.push(`Remote: ${diagnostics.hostName}`);
182+
output.push(this.formatMachineInfo(diagnostics.machineInfo));
183+
184+
if (diagnostics.processes) {
185+
output.push(this.formatProcessList(info, diagnostics.processes));
186+
}
175187

176-
if (diagnostics.processes) {
177-
output.push(this.formatProcessList(info, diagnostics.processes));
178-
}
188+
if (diagnostics.workspaceMetadata) {
189+
for (const folder of Object.keys(diagnostics.workspaceMetadata)) {
190+
const metadata = diagnostics.workspaceMetadata[folder];
179191

180-
if (diagnostics.workspaceMetadata) {
181-
for (const folder of Object.keys(diagnostics.workspaceMetadata)) {
182-
const metadata = diagnostics.workspaceMetadata[folder];
192+
let countMessage = `${metadata.fileCount} files`;
193+
if (metadata.maxFilesReached) {
194+
countMessage = `more than ${countMessage}`;
195+
}
183196

184-
let countMessage = `${metadata.fileCount} files`;
185-
if (metadata.maxFilesReached) {
186-
countMessage = `more than ${countMessage}`;
197+
output.push(`Folder (${folder}): ${countMessage}`);
198+
output.push(this.formatWorkspaceStats(metadata));
187199
}
188-
189-
output.push(`Folder (${folder}): ${countMessage}`);
190-
output.push(this.formatWorkspaceStats(metadata));
191200
}
192201
}
193202
});

‎src/vs/platform/issue/electron-main/issueService.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { ILogService } from 'vs/platform/log/common/log';
1616
import { IWindowsService } from 'vs/platform/windows/common/windows';
1717
import { IWindowState } from 'vs/platform/windows/electron-main/windows';
1818
import { listProcesses } from 'vs/base/node/ps';
19+
import { isRemoteDiagnosticError } from 'vs/platform/diagnostics/common/diagnosticsService';
1920

2021
const DEFAULT_BACKGROUND_COLOR = '#1E1E1E';
2122

@@ -53,8 +54,12 @@ export class IssueService implements IIssueService {
5354
processesMap[localize('local', "Local")] = await listProcesses(mainPid);
5455
(await this.launchService.getRemoteDiagnostics({ includeProcesses: true }))
5556
.forEach(data => {
56-
if (data.processes) {
57-
processesMap[data.hostName] = data.processes;
57+
if (isRemoteDiagnosticError(data)) {
58+
processesMap[data.hostName] = data;
59+
} else {
60+
if (data.processes) {
61+
processesMap[data.hostName] = data.processes;
62+
}
5863
}
5964
});
6065
} catch (e) {

‎src/vs/platform/launch/electron-main/launchService.ts‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { BrowserWindow, ipcMain, Event as IpcEvent } from 'electron';
1919
import { Event } from 'vs/base/common/event';
2020
import { hasArgs } from 'vs/platform/environment/node/argv';
2121
import { coalesce } from 'vs/base/common/arrays';
22-
import { IDiagnosticInfoOptions, IDiagnosticInfo, IRemoteDiagnosticInfo } from 'vs/platform/diagnostics/common/diagnosticsService';
22+
import { IDiagnosticInfoOptions, IDiagnosticInfo, IRemoteDiagnosticInfo, IRemoteDiagnosticError } from 'vs/platform/diagnostics/common/diagnosticsService';
2323

2424
export const ID = 'launchService';
2525
export const ILaunchService = createDecorator<ILaunchService>(ID);
@@ -71,7 +71,7 @@ export interface ILaunchService {
7171
getMainProcessId(): Promise<number>;
7272
getMainProcessInfo(): Promise<IMainProcessInfo>;
7373
getLogsPath(): Promise<string>;
74-
getRemoteDiagnostics(options: IRemoteDiagnosticOptions): Promise<IRemoteDiagnosticInfo[]>;
74+
getRemoteDiagnostics(options: IRemoteDiagnosticOptions): Promise<(IRemoteDiagnosticInfo | IRemoteDiagnosticError)[]>;
7575
}
7676

7777
export class LaunchChannel implements IServerChannel {
@@ -290,9 +290,9 @@ export class LaunchService implements ILaunchService {
290290
return Promise.resolve(this.environmentService.logsPath);
291291
}
292292

293-
getRemoteDiagnostics(options: IRemoteDiagnosticOptions): Promise<IRemoteDiagnosticInfo[]> {
293+
getRemoteDiagnostics(options: IRemoteDiagnosticOptions): Promise<(IRemoteDiagnosticInfo | IRemoteDiagnosticError)[]> {
294294
const windows = this.windowsMainService.getWindows();
295-
const promises: Promise<IDiagnosticInfo | undefined>[] = windows.map(window => {
295+
const promises: Promise<IDiagnosticInfo | IRemoteDiagnosticError | undefined>[] = windows.map(window => {
296296
return new Promise((resolve, reject) => {
297297
if (window.remoteAuthority) {
298298
const replyChannel = `vscode:getDiagnosticInfoResponse${window.id}`;
@@ -306,26 +306,22 @@ export class LaunchService implements ILaunchService {
306306
ipcMain.once(replyChannel, (_: IpcEvent, data: IRemoteDiagnosticInfo) => {
307307
// No data is returned if getting the connection fails.
308308
if (!data) {
309-
resolve();
310-
}
311-
312-
if (typeof (data) === 'string') {
313-
reject(new Error(data));
309+
resolve({ hostName: window.remoteAuthority!, errorMessage: `Unable to resolve connection to '${window.remoteAuthority}'.` });
314310
}
315311

316312
resolve(data);
317313
});
318314

319315
setTimeout(() => {
320-
resolve();
316+
resolve({ hostName: window.remoteAuthority!, errorMessage: `Fetching remote diagnostics for '${window.remoteAuthority}' timed out.` });
321317
}, 5000);
322318
} else {
323319
resolve();
324320
}
325321
});
326322
});
327323

328-
return Promise.all(promises).then(diagnostics => diagnostics.filter((x): x is IRemoteDiagnosticInfo => !!x));
324+
return Promise.all(promises).then(diagnostics => diagnostics.filter((x): x is IRemoteDiagnosticInfo | IRemoteDiagnosticError => !!x));
329325
}
330326

331327
private getFolderURIs(window: ICodeWindow): URI[] {

0 commit comments

Comments
 (0)