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

Display Kernel startup Errors in cell output for more visibility #8304

Merged
merged 4 commits into from
Nov 19, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved out of interactive window class, this is not specific to just interactive window anymore.

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