Skip to content

Commit

Permalink
Display Kernel startup Errors in cell output for more visibility (#8304)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored Nov 19, 2021
1 parent d9de7a5 commit f3f77cf
Show file tree
Hide file tree
Showing 14 changed files with 347 additions and 218 deletions.
337 changes: 208 additions & 129 deletions src/client/datascience/errors/errorHandler.ts

Large diffs are not rendered by default.

85 changes: 85 additions & 0 deletions src/client/datascience/errors/errorRendererComms.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { commands, notebooks, Position, Range, Selection, TextEditorRevealType, Uri } from 'vscode';
import { IExtensionSyncActivationService } from '../../activation/types';
import { IApplicationShell, ICommandManager, IDocumentManager } from '../../common/application/types';
import { IFileSystem } from '../../common/platform/types';
import { IDisposableRegistry } from '../../common/types';
import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes';
import { LineQueryRegex, linkCommandAllowList } from '../interactive-common/linkProvider';

@injectable()
export class ErrorRendererCommunicationHandler implements IExtensionSyncActivationService {
constructor(
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
@inject(ICommandManager) private readonly commandManager: ICommandManager,
@inject(IFileSystem) private readonly fs: IFileSystem,
@inject(IApplicationShell) private readonly applicationShell: IApplicationShell
) {}

activate(): void {
const messageChannel = notebooks.createRendererMessaging('jupyter-error-renderer');
this.disposables.push(
messageChannel.onDidReceiveMessage(async (e) => {
const message = e.message;
if (message.message === InteractiveWindowMessages.OpenLink) {
const href = message.payload;
if (href.startsWith('file')) {
await this.openFile(href);
} else if (href.startsWith('https://command:') || href.startsWith('command:')) {
const temp: string = href.startsWith('https://command:')
? href.split(':')[2]
: href.split(':')[1];
const params: string[] = temp.includes('/?') ? temp.split('/?')[1].split(',') : [];
let command = temp.split('/?')[0];
if (command.endsWith('/')) {
command = command.substring(0, command.length - 1);
}
if (linkCommandAllowList.includes(command)) {
await commands.executeCommand(command, params);
}
} else {
this.applicationShell.openUrl(href);
}
}
})
);
}

private async openFile(fileUri: string) {
const uri = Uri.parse(fileUri);
let selection: Range = new Range(new Position(0, 0), new Position(0, 0));
if (uri.query) {
// Might have a line number query on the file name
const lineMatch = LineQueryRegex.exec(uri.query);
if (lineMatch) {
const lineNumber = parseInt(lineMatch[1], 10);
selection = new Range(new Position(lineNumber, 0), new Position(lineNumber, 0));
}
}

// Show the matching editor if there is one
let editor = this.documentManager.visibleTextEditors.find((e) => this.fs.arePathsSame(e.document.uri, uri));
if (editor) {
return this.documentManager
.showTextDocument(editor.document, { selection, viewColumn: editor.viewColumn })
.then((e) => {
e.revealRange(selection, TextEditorRevealType.InCenter);
});
} else {
// Not a visible editor, try opening otherwise
return this.commandManager.executeCommand('vscode.open', uri).then(() => {
// See if that opened a text document
editor = this.documentManager.visibleTextEditors.find((e) => this.fs.arePathsSame(e.document.uri, uri));
if (editor) {
// Force the selection to change
editor.revealRange(selection);
editor.selection = new Selection(selection.start, selection.start);
}
});
}
}
}
2 changes: 2 additions & 0 deletions src/client/datascience/interactive-common/linkProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ export const LineQueryRegex = /line=(\d+)/;
// in a markdown cell using the syntax: https://command:[my.vscode.command].
export const linkCommandAllowList = [
'jupyter.latestExtension',
'jupyter.viewOutput',
'workbench.action.openSettings',
'jupyter.enableLoadingWidgetScriptsFromThirdPartySource'
];
80 changes: 2 additions & 78 deletions src/client/datascience/interactive-window/interactiveWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,27 @@ import {
workspace,
WorkspaceEdit,
notebooks,
Position,
Range,
Selection,
commands,
TextEditorRevealType,
ViewColumn,
NotebookEditor,
Disposable,
window,
ThemeColor
} from 'vscode';
import { IPythonExtensionChecker } from '../../api/types';
import {
IApplicationShell,
ICommandManager,
IDocumentManager,
IWorkspaceService
} from '../../common/application/types';
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
import { JVSC_EXTENSION_ID, MARKDOWN_LANGUAGE, PYTHON_LANGUAGE } from '../../common/constants';
import '../../common/extensions';
import { traceInfo, traceInfoIfCI } from '../../common/logger';
import { IFileSystem } from '../../common/platform/types';
import * as uuid from 'uuid/v4';

import { IConfigurationService, IDisposableRegistry, InteractiveWindowMode, Resource } from '../../common/types';
import { IConfigurationService, InteractiveWindowMode, Resource } from '../../common/types';
import { createDeferred, Deferred } from '../../common/utils/async';
import { noop } from '../../common/utils/misc';
import { generateCellsFromNotebookDocument } from '../cellFactory';
import { CellMatcher } from '../cellMatcher';
import { Commands, defaultNotebookFormat } from '../constants';
import { ExportFormat, IExportDialog } from '../export/types';
import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes';
import { IKernel, IKernelProvider, NotebookCellRunState } from '../jupyter/kernels/types';
import { INotebookControllerManager } from '../notebook/types';
import { VSCodeNotebookController } from '../notebook/vscodeNotebookController';
Expand All @@ -55,7 +44,6 @@ import { IInteractiveWindowLoadable, IInteractiveWindowDebugger, INotebookExport
import { getInteractiveWindowTitle } from './identity';
import { generateMarkdownFromCodeLines } from '../../../datascience-ui/common';
import { chainWithPendingUpdates } from '../notebook/helpers/notebookUpdater';
import { LineQueryRegex, linkCommandAllowList } from '../interactive-common/linkProvider';
import { INativeInteractiveWindow } from './types';
import { generateInteractiveCode } from '../../../datascience-ui/common/cellFactory';
import { initializeInteractiveOrNotebookTelemetryBasedOnUserAction } from '../telemetry/telemetry';
Expand Down Expand Up @@ -120,7 +108,6 @@ export class InteractiveWindow implements IInteractiveWindowLoadable {
private _inputUri: Uri | undefined;

constructor(
private readonly applicationShell: IApplicationShell,
private readonly documentManager: IDocumentManager,
private readonly fs: IFileSystem,
private readonly configuration: IConfigurationService,
Expand All @@ -133,7 +120,6 @@ export class InteractiveWindow implements IInteractiveWindowLoadable {
private readonly exportDialog: IExportDialog,
private readonly notebookControllerManager: INotebookControllerManager,
private readonly kernelProvider: IKernelProvider,
private readonly disposables: IDisposableRegistry,
private readonly interactiveWindowDebugger: IInteractiveWindowDebugger
) {
// Set our owner and first submitter
Expand Down Expand Up @@ -220,71 +206,9 @@ export class InteractiveWindow implements IInteractiveWindowLoadable {
})
);
this.listenForControllerSelection(notebookEditor.document);
this.initializeRendererCommunication();
return notebookEditor;
}

private initializeRendererCommunication() {
const messageChannel = notebooks.createRendererMessaging('jupyter-error-renderer');
this.disposables.push(
messageChannel.onDidReceiveMessage(async (e) => {
const message = e.message;
if (message.message === InteractiveWindowMessages.OpenLink) {
const href = message.payload;
if (href.startsWith('file')) {
await this.openFile(href);
} else if (href.startsWith('https://command:')) {
const temp: string = href.split(':')[2];
const params: string[] = temp.includes('/?') ? temp.split('/?')[1].split(',') : [];
let command = temp.split('/?')[0];
if (command.endsWith('/')) {
command = command.substring(0, command.length - 1);
}
if (linkCommandAllowList.includes(command)) {
await commands.executeCommand(command, params);
}
} else {
this.applicationShell.openUrl(href);
}
}
})
);
}

private async openFile(fileUri: string) {
const uri = Uri.parse(fileUri);
let selection: Range = new Range(new Position(0, 0), new Position(0, 0));
if (uri.query) {
// Might have a line number query on the file name
const lineMatch = LineQueryRegex.exec(uri.query);
if (lineMatch) {
const lineNumber = parseInt(lineMatch[1], 10);
selection = new Range(new Position(lineNumber, 0), new Position(lineNumber, 0));
}
}

// Show the matching editor if there is one
let editor = this.documentManager.visibleTextEditors.find((e) => this.fs.arePathsSame(e.document.uri, uri));
if (editor) {
return this.documentManager
.showTextDocument(editor.document, { selection, viewColumn: editor.viewColumn })
.then((e) => {
e.revealRange(selection, TextEditorRevealType.InCenter);
});
} else {
// Not a visible editor, try opening otherwise
return this.commandManager.executeCommand('vscode.open', uri).then(() => {
// See if that opened a text document
editor = this.documentManager.visibleTextEditors.find((e) => this.fs.arePathsSame(e.document.uri, uri));
if (editor) {
// Force the selection to change
editor.revealRange(selection);
editor.selection = new Selection(selection.start, selection.start);
}
});
}
}

private registerControllerChangeListener(controller: VSCodeNotebookController, notebookDocument: NotebookDocument) {
const controllerChangeListener = controller.controller.onDidChangeSelectedNotebooks(
(selectedEvent: { notebook: NotebookDocument; selected: boolean }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
// Set it as soon as we create it. The .ctor for the interactive window
// may cause a subclass to talk to the IInteractiveWindowProvider to get the active interactive window.
const result = new InteractiveWindow(
this.serviceContainer.get<IApplicationShell>(IApplicationShell),
this.serviceContainer.get<IDocumentManager>(IDocumentManager),
this.serviceContainer.get<IFileSystem>(IFileSystem),
this.serviceContainer.get<IConfigurationService>(IConfigurationService),
Expand All @@ -133,7 +132,6 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
this.serviceContainer.get<IExportDialog>(IExportDialog),
this.notebookControllerManager,
this.kernelProvider,
this.disposables,
this.serviceContainer.get<IInteractiveWindowDebugger>(IInteractiveWindowDebugger)
);
this._windows.push(result);
Expand Down
4 changes: 4 additions & 0 deletions src/client/datascience/jupyter/kernels/cellExecutionQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export class CellExecutionQueue implements Disposable {
public get failed(): boolean {
return this.cancelledOrCompletedWithErrors;
}
public get queue(): Readonly<NotebookCell[]> {
return this.queueOfCellsToExecute.map((cell) => cell.cell);
}
constructor(
private readonly session: Promise<IJupyterSession>,
private readonly executionFactory: CellExecutionFactory,
Expand Down Expand Up @@ -76,6 +79,7 @@ export class CellExecutionQueue implements Disposable {
this.cancelledOrCompletedWithErrors = true;
traceInfo('Cancel pending cells');
await Promise.all(this.queueOfCellsToExecute.map((item) => item.cancel(forced)));
this.queueOfCellsToExecute.splice(0, this.queueOfCellsToExecute.length);
}
/**
* Wait for cells to complete (for for the queue of cells to be processed)
Expand Down
15 changes: 13 additions & 2 deletions src/client/datascience/jupyter/kernels/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ export class Kernel implements IKernel {
public get session(): IJupyterSession | undefined {
return this.notebook?.session;
}
public get hasPendingCells() {
return this.kernelExecution.queue.length > 0;
}
private _disposed?: boolean;
private _disposing?: boolean;
private _ignoreNotebookDisposedErrors?: boolean;
Expand Down Expand Up @@ -410,9 +413,17 @@ export class Kernel implements IKernel {
// errors about startup failures.
traceWarning(`Ignoring kernel startup failure as kernel was disposed`, ex);
} else {
void this.errorHandler
const cellForErrorDisplay = this.kernelExecution.queue.length
? this.kernelExecution.queue[0]
: undefined;
void this.errorHandler.handleKernelError(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
.handleKernelError(ex as any, 'start', this.kernelConnectionMetadata, this.resourceUri); // Just a notification, so don't await this
ex as any,
'start',
this.kernelConnectionMetadata,
this.resourceUri,
cellForErrorDisplay
); // Just a notification, so don't await this
}
traceError(`failed to start INotebook in kernel, UI Disabled = ${this.startupUI.disableUI}`, ex);
this.startCancellation.cancel();
Expand Down
4 changes: 3 additions & 1 deletion src/client/datascience/jupyter/kernels/kernelExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ export class KernelExecution implements IDisposable {
public get onPreExecute() {
return this._onPreExecute.event;
}

public get queue() {
return this.documentExecutions.get(this.kernel.notebookDocument)?.queue || [];
}
public async executeCell(
sessionPromise: Promise<IJupyterSession>,
cell: NotebookCell
Expand Down
1 change: 1 addition & 0 deletions src/client/datascience/jupyter/kernels/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export interface IKernel extends IAsyncDisposable {
readonly onWillInterrupt: Event<void>;
readonly onPreExecute: Event<NotebookCell>;
readonly status: KernelMessage.Status;
readonly hasPendingCells: boolean;
readonly disposed: boolean;
readonly disposing: boolean;
/**
Expand Down
3 changes: 3 additions & 0 deletions src/client/datascience/kernel-launcher/kernelProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ export class KernelProcess implements IKernelProcess {
tcpPortUsed.waitUntilUsed(this.connection.shell_port, 200, timeout),
tcpPortUsed.waitUntilUsed(this.connection.iopub_port, 200, timeout)
]).catch((ex) => {
if (cancelToken.isCancellationRequested) {
return;
}
traceError(`waitUntilUsed timed out`, ex);
// Throw an error we recognize.
return Promise.reject(new KernelPortNotUsedTimeoutError(this.kernelConnectionMetadata));
Expand Down
2 changes: 2 additions & 0 deletions src/client/datascience/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ import { HostRawNotebookProvider } from './raw-kernel/liveshare/hostRawNotebookP
import { KernelCommandListener } from './jupyter/kernels/kernelCommandListener';
import { CellHashProviderFactory } from './editor-integration/cellHashProviderFactory';
import { ExportToPythonPlain } from './export/exportToPythonPlain';
import { ErrorRendererCommunicationHandler } from './errors/errorRendererComms';

// README: Did you make sure "dataScienceIocContainer.ts" has also been updated appropriately?

Expand Down Expand Up @@ -292,6 +293,7 @@ export function registerTypes(serviceManager: IServiceManager, inNotebookApiExpe
serviceManager.addSingleton<IJupyterServerUriStorage>(IJupyterServerUriStorage, JupyterServerUriStorage);
serviceManager.addSingleton<INotebookWatcher>(INotebookWatcher, NotebookWatcher);
serviceManager.addSingleton<IExtensionSyncActivationService>(IExtensionSyncActivationService, ExtensionRecommendationService);
serviceManager.addSingleton<IExtensionSyncActivationService>(IExtensionSyncActivationService, ErrorRendererCommunicationHandler);
serviceManager.addSingleton<IDebuggingManager>(IDebuggingManager, DebuggingManager, undefined, [IExtensionSingleActivationService]);

registerNotebookTypes(serviceManager);
Expand Down
3 changes: 2 additions & 1 deletion src/client/datascience/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,8 @@ export interface IDataScienceErrorHandler {
err: Error,
context: 'start' | 'restart' | 'interrupt' | 'execution',
kernelConnection: KernelConnectionMetadata,
resource: Resource
resource: Resource,
cellToDisplayErrors?: NotebookCell
): Promise<void>;
}

Expand Down
Loading

0 comments on commit f3f77cf

Please sign in to comment.