From 278b2dc34ff9bc1ee8874369b24ea7252734f8cb Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 22 Mar 2022 04:10:32 +1100 Subject: [PATCH] Fixes to kernel installation prmopts and tests --- src/client/common/utils/async.ts | 4 +- src/client/telemetry/index.ts | 5 +- src/extension/errors/errorHandler.ts | 7 +- src/extension/errors/kernelDeadError.ts | 19 ++ src/kernels/helpers.ts | 165 +++++++++++++++--- src/kernels/installer/moduleInstaller.ts | 23 ++- src/kernels/installer/productInstaller.ts | 12 +- src/kernels/installer/types.ts | 3 +- .../jupyterInterpreterDependencyService.ts | 4 +- src/kernels/jupyter/jupyterKernelService.ts | 6 +- .../launcher/jupyterNotebookProvider.ts | 3 +- .../launcher/liveshare/hostJupyterServer.ts | 24 ++- .../jupyter/launcher/notebookProvider.ts | 3 +- .../jupyter/launcher/notebookStarter.ts | 10 +- src/kernels/jupyter/session/jupyterSession.ts | 25 ++- src/kernels/kernel.ts | 65 +++++-- src/kernels/kernelCommandListener.ts | 67 ++++--- src/kernels/kernelDependencyService.ts | 37 +++- src/kernels/kernelProvider.ts | 6 +- src/kernels/raw/launcher/kernelLauncher.ts | 6 +- src/kernels/raw/launcher/kernelProcess.ts | 11 +- .../raw/session/hostRawNotebookProvider.ts | 6 +- src/kernels/raw/session/rawJupyterSession.ts | 6 +- src/kernels/types.ts | 3 +- .../controllers/vscodeNotebookController.ts | 50 ++++-- src/notebooks/execution/cellExecution.ts | 3 + src/notebooks/execution/cellExecutionQueue.ts | 23 ++- src/notebooks/execution/kernelExecution.ts | 8 +- src/notebooks/helpers.ts | 4 +- .../installationPrompts.vscode.test.ts | 23 +-- 30 files changed, 476 insertions(+), 155 deletions(-) create mode 100644 src/extension/errors/kernelDeadError.ts diff --git a/src/client/common/utils/async.ts b/src/client/common/utils/async.ts index 299186f953d..35faf78c45b 100644 --- a/src/client/common/utils/async.ts +++ b/src/client/common/utils/async.ts @@ -138,9 +138,9 @@ export function createDeferred(scope: any = null): Deferred { return new DeferredImpl(scope); } -export function createDeferredFrom(...promises: Promise[]): Deferred { +export function createDeferredFrom(promise: Promise): Deferred { const deferred = createDeferred(); - Promise.all(promises) + promise // eslint-disable-next-line @typescript-eslint/no-explicit-any .then(deferred.resolve.bind(deferred) as any) // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c039081e5fb..d023051f34b 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -18,7 +18,7 @@ import { } from '../datascience/constants'; import { ResourceSpecificTelemetryProperties } from '../datascience/telemetry/types'; import { ExportFormat } from '../datascience/export/types'; -import { InterruptResult } from '../datascience/types'; +import { InterruptResult, KernelInterpreterDependencyResponse } from '../datascience/types'; import { CheckboxState, EventName, PlatformErrors, SliceOperationSource } from './constants'; import { noop } from '../common/utils/misc'; import { isPromise } from 'rxjs/internal-compatibility'; @@ -846,6 +846,7 @@ export interface IEventNamePropertyMapping { */ isModulePresent?: 'true' | undefined; action: + | 'cancelled' // User cancelled the installation or closed the notebook or the like. | 'displayed' // Install prompt may have been displayed. | 'prompted' // Install prompt was displayed. | 'installed' // Installation disabled (this is what python extension returns). @@ -1309,7 +1310,7 @@ export interface IEventNamePropertyMapping { [Telemetry.RawKernelSessionStartTimeout]: never | undefined; [Telemetry.RawKernelSessionStartUserCancel]: never | undefined; [Telemetry.RawKernelSessionStartNoIpykernel]: { - reason: number; + reason: KernelInterpreterDependencyResponse; } & TelemetryErrorProperties; /** * This event is sent when the underlying kernelProcess for a diff --git a/src/extension/errors/errorHandler.ts b/src/extension/errors/errorHandler.ts index 75283a144d4..87c35f4d055 100644 --- a/src/extension/errors/errorHandler.ts +++ b/src/extension/errors/errorHandler.ts @@ -39,6 +39,7 @@ import { JupyterKernelDependencyError } from './jupyterKernelDependencyError'; import { WrappedError, BaseKernelError, WrappedKernelError, BaseError } from './types'; import { noop } from '../../client/common/utils/misc'; import { EnvironmentType } from '../../client/pythonEnvironments/info'; +import { KernelDeadError } from './kernelDeadError'; @injectable() export class DataScienceErrorHandler implements IDataScienceErrorHandler { @@ -97,7 +98,11 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { context: 'start' | 'restart' | 'interrupt' | 'execution' ) { error = WrappedError.unwrap(error); - if (error instanceof JupyterKernelDependencyError) { + if (error instanceof KernelDeadError) { + // When we get this we've already asked the user to restart the kernel, + // No need to display errors in each cell. + return ''; + } else if (error instanceof JupyterKernelDependencyError) { return getIPyKernelMissingErrorMessageForCell(error.kernelConnectionMetadata) || error.message; } else if (error instanceof JupyterInstallError) { return getJupyterMissingErrorMessageForCell(error) || error.message; diff --git a/src/extension/errors/kernelDeadError.ts b/src/extension/errors/kernelDeadError.ts new file mode 100644 index 00000000000..ebc8d973247 --- /dev/null +++ b/src/extension/errors/kernelDeadError.ts @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { DataScience } from '../../client/common/utils/localize'; +import { getDisplayNameOrNameOfKernelConnection } from '../../kernels/helpers'; +import { KernelConnectionMetadata } from '../../kernels/types'; +import { WrappedKernelError } from './types'; + +export class KernelDeadError extends WrappedKernelError { + constructor(kernelConnectionMetadata: KernelConnectionMetadata) { + super( + DataScience.kernelDiedWithoutError().format( + getDisplayNameOrNameOfKernelConnection(kernelConnectionMetadata) + ), + undefined, + kernelConnectionMetadata + ); + } +} diff --git a/src/kernels/helpers.ts b/src/kernels/helpers.ts index c0f6ea211e7..516fc9ae304 100644 --- a/src/kernels/helpers.ts +++ b/src/kernels/helpers.ts @@ -31,7 +31,7 @@ import { traceError, traceInfo, traceInfoIfCI, traceVerbose, traceWarning } from import { getDisplayPath } from '../client/common/platform/fs-paths'; import { IPythonExecutionFactory } from '../client/common/process/types'; import { IPathUtils, IConfigurationService, Resource, IMemento, GLOBAL_MEMENTO } from '../client/common/types'; -import { createDeferred } from '../client/common/utils/async'; +import { createDeferred, createDeferredFromPromise, Deferred } from '../client/common/utils/async'; import { DataScience } from '../client/common/utils/localize'; import { getResourceType } from '../client/datascience/common'; import { Settings } from '../client/datascience/constants'; @@ -44,7 +44,8 @@ import { IDataScienceErrorHandler, IRawNotebookProvider, KernelInterpreterDependencyResponse, - IJupyterKernelSpec + IJupyterKernelSpec, + IDisplayOptions } from '../client/datascience/types'; import { IServiceContainer } from '../client/ioc/types'; import { @@ -71,6 +72,8 @@ import { isPythonNotebook } from '../notebooks/helpers'; import { INotebookControllerManager } from '../notebooks/types'; import { PreferredRemoteKernelIdProvider } from './raw/finder/preferredRemoteKernelIdProvider'; import { findNotebookEditor, selectKernel } from '../notebooks/controllers/kernelSelector'; +import { DisplayOptions } from '../client/datascience/displayOptions'; +import { KernelDeadError } from '../extension/errors/kernelDeadError'; // Helper functions for dealing with kernels and kernelspecs @@ -1975,10 +1978,10 @@ export async function handleKernelError( return resultController; } -function convertContextToFunction(context: 'start' | 'interrupt' | 'restart') { +function convertContextToFunction(context: 'start' | 'interrupt' | 'restart', options?: IDisplayOptions) { switch (context) { case 'start': - return (k: IKernel) => k.start(); + return (k: IKernel) => k.start(options); case 'interrupt': return (k: IKernel) => k.interrupt(); @@ -1988,17 +1991,117 @@ function convertContextToFunction(context: 'start' | 'interrupt' | 'restart') { } } +const connections = new WeakMap< + NotebookDocument, + { + kernel: Deferred<{ + kernel: IKernel; + controller: VSCodeNotebookController; + deadKernelAction?: 'deadKernelWasRestarted' | 'deadKernelWasNoRestarted'; + }>; + options: IDisplayOptions; + } +>(); + export async function wrapKernelMethod( initialController: VSCodeNotebookController, initialContext: 'start' | 'interrupt' | 'restart', serviceContainer: IServiceContainer, resource: Resource, - notebook: NotebookDocument -) { + notebook: NotebookDocument, + options: IDisplayOptions = new DisplayOptions(false) +): Promise { + let currentPromise = connections.get(notebook); + if (!options.disableUI && currentPromise?.options.disableUI) { + currentPromise.options.disableUI = false; + } + // If the current kernel has been disposed or in the middle of being disposed, then create another one. + // But do that only if we require a UI, else we can just use the current one. + if ( + !options.disableUI && + currentPromise?.kernel.resolved && + (currentPromise?.kernel.value?.kernel?.disposed || currentPromise?.kernel.value?.kernel?.disposing) + ) { + connections.delete(notebook); + currentPromise = undefined; + } + const verifyKernelState = async ( + promise: Promise<{ + kernel: IKernel; + controller: VSCodeNotebookController; + deadKernelAction?: 'deadKernelWasRestarted' | 'deadKernelWasNoRestarted'; + }> + ): Promise => { + const { kernel, controller, deadKernelAction } = await promise; + // Before returning, but without disposing the kernel, double check it's still valid + // If a restart didn't happen, then we can't connect. Throw an error. + // Do this outside of the loop so that subsequent calls will still ask because the kernel isn't disposed + if (kernel.status === 'dead' || (kernel.status === 'terminating' && !kernel.disposed && !kernel.disposing)) { + // If the kernel is dead, then remove the cached promise, & try to get the kernel again. + // At that point, it will get restarted. + if (connections.get(notebook)?.kernel.promise === promise) { + connections.delete(notebook); + } + if (deadKernelAction === 'deadKernelWasNoRestarted') { + throw new KernelDeadError(kernel.kernelConnectionMetadata); + } else if (deadKernelAction === 'deadKernelWasRestarted') { + return kernel; + } + // Kernel is dead and we didn't prompt the user to restart it, hence re-run the code that will prompt the user for a restart. + return wrapKernelMethod(controller, 'start', serviceContainer, resource, notebook, options); + } + return kernel; + }; + + // Wrap the kernel method again to interrupt/restart this kernel. + if (currentPromise && initialContext !== 'restart' && initialContext !== 'interrupt') { + return verifyKernelState(currentPromise.kernel.promise); + } + + const promise = wrapKernelMethodImpl( + initialController, + initialContext, + serviceContainer, + resource, + notebook, + options + ); + const deferred = createDeferredFromPromise(promise); + // If the kernel gets disposed or we fail to create the kernel, then ensure we remove the cached result. + promise + .then((result) => { + result.kernel.onDisposed(() => { + if (connections.get(notebook)?.kernel === deferred) { + connections.delete(notebook); + } + }); + }) + .catch(() => { + if (connections.get(notebook)?.kernel === deferred) { + connections.delete(notebook); + } + }); + + connections.set(notebook, { kernel: deferred, options }); + return verifyKernelState(deferred.promise); +} + +export async function wrapKernelMethodImpl( + initialController: VSCodeNotebookController, + initialContext: 'start' | 'interrupt' | 'restart', + serviceContainer: IServiceContainer, + resource: Resource, + notebook: NotebookDocument, + options: IDisplayOptions = new DisplayOptions(false) +): Promise<{ + kernel: IKernel; + controller: VSCodeNotebookController; + deadKernelAction?: 'deadKernelWasRestarted' | 'deadKernelWasNoRestarted'; +}> { const kernelProvider = serviceContainer.get(IKernelProvider); let kernel: IKernel | undefined; let controller: VSCodeNotebookController = initialController; - let currentMethod = convertContextToFunction(initialContext); + let currentMethod = convertContextToFunction(initialContext, options); let context = initialContext; while (kernel === undefined) { // Try to create the kernel (possibly again) @@ -2008,46 +2111,52 @@ export async function wrapKernelMethod( resourceUri: resource }); + const isKernelDead = (k: IKernel) => + k.status === 'dead' || (k.status === 'terminating' && !k.disposed && !k.disposing); + try { - await currentMethod(kernel); + // If the kernel is dead, ask the user if they want to restart. + // We need to perform this check first, as its possible we'd call this method for dead kernels. + // & if the kernel is dead, prompt to restart. + if (initialContext !== 'restart' && isKernelDead(kernel) && !options.disableUI) { + const restarted = await notifyAndRestartDeadKernel(kernel, serviceContainer); + return { + kernel, + controller, + deadKernelAction: restarted ? 'deadKernelWasRestarted' : 'deadKernelWasNoRestarted' + }; + } else { + await currentMethod(kernel); - // If the kernel is dead, ask the user if they want to restart - if ( - kernel.status === 'dead' || - (kernel.status === 'terminating' && !kernel.disposed && !kernel.disposing) - ) { - await notifyAndRestartDeadKernel(kernel, serviceContainer); + // If the kernel is dead, ask the user if they want to restart + if (isKernelDead(kernel) && !options.disableUI) { + await notifyAndRestartDeadKernel(kernel, serviceContainer); + } } } catch (error) { + if (options.disableUI) { + throw error; + } controller = await handleKernelError(serviceContainer, error, context, resource, kernel, controller); // When we wrap around, update the current method to start. This // means if we're handling a restart or an interrupt that fails, we move onto trying to start the kernel. - currentMethod = (k) => k.start(); + currentMethod = (k) => k.start(options); context = 'start'; // Since an error occurred, we have to try again (controller may have switched so we have to pick a new kernel) kernel = undefined; } } - // Before returning, but without disposing the kernel, double check it's still valid - // If a restart didn't happen, then we can't connect. Throw an error. - // Do this outside of the loop so that subsequent calls will still ask because the kernel isn't disposed - if (kernel.status === 'dead' || (kernel.status === 'terminating' && !kernel.disposed && !kernel.disposing)) { - throw new Error( - DataScience.kernelDiedWithoutError().format( - getDisplayNameOrNameOfKernelConnection(kernel.kernelConnectionMetadata) - ) - ); - } - return kernel; + return { kernel, controller }; } export async function connectToKernel( initialController: VSCodeNotebookController, serviceContainer: IServiceContainer, resource: Resource, - notebook: NotebookDocument + notebook: NotebookDocument, + options: IDisplayOptions = new DisplayOptions(false) ): Promise { - return wrapKernelMethod(initialController, 'start', serviceContainer, resource, notebook); + return wrapKernelMethod(initialController, 'start', serviceContainer, resource, notebook, options); } diff --git a/src/kernels/installer/moduleInstaller.ts b/src/kernels/installer/moduleInstaller.ts index ee4238e541b..ea3da688220 100644 --- a/src/kernels/installer/moduleInstaller.ts +++ b/src/kernels/installer/moduleInstaller.ts @@ -55,6 +55,9 @@ export abstract class ModuleInstaller implements IModuleInstaller { const environmentService = this.serviceContainer.get( IEnvironmentVariablesService ); + if (cancelTokenSource.token.isCancellationRequested) { + return; + } const install = async ( progress: Progress<{ message?: string | undefined; @@ -62,18 +65,21 @@ export abstract class ModuleInstaller implements IModuleInstaller { }>, token: CancellationToken ) => { + const deferred = createDeferred(); // When the progress is canceled notify caller token.onCancellationRequested(() => { cancelTokenSource.cancel(); + deferred.resolve(); }); - // Args can be for a specific exe or for the interpreter. Both need to - // use an activated environment though - const deferred = createDeferred(); + let observable: ObservableExecutionResult | undefined; // Some installers only work with shellexec if (args.useShellExec) { const proc = await procFactory.create(undefined); + if (cancelTokenSource.token.isCancellationRequested) { + return; + } try { const results = await proc.shellExec(args.args.join(' '), { cwd: args.cwd }); traceInfo(results.stdout); @@ -82,15 +88,26 @@ export abstract class ModuleInstaller implements IModuleInstaller { deferred.reject(ex); } } else if (args.exe) { + // Args can be for a specific exe or for the interpreter. Both need to + // use an activated environment though // For the exe, just figure out the environment variables. const envVars = await activationHelper.getActivatedEnvironmentVariables(undefined, interpreter, false); + if (cancelTokenSource.token.isCancellationRequested) { + return; + } const env = { ...process.env }; environmentService.mergeVariables(envVars || {}, env); environmentService.mergePaths(envVars || {}, env); const proc = await procFactory.create(undefined); + if (cancelTokenSource.token.isCancellationRequested) { + return; + } observable = proc.execObservable(args.exe, args.args, { encoding: 'utf-8', token, env, cwd: args.cwd }); } else { const proc = await pythonFactory.createActivatedEnvironment({ interpreter }); + if (cancelTokenSource.token.isCancellationRequested) { + return; + } observable = proc.execObservable(args.args, { encoding: 'utf-8', token, diff --git a/src/kernels/installer/productInstaller.ts b/src/kernels/installer/productInstaller.ts index c792b1101cb..03b963ba6af 100644 --- a/src/kernels/installer/productInstaller.ts +++ b/src/kernels/installer/productInstaller.ts @@ -117,6 +117,9 @@ abstract class BaseInstaller { if (!installer) { return InstallerResponse.Ignore; } + if (cancelTokenSource.token.isCancellationRequested) { + return InstallerResponse.Cancelled; + } let flags = reInstallAndUpdate === true ? ModuleInstallFlags.updateDependencies | ModuleInstallFlags.reInstall @@ -125,7 +128,9 @@ abstract class BaseInstaller { flags = flags ? flags | ModuleInstallFlags.installPipIfRequired : ModuleInstallFlags.installPipIfRequired; } await installer.installModule(product, interpreter, cancelTokenSource, flags); - + if (cancelTokenSource.token.isCancellationRequested) { + return InstallerResponse.Cancelled; + } return this.isInstalled(product, interpreter).then((isInstalled) => { return isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore; }); @@ -257,7 +262,7 @@ export class ProductInstaller implements IInstaller { if (interpreter) { this.interpreterPackages.trackPackages(interpreter); } - let action: 'installed' | 'failed' | 'disabled' | 'ignored' = 'installed'; + let action: 'installed' | 'failed' | 'disabled' | 'ignored' | 'cancelled' = 'installed'; try { const result = await this.createInstaller(product).install( product, @@ -271,6 +276,9 @@ export class ProductInstaller implements IInstaller { this._onInstalled.fire({ product, resource: interpreter }); } switch (result) { + case InstallerResponse.Cancelled: + action = 'cancelled'; + break; case InstallerResponse.Installed: action = 'installed'; break; diff --git a/src/kernels/installer/types.ts b/src/kernels/installer/types.ts index 4a1d91e581a..60a71b24a0e 100644 --- a/src/kernels/installer/types.ts +++ b/src/kernels/installer/types.ts @@ -8,7 +8,8 @@ import { PythonEnvironment } from '../../client/pythonEnvironments/info'; export enum InstallerResponse { Installed, Disabled, - Ignore + Ignore, + Cancelled } export enum Product { diff --git a/src/kernels/jupyter/interpreter/jupyterInterpreterDependencyService.ts b/src/kernels/jupyter/interpreter/jupyterInterpreterDependencyService.ts index 7ac788d6965..1105448874e 100644 --- a/src/kernels/jupyter/interpreter/jupyterInterpreterDependencyService.ts +++ b/src/kernels/jupyter/interpreter/jupyterInterpreterDependencyService.ts @@ -174,7 +174,7 @@ export class JupyterInterpreterDependencyService { sortProductsInOrderForInstallation(productsToInstall); let productToInstall = productsToInstall.shift(); - const cancellatonPromise = createPromiseFromCancellation({ + const cancellationPromise = createPromiseFromCancellation({ cancelAction: 'resolve', defaultValue: InstallerResponse.Ignore, token: tokenSource.token @@ -189,7 +189,7 @@ export class JupyterInterpreterDependencyService { undefined, pipInstalledInNonCondaEnv === false ), - cancellatonPromise + cancellationPromise ]); if (response === InstallerResponse.Installed) { productToInstall = productsToInstall.shift(); diff --git a/src/kernels/jupyter/jupyterKernelService.ts b/src/kernels/jupyter/jupyterKernelService.ts index ebb9a8489a1..869c6758a3d 100644 --- a/src/kernels/jupyter/jupyterKernelService.ts +++ b/src/kernels/jupyter/jupyterKernelService.ts @@ -9,7 +9,7 @@ import * as path from 'path'; import { CancellationToken } from 'vscode'; import { Cancellation } from '../../client/common/cancellation'; import '../../client/common/extensions'; -import { traceInfoIfCI, traceInfo } from '../../client/common/logger'; +import { traceInfoIfCI, traceInfo, traceVerbose } from '../../client/common/logger'; import { getDisplayPath } from '../../client/common/platform/fs-paths'; import { IFileSystem } from '../../client/common/platform/types'; import { Resource, ReadWrite } from '../../client/common/types'; @@ -55,13 +55,13 @@ export class JupyterKernelService { * @param resource * @param kernel */ - @traceDecorators.verbose('Check if a kernel is usable') public async ensureKernelIsUsable( resource: Resource, @logValue('id') kernel: KernelConnectionMetadata, - @ignoreLogging() ui: IDisplayOptions, + @logValue('disableUI') ui: IDisplayOptions, @ignoreLogging() cancelToken: CancellationToken ): Promise { + traceVerbose('Check if a kernel is usable'); // If we have an interpreter, make sure it has the correct dependencies installed if ( kernel.kind !== 'connectToLiveKernel' && diff --git a/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts b/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts index ad3e1f3bd8c..b2735f708b7 100644 --- a/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts +++ b/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts @@ -15,6 +15,7 @@ import { INotebook } from '../../../client/datascience/types'; import { isLocalConnection } from '../../types'; +import { Cancellation } from '../../../client/common/cancellation'; // When the NotebookProvider looks to create a notebook it uses this class to create a Jupyter notebook @injectable() @@ -51,7 +52,7 @@ export class JupyterNotebookProvider implements IJupyterNotebookProvider { token: options.token, localJupyter: isLocalConnection(options.kernelConnection) }); - + Cancellation.throwIfCanceled(options.token); if (server) { return server.createNotebook(options.resource, options.kernelConnection, options.token, options.ui); } diff --git a/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts b/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts index a0049a6d506..ae168d53cdf 100644 --- a/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts +++ b/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts @@ -7,7 +7,7 @@ import { CancellationToken } from 'vscode-jsonrpc'; import { injectable, inject, named } from 'inversify'; import { IWorkspaceService } from '../../../../client/common/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../../../../client/common/constants'; -import { traceInfo, traceError } from '../../../../client/common/logger'; +import { traceInfo, traceError, traceInfoIfCI } from '../../../../client/common/logger'; import { IAsyncDisposableRegistry, IOutputChannel, @@ -33,6 +33,8 @@ import { computeWorkingDirectory } from '../../jupyterUtils'; import { JupyterSessionManager } from '../../session/jupyterSessionManager'; import { JupyterNotebook } from '../jupyterNotebook'; import { noop } from '../../../../client/common/utils/misc'; +import { getDisplayPath } from '../../../../client/common/platform/fs-paths'; +import { Cancellation } from '../../../../client/common/cancellation'; /* eslint-disable @typescript-eslint/no-explicit-any */ @injectable() @@ -66,7 +68,12 @@ export class HostJupyterServer implements INotebookServer { private get isDisposed() { return this.disposed; } - + private throwIfDisposedOrCancelled(token?: CancellationToken) { + if (this.isDisposed) { + throw new SessionDisposedError(); + } + Cancellation.throwIfCanceled(token); + } private async createNotebookInstance( resource: Resource, sessionManager: JupyterSessionManager, @@ -74,17 +81,19 @@ export class HostJupyterServer implements INotebookServer { cancelToken: CancellationToken, ui: IDisplayOptions ): Promise { + this.throwIfDisposedOrCancelled(cancelToken); // Compute launch information from the resource and the notebook metadata const notebookPromise = createDeferred(); // Save the notebook this.trackDisposable(notebookPromise.promise); const getExistingSession = async () => { const connection = await this.computeLaunchInfo(); - + this.throwIfDisposedOrCancelled(cancelToken); // Figure out the working directory we need for our new notebook. This is only necessary for local. const workingDirectory = isLocalConnection(kernelConnection) ? await computeWorkingDirectory(resource, this.workspaceService) : ''; + this.throwIfDisposedOrCancelled(cancelToken); // Start a session (or use the existing one if allowed) const session = await sessionManager.startNew( resource, @@ -93,12 +102,14 @@ export class HostJupyterServer implements INotebookServer { ui, cancelToken ); + this.throwIfDisposedOrCancelled(cancelToken); traceInfo(`Started session for kernel ${kernelConnection.id}`); return { connection, session }; }; try { const { connection, session } = await getExistingSession(); + this.throwIfDisposedOrCancelled(cancelToken); if (session) { // Create our notebook @@ -158,6 +169,12 @@ export class HostJupyterServer implements INotebookServer { cancelToken: CancellationToken, ui: IDisplayOptions ): Promise { + this.throwIfDisposedOrCancelled(cancelToken); + traceInfoIfCI( + `HostJupyterServer.createNotebook for ${getDisplayPath(resource)} with ui.disableUI=${ + ui.disableUI + }, cancelToken.isCancellationRequested=${cancelToken.isCancellationRequested}` + ); if (!this.sessionManager || this.isDisposed) { throw new SessionDisposedError(); } @@ -171,6 +188,7 @@ export class HostJupyterServer implements INotebookServer { cancelToken, ui ); + this.throwIfDisposedOrCancelled(cancelToken); const baseUrl = this.connection?.baseUrl || ''; this.logRemoteOutput(DataScience.createdNewNotebook().format(baseUrl)); sendKernelTelemetryEvent(resource, Telemetry.JupyterCreatingNotebook, stopWatch.elapsedTime); diff --git a/src/kernels/jupyter/launcher/notebookProvider.ts b/src/kernels/jupyter/launcher/notebookProvider.ts index 25cc946c072..089b93471db 100644 --- a/src/kernels/jupyter/launcher/notebookProvider.ts +++ b/src/kernels/jupyter/launcher/notebookProvider.ts @@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify'; import { IPythonExtensionChecker } from '../../../client/api/types'; +import { Cancellation } from '../../../client/common/cancellation'; import { IConfigurationService } from '../../../client/common/types'; import { Settings } from '../../../client/datascience/constants'; import { DisplayOptions } from '../../../client/datascience/displayOptions'; @@ -78,7 +79,7 @@ export class NotebookProvider implements INotebookProvider { return undefined; } } - + Cancellation.throwIfCanceled(options.token); trackKernelResourceInformation(options.resource, { kernelConnection: options.kernelConnection }); const promise = rawLocalKernel ? this.rawNotebookProvider.createNotebook( diff --git a/src/kernels/jupyter/launcher/notebookStarter.ts b/src/kernels/jupyter/launcher/notebookStarter.ts index 5f2237892e6..93560d19c57 100644 --- a/src/kernels/jupyter/launcher/notebookStarter.ts +++ b/src/kernels/jupyter/launcher/notebookStarter.ts @@ -11,7 +11,7 @@ import * as path from 'path'; import * as uuid from 'uuid/v4'; import { CancellationError, CancellationToken, Disposable } from 'vscode'; import { IDisposable } from '@fluentui/react'; -import { createPromiseFromCancellation } from '../../../client/common/cancellation'; +import { Cancellation, createPromiseFromCancellation } from '../../../client/common/cancellation'; import { disposeAllDisposables } from '../../../client/common/helpers'; import { traceInfo, traceError } from '../../../client/common/logger'; import { IFileSystem, TemporaryDirectory } from '../../../client/common/platform/types'; @@ -91,9 +91,7 @@ export class NotebookStarter implements Disposable { ); // Make sure we haven't canceled already. - if (cancelToken.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(cancelToken); // Then use this to launch our notebook process. traceInfo('Starting Jupyter Notebook'); @@ -135,9 +133,7 @@ export class NotebookStarter implements Disposable { cancelToken ); // Make sure we haven't canceled already. - if (cancelToken && cancelToken.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(cancelToken); const connection = await Promise.race([ starter.waitForConnection(), createPromiseFromCancellation({ diff --git a/src/kernels/jupyter/session/jupyterSession.ts b/src/kernels/jupyter/session/jupyterSession.ts index a05c9c69771..8c660a7e75a 100644 --- a/src/kernels/jupyter/session/jupyterSession.ts +++ b/src/kernels/jupyter/session/jupyterSession.ts @@ -29,6 +29,7 @@ import { getNameOfKernelConnection } from '../../helpers'; import { KernelConnectionMetadata, isLocalConnection } from '../../types'; import { JupyterKernelService } from '../jupyterKernelService'; import { JupyterWebSockets } from './jupyterWebSocket'; +import { getDisplayPath } from '../../../client/common/platform/fs-paths'; const jvscIdentifier = '-jvsc-'; function getRemoteIPynbSuffix(): string { @@ -145,12 +146,15 @@ export class JupyterSession extends BaseJupyterSession { await this.waitForIdleOnSession(newSession, this.idleTimeout); } } catch (exc) { + // Don't log errors if UI is disabled (e.g. auto starting a kernel) + // Else we just pollute the logs with lots of noise. + const loggerFn = options.ui.disableUI ? traceVerbose : traceError; // Don't swallow known exceptions. if (exc instanceof BaseError) { - traceError('Failed to change kernel, re-throwing', exc); + loggerFn('Failed to change kernel, re-throwing', exc); throw exc; } else { - traceError('Failed to change kernel', exc); + loggerFn('Failed to change kernel', exc); // Throw a new exception indicating we cannot change. throw new JupyterInvalidKernelError(this.kernelConnectionMetadata); } @@ -252,6 +256,11 @@ export class JupyterSession extends BaseJupyterSession { token: CancellationToken; ui: IDisplayOptions; }): Promise { + traceInfoIfCI( + `JupyterSession.createSession for ${getDisplayPath(this.resource)}, options.ui.disableUI=${ + options.ui.disableUI + }` + ); // Create our backing file for the notebook const backingFile = await this.createBackingFile(); @@ -259,7 +268,11 @@ export class JupyterSession extends BaseJupyterSession { if (this.kernelConnectionMetadata?.interpreter && isLocalConnection(this.kernelConnectionMetadata)) { // Make sure the kernel actually exists and is up to date. try { - traceInfoIfCI(`JupyterSession.createSession ${this.kernelConnectionMetadata.id}`); + traceInfoIfCI( + `JupyterSession.createSession ${this.kernelConnectionMetadata.id} for ${getDisplayPath( + this.resource + )}, options.ui.disableUI=${options.ui.disableUI}` + ); await this.kernelService.ensureKernelIsUsable( this.resource, this.kernelConnectionMetadata, @@ -291,7 +304,11 @@ export class JupyterSession extends BaseJupyterSession { type: 'notebook' }; - traceInfo(`Starting a new session for kernel id = ${this.kernelConnectionMetadata?.id}, name = ${kernelName}`); + traceInfo( + `Starting a new session for kernel id = ${ + this.kernelConnectionMetadata?.id + }, name = ${kernelName} for ${getDisplayPath(this.resource)}` + ); return Cancellation.race( () => this.sessionManager!.startNew(sessionOptions, { diff --git a/src/kernels/kernel.ts b/src/kernels/kernel.ts index 71eb77f8faa..3a2da1d158a 100644 --- a/src/kernels/kernel.ts +++ b/src/kernels/kernel.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. 'use strict'; +import * as uuid from 'uuid/v4'; import type * as nbformat from '@jupyterlab/nbformat'; import type { KernelMessage } from '@jupyterlab/services'; import { Observable } from 'rxjs/Observable'; @@ -14,8 +15,7 @@ import { NotebookController, NotebookDocument, ColorThemeKind, - Disposable, - CancellationError + Disposable } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../client/common/application/types'; import { WrappedError } from '../client/../extension/errors/types'; @@ -47,7 +47,8 @@ import { IJupyterSession, INotebookProvider, IStatusProvider, - InterruptResult + InterruptResult, + IDisplayOptions } from '../client/datascience/types'; import { calculateWorkingDirectory } from '../client/datascience/utils'; import { sendTelemetryEvent } from '../client/telemetry'; @@ -70,8 +71,14 @@ import { NotebookCellRunState } from './types'; import { KernelExecution } from '../notebooks/execution/kernelExecution'; +import { traceCellMessage } from '../notebooks/helpers'; +import { Cancellation } from '../client/common/cancellation'; export class Kernel implements IKernel { + /** + * Used for debugging purposes, ability to uniquely identify kernels. + */ + public readonly id: string; get connection(): INotebookProviderConnection | undefined { return this.notebook?.connection; } @@ -162,6 +169,7 @@ export class Kernel implements IKernel { private readonly pythonExecutionFactory: IPythonExecutionFactory, private statusProvider: IStatusProvider ) { + this.id = `${uuid()}#${kernelConnectionMetadata.id}`; this.kernelExecution = new KernelExecution( this, appShell, @@ -185,6 +193,7 @@ export class Kernel implements IKernel { } public async executeCell(cell: NotebookCell): Promise { + traceCellMessage(cell, `kernel.executeCell, ${getDisplayPath(cell.notebook.uri)}`); sendKernelTelemetryEvent(this.resourceUri, Telemetry.ExecuteCell); const stopWatch = new StopWatch(); const sessionPromise = this.startNotebook().then((nb) => nb.session); @@ -194,13 +203,14 @@ export class Kernel implements IKernel { return promise; } public async executeHidden(code: string): Promise { + traceInfoIfCI(`Execute hidden code ${code}`); const stopWatch = new StopWatch(); const sessionPromise = this.startNotebook().then((nb) => nb.session); const promise = sessionPromise.then((session) => executeSilently(session, code)); this.trackNotebookCellPerceivedColdTime(stopWatch, sessionPromise, promise).catch(noop); return promise; } - public async start(options?: { disableUI?: boolean }): Promise { + public async start(options?: IDisplayOptions): Promise { await this.startNotebook(options); } public async interrupt(): Promise { @@ -239,6 +249,7 @@ export class Kernel implements IKernel { } } public async dispose(): Promise { + traceInfoIfCI(`Dispose Kernel ${getDisplayPath(this.notebookDocument.uri)}`); this._disposing = true; if (this.disposingPromise) { return this.disposingPromise; @@ -248,13 +259,14 @@ export class Kernel implements IKernel { const disposeImpl = async () => { traceInfo(`Dispose kernel ${(this.resourceUri || this.notebookDocument.uri).toString()}`); this.restarting = undefined; + const promises: Promise[] = []; + promises.push(this.kernelExecution.cancel()); this.notebook = this.notebook ? this.notebook : this._notebookPromise ? await this._notebookPromise : undefined; this._notebookPromise = undefined; - const promises: Promise[] = []; if (this.notebook) { promises.push(this.notebook.session.dispose().catch(noop)); this.notebook = undefined; @@ -289,7 +301,7 @@ export class Kernel implements IKernel { // If the notebook died, then start a new notebook. await (this._notebookPromise ? this.kernelExecution.restart(this._notebookPromise?.then((item) => item.session)) - : this.start({ disableUI: false })); + : this.start(new DisplayOptions(false))); traceInfoIfCI(`Restarted ${getDisplayPath(this.notebookDocument.uri)}`); sendKernelTelemetryEvent(this.resourceUri, Telemetry.NotebookRestart, stopWatch.elapsedTime); } catch (ex) { @@ -340,11 +352,19 @@ export class Kernel implements IKernel { ); } } - private async startNotebook(options: { disableUI?: boolean } = { disableUI: false }): Promise { + private async startNotebook(options: IDisplayOptions = new DisplayOptions(false)): Promise { this._startedAtLeastOnce = true; + traceInfoIfCI( + `Start Notebook (options.disableUI=${options.disableUI}) for ${getDisplayPath(this.notebookDocument.uri)}.` + ); if (!options.disableUI) { this.startupUI.disableUI = false; } + options.onDidChangeDisableUI(() => { + if (!options.disableUI && this.startupUI.disableUI) { + this.startupUI.disableUI = false; + } + }); if (!this.startupUI.disableUI) { // This means the user is actually running something against the kernel (deliberately). initializeInteractiveOrNotebookTelemetryBasedOnUserAction(this.resourceUri, this.kernelConnectionMetadata); @@ -368,8 +388,15 @@ export class Kernel implements IKernel { await this.restarting.promise; } if (!this._notebookPromise) { - this.startCancellation = new CancellationTokenSource(); + // Don't create a new one unnecessarily. + if (this.startCancellation.token.isCancellationRequested) { + traceInfoIfCI(`Create new cancellation token for ${getDisplayPath(this.notebookDocument.uri)}`); + this.startCancellation = new CancellationTokenSource(); + } this._notebookPromise = this.createNotebook(new StopWatch()).catch((ex) => { + traceInfoIfCI( + `Failed to create Notebook in Kernel.startNotebook for ${getDisplayPath(this.notebookDocument.uri)}` + ); // If we fail also clear the promise. this.startCancellation.cancel(); this._notebookPromise = undefined; @@ -383,7 +410,11 @@ export class Kernel implements IKernel { let disposables: Disposable[] = []; try { // No need to block kernel startup on UI updates. - traceInfo(`Starting Notebook in kernel.ts id = ${this.kernelConnectionMetadata.id}`); + traceInfo( + `Starting Notebook in kernel.ts id = ${this.kernelConnectionMetadata.id} for ${getDisplayPath( + this.notebookDocument.uri + )}` + ); this.createProgressIndicator(disposables); this.isKernelDead = false; this._onStatusChanged.fire('starting'); @@ -394,9 +425,7 @@ export class Kernel implements IKernel { kernelConnection: this.kernelConnectionMetadata, token: this.startCancellation.token }); - if (this.startCancellation.token.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(this.startCancellation.token); if (!notebook) { // This is an unlikely case. // getOrCreateNotebook would return undefined only if getOnly = true (an issue with typings). @@ -413,10 +442,14 @@ export class Kernel implements IKernel { this._onStarted.fire(); return notebook; } catch (ex) { - traceError(`failed to create INotebook in kernel, UI Disabled = ${this.startupUI.disableUI}`, ex); - if (this.startCancellation.token.isCancellationRequested) { - throw new CancellationError(); + // Don't log errors if UI is disabled (e.g. auto starting a kernel) + // Else we just pollute the logs with lots of noise. + if (this.startupUI.disableUI) { + traceVerbose(`failed to create INotebook in kernel, UI Disabled = ${this.startupUI.disableUI}`, ex); + } else { + traceError(`failed to create INotebook in kernel, UI Disabled = ${this.startupUI.disableUI}`, ex); } + Cancellation.throwIfCanceled(this.startCancellation.token); if (ex instanceof JupyterConnectError) { throw ex; } @@ -459,7 +492,7 @@ export class Kernel implements IKernel { } private async initializeAfterStart(notebook: INotebook | undefined, notebookDocument: NotebookDocument) { - traceVerbose('Started running kernel initialization'); + traceVerbose(`Started running kernel initialization for ${getDisplayPath(this.notebookDocument.uri)}`); if (!notebook) { traceVerbose('Not running kernel initialization'); return; diff --git a/src/kernels/kernelCommandListener.ts b/src/kernels/kernelCommandListener.ts index 62b89f9aa3c..3135cd98c49 100644 --- a/src/kernels/kernelCommandListener.ts +++ b/src/kernels/kernelCommandListener.ts @@ -3,7 +3,7 @@ 'use strict'; import type { KernelMessage } from '@jupyterlab/services'; import { inject, injectable } from 'inversify'; -import { ConfigurationTarget, NotebookDocument, Uri, window, workspace } from 'vscode'; +import { ConfigurationTarget, Uri, window, workspace } from 'vscode'; import { IApplicationShell, ICommandManager } from '../client/common/application/types'; import { displayErrorsInCell } from '../client/../extension/errors/errorUtils'; import { traceInfo } from '../client/common/logger'; @@ -117,7 +117,7 @@ export class KernelCommandListener implements IDataScienceCommandListener { traceInfo(`Interrupt requested & no kernel.`); return; } - await this.wrapKernelMethod('interrupt', document, kernel); + await this.wrapKernelMethod('interrupt', kernel); } private async restartKernel(notebookUri: Uri | undefined) { @@ -154,36 +154,57 @@ export class KernelCommandListener implements IDataScienceCommandListener { ); if (response === dontAskAgain) { await this.disableAskForRestart(document.uri); - void this.wrapKernelMethod('restart', document, kernel); + void this.wrapKernelMethod('restart', kernel); } else if (response === yes) { - void this.wrapKernelMethod('restart', document, kernel); + void this.wrapKernelMethod('restart', kernel); } } else { - void this.wrapKernelMethod('restart', document, kernel); + void this.wrapKernelMethod('restart', kernel); } } } - private async wrapKernelMethod(context: 'interrupt' | 'restart', notebook: NotebookDocument, kernel: IKernel) { - // Get currently executing cell and controller - const currentCell = kernel.pendingCells[0]; - const controller = this.notebookControllerManager.getSelectedNotebookController(notebook); - try { - // Wrap the restart/interrupt in a loop that allows the user to switch - await wrapKernelMethod(controller!, context, this.serviceContainer, notebook.uri, notebook); - } catch (ex) { - if (currentCell) { - const cellExecution = CellExecutionCreator.getOrCreate(currentCell, kernel.controller); - displayErrorsInCell( - currentCell, - cellExecution, - await this.errorHandler.getErrorMessageForDisplayInCell(ex, context) + private readonly pendingRestartInterrupt = new WeakMap>(); + private async wrapKernelMethod(context: 'interrupt' | 'restart', kernel: IKernel) { + // We don't want to create multiple restarts/interrupt requests for the same kernel. + const pendingPromise = this.pendingRestartInterrupt.get(kernel); + if (pendingPromise) { + return pendingPromise; + } + const promise = (async () => { + // Get currently executing cell and controller + const currentCell = kernel.pendingCells[0]; + const controller = this.notebookControllerManager.getSelectedNotebookController(kernel.notebookDocument); + try { + // Wrap the restart/interrupt in a loop that allows the user to switch + await wrapKernelMethod( + controller!, + context, + this.serviceContainer, + kernel.resourceUri, + kernel.notebookDocument ); - cellExecution.end(false); - } else { - void this.applicationShell.showErrorMessage(ex.toString()); + } catch (ex) { + if (currentCell) { + const cellExecution = CellExecutionCreator.getOrCreate(currentCell, kernel.controller); + displayErrorsInCell( + currentCell, + cellExecution, + await this.errorHandler.getErrorMessageForDisplayInCell(ex, context) + ); + cellExecution.end(false); + } else { + void this.applicationShell.showErrorMessage(ex.toString()); + } } - } + })(); + promise.finally(() => { + if (this.pendingRestartInterrupt.get(kernel) === promise) { + this.pendingRestartInterrupt.delete(kernel); + } + }); + this.pendingRestartInterrupt.set(kernel, promise); + return promise; } private async shouldAskForRestart(notebookUri: Uri): Promise { diff --git a/src/kernels/kernelDependencyService.ts b/src/kernels/kernelDependencyService.ts index d7f33883b80..9ce4c6b9d7c 100644 --- a/src/kernels/kernelDependencyService.ts +++ b/src/kernels/kernelDependencyService.ts @@ -7,10 +7,11 @@ import { inject, injectable, named } from 'inversify'; import { CancellationToken, CancellationTokenSource, Memento } from 'vscode'; import { IApplicationShell } from '../client/common/application/types'; import { createPromiseFromCancellation } from '../client/common/cancellation'; -import { traceInfo, traceError } from '../client/common/logger'; +import { traceInfo, traceError, traceInfoIfCI } from '../client/common/logger'; import { getDisplayPath } from '../client/common/platform/fs-paths'; import { IMemento, GLOBAL_MEMENTO, IsCodeSpace, Resource } from '../client/common/types'; import { DataScience, Common } from '../client/common/utils/localize'; +import { noop } from '../client/common/utils/misc'; import { getResourceType } from '../client/datascience/common'; import { KernelProgressReporter } from '../client/datascience/progress/kernelProgressReporter'; import { @@ -21,7 +22,7 @@ import { } from '../client/datascience/types'; import { IServiceContainer } from '../client/ioc/types'; import { traceDecorators } from '../client/logging'; -import { ignoreLogging, logValue, TraceOptions } from '../client/logging/trace'; +import { ignoreLogging, logValue } from '../client/logging/trace'; import { EnvironmentType, PythonEnvironment } from '../client/pythonEnvironments/info'; import { sendTelemetryEvent } from '../client/telemetry'; import { getTelemetrySafeHashedString } from '../client/telemetry/helpers'; @@ -54,7 +55,7 @@ export class KernelDependencyService implements IKernelDependencyService { * Configures the python interpreter to ensure it can run a Jupyter Kernel by installing any missing dependencies. * If user opts not to install they can opt to select another interpreter. */ - @traceDecorators.verbose('Install Missing Dependencies', TraceOptions.ReturnValue) + @traceDecorators.verbose('Install Missing Dependencies') public async installMissingDependencies( resource: Resource, kernelConnection: KernelConnectionMetadata, @@ -62,7 +63,11 @@ export class KernelDependencyService implements IKernelDependencyService { @ignoreLogging() token: CancellationToken, ignoreCache?: boolean ): Promise { - traceInfo(`installMissingDependencies ${getDisplayPath(kernelConnection.interpreter?.path)}`); + traceInfo( + `installMissingDependencies ${getDisplayPath(kernelConnection.interpreter?.path)}, ui.disabled=${ + ui.disableUI + } for resource ${getDisplayPath(resource)}` + ); if ( kernelConnection.kind === 'connectToLiveKernel' || kernelConnection.kind === 'startUsingRemoteKernelSpec' || @@ -83,14 +88,25 @@ export class KernelDependencyService implements IKernelDependencyService { } // Cache the install run - const cancelTokenSource = new CancellationTokenSource(); let promise = this.installPromises.get(kernelConnection.interpreter.path); + let cancelTokenSource: CancellationTokenSource | undefined; if (!promise) { + const cancelTokenSource = new CancellationTokenSource(); + const disposable = token.onCancellationRequested(() => { + cancelTokenSource.cancel(); + disposable.dispose(); + }); promise = KernelProgressReporter.wrapAndReportProgress( resource, DataScience.installingMissingDependencies(), () => this.runInstaller(resource, kernelConnection.interpreter!, ui, cancelTokenSource) ); + promise + .finally(() => { + disposable.dispose(); + cancelTokenSource.dispose(); + }) + .catch(noop); this.installPromises.set(kernelConnection.interpreter.path, promise); } @@ -99,10 +115,11 @@ export class KernelDependencyService implements IKernelDependencyService { try { // This can throw an exception (if say it fails to install) or it can cancel dependencyResponse = await promise; - if (cancelTokenSource.token?.isCancellationRequested || token.isCancellationRequested) { + if (cancelTokenSource?.token?.isCancellationRequested || token.isCancellationRequested) { dependencyResponse = KernelInterpreterDependencyResponse.cancel; } } catch (ex) { + traceInfoIfCI(`Failed to install kernel dependency`, ex); // Failure occurred dependencyResponse = KernelInterpreterDependencyResponse.failed; } finally { @@ -163,6 +180,11 @@ export class KernelDependencyService implements IKernelDependencyService { ui: IDisplayOptions, cancelTokenSource: CancellationTokenSource ): Promise { + traceInfoIfCI( + `Run Installer for ${getDisplayPath(resource)} ui.disableUI=${ + ui.disableUI + }, cancelTokenSource.token.isCancellationRequested=${cancelTokenSource.token.isCancellationRequested}` + ); // If there's no UI, then cancel installation. if (ui.disableUI) { return KernelInterpreterDependencyResponse.uiHidden; @@ -213,6 +235,7 @@ export class KernelDependencyService implements IKernelDependencyService { pythonEnvType: interpreter.envType }); } + traceInfoIfCI(`Prompting user for install (this.isCodeSpace=${this.isCodeSpace}).`); const selection = this.isCodeSpace ? Common.install() : await Promise.race([ @@ -248,7 +271,7 @@ export class KernelDependencyService implements IKernelDependencyService { }); const cancellationPromise = createPromiseFromCancellation({ cancelAction: 'resolve', - defaultValue: InstallerResponse.Ignore, + defaultValue: InstallerResponse.Cancelled, token: cancelTokenSource.token }); // Always pass a cancellation token to `install`, to ensure it waits until the module is installed. diff --git a/src/kernels/kernelProvider.ts b/src/kernels/kernelProvider.ts index c407f8d0aae..6f8b90af255 100644 --- a/src/kernels/kernelProvider.ts +++ b/src/kernels/kernelProvider.ts @@ -6,7 +6,7 @@ import type { KernelMessage } from '@jupyterlab/services'; import { inject, injectable } from 'inversify'; import { Event, EventEmitter, NotebookDocument } from 'vscode'; import { IApplicationShell, IWorkspaceService, IVSCodeNotebook } from '../client/common/application/types'; -import { traceVerbose, traceWarning } from '../client/common/logger'; +import { traceInfoIfCI, traceVerbose, traceWarning } from '../client/common/logger'; import { getDisplayPath } from '../client/common/platform/fs-paths'; import { IFileSystem } from '../client/common/platform/types'; import { IPythonExecutionFactory } from '../client/common/process/types'; @@ -77,6 +77,7 @@ export class KernelProvider implements IKernelProvider { return this.kernelsByNotebook.get(notebook)?.kernel; } public async dispose() { + traceInfoIfCI(`Disposing all kernels from kernel provider`); const items = Array.from(this.pendingDisposables.values()); this.pendingDisposables.clear(); await Promise.all(items); @@ -149,6 +150,9 @@ export class KernelProvider implements IKernelProvider { ); } private disposeOldKernel(notebook: NotebookDocument) { + traceInfoIfCI( + `Disposing kernel associated with ${getDisplayPath(notebook.uri)}, isClosed=${notebook.isClosed}` + ); const kernelToDispose = this.kernelsByNotebook.get(notebook); if (kernelToDispose) { this.pendingDisposables.add(kernelToDispose.kernel); diff --git a/src/kernels/raw/launcher/kernelLauncher.ts b/src/kernels/raw/launcher/kernelLauncher.ts index bd5d3cac42f..6d27a79c41e 100644 --- a/src/kernels/raw/launcher/kernelLauncher.ts +++ b/src/kernels/raw/launcher/kernelLauncher.ts @@ -11,7 +11,7 @@ import { promisify } from 'util'; import * as uuid from 'uuid/v4'; import { CancellationError, CancellationToken, window } from 'vscode'; import { IPythonExtensionChecker } from '../../../client/api/types'; -import { createPromiseFromCancellation } from '../../../client/common/cancellation'; +import { Cancellation, createPromiseFromCancellation } from '../../../client/common/cancellation'; import { isTestExecution } from '../../../client/common/constants'; import { getTelemetrySafeErrorMessageFromPythonTraceback } from '../../../client/../extension/errors/errorUtils'; import { traceInfo, traceWarning } from '../../../client/common/logger'; @@ -204,9 +204,7 @@ export class KernelLauncher implements IKernelLauncher { ]); } catch (ex) { void kernelProcess.dispose(); - if (ex instanceof CancellationError || cancelToken?.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(cancelToken); throw ex; } diff --git a/src/kernels/raw/launcher/kernelProcess.ts b/src/kernels/raw/launcher/kernelProcess.ts index 7aa9f83340d..2ea05e35542 100644 --- a/src/kernels/raw/launcher/kernelProcess.ts +++ b/src/kernels/raw/launcher/kernelProcess.ts @@ -17,7 +17,7 @@ import { LocalKernelSpecConnectionMetadata, PythonKernelConnectionMetadata } fro import { IKernelConnection, IKernelProcess } from '../types'; import { KernelEnvironmentVariablesService } from './kernelEnvVarsService'; import { IPythonExtensionChecker } from '../../../client/api/types'; -import { createPromiseFromCancellation } from '../../../client/common/cancellation'; +import { Cancellation, createPromiseFromCancellation } from '../../../client/common/cancellation'; import { getTelemetrySafeErrorMessageFromPythonTraceback, getErrorMessageFromPythonTraceback @@ -125,9 +125,7 @@ export class KernelProcess implements IKernelProcess { // Update our connection arguments in the kernel spec await this.updateConnectionArgs(); - if (cancelToken.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(cancelToken); const exeObs = await this.launchAsObservable(workingDirectory, cancelToken); if (cancelToken.isCancellationRequested) { throw new CancellationError(); @@ -431,10 +429,7 @@ export class KernelProcess implements IKernelProcess { cwd: wdExists ? workingDirectory : process.cwd(), env }); - - if (cancelToken.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(cancelToken); } // If we are not python just use the ProcessExecutionFactory diff --git a/src/kernels/raw/session/hostRawNotebookProvider.ts b/src/kernels/raw/session/hostRawNotebookProvider.ts index abe89cd3d7e..642bc96763b 100644 --- a/src/kernels/raw/session/hostRawNotebookProvider.ts +++ b/src/kernels/raw/session/hostRawNotebookProvider.ts @@ -38,6 +38,7 @@ import { KernelConnectionMetadata } from '../../types'; import { IKernelLauncher } from '../types'; import { RawJupyterSession } from './rawJupyterSession'; import { noop } from '../../../client/common/utils/misc'; +import { Cancellation } from '../../../client/common/cancellation'; // eslint-disable-next-line @typescript-eslint/no-require-imports /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -118,6 +119,7 @@ export class HostRawNotebookProvider implements IRawNotebookProvider { } traceInfo(`Computing working directory ${getDisplayPath(document.uri)}`); const workingDirectory = await computeWorkingDirectory(resource, this.workspaceService); + Cancellation.throwIfCanceled(cancelToken); const launchTimeout = this.configService.getSettings(resource).jupyterLaunchTimeout; const interruptTimeout = this.configService.getSettings(resource).jupyterInterruptTimeout; rawSession = new RawJupyterSession( @@ -140,7 +142,9 @@ export class HostRawNotebookProvider implements IRawNotebookProvider { `Connecting to raw session for ${getDisplayPath(document.uri)} with connection ${kernelConnection.id}` ); await rawSession.connect({ token: cancelToken, ui }); - + if (cancelToken.isCancellationRequested) { + throw new vscode.CancellationError(); + } if (rawSession.isConnected) { // Create our notebook const notebook = new JupyterNotebook(rawSession, this.rawConnection); diff --git a/src/kernels/raw/session/rawJupyterSession.ts b/src/kernels/raw/session/rawJupyterSession.ts index 0c1084d3af9..9954a74627a 100644 --- a/src/kernels/raw/session/rawJupyterSession.ts +++ b/src/kernels/raw/session/rawJupyterSession.ts @@ -5,7 +5,7 @@ import type { Kernel, KernelMessage } from '@jupyterlab/services'; import type { Slot } from '@lumino/signaling'; import { CancellationError, CancellationTokenSource } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; -import { createPromiseFromCancellation } from '../../../client/common/cancellation'; +import { Cancellation, createPromiseFromCancellation } from '../../../client/common/cancellation'; import { getTelemetrySafeErrorMessageFromPythonTraceback } from '../../../client/../extension/errors/errorUtils'; import { traceInfo, traceError, traceVerbose, traceWarning } from '../../../client/common/logger'; import { getDisplayPath } from '../../../client/common/platform/fs-paths'; @@ -84,9 +84,7 @@ export class RawJupyterSession extends BaseJupyterSession { // Try to start up our raw session, allow for cancellation or timeout // Notebook Provider level will handle the thrown error newSession = await this.startRawSession(options); - if (options.token?.isCancellationRequested) { - throw new CancellationError(); - } + Cancellation.throwIfCanceled(options.token); // Only connect our session if we didn't cancel or timeout sendKernelTelemetryEvent(this.resource, Telemetry.RawKernelSessionStartSuccess); sendKernelTelemetryEvent(this.resource, Telemetry.RawKernelSessionStart, stopWatch.elapsedTime); diff --git a/src/kernels/types.ts b/src/kernels/types.ts index 3ad7674b85b..331fe3880ee 100644 --- a/src/kernels/types.ts +++ b/src/kernels/types.ts @@ -9,6 +9,7 @@ import type { Event, NotebookCell, NotebookController, NotebookDocument, QuickPi import type * as nbformat from '@jupyterlab/nbformat'; import * as url from 'url'; import { + IDisplayOptions, IJupyterKernel, IJupyterKernelSpec, IJupyterSession, @@ -178,7 +179,7 @@ export interface IKernel extends IAsyncDisposable { * Controller associated with this kernel */ readonly controller: NotebookController; - start(options?: { disableUI?: boolean }): Promise; + start(options?: IDisplayOptions): Promise; interrupt(): Promise; restart(): Promise; executeCell(cell: NotebookCell): Promise; diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index ce146bb320d..19f8e2eaaa8 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -29,7 +29,7 @@ import { } from '../../client/common/application/types'; import { PYTHON_LANGUAGE } from '../../client/common/constants'; import { disposeAllDisposables } from '../../client/common/helpers'; -import { traceInfoIfCI, traceInfo, traceVerbose } from '../../client/common/logger'; +import { traceInfoIfCI, traceInfo, traceVerbose, traceWarning } from '../../client/common/logger'; import { getDisplayPath } from '../../client/common/platform/fs-paths'; import { IBrowserService, @@ -44,11 +44,12 @@ import { chainable } from '../../client/common/utils/decorators'; import { DataScience, Common } from '../../client/common/utils/localize'; import { noop } from '../../client/common/utils/misc'; import { sendNotebookOrKernelLanguageTelemetry } from '../../client/datascience/common'; +import { DisplayOptions } from '../../client/datascience/displayOptions'; import { initializeInteractiveOrNotebookTelemetryBasedOnUserAction, sendKernelTelemetryEvent } from '../../client/datascience/telemetry/telemetry'; -import { IDataScienceErrorHandler, KernelSocketInformation } from '../../client/datascience/types'; +import { IDataScienceErrorHandler, IDisplayOptions, KernelSocketInformation } from '../../client/datascience/types'; import { IServiceContainer } from '../../client/ioc/types'; import { traceDecorators } from '../../client/logging'; import { TraceOptions } from '../../client/logging/trace'; @@ -56,6 +57,7 @@ import { ConsoleForegroundColors } from '../../client/logging/_global'; import { EnvironmentType } from '../../client/pythonEnvironments/info'; import { Telemetry, Commands } from '../../datascience-ui/common/constants'; import { displayErrorsInCell } from '../../extension/errors/errorUtils'; +import { KernelDeadError } from '../../extension/errors/kernelDeadError'; import { WrappedError } from '../../extension/errors/types'; import { IPyWidgetMessages } from '../../extension/messageTypes'; import { NotebookCellLanguageService } from '../../intellisense/cellLanguageService'; @@ -226,6 +228,24 @@ export class VSCodeNotebookController implements Disposable { traceInfoIfCI('No cells passed to handleExecution'); return; } + // Found on CI that sometimes VS Code calls this with old deleted cells. + // See here https://github.com/microsoft/vscode-jupyter/runs/5581627878?check_suite_focus=true + cells = cells.filter((cell) => { + if (cell.index < 0) { + traceWarning( + `Attempting to run a cell with index ${cell.index}, kind ${ + cell.kind + }, text = ${cell.document.getText()}` + ); + return false; + } + return true; + }); + traceInfoIfCI( + `VSCodeNotebookController::handleExecution for ${getDisplayPath(notebook.uri)} for cells ${ + cells.length + } with data ${cells.map((cell) => cell.document.getText()).join('\n#CELL\n')}` + ); // When we receive a cell execute request, first ensure that the notebook is trusted. // If it isn't already trusted, block execution until the user trusts it. if (!this.workspace.isTrusted) { @@ -404,16 +424,23 @@ export class VSCodeNotebookController implements Disposable { private async executeCell(doc: NotebookDocument, cell: NotebookCell) { traceInfo(`Execute Cell ${cell.index} ${getDisplayPath(cell.notebook.uri)}`); - + const startTime = new Date().getTime(); // Start execution now (from the user's point of view) - const execution = CellExecutionCreator.getOrCreate(cell, this.controller); - execution.start(new Date().getTime()); + let execution = CellExecutionCreator.getOrCreate(cell, this.controller); + execution.start(startTime); void execution.clearOutput(cell); // Connect to a matching kernel if possible (but user may pick a different one) let context: 'start' | 'execution' = 'start'; + let kernel: IKernel | undefined; try { - const kernel = await this.connectToKernel(doc); + kernel = await this.connectToKernel(doc, new DisplayOptions(false)); + // If the controller changed, then ensure to create a new cell execution object. + if (kernel && kernel.controller.id !== execution.controllerId) { + execution.end(undefined); + execution = CellExecutionCreator.getOrCreate(cell, kernel.controller); + execution.start(startTime); + } context = 'execution'; if (kernel.controller.id === this.id) { this.updateKernelInfoInNotebookWhenAvailable(kernel, doc); @@ -421,12 +448,11 @@ export class VSCodeNotebookController implements Disposable { return await kernel.executeCell(cell); } catch (ex) { const errorHandler = this.serviceContainer.get(IDataScienceErrorHandler); - // If there was a failure connecting or executing the kernel, stick it in this cell displayErrorsInCell(cell, execution, await errorHandler.getErrorMessageForDisplayInCell(ex, context)); + ex = WrappedError.unwrap(ex); const isCancelled = - (WrappedError.unwrap(ex) || ex) instanceof CancellationError || - (WrappedError.unwrap(ex) || ex) instanceof VscCancellationError; + ex instanceof CancellationError || ex instanceof VscCancellationError || ex instanceof KernelDeadError; // If user cancels the execution, then don't show error status against cell. execution.end(isCancelled ? undefined : false); return NotebookCellExecutionState.Idle; @@ -436,11 +462,11 @@ export class VSCodeNotebookController implements Disposable { } @chainable() - private async connectToKernel(doc: NotebookDocument) { + private async connectToKernel(doc: NotebookDocument, options: IDisplayOptions) { // executeCell can get called multiple times before the first one is resolved. Since we only want // one of the calls to connect to the kernel, chain these together. The chained promise will then fail out // all of the cells if it fails. - return connectToKernel(this, this.serviceContainer, doc.uri, doc); + return connectToKernel(this, this.serviceContainer, doc.uri, doc, options); } private updateKernelInfoInNotebookWhenAvailable(kernel: IKernel, doc: NotebookDocument) { @@ -584,7 +610,7 @@ export class VSCodeNotebookController implements Disposable { isLocalConnection(this.kernelConnection) ) { // Startup could fail due to missing dependencies or the like. - void newKernel.start({ disableUI: true }); + this.connectToKernel(document, new DisplayOptions(true)).catch(noop); } } } diff --git a/src/notebooks/execution/cellExecution.ts b/src/notebooks/execution/cellExecution.ts index ea143b8e110..ed7735d3439 100644 --- a/src/notebooks/execution/cellExecution.ts +++ b/src/notebooks/execution/cellExecution.ts @@ -266,6 +266,9 @@ export class CellExecution implements IDisposable { * Hence `forced=true` is more like a hard kill. */ public async cancel(forced = false) { + if (this.cancelHandled) { + return; + } // Close all of the prompts (if we any any UI prompts asking user for input). this.prompts.forEach((item) => item.cancel()); if (this.started && !forced) { diff --git a/src/notebooks/execution/cellExecutionQueue.ts b/src/notebooks/execution/cellExecutionQueue.ts index fc55158b496..57d5189b3a4 100644 --- a/src/notebooks/execution/cellExecutionQueue.ts +++ b/src/notebooks/execution/cellExecutionQueue.ts @@ -116,6 +116,7 @@ export class CellExecutionQueue implements Disposable { } } private async executeQueuedCells() { + let notebookClosed: boolean | undefined; const kernelConnection = await this.session; this.queueOfCellsToExecute.forEach((exec) => traceCellMessage(exec.cell, 'Ready to execute')); while (this.queueOfCellsToExecute.length) { @@ -127,8 +128,14 @@ export class CellExecutionQueue implements Disposable { let executionResult: NotebookCellRunState | undefined; try { - await cellToExecute.start(kernelConnection); - executionResult = await cellToExecute.result; + if (cellToExecute.cell.notebook.isClosed) { + notebookClosed = true; + } else if (this.cancelledOrCompletedWithErrors) { + // Noop. + } else { + await cellToExecute.start(kernelConnection); + executionResult = await cellToExecute.result; + } } finally { // Once the cell has completed execution, remove it from the queue. traceCellMessage(cellToExecute.cell, `After Execute individual cell ${executionResult}`); @@ -138,10 +145,16 @@ export class CellExecutionQueue implements Disposable { } } - // If a cell has failed the get out. - if (this.cancelledOrCompletedWithErrors || executionResult === NotebookCellRunState.Error) { + // If notebook was closed or a cell has failed the get out. + if ( + notebookClosed || + this.cancelledOrCompletedWithErrors || + executionResult === NotebookCellRunState.Error + ) { this.cancelledOrCompletedWithErrors = true; - traceInfo(`Cancel all remaining cells ${this.cancelledOrCompletedWithErrors} || ${executionResult}`); + traceInfo( + `Cancel all remaining cells ${this.cancelledOrCompletedWithErrors} || ${executionResult} || ${notebookClosed}` + ); await this.cancel(); break; } diff --git a/src/notebooks/execution/kernelExecution.ts b/src/notebooks/execution/kernelExecution.ts index 53583987b62..0aff62ec6d2 100644 --- a/src/notebooks/execution/kernelExecution.ts +++ b/src/notebooks/execution/kernelExecution.ts @@ -8,7 +8,7 @@ import { CellExecutionFactory } from './cellExecution'; import { CellExecutionQueue } from './cellExecutionQueue'; import { KernelMessage } from '@jupyterlab/services'; import { IApplicationShell } from '../../client/common/application/types'; -import { traceInfo, traceWarning } from '../../client/common/logger'; +import { traceInfo, traceInfoIfCI, traceWarning } from '../../client/common/logger'; import { IDisposable, IDisposableRegistry } from '../../client/common/types'; import { createDeferred, waitForPromise } from '../../client/common/utils/async'; import { StopWatch } from '../../client/common/utils/stopWatch'; @@ -19,6 +19,8 @@ import { captureTelemetry } from '../../client/telemetry'; import { Telemetry } from '../../datascience-ui/common/constants'; import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; import { IKernel, KernelConnectionMetadata, NotebookCellRunState } from '../../kernels/types'; +import { traceCellMessage } from '../helpers'; +import { getDisplayPath } from '../../client/common/platform/fs-paths'; /** * Separate class that deals just with kernel execution. @@ -61,6 +63,7 @@ export class KernelExecution implements IDisposable { sessionPromise: Promise, cell: NotebookCell ): Promise { + traceCellMessage(cell, `KernelExecution.executeCell (1), ${getDisplayPath(cell.notebook.uri)}`); if (cell.kind == NotebookCellKind.Markup) { return NotebookCellRunState.Success; } @@ -70,9 +73,11 @@ export class KernelExecution implements IDisposable { await this._restartPromise; } + traceCellMessage(cell, `KernelExecution.executeCell (2), ${getDisplayPath(cell.notebook.uri)}`); const executionQueue = this.getOrCreateCellExecutionQueue(cell.notebook, sessionPromise); executionQueue.queueCell(cell); const result = await executionQueue.waitForCompletion([cell]); + traceCellMessage(cell, `KernelExecution.executeCell completed (3), ${getDisplayPath(cell.notebook.uri)}`); return result[0]; } public async cancel() { @@ -157,6 +162,7 @@ export class KernelExecution implements IDisposable { await this._restartPromise; } public dispose() { + traceInfoIfCI(`Dispose KernelExecution`); this.disposables.forEach((d) => d.dispose()); } private getOrCreateCellExecutionQueue(document: NotebookDocument, sessionPromise: Promise) { diff --git a/src/notebooks/helpers.ts b/src/notebooks/helpers.ts index 8300eb24980..08d54ca8a89 100644 --- a/src/notebooks/helpers.ts +++ b/src/notebooks/helpers.ts @@ -311,7 +311,9 @@ export class NotebookCellStateTracker { export function traceCellMessage(cell: NotebookCell, message: string) { traceInfoIfCI( - `Cell Index:${cell.index}, state:${NotebookCellStateTracker.getCellState(cell)}, exec: ${ + `Cell Index:${cell.index}, of document ${path.basename( + cell.notebook.uri.fsPath + )} with state:${NotebookCellStateTracker.getCellState(cell)}, exec: ${ cell.executionSummary?.executionOrder }. ${message}` ); diff --git a/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts b/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts index e9d2611b6ef..731690bbebd 100644 --- a/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts +++ b/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts @@ -33,7 +33,7 @@ import { closeActiveWindows, initialize } from '../../../initialize'; import { openNotebook, submitFromPythonFile } from '../../helpers'; import { JupyterNotebookView } from '../../../../notebooks/constants'; import { INotebookControllerManager } from '../../../../notebooks/types'; -import { WrappedError } from '../../../../client/../extension/errors/types'; +import { BaseKernelError, WrappedError } from '../../../../client/../extension/errors/types'; import { Commands } from '../../../../client/datascience/constants'; import { clearInstalledIntoInterpreterMemento } from '../../../../kernels/installer/productInstaller'; import { ProductNames } from '../../../../kernels/installer/productNames'; @@ -47,10 +47,10 @@ import { assertVSCCellIsNotRunning, defaultNotebookTestTimeout, waitForKernelToChange, - insertCodeCell, waitForExecutionCompletedSuccessfully, getCellOutputs, - waitForCellHavingOutput + waitForCellHavingOutput, + insertCodeCell } from '../../notebook/helper'; import * as kernelSelector from '../../../../notebooks/controllers/kernelSelector'; @@ -137,6 +137,7 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { .toString('utf8') .replace('', getInterpreterHash({ path: venvPythonPath })) ); + await installIPyKernel(venvKernelPath); await uninstallIPyKernel(venvPythonPath); await closeActiveWindows(); await Promise.all([ @@ -365,7 +366,7 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { waitForIPyKernelToGetInstalled() ]); }); - test.skip('Ensure ipykernel install prompt is displayed and we can select another kernel after uninstalling IPyKernel from a live notebook and then restarting the kernel (VSCode Notebook)', async function () { + test('Ensure ipykernel install prompt is displayed and we can select another kernel after uninstalling IPyKernel from a live notebook and then restarting the kernel (VSCode Notebook)', async function () { if (IS_REMOTE_NATIVE_TEST) { return this.skip(); } @@ -436,11 +437,11 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { await openNotebook(nbFile); await waitForKernelToGetAutoSelected(); - let cell = vscodeNotebook.activeNotebookEditor?.document.cellAt(0)!; + const cell = vscodeNotebook.activeNotebookEditor?.document.cellAt(0)!; assert.equal(cell.outputs.length, 0); // Insert another cell so we can test run all - await insertCodeCell('print("foo")'); + const cell2 = await insertCodeCell('print("foo")'); // The prompt should be displayed when we run a cell. const runPromise = runAllCellsInActiveNotebook(); @@ -448,11 +449,10 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { // Now the run should finish await runPromise; - cell = vscodeNotebook.activeNotebookEditor?.document.cellAt(0)!; - await waitForExecutionCompletedSuccessfully(cell); + await Promise.all([waitForExecutionCompletedSuccessfully(cell), waitForExecutionCompletedSuccessfully(cell2)]); }); - test.skip('Ensure ipykernel install prompt is NOT displayed when auto start is enabled & ipykernel is missing (VSCode Notebook)', async function () { + test('Ensure ipykernel install prompt is NOT displayed when auto start is enabled & ipykernel is missing (VSCode Notebook)', async function () { // Ensure we have auto start enabled, and verify kernel startup fails silently without any notifications. // When running a cell we should get an install prompt. configSettings.disableJupyterAutoStart = false; @@ -476,8 +476,9 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { await kernelStartSpy.getCall(0).returnValue; assert.fail('Did not fail as expected'); } catch (ex) { - const err = WrappedError.unwrap(ex); - assert.strictEqual(err.category, 'kerneldied'); + const err = WrappedError.unwrap(ex) as BaseKernelError; + // Depending on whether its jupter or raw kernels, we could get a different error thrown. + assert.include('noipykernel kerneldied', err.category); } console.log('Step6');