Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

electron - better handle window errors #160427

Merged
merged 1 commit into from Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/vs/code/electron-main/app.ts
Expand Up @@ -1187,8 +1187,8 @@ export class CodeApplication extends Disposable {
case WindowError.UNRESPONSIVE:
message = 'SharedProcess: detected unresponsive window';
break;
case WindowError.CRASHED:
message = `SharedProcess: crashed (detail: ${details?.reason ?? '<unknown>'}, code: ${details?.exitCode ?? '<unknown>'})`;
case WindowError.PROCESS_GONE:
message = `SharedProcess: renderer process gone (detail: ${details?.reason ?? '<unknown>'}, code: ${details?.exitCode ?? '<unknown>'})`;
break;
case WindowError.LOAD:
message = `SharedProcess: failed to load (detail: ${details?.reason ?? '<unknown>'}, code: ${details?.exitCode ?? '<unknown>'})`;
Expand Down
Expand Up @@ -276,7 +276,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
// Crashes & Unresponsive & Failed to load
// We use `onUnexpectedError` explicitly because the error handler
// will send the error to the active window to log in devtools too
this.window.webContents.on('render-process-gone', (event, details) => this._onDidError.fire({ type: WindowError.CRASHED, details }));
this.window.webContents.on('render-process-gone', (event, details) => this._onDidError.fire({ type: WindowError.PROCESS_GONE, details }));
this.window.on('unresponsive', () => this._onDidError.fire({ type: WindowError.UNRESPONSIVE }));
this.window.webContents.on('did-fail-load', (event, exitCode, reason) => this._onDidError.fire({ type: WindowError.LOAD, details: { reason, exitCode } }));
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/window/electron-main/window.ts
Expand Up @@ -155,9 +155,9 @@ export const enum WindowError {
UNRESPONSIVE = 1,

/**
* Maps to the `render-proces-gone` event on a `WebContents`.
* Maps to the `render-process-gone` event on a `WebContents`.
*/
CRASHED = 2,
PROCESS_GONE = 2,

/**
* Maps to the `did-fail-load` event on a `WebContents`.
Expand Down
43 changes: 23 additions & 20 deletions src/vs/platform/windows/electron-main/windowImpl.ts
Expand Up @@ -517,9 +517,9 @@ export class CodeWindow extends Disposable implements ICodeWindow {

private registerListeners(): void {

// Crashes & Unresponsive & Failed to load
// Window error conditions to handle
this._win.on('unresponsive', () => this.onWindowError(WindowError.UNRESPONSIVE));
this._win.webContents.on('render-process-gone', (event, details) => this.onWindowError(WindowError.CRASHED, details));
this._win.webContents.on('render-process-gone', (event, details) => this.onWindowError(WindowError.PROCESS_GONE, details));
this._win.webContents.on('did-fail-load', (event, exitCode, reason) => this.onWindowError(WindowError.LOAD, { reason, exitCode }));

// Prevent windows/iframes from blocking the unload
Expand Down Expand Up @@ -618,13 +618,13 @@ export class CodeWindow extends Disposable implements ICodeWindow {
}

private async onWindowError(error: WindowError.UNRESPONSIVE): Promise<void>;
private async onWindowError(error: WindowError.CRASHED, details: { reason: string; exitCode: number }): Promise<void>;
private async onWindowError(error: WindowError.PROCESS_GONE, details: { reason: string; exitCode: number }): Promise<void>;
private async onWindowError(error: WindowError.LOAD, details: { reason: string; exitCode: number }): Promise<void>;
private async onWindowError(type: WindowError, details?: { reason: string; exitCode: number }): Promise<void> {

switch (type) {
case WindowError.CRASHED:
this.logService.error(`CodeWindow: renderer process crashed (reason: ${details?.reason || '<unknown>'}, code: ${details?.exitCode || '<unknown>'})`);
case WindowError.PROCESS_GONE:
this.logService.error(`CodeWindow: renderer process gone (reason: ${details?.reason || '<unknown>'}, code: ${details?.exitCode || '<unknown>'})`);
break;
case WindowError.UNRESPONSIVE:
this.logService.error('CodeWindow: detected unresponsive');
Expand All @@ -636,11 +636,11 @@ export class CodeWindow extends Disposable implements ICodeWindow {

// Telemetry
type WindowErrorClassification = {
type: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The type of window crash to understand the nature of the crash better.' };
reason: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The reason of the window crash to understand the nature of the crash better.' };
code: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The exit code of the window process to understand the nature of the crash better' };
type: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The type of window error to understand the nature of the error better.' };
reason: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The reason of the window error to understand the nature of the error better.' };
code: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'The exit code of the window process to understand the nature of the error better' };
owner: 'bpasero';
comment: 'Provides insight into reasons the vscode window crashes.';
comment: 'Provides insight into reasons the vscode window had an error.';
};
type WindowErrorEvent = {
type: WindowError;
Expand All @@ -652,19 +652,22 @@ export class CodeWindow extends Disposable implements ICodeWindow {
// Inform User if non-recoverable
switch (type) {
case WindowError.UNRESPONSIVE:
case WindowError.CRASHED:
case WindowError.PROCESS_GONE:

// If we run extension tests from CLI, showing a dialog is not
// very helpful in this case. Rather, we bring down the test run
// to signal back a failing run.
// If we run extension tests from CLI, we want to signal
// back this state to the test runner by exiting with a
// non-zero exit code.
if (this.isExtensionDevelopmentTestFromCli) {
this.lifecycleMainService.kill(1);
return;
}

// If we run smoke tests, we never want to show a blocking dialog
// If we run smoke tests, want to proceed with an orderly
// shutdown as much as possible by destroying the window
// and then calling the normal `quit` routine.
if (this.environmentMainService.args['enable-smoke-test-driver']) {
this.destroyWindow(false, false);
this.lifecycleMainService.quit(); // still allow for an orderly shutdown
return;
}

Expand Down Expand Up @@ -704,13 +707,13 @@ export class CodeWindow extends Disposable implements ICodeWindow {
}
}

// Crashed
else if (type === WindowError.CRASHED) {
// Process gone
else if (type === WindowError.PROCESS_GONE) {
let message: string;
if (!details) {
message = localize('appCrashed', "The window has crashed");
message = localize('appGone', "The window terminated unexpectedly");
} else {
message = localize('appCrashedDetails', "The window has crashed (reason: '{0}', code: '{1}')", details.reason, details.exitCode ?? '<unknown>');
message = localize('appGoneDetails', "The window terminated unexpectedly (reason: '{0}', code: '{1}')", details.reason, details.exitCode ?? '<unknown>');
}

// Show Dialog
Expand All @@ -722,7 +725,7 @@ export class CodeWindow extends Disposable implements ICodeWindow {
mnemonicButtonLabel(localize({ key: 'close', comment: ['&& denotes a mnemonic'] }, "&&Close"))
],
message,
detail: localize('appCrashedDetail', "We are sorry for the inconvenience. You can reopen the window to continue where you left off."),
detail: localize('appGoneDetail', "We are sorry for the inconvenience. You can reopen the window to continue where you left off."),
noLink: true,
defaultId: 0,
checkboxLabel: this._config?.workspace ? localize('doNotRestoreEditors', "Don't restore editors") : undefined
Expand Down Expand Up @@ -754,7 +757,7 @@ export class CodeWindow extends Disposable implements ICodeWindow {
// 'close' event will not be fired on destroy(), so signal crash via explicit event
this._onDidDestroy.fire();

// make sure to destroy the window as it has crashed
// make sure to destroy the window as its renderer process is gone
this._win?.destroy();

// ask the windows service to open a new fresh window if specified
Expand Down