diff --git a/package.nls.json b/package.nls.json index f940f207483..8a9e38c64f1 100644 --- a/package.nls.json +++ b/package.nls.json @@ -230,6 +230,7 @@ "CommonSurvey.remindMeLaterLabel": "Remind me later", "CommonSurvey.yesLabel": "Yes, take survey now", "CommonSurvey.noLabel": "No, thanks", + "Common.clickHereForMoreInfoWithHtml": "Click here for more info.", "OutputChannelNames.languageServer": "Python Language Server", "OutputChannelNames.python": "Python", "OutputChannelNames.pythonTest": "Python Test Log", @@ -261,7 +262,6 @@ "DataScience.restartingKernelHeader": "_Restarting kernel..._", "DataScience.startedNewKernelHeader": "Started '{0}' kernel", "DataScience.connectKernelHeader": "Connected to '{0}' kernel", - "DataScience.canceledKernelHeader": "Canceled connection to '{0}' kernel", "DataScience.executingCodeFailure": "Executing code failed : {0}", "DataScience.inputWatermark": "Type code here and press shift-enter to run", "DataScience.deleteButtonTooltip": "Remove cell", diff --git a/src/client/common/errors/errorUtils.ts b/src/client/common/errors/errorUtils.ts index 1e8c1e16505..2a6823860f9 100644 --- a/src/client/common/errors/errorUtils.ts +++ b/src/client/common/errors/errorUtils.ts @@ -572,7 +572,7 @@ function isBuiltInModuleOverwritten( }; } -export async function displayErrorsInCell(cell: NotebookCell, execution: NotebookCellExecution, errorMessage: string) { +export function displayErrorsInCell(cell: NotebookCell, execution: NotebookCellExecution, errorMessage: string) { if (!errorMessage) { return; } @@ -593,5 +593,4 @@ export async function displayErrorsInCell(cell: NotebookCell, execution: Noteboo }) ]); void execution.appendOutput(output); - execution.end(false); } diff --git a/src/client/common/errors/types.ts b/src/client/common/errors/types.ts index d7bcfbfc686..e5b807cc959 100644 --- a/src/client/common/errors/types.ts +++ b/src/client/common/errors/types.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { EOL } from 'os'; +import { KernelConnectionMetadata } from '../../datascience/jupyter/kernels/types'; export abstract class BaseError extends Error { public stdErr?: string; @@ -10,6 +11,17 @@ export abstract class BaseError extends Error { } } +export abstract class BaseKernelError extends BaseError { + public stdErr?: string; + constructor( + category: ErrorCategory, + message: string, + public readonly kernelConnectionMetadata: KernelConnectionMetadata + ) { + super(category, message); + } +} + /** * Wraps an error with a custom error message, retaining the call stack information. */ @@ -44,6 +56,15 @@ export class WrappedError extends BaseError { return err; } } +export class WrappedKernelError extends WrappedError { + constructor( + message: string, + originalException: Error | undefined, + public readonly kernelConnectionMetadata: KernelConnectionMetadata + ) { + super(message, originalException); + } +} export function getErrorCategory(error?: Error): ErrorCategory { if (!error) { diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index ccbba3ceb83..880adbe312b 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -29,6 +29,10 @@ export namespace Common { export const learnMore = localize('Common.learnMore', 'Learn more'); export const and = localize('Common.and', 'and'); export const reportThisIssue = localize('Common.reportThisIssue', 'Report this issue'); + export const clickHereForMoreInfoWithHtml = localize( + 'Common.clickHereForMoreInfoWithHtml', + "Click here for more info." + ); } export namespace CommonSurvey { @@ -453,7 +457,6 @@ export namespace DataScience { export const startingNewKernelHeader = localize('DataScience.kernelStarting', '_Connecting to kernel..._'); export const startingNewKernelCustomHeader = localize('DataScience.kernelStartingCustom', '_Connecting to {0}..._'); export const connectKernelHeader = localize('DataScience.connectKernelHeader', 'Connected to {0}'); - export const canceledKernelHeader = localize('DataScience.canceledKernelHeader', 'Canceled connection to {0}'); export const jupyterSelectURIPrompt = localize( 'DataScience.jupyterSelectURIPrompt', @@ -978,10 +981,6 @@ export namespace DataScience { 'DataScience.ipykernelNotInstalled', 'IPyKernel not installed into interpreter {0}' ); - export const ipykernelNotInstalledBecauseCanceled = localize( - 'DataScience.ipykernelNotInstalledBecauseCanceled', - 'IPyKernel not installed into interpreter. Installation canceled.' - ); export const needIpykernel6 = localize('DataScience.needIpykernel6', 'Ipykernel setup required for this feature'); export const setup = localize('DataScience.setup', 'Setup'); export const startingRunByLine = localize('DataScience.startingRunByLine', 'Starting Run by Line'); diff --git a/src/client/datascience/baseJupyterSession.ts b/src/client/datascience/baseJupyterSession.ts index 52b342232e4..805ecb0e0ec 100644 --- a/src/client/datascience/baseJupyterSession.ts +++ b/src/client/datascience/baseJupyterSession.ts @@ -300,7 +300,6 @@ export abstract class BaseJupyterSession implements IJupyterSession { if (session.kernel.status == 'idle') { deferred.resolve(session.kernel.status); } - const result = await Promise.race([deferred.promise, sleep(timeout)]); session.kernel.statusChanged?.disconnect(handler); traceInfo(`Finished waiting for idle on (kernel): ${session.kernel.id} -> ${session.kernel.status}`); @@ -313,12 +312,12 @@ export abstract class BaseJupyterSession implements IJupyterSession { ); // If we throw an exception, make sure to shutdown the session as it's not usable anymore this.shutdownSession(session, this.statusHandler, isRestartSession).ignoreErrors(); - throw new JupyterWaitForIdleError(localize.DataScience.jupyterLaunchTimedOut()); + throw new JupyterWaitForIdleError(this.kernelConnectionMetadata); } finally { progress?.dispose(); } } else { - throw new JupyterInvalidKernelError(undefined); + throw new JupyterInvalidKernelError(this.kernelConnectionMetadata); } } diff --git a/src/client/datascience/errors/errorHandler.ts b/src/client/datascience/errors/errorHandler.ts index 307d49aa747..1d23c143f8c 100644 --- a/src/client/datascience/errors/errorHandler.ts +++ b/src/client/datascience/errors/errorHandler.ts @@ -3,7 +3,7 @@ import type * as nbformat from '@jupyterlab/nbformat'; import { inject, injectable } from 'inversify'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; -import { WrappedError } from '../../common/errors/types'; +import { BaseError, BaseKernelError, WrappedError, WrappedKernelError } from '../../common/errors/types'; import { traceWarning } from '../../common/logger'; import { Common, DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -17,22 +17,34 @@ import { IKernelDependencyService, KernelInterpreterDependencyResponse } from '../types'; -import { CancellationError as VscCancellationError, CancellationTokenSource, ConfigurationTarget } from 'vscode'; +import { + CancellationError as VscCancellationError, + CancellationTokenSource, + ConfigurationTarget, + workspace +} from 'vscode'; import { CancellationError } from '../../common/cancellation'; import { KernelConnectionTimeoutError } from './kernelConnectionTimeoutError'; import { KernelDiedError } from './kernelDiedError'; import { KernelPortNotUsedTimeoutError } from './kernelPortNotUsedTimeoutError'; import { KernelProcessExitedError } from './kernelProcessExitedError'; -import { PythonKernelDiedError } from './pythonKernelDiedError'; -import { analyzeKernelErrors, getErrorMessageFromPythonTraceback } from '../../common/errors/errorUtils'; +import { + analyzeKernelErrors, + getErrorMessageFromPythonTraceback, + KernelFailureReason +} from '../../common/errors/errorUtils'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; import { IBrowserService, IConfigurationService, Resource } from '../../common/types'; import { Commands, Telemetry } from '../constants'; import { sendTelemetryEvent } from '../../telemetry'; import { JupyterConnectError } from './jupyterConnectError'; -import { JupyterKernelDependencyError } from '../jupyter/kernels/jupyterKernelDependencyError'; import { JupyterInterpreterDependencyResponse } from '../jupyter/interpreter/jupyterInterpreterDependencyService'; import { DisplayOptions } from '../displayOptions'; +import { JupyterKernelDependencyError } from './jupyterKernelDependencyError'; +import { EnvironmentType } from '../../pythonEnvironments/info'; +import { translateProductToModule } from '../../../kernels/installer/moduleInstaller'; +import { ProductNames } from '../../../kernels/installer/productNames'; +import { Product } from '../../../kernels/installer/types'; @injectable() export class DataScienceErrorHandler implements IDataScienceErrorHandler { @@ -77,12 +89,10 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { } else if ( err instanceof KernelDiedError || err instanceof KernelProcessExitedError || - err instanceof PythonKernelDiedError || err instanceof JupyterConnectError ) { const defaultErrorMessage = getCombinedErrorMessage( - // PythonKernelDiedError has an `errorMessage` property, use that over `err.stdErr` for user facing error messages. - 'errorMessage' in err ? err.errorMessage : getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr + getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr ); this.applicationShell.showErrorMessage(defaultErrorMessage).then(noop, noop); } else { @@ -91,7 +101,58 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { this.applicationShell.showErrorMessage(message).then(noop, noop); } } - + public async getErrorMessageForDisplayInCell(error: Error) { + let message: string = error.message; + error = WrappedError.unwrap(error); + if (error instanceof JupyterKernelDependencyError) { + message = getIPyKernelMissingErrorMessageForCell(error.kernelConnectionMetadata) || message; + } else if (error instanceof JupyterInstallError) { + message = getJupyterMissingErrorMessageForCell(error) || message; + } else if (error instanceof VscCancellationError || error instanceof CancellationError) { + // Don't show the message for cancellation errors + traceWarning(`Cancelled by user`, error); + return ''; + } else if ( + error instanceof KernelDiedError && + (error.kernelConnectionMetadata.kind === 'startUsingLocalKernelSpec' || + error.kernelConnectionMetadata.kind === 'startUsingPythonInterpreter') && + error.kernelConnectionMetadata.interpreter && + !(await this.kernelDependency.areDependenciesInstalled(error.kernelConnectionMetadata, undefined, true)) + ) { + // We don't look for ipykernel dependencies before we start a kernel, hence + // its possible the kernel failed to start due to missing dependencies. + message = getIPyKernelMissingErrorMessageForCell(error.kernelConnectionMetadata) || message; + } else if (error instanceof BaseKernelError || error instanceof WrappedKernelError) { + const failureInfo = analyzeKernelErrors( + workspace.workspaceFolders || [], + error, + getDisplayNameOrNameOfKernelConnection(error.kernelConnectionMetadata), + error.kernelConnectionMetadata.interpreter?.sysPrefix + ); + if (failureInfo) { + // Special case for ipykernel module missing. + if ( + failureInfo.reason === KernelFailureReason.moduleNotFoundFailure && + ['ipykernel_launcher', 'ipykernel'].includes(failureInfo.moduleName) + ) { + return getIPyKernelMissingErrorMessageForCell(error.kernelConnectionMetadata) || message; + } + const messageParts = [failureInfo.message]; + if (failureInfo.moreInfoLink) { + messageParts.push(Common.clickHereForMoreInfoWithHtml().format(failureInfo.moreInfoLink)); + } + return messageParts.join('\n'); + } + return getCombinedErrorMessage( + getErrorMessageFromPythonTraceback(error.stdErr) || error.stdErr || error.message + ); + } else if (error instanceof BaseError) { + return getCombinedErrorMessage( + getErrorMessageFromPythonTraceback(error.stdErr) || error.stdErr || error.message + ); + } + return message; + } public async handleKernelError( err: Error, purpose: 'start' | 'restart' | 'interrupt' | 'execution', @@ -110,6 +171,30 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { return response === JupyterInterpreterDependencyResponse.ok ? KernelInterpreterDependencyResponse.ok : KernelInterpreterDependencyResponse.cancel; + } else if (err instanceof JupyterSelfCertsError) { + // On a self cert error, warn the user and ask if they want to change the setting + const enableOption: string = DataScience.jupyterSelfCertEnable(); + const closeOption: string = DataScience.jupyterSelfCertClose(); + void this.applicationShell + .showErrorMessage(DataScience.jupyterSelfCertFail().format(err.message), enableOption, closeOption) + .then((value) => { + if (value === enableOption) { + sendTelemetryEvent(Telemetry.SelfCertsMessageEnabled); + void this.configuration.updateSetting( + 'allowUnauthorizedRemoteConnection', + true, + undefined, + ConfigurationTarget.Workspace + ); + } else if (value === closeOption) { + sendTelemetryEvent(Telemetry.SelfCertsMessageClose); + } + }); + return KernelInterpreterDependencyResponse.failed; + } else if (err instanceof VscCancellationError || err instanceof CancellationError) { + // Don't show the message for cancellation errors + traceWarning(`Cancelled by user`, err); + return KernelInterpreterDependencyResponse.cancel; } else if ( (purpose === 'start' || purpose === 'restart') && !(await this.kernelDependency.areDependenciesInstalled(kernelConnection, undefined, true)) @@ -135,11 +220,18 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { ); if (failureInfo) { void this.showMessageWithMoreInfo(failureInfo?.message, failureInfo?.moreInfoLink); + } else if (err instanceof BaseError) { + const message = getCombinedErrorMessage( + getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr || err.message + ); + void this.showMessageWithMoreInfo(message); + } else { + void this.showMessageWithMoreInfo(err.message); } return KernelInterpreterDependencyResponse.failed; } } - private async showMessageWithMoreInfo(message: string, moreInfoLink: string | undefined) { + private async showMessageWithMoreInfo(message: string, moreInfoLink?: string) { if (!message.includes(Commands.ViewJupyterOutput)) { message = `${message} \n${DataScience.viewJupyterLogForFurtherInfo()}`; } @@ -172,3 +264,52 @@ export function getKernelNotInstalledErrorMessage(notebookMetadata?: nbformat.IN return DataScience.kernelNotInstalled().format(kernelName); } } + +function getIPyKernelMissingErrorMessageForCell(kernelConnection: KernelConnectionMetadata) { + if ( + kernelConnection.kind === 'connectToLiveKernel' || + kernelConnection.kind === 'startUsingRemoteKernelSpec' || + !kernelConnection.interpreter + ) { + return; + } + const displayNameOfKernel = kernelConnection.interpreter.displayName || kernelConnection.interpreter.path; + const ipyKernelName = ProductNames.get(Product.ipykernel)!; + const ipyKernelModuleName = translateProductToModule(Product.ipykernel); + + let installerCommand = `${kernelConnection.interpreter.path.fileToCommandArgument()} -m pip install ${ipyKernelModuleName} -U --force-reinstall`; + if (kernelConnection.interpreter?.envType === EnvironmentType.Conda) { + if (kernelConnection.interpreter?.envName) { + installerCommand = `conda install -n ${kernelConnection.interpreter?.envName} ${ipyKernelModuleName} --update-deps --force-reinstall`; + } else if (kernelConnection.interpreter?.envPath) { + installerCommand = `conda install -p ${kernelConnection.interpreter?.envPath} ${ipyKernelModuleName} --update-deps --force-reinstall`; + } + } else if ( + kernelConnection.interpreter?.envType === EnvironmentType.Global || + kernelConnection.interpreter?.envType === EnvironmentType.WindowsStore || + kernelConnection.interpreter?.envType === EnvironmentType.System + ) { + installerCommand = `${kernelConnection.interpreter.path.fileToCommandArgument()} -m pip install ${ipyKernelModuleName} -U --user --force-reinstall`; + } + const message = DataScience.libraryRequiredToLaunchJupyterKernelNotInstalledInterpreter().format( + displayNameOfKernel, + ProductNames.get(Product.ipykernel)! + ); + const installationInstructions = DataScience.installPackageInstructions().format(ipyKernelName, installerCommand); + return message + '\n' + installationInstructions; +} +function getJupyterMissingErrorMessageForCell(err: JupyterInstallError) { + const productNames = `${ProductNames.get(Product.jupyter)} ${Common.and()} ${ProductNames.get(Product.notebook)}`; + const moduleNames = [Product.jupyter, Product.notebook].map(translateProductToModule).join(' '); + + const installerCommand = `python -m pip install ${moduleNames} -U\nor\nconda install ${moduleNames} -U`; + const installationInstructions = DataScience.installPackageInstructions().format(productNames, installerCommand); + + return ( + err.message + + '\n' + + installationInstructions + + '\n' + + Common.clickHereForMoreInfoWithHtml().format('https://aka.ms/installJupyterForVSCode') + ); +} diff --git a/src/client/datascience/errors/jupyterDebuggerNotInstalledError.ts b/src/client/datascience/errors/jupyterDebuggerNotInstalledError.ts index f9330ed8b48..cf4c87034c9 100644 --- a/src/client/datascience/errors/jupyterDebuggerNotInstalledError.ts +++ b/src/client/datascience/errors/jupyterDebuggerNotInstalledError.ts @@ -1,15 +1,16 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import '../../common/extensions'; import * as localize from '../../common/utils/localize'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class JupyterDebuggerNotInstalledError extends BaseError { - constructor(debuggerPkg: string, message?: string) { +export class JupyterDebuggerNotInstalledError extends BaseKernelError { + constructor(debuggerPkg: string, message: string | undefined, kernelConnectionMetadata: KernelConnectionMetadata) { const errorMessage = message ? message : localize.DataScience.jupyterDebuggerNotInstalledError().format(debuggerPkg); - super('notinstalled', errorMessage); + super('notinstalled', errorMessage, kernelConnectionMetadata); } } diff --git a/src/client/datascience/errors/jupyterDebuggerPortBlockedError.ts b/src/client/datascience/errors/jupyterDebuggerPortBlockedError.ts deleted file mode 100644 index c9d1824aa66..00000000000 --- a/src/client/datascience/errors/jupyterDebuggerPortBlockedError.ts +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. -'use strict'; -import { BaseError } from '../../common/errors/types'; -import '../../common/extensions'; -import * as localize from '../../common/utils/localize'; - -export class JupyterDebuggerPortBlockedError extends BaseError { - constructor(portNumber: number, rangeBegin: number, rangeEnd: number) { - super( - 'debugger', - portNumber === -1 - ? localize.DataScience.jupyterDebuggerPortBlockedSearchError().format( - rangeBegin.toString(), - rangeEnd.toString() - ) - : localize.DataScience.jupyterDebuggerPortBlockedError().format(portNumber.toString()) - ); - } -} diff --git a/src/client/datascience/errors/jupyterDebuggerPortNotAvailableError.ts b/src/client/datascience/errors/jupyterDebuggerPortNotAvailableError.ts deleted file mode 100644 index 4007669a8c0..00000000000 --- a/src/client/datascience/errors/jupyterDebuggerPortNotAvailableError.ts +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. -'use strict'; -import { BaseError } from '../../common/errors/types'; -import '../../common/extensions'; -import * as localize from '../../common/utils/localize'; - -export class JupyterDebuggerPortNotAvailableError extends BaseError { - constructor(portNumber: number, rangeBegin: number, rangeEnd: number) { - super( - 'debugger', - portNumber === -1 - ? localize.DataScience.jupyterDebuggerPortNotAvailableSearchError().format( - rangeBegin.toString(), - rangeEnd.toString() - ) - : localize.DataScience.jupyterDebuggerPortNotAvailableError().format(portNumber.toString()) - ); - } -} diff --git a/src/client/datascience/errors/jupyterDebuggerRemoteNotSupportedError.ts b/src/client/datascience/errors/jupyterDebuggerRemoteNotSupportedError.ts index bf0e1b2649b..b2b11cf71e3 100644 --- a/src/client/datascience/errors/jupyterDebuggerRemoteNotSupportedError.ts +++ b/src/client/datascience/errors/jupyterDebuggerRemoteNotSupportedError.ts @@ -1,12 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import '../../common/extensions'; import * as localize from '../../common/utils/localize'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class JupyterDebuggerRemoteNotSupportedError extends BaseError { - constructor() { - super('debugger', localize.DataScience.remoteDebuggerNotSupported()); +export class JupyterDebuggerRemoteNotSupportedError extends BaseKernelError { + constructor(kernelConnectionMetadata: KernelConnectionMetadata) { + super('debugger', localize.DataScience.remoteDebuggerNotSupported(), kernelConnectionMetadata); } } diff --git a/src/client/datascience/errors/jupyterInvalidKernelError.ts b/src/client/datascience/errors/jupyterInvalidKernelError.ts index d16cc8e66ab..c4645b0926c 100644 --- a/src/client/datascience/errors/jupyterInvalidKernelError.ts +++ b/src/client/datascience/errors/jupyterInvalidKernelError.ts @@ -1,20 +1,21 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import * as localize from '../../common/utils/localize'; import { sendTelemetryEvent } from '../../telemetry'; import { Telemetry } from '../constants'; import { getDisplayNameOrNameOfKernelConnection } from '../jupyter/kernels/helpers'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class JupyterInvalidKernelError extends BaseError { - constructor(public readonly kernelConnectionMetadata: KernelConnectionMetadata | undefined) { +export class JupyterInvalidKernelError extends BaseKernelError { + constructor(kernelConnectionMetadata: KernelConnectionMetadata) { super( 'invalidkernel', localize.DataScience.kernelInvalid().format( getDisplayNameOrNameOfKernelConnection(kernelConnectionMetadata) - ) + ), + kernelConnectionMetadata ); sendTelemetryEvent(Telemetry.KernelInvalid); } diff --git a/src/client/datascience/errors/jupyterKernelDependencyError.ts b/src/client/datascience/errors/jupyterKernelDependencyError.ts new file mode 100644 index 00000000000..d67e7c85c98 --- /dev/null +++ b/src/client/datascience/errors/jupyterKernelDependencyError.ts @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; +import { BaseKernelError } from '../../common/errors/types'; +import { DataScience } from '../../common/utils/localize'; +import { KernelInterpreterDependencyResponse } from '../types'; +import { getDisplayNameOrNameOfKernelConnection } from '../jupyter/kernels/helpers'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; + +export class JupyterKernelDependencyError extends BaseKernelError { + constructor( + public reason: KernelInterpreterDependencyResponse, + kernelConnectionMetadata: KernelConnectionMetadata + ) { + super( + 'noipykernel', + DataScience.kernelInvalid().format(getDisplayNameOrNameOfKernelConnection(kernelConnectionMetadata)), + kernelConnectionMetadata + ); + } +} diff --git a/src/client/datascience/errors/jupyterWaitForIdleError.ts b/src/client/datascience/errors/jupyterWaitForIdleError.ts index 095ea32dff6..6f2e834ed62 100644 --- a/src/client/datascience/errors/jupyterWaitForIdleError.ts +++ b/src/client/datascience/errors/jupyterWaitForIdleError.ts @@ -2,13 +2,15 @@ // Licensed under the MIT License. 'use strict'; -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; +import { DataScience } from '../../common/utils/localize'; import { sendTelemetryEvent } from '../../telemetry'; import { Telemetry } from '../constants'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class JupyterWaitForIdleError extends BaseError { - constructor(message: string) { - super('timeout', message); +export class JupyterWaitForIdleError extends BaseKernelError { + constructor(kernelConnectionMetadata: KernelConnectionMetadata) { + super('timeout', DataScience.jupyterLaunchTimedOut(), kernelConnectionMetadata); sendTelemetryEvent(Telemetry.SessionIdleTimeout); } } diff --git a/src/client/datascience/errors/kernelConnectionTimeoutError.ts b/src/client/datascience/errors/kernelConnectionTimeoutError.ts index c697036e42f..d0a1752d73b 100644 --- a/src/client/datascience/errors/kernelConnectionTimeoutError.ts +++ b/src/client/datascience/errors/kernelConnectionTimeoutError.ts @@ -1,18 +1,19 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import { DataScience } from '../../common/utils/localize'; import { getDisplayNameOrNameOfKernelConnection } from '../jupyter/kernels/helpers'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class KernelConnectionTimeoutError extends BaseError { - constructor(public readonly kernelConnection: KernelConnectionMetadata) { +export class KernelConnectionTimeoutError extends BaseKernelError { + constructor(kernelConnection: KernelConnectionMetadata) { super( 'timeout', DataScience.rawKernelStartFailedDueToTimeout().format( getDisplayNameOrNameOfKernelConnection(kernelConnection) - ) + ), + kernelConnection ); } } diff --git a/src/client/datascience/errors/kernelDiedError.ts b/src/client/datascience/errors/kernelDiedError.ts index 032cde34531..069aeefb501 100644 --- a/src/client/datascience/errors/kernelDiedError.ts +++ b/src/client/datascience/errors/kernelDiedError.ts @@ -1,10 +1,16 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { WrappedError } from '../../common/errors/types'; +import { WrappedKernelError } from '../../common/errors/types'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class KernelDiedError extends WrappedError { - constructor(message: string, public readonly stdErr: string, originalException?: Error) { - super(message, originalException); +export class KernelDiedError extends WrappedKernelError { + constructor( + message: string, + public readonly stdErr: string, + originalException: Error | undefined, + kernelConnectionMetadata: KernelConnectionMetadata + ) { + super(message, originalException, kernelConnectionMetadata); } } diff --git a/src/client/datascience/errors/kernelInterruptTimeoutError.ts b/src/client/datascience/errors/kernelInterruptTimeoutError.ts index bae8aabc0e6..43644f48a13 100644 --- a/src/client/datascience/errors/kernelInterruptTimeoutError.ts +++ b/src/client/datascience/errors/kernelInterruptTimeoutError.ts @@ -1,12 +1,12 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import { DataScience } from '../../common/utils/localize'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class KernelInterruptTimeoutError extends BaseError { - constructor(public readonly kernelConnection: KernelConnectionMetadata) { - super('kernelpromisetimeout', DataScience.interruptingKernelFailed()); +export class KernelInterruptTimeoutError extends BaseKernelError { + constructor(kernelConnection: KernelConnectionMetadata) { + super('kernelpromisetimeout', DataScience.interruptingKernelFailed(), kernelConnection); } } diff --git a/src/client/datascience/errors/kernelPortNotUsedTimeoutError.ts b/src/client/datascience/errors/kernelPortNotUsedTimeoutError.ts index 84b4d4857d8..7204a04287b 100644 --- a/src/client/datascience/errors/kernelPortNotUsedTimeoutError.ts +++ b/src/client/datascience/errors/kernelPortNotUsedTimeoutError.ts @@ -1,18 +1,19 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import { DataScience } from '../../common/utils/localize'; import { getDisplayNameOrNameOfKernelConnection } from '../jupyter/kernels/helpers'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class KernelPortNotUsedTimeoutError extends BaseError { - constructor(public readonly kernelConnection: KernelConnectionMetadata) { +export class KernelPortNotUsedTimeoutError extends BaseKernelError { + constructor(kernelConnection: KernelConnectionMetadata) { super( 'timeout', DataScience.rawKernelStartFailedDueToTimeout().format( getDisplayNameOrNameOfKernelConnection(kernelConnection) - ) + ), + kernelConnection ); } } diff --git a/src/client/datascience/errors/kernelProcessExitedError.ts b/src/client/datascience/errors/kernelProcessExitedError.ts index 8153e286715..1f91a0c1076 100644 --- a/src/client/datascience/errors/kernelProcessExitedError.ts +++ b/src/client/datascience/errors/kernelProcessExitedError.ts @@ -1,11 +1,16 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { BaseError } from '../../common/errors/types'; +import { BaseKernelError } from '../../common/errors/types'; import { DataScience } from '../../common/utils/localize'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; -export class KernelProcessExitedError extends BaseError { - constructor(public readonly exitCode: number = -1, public readonly stdErr: string) { - super('kerneldied', DataScience.kernelDied().format(stdErr.trim())); +export class KernelProcessExitedError extends BaseKernelError { + constructor( + public readonly exitCode: number = -1, + public readonly stdErr: string, + kernelConnectionMetadata: KernelConnectionMetadata + ) { + super('kerneldied', DataScience.kernelDied().format(stdErr.trim()), kernelConnectionMetadata); } } diff --git a/src/client/datascience/errors/pythonKernelDiedError.ts b/src/client/datascience/errors/pythonKernelDiedError.ts deleted file mode 100644 index 893a37dad49..00000000000 --- a/src/client/datascience/errors/pythonKernelDiedError.ts +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { BaseError } from '../../common/errors/types'; -import { DataScience } from '../../common/utils/localize'; - -export class PythonKernelDiedError extends BaseError { - public readonly exitCode: number; - public readonly reason?: string; - public readonly errorMessage: string; - constructor(options: { exitCode: number; reason?: string; stdErr: string } | { error: Error; stdErr: string }) { - // Last line in stack traces generally contains the error message. - // Display that in the error message. - let reason = ('reason' in options ? options.reason || '' : options.stdErr).trim().split('\n').reverse()[0]; - reason = reason ? reason.trim() : ''; - reason = reason.endsWith('.') ? reason : `${reason}.`; - reason = reason ? `${reason} \n` : ''; - // No point displaying exit code if its 1 (thats not useful information). - const exitCodeMessage = 'exitCode' in options && options.exitCode > 1 ? ` (code: ${options.exitCode}). ` : ''; - const message = - 'exitCode' in options - ? `${exitCodeMessage}${reason}${options.reason === options.stdErr ? '' : options.reason}` - : options.error.message; - super('kerneldied', DataScience.kernelDied().format(message.trim())); - this.errorMessage = message; - this.stdErr = options.stdErr; - if ('exitCode' in options) { - this.exitCode = options.exitCode; - this.reason = options.reason; - } else { - this.exitCode = -1; - this.reason = options.error.message; - this.stack = options.error.stack; - this.name = options.error.name; - } - } -} diff --git a/src/client/datascience/interactive-common/notebookServerProvider.ts b/src/client/datascience/interactive-common/notebookServerProvider.ts index 8c928051fa0..4ca98eadb31 100644 --- a/src/client/datascience/interactive-common/notebookServerProvider.ts +++ b/src/client/datascience/interactive-common/notebookServerProvider.ts @@ -85,6 +85,10 @@ export class NotebookServerProvider implements IJupyterServerProvider { } try { const value = await this.serverPromise[property]; + // If we cancelled starting of the server, then don't cache the result. + if (!value && options.token.isCancellationRequested) { + delete this.serverPromise[property]; + } return value; } catch (e) { // Don't cache the error diff --git a/src/client/datascience/jupyter/interactiveWindowDebugger.ts b/src/client/datascience/jupyter/interactiveWindowDebugger.ts index fbbf1603fa3..cde53c63df7 100644 --- a/src/client/datascience/jupyter/interactiveWindowDebugger.ts +++ b/src/client/datascience/jupyter/interactiveWindowDebugger.ts @@ -327,16 +327,22 @@ export class InteractiveWindowDebugger implements IInteractiveWindowDebugger, IC // if we cannot parse the connect information, throw so we exit out of debugging if (outputs.length > 0 && outputs[0].output_type === 'error') { const error = outputs[0] as nbformat.IError; - throw new JupyterDebuggerNotInstalledError(this.debuggerPackage, error.ename); + throw new JupyterDebuggerNotInstalledError( + this.debuggerPackage, + error.ename, + kernel.kernelConnectionMetadata + ); } throw new JupyterDebuggerNotInstalledError( - localize.DataScience.jupyterDebuggerOutputParseError().format(this.debuggerPackage) + localize.DataScience.jupyterDebuggerOutputParseError().format(this.debuggerPackage), + undefined, + kernel.kernelConnectionMetadata ); } - private async connectToRemote(_kernel: IKernel): Promise<{ port: number; host: string }> { + private async connectToRemote(kernel: IKernel): Promise<{ port: number; host: string }> { // We actually need a token. This isn't supported at the moment - throw new JupyterDebuggerRemoteNotSupportedError(); + throw new JupyterDebuggerRemoteNotSupportedError(kernel.kernelConnectionMetadata); // let portNumber = this.configService.getSettings().remoteDebuggerPort; // if (!portNumber) { diff --git a/src/client/datascience/jupyter/kernels/helpers.ts b/src/client/datascience/jupyter/kernels/helpers.ts index 4a36a874dfa..481a56ccbad 100644 --- a/src/client/datascience/jupyter/kernels/helpers.ts +++ b/src/client/datascience/jupyter/kernels/helpers.ts @@ -64,7 +64,6 @@ import { removeNotebookSuffixAddedByExtension } from '../jupyterSession'; import { SilentExecutionErrorOptions } from './kernel'; import { Memento, NotebookDocument, Uri } from 'vscode'; import { IServiceContainer } from '../../../ioc/types'; -import { CancellationError } from '../../../common/cancellation'; import { INotebookControllerManager } from '../../notebook/types'; import { VSCodeNotebookController } from '../../notebook/vscodeNotebookController'; import { findNotebookEditor, selectKernel } from './kernelSelector'; @@ -1955,10 +1954,6 @@ export async function handleKernelError( switch (handleResult) { case KernelInterpreterDependencyResponse.cancel: - throw new CancellationError( - DataScience.canceledKernelHeader().format(controller.connection.interpreter?.displayName || '') - ); - case KernelInterpreterDependencyResponse.failed: throw error; diff --git a/src/client/datascience/jupyter/kernels/jupyterKernelDependencyError.ts b/src/client/datascience/jupyter/kernels/jupyterKernelDependencyError.ts deleted file mode 100644 index 01bce7c44bc..00000000000 --- a/src/client/datascience/jupyter/kernels/jupyterKernelDependencyError.ts +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; -import { BaseError } from '../../../common/errors/types'; -import { DataScience } from '../../../common/utils/localize'; -import { KernelInterpreterDependencyResponse } from '../../types'; -import { getDisplayNameOrNameOfKernelConnection } from './helpers'; -import { KernelConnectionMetadata } from './types'; - -export class JupyterKernelDependencyError extends BaseError { - constructor( - public reason: KernelInterpreterDependencyResponse, - kernelConnectionMetadata: KernelConnectionMetadata - ) { - super( - 'noipykernel', - DataScience.kernelInvalid().format(getDisplayNameOrNameOfKernelConnection(kernelConnectionMetadata)) - ); - } -} diff --git a/src/client/datascience/jupyter/kernels/jupyterKernelService.ts b/src/client/datascience/jupyter/kernels/jupyterKernelService.ts index 0072d328b2f..47e7edd62bb 100644 --- a/src/client/datascience/jupyter/kernels/jupyterKernelService.ts +++ b/src/client/datascience/jupyter/kernels/jupyterKernelService.ts @@ -29,7 +29,7 @@ import { } from '../../types'; import { JupyterPaths } from '../../kernel-launcher/jupyterPaths'; import { cleanEnvironment, getKernelRegistrationInfo } from './helpers'; -import { JupyterKernelDependencyError } from './jupyterKernelDependencyError'; +import { JupyterKernelDependencyError } from '../../errors/jupyterKernelDependencyError'; import { JupyterKernelSpec } from './jupyterKernelSpec'; import { KernelConnectionMetadata, LocalKernelConnectionMetadata } from './types'; diff --git a/src/client/datascience/jupyter/kernels/kernel.ts b/src/client/datascience/jupyter/kernels/kernel.ts index 59653adadd5..97797484b34 100644 --- a/src/client/datascience/jupyter/kernels/kernel.ts +++ b/src/client/datascience/jupyter/kernels/kernel.ts @@ -67,6 +67,7 @@ import { DisplayOptions } from '../../displayOptions'; import { JupyterConnectError } from '../../errors/jupyterConnectError'; import { KernelProgressReporter } from '../../progress/kernelProgressReporter'; import { disposeAllDisposables } from '../../../common/helpers'; +import { CancellationError } from '../../../common/cancellation'; export class Kernel implements IKernel { get connection(): INotebookProviderConnection | undefined { @@ -391,6 +392,9 @@ export class Kernel implements IKernel { kernelConnection: this.kernelConnectionMetadata, token: this.startCancellation.token }); + if (this.startCancellation.token.isCancellationRequested) { + throw new CancellationError(); + } if (!notebook) { // This is an unlikely case. // getOrCreateNotebook would return undefined only if getOnly = true (an issue with typings). @@ -408,6 +412,9 @@ export class Kernel implements IKernel { 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(); + } if (ex instanceof JupyterConnectError) { throw ex; } diff --git a/src/client/datascience/jupyter/kernels/kernelCommandListener.ts b/src/client/datascience/jupyter/kernels/kernelCommandListener.ts index 04bf48f4cab..a31c2bfa8fc 100644 --- a/src/client/datascience/jupyter/kernels/kernelCommandListener.ts +++ b/src/client/datascience/jupyter/kernels/kernelCommandListener.ts @@ -15,7 +15,12 @@ import { Commands, Telemetry } from '../../constants'; import { INotebookControllerManager } from '../../notebook/types'; import { RawJupyterSession } from '../../raw-kernel/rawJupyterSession'; import { trackKernelResourceInformation } from '../../telemetry/telemetry'; -import { IDataScienceCommandListener, IInteractiveWindowProvider, IStatusProvider } from '../../types'; +import { + IDataScienceCommandListener, + IDataScienceErrorHandler, + IInteractiveWindowProvider, + IStatusProvider +} from '../../types'; import { JupyterSession } from '../jupyterSession'; import { CellExecutionCreator } from './cellExecutionCreator'; import { getDisplayNameOrNameOfKernelConnection, wrapKernelMethod } from './helpers'; @@ -35,7 +40,8 @@ export class KernelCommandListener implements IDataScienceCommandListener { @inject(IInteractiveWindowProvider) private interactiveWindowProvider: IInteractiveWindowProvider, @inject(IConfigurationService) private configurationService: IConfigurationService, @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(INotebookControllerManager) private notebookControllerManager: INotebookControllerManager + @inject(INotebookControllerManager) private notebookControllerManager: INotebookControllerManager, + @inject(IDataScienceErrorHandler) private errorHandler: IDataScienceErrorHandler ) {} public register(commandManager: ICommandManager): void { @@ -168,7 +174,12 @@ export class KernelCommandListener implements IDataScienceCommandListener { } catch (ex) { if (currentCell) { const cellExecution = CellExecutionCreator.getOrCreate(currentCell, kernel.controller); - displayErrorsInCell(currentCell, cellExecution, ex).ignoreErrors(); + displayErrorsInCell( + currentCell, + cellExecution, + await this.errorHandler.getErrorMessageForDisplayInCell(ex) + ); + cellExecution.end(false); } else { void this.applicationShell.showErrorMessage(ex.toString()); } diff --git a/src/client/datascience/jupyter/notebookStarter.ts b/src/client/datascience/jupyter/notebookStarter.ts index 92406e927a9..42a4c3f7bf7 100644 --- a/src/client/datascience/jupyter/notebookStarter.ts +++ b/src/client/datascience/jupyter/notebookStarter.ts @@ -9,12 +9,11 @@ import { inject, injectable, named } from 'inversify'; import * as os from 'os'; import * as path from 'path'; import * as uuid from 'uuid/v4'; -import { CancellationToken, Disposable, ExtensionMode } from 'vscode'; +import { CancellationToken, Disposable } from 'vscode'; import { CancellationError, createPromiseFromCancellation } from '../../common/cancellation'; import { WrappedError } from '../../common/errors/types'; import { traceError, traceInfo } from '../../common/logger'; import { IFileSystem, TemporaryDirectory } from '../../common/platform/types'; -import { IDisposable, IExtensionContext, IOutputChannel, Resource } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { StopWatch } from '../../common/utils/stopWatch'; import { IServiceContainer } from '../../ioc/types'; @@ -27,6 +26,7 @@ import { JupyterInstallError } from '../errors/jupyterInstallError'; import { disposeAllDisposables } from '../../common/helpers'; import { JupyterConnectError } from '../errors/jupyterConnectError'; import { KernelProgressReporter } from '../progress/kernelProgressReporter'; +import { IDisposable, IOutputChannel, Resource } from '../../common/types'; /** * Responsible for starting a notebook. @@ -48,8 +48,9 @@ export class NotebookStarter implements Disposable { private readonly jupyterInterpreterService: IJupyterSubCommandExecutionService, @inject(IFileSystem) private readonly fs: IFileSystem, @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, - @inject(IExtensionContext) private readonly context: IExtensionContext, - @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) private readonly jupyterOutputChannel: IOutputChannel + @inject(IOutputChannel) + @named(JUPYTER_OUTPUT_CHANNEL) + private readonly jupyterOutputChannel: IOutputChannel ) {} public dispose() { while (this.disposables.length > 0) { @@ -198,12 +199,11 @@ export class NotebookStarter implements Disposable { const promisedArgs: Promise[] = []; promisedArgs.push(Promise.resolve('--no-browser')); promisedArgs.push(Promise.resolve(this.getNotebookDirArgument(workingDirectory))); - if (this.context.extensionMode === ExtensionMode.Test) { - // When kernels fail to start, Jupyter will attempt to restart 5 times, - // & this is very slow in the tests. - // Hence disable automatic starts of failed kernels in tests. - promisedArgs.push(Promise.resolve('--KernelManager.autorestart=False')); - } + // When kernels fail to start, Jupyter will attempt to restart 5 times, + // & this is very slow (we dont want users to wait because of kernel failures). + // Also when kernels die, we don't restart automatically with raw kernels, + // We should'nt do the same with jupyter (else startup code will not run). + promisedArgs.push(Promise.resolve('--KernelManager.autorestart=False')); if (useDefaultConfig) { promisedArgs.push(this.getConfigArgument(tempDirPromise)); } @@ -223,7 +223,15 @@ export class NotebookStarter implements Disposable { private async generateCustomArguments(customCommandLine: string[]): Promise { // We still have a bunch of args we have to pass - const requiredArgs = ['--no-browser', '--NotebookApp.iopub_data_rate_limit=10000000000.0']; + const requiredArgs = [ + '--no-browser', + '--NotebookApp.iopub_data_rate_limit=10000000000.0', // When kernels fail to start, Jupyter will attempt to restart 5 times, + // When kernels fail to start, Jupyter will attempt to restart 5 times, + // & this is very slow (we dont want users to wait because of kernel failures). + // Also when kernels die, we don't restart automatically with raw kernels, + // We should'nt do the same with jupyter (else startup code will not run). + '--KernelManager.autorestart=False' + ]; return [...requiredArgs, ...customCommandLine]; } diff --git a/src/client/datascience/kernel-launcher/kernelProcess.ts b/src/client/datascience/kernel-launcher/kernelProcess.ts index d4d20c19f64..822d9a6637d 100644 --- a/src/client/datascience/kernel-launcher/kernelProcess.ts +++ b/src/client/datascience/kernel-launcher/kernelProcess.ts @@ -39,7 +39,6 @@ import { KernelEnvironmentVariablesService } from './kernelEnvVarsService'; import { IKernelConnection, IKernelProcess } from './types'; import { BaseError } from '../../common/errors/types'; import { KernelProcessExitedError } from '../errors/kernelProcessExitedError'; -import { PythonKernelDiedError } from '../errors/pythonKernelDiedError'; import { KernelDiedError } from '../errors/kernelDiedError'; import { KernelPortNotUsedTimeoutError } from '../errors/kernelPortNotUsedTimeoutError'; import { ignoreLogging, TraceOptions } from '../../logging/trace'; @@ -152,7 +151,7 @@ export class KernelProcess implements IKernelProcess { exitEventFired = true; } if (!cancelToken.isCancellationRequested) { - deferred.reject(new KernelProcessExitedError(exitCode || -1, stderr)); + deferred.reject(new KernelProcessExitedError(exitCode || -1, stderr, this.kernelConnectionMetadata)); } }); @@ -191,29 +190,7 @@ export class KernelProcess implements IKernelProcess { return; } traceError('Kernel died', error, stderr); - if (error instanceof PythonKernelDiedError) { - providedExitCode = error.exitCode; - this.sendToOutput(`Exit - ${error.exitCode}, ${error.reason}`); - if (this.disposed) { - traceInfo('KernelProcess Exit', `Exit - ${error.exitCode}, ${error.reason}`, error); - return; - } else { - traceError('KernelProcess Exit', `Exit - ${error.exitCode}, ${error.reason}`, error); - } - if (!stderrProc && (error.stdErr || error.reason || error.message)) { - // This is used when process exits. - stderrProc = error.stdErr || error.reason || error.message; - } - if (!exitEventFired) { - let reason = error.reason || error.message; - this.exitEvent.fire({ - exitCode: error.exitCode, - reason: getTelemetrySafeErrorMessageFromPythonTraceback(reason) - }); - exitEventFired = true; - } - deferred.reject(error); - } + deferred.reject(error); }, () => { console.error('Completed'); @@ -267,7 +244,8 @@ export class KernelProcess implements IKernelProcess { // Include what ever we have as the stderr. stderrProc + '\n' + stderr + '\n', // eslint-disable-next-line @typescript-eslint/no-explicit-any - e as any + e as any, + this.kernelConnectionMetadata ); } } diff --git a/src/client/datascience/notebook/vscodeNotebookController.ts b/src/client/datascience/notebook/vscodeNotebookController.ts index 98ed9971096..afdb269b2bf 100644 --- a/src/client/datascience/notebook/vscodeNotebookController.ts +++ b/src/client/datascience/notebook/vscodeNotebookController.ts @@ -3,11 +3,13 @@ import { join } from 'path'; import { + CancellationError as VscCancellationError, Disposable, EventEmitter, ExtensionMode, languages, NotebookCell, + NotebookCellExecutionState, NotebookCellKind, NotebookController, NotebookControllerAffinity, @@ -24,8 +26,10 @@ import { IVSCodeNotebook, IWorkspaceService } from '../../common/application/types'; +import { CancellationError } from '../../common/cancellation'; import { PYTHON_LANGUAGE } from '../../common/constants'; import { displayErrorsInCell } from '../../common/errors/errorUtils'; +import { WrappedError } from '../../common/errors/types'; import { disposeAllDisposables } from '../../common/helpers'; import { traceInfo, traceInfoIfCI, traceVerbose } from '../../common/logger'; import { getDisplayPath } from '../../common/platform/fs-paths'; @@ -73,7 +77,7 @@ import { initializeInteractiveOrNotebookTelemetryBasedOnUserAction, sendKernelTelemetryEvent } from '../telemetry/telemetry'; -import { KernelSocketInformation } from '../types'; +import { IDataScienceErrorHandler, KernelSocketInformation } from '../types'; import { NotebookCellLanguageService } from './cellLanguageService'; import { InteractiveWindowView } from './constants'; import { isJupyterNotebook, traceCellMessage, updateNotebookDocumentMetadata } from './helpers/helpers'; @@ -385,8 +389,16 @@ 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 - return displayErrorsInCell(cell, execution, ex.toString()); + displayErrorsInCell(cell, execution, await errorHandler.getErrorMessageForDisplayInCell(ex)); + const isCancelled = + (WrappedError.unwrap(ex) || ex) instanceof CancellationError || + (WrappedError.unwrap(ex) || ex) instanceof VscCancellationError; + // If user cancels the execution, then don't show error status against cell. + execution.end(isCancelled ? undefined : false); + return NotebookCellExecutionState.Idle; } // Execution should be ended elsewhere diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index b2bede89af7..d2fa8274967 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -421,6 +421,7 @@ export interface IDataScienceErrorHandler { kernelConnection: KernelConnectionMetadata, resource: Resource ): Promise; + getErrorMessageForDisplayInCell(err: Error): Promise; } /** diff --git a/src/test/datascience/errorHandler.unit.test.ts b/src/test/datascience/errorHandler.unit.test.ts index 21b30bf505e..07a5f156e1d 100644 --- a/src/test/datascience/errorHandler.unit.test.ts +++ b/src/test/datascience/errorHandler.unit.test.ts @@ -215,7 +215,12 @@ suite('DataScience Error Handler Unit Tests', () => { }; test('Unable to import from user overriding module (windows)', async () => { await dataScienceErrorHandler.handleKernelError( - new KernelDiedError('Hello', stdErrorMessages.userOrverridingRandomPyFile_Windows, undefined), + new KernelDiedError( + 'Hello', + stdErrorMessages.userOrverridingRandomPyFile_Windows, + undefined, + kernelConnection + ), 'start', kernelConnection, undefined @@ -238,7 +243,12 @@ suite('DataScience Error Handler Unit Tests', () => { ]; when(workspaceService.workspaceFolders).thenReturn(workspaceFolders); await dataScienceErrorHandler.handleKernelError( - new KernelDiedError('Hello', stdErrorMessages.userOrverridingRandomPyFile_Windows, undefined), + new KernelDiedError( + 'Hello', + stdErrorMessages.userOrverridingRandomPyFile_Windows, + undefined, + kernelConnection + ), 'start', kernelConnection, undefined @@ -255,7 +265,12 @@ suite('DataScience Error Handler Unit Tests', () => { }); test('Unable to import from user overriding module (linux)', async () => { await dataScienceErrorHandler.handleKernelError( - new KernelDiedError('Hello', stdErrorMessages.userOrverridingRandomPyFile_Unix, undefined), + new KernelDiedError( + 'Hello', + stdErrorMessages.userOrverridingRandomPyFile_Unix, + undefined, + kernelConnection + ), 'start', kernelConnection, undefined @@ -282,7 +297,12 @@ suite('DataScience Error Handler Unit Tests', () => { ]; when(workspaceService.workspaceFolders).thenReturn(workspaceFolders); await dataScienceErrorHandler.handleKernelError( - new KernelDiedError('Hello', stdErrorMessages.userOrverridingRandomPyFile_Unix, undefined), + new KernelDiedError( + 'Hello', + stdErrorMessages.userOrverridingRandomPyFile_Unix, + undefined, + kernelConnection + ), 'start', kernelConnection, undefined @@ -302,7 +322,8 @@ suite('DataScience Error Handler Unit Tests', () => { import win32api ImportError: No module named 'win32api' `, - undefined + undefined, + kernelConnection ), 'start', kernelConnection, @@ -322,7 +343,8 @@ ImportError: No module named 'win32api' import xyz ImportError: No module named 'xyz' `, - undefined + undefined, + kernelConnection ), 'start', kernelConnection, @@ -338,7 +360,8 @@ ImportError: No module named 'xyz' new KernelDiedError( 'Hello', `ImportError: cannot import name 'constants' from partially initialized module 'zmq.backend.cython' (most likely due to a circular import) (C:\\Users\\\\AppData\\Roaming\\Python\\Python38\\site-packages\\zmq\\backend\\cython\\__init__.py)`, - undefined + undefined, + kernelConnection ), 'start', kernelConnection, @@ -351,7 +374,7 @@ ImportError: No module named 'xyz' }); test('Unknown Dll load failure', async () => { await dataScienceErrorHandler.handleKernelError( - new KernelDiedError('Hello', `ImportError: DLL load failed`, undefined), + new KernelDiedError('Hello', `ImportError: DLL load failed`, undefined, kernelConnection), 'start', kernelConnection, undefined @@ -363,7 +386,7 @@ ImportError: No module named 'xyz' }); test('Dll load failure', async () => { await dataScienceErrorHandler.handleKernelError( - new KernelDiedError('Hello', `import XYZ\nImportError: DLL load failed`, undefined), + new KernelDiedError('Hello', `import XYZ\nImportError: DLL load failed`, undefined, kernelConnection), 'start', kernelConnection, undefined diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index 2d1d2a9c68f..3d68901eb5f 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -11,7 +11,7 @@ import { anything, instance, match, mock, reset, when } from 'ts-mockito'; import { Matcher } from 'ts-mockito/lib/matcher/type/Matcher'; import * as TypeMoq from 'typemoq'; import * as uuid from 'uuid/v4'; -import { CancellationTokenSource, ConfigurationChangeEvent, Disposable, EventEmitter, ExtensionMode } from 'vscode'; +import { CancellationTokenSource, ConfigurationChangeEvent, Disposable, EventEmitter } from 'vscode'; import { ApplicationShell } from '../../client/common/application/applicationShell'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import { WorkspaceService } from '../../client/common/application/workspace'; @@ -31,13 +31,7 @@ import { ObservableExecutionResult, Output } from '../../client/common/process/types'; -import { - IAsyncDisposableRegistry, - IConfigurationService, - IExtensionContext, - IOutputChannel, - IPathUtils -} from '../../client/common/types'; +import { IAsyncDisposableRegistry, IConfigurationService, IOutputChannel, IPathUtils } from '../../client/common/types'; import { EXTENSION_ROOT_DIR } from '../../client/constants'; import { DisplayOptions } from '../../client/datascience/displayOptions'; import { JupyterInterpreterDependencyService } from '../../client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService'; @@ -480,6 +474,7 @@ suite('Jupyter Execution', async () => { 'notebook', '--no-browser', /--notebook-dir=.*/, + '--KernelManager.autorestart=False', /--config=.*/, '--NotebookApp.iopub_data_rate_limit=10000000000.0', ...dockerArgs @@ -523,6 +518,7 @@ suite('Jupyter Execution', async () => { 'notebook', '--no-browser', /--notebook-dir=.*/, + '--KernelManager.autorestart=False', /--config=.*/, '--NotebookApp.iopub_data_rate_limit=10000000000.0' ], @@ -638,6 +634,7 @@ suite('Jupyter Execution', async () => { 'notebook', '--no-browser', /--notebook-dir=.*/, + '--KernelManager.autorestart=False', /--config=.*/, '--NotebookApp.iopub_data_rate_limit=10000000000.0' ], @@ -682,6 +679,7 @@ suite('Jupyter Execution', async () => { 'notebook', '--no-browser', /--notebook-dir=.*/, + '--KernelManager.autorestart=False', /--config=.*/, '--NotebookApp.iopub_data_rate_limit=10000000000.0' ], @@ -723,6 +721,7 @@ suite('Jupyter Execution', async () => { 'notebook', '--no-browser', /--notebook-dir=.*/, + '--KernelManager.autorestart=False', /--config=.*/, '--NotebookApp.iopub_data_rate_limit=10000000000.0' ], @@ -982,13 +981,10 @@ suite('Jupyter Execution', async () => { when(serviceContainer.get(IJupyterSubCommandExecutionService)).thenReturn( jupyterCmdExecutionService ); - const context = mock(); - when(context.extensionMode).thenReturn(ExtensionMode.Production); notebookStarter = new NotebookStarter( jupyterCmdExecutionService, instance(fileSystem), instance(serviceContainer), - instance(context), instance(jupyterOutputChannel) ); const kernelFinder = mock(LocalKernelFinder); diff --git a/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts b/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts index db3c87e560d..91adcd1092c 100644 --- a/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts +++ b/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts @@ -5,7 +5,7 @@ import { assert } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; import * as sinon from 'sinon'; -import { commands, Memento, workspace, window, Uri } from 'vscode'; +import { commands, Memento, workspace, window, Uri, NotebookCell } from 'vscode'; import { IPythonApiProvider } from '../../../../client/api/types'; import { ICommandManager, IVSCodeNotebook } from '../../../../client/common/application/types'; import { Kernel } from '../../../../client/datascience/jupyter/kernels/kernel'; @@ -23,7 +23,7 @@ import { import { createDeferred } from '../../../../client/common/utils/async'; import { Common, DataScience } from '../../../../client/common/utils/localize'; import { InteractiveWindowProvider } from '../../../../client/datascience/interactive-window/interactiveWindowProvider'; -import { hasErrorOutput } from '../../../../client/datascience/notebook/helpers/helpers'; +import { hasErrorOutput, translateCellErrorOutput } from '../../../../client/datascience/notebook/helpers/helpers'; import { IInteractiveWindowProvider } from '../../../../client/datascience/types'; import { IInterpreterService } from '../../../../client/interpreter/contracts'; import { areInterpreterPathsSame, getInterpreterHash } from '../../../../client/pythonEnvironments/info/interpreter'; @@ -180,7 +180,7 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { test(`Ensure prompt is displayed when ipykernel module is not found and it gets installed for '${path.basename( venvNoRegPath )}'`, async () => openNotebookAndInstallIpyKernelWhenRunningCell(venvPythonPath)); - test('Ensure ipykernel install prompt is displayed every time you try to run a cell (VSCode Notebook)', async function () { + test('Ensure ipykernel install prompt is displayed every time you try to run a cell in a Notebook', async function () { if (IS_REMOTE_NATIVE_TEST) { return this.skip(); } @@ -204,7 +204,10 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { // Once ipykernel prompt has been dismissed, execution should stop due to missing dependencies. await waitForCondition( - async () => hasErrorOutput(cell.outputs) && assertVSCCellIsNotRunning(cell), + async () => + hasErrorOutput(cell.outputs) && + assertVSCCellIsNotRunning(cell) && + verifyInstallIPyKernelInstructionsInOutput(cell), defaultNotebookTestTimeout, 'No errors in cell' ); @@ -241,7 +244,7 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { 'No errors in cell' ); }); - test('Ensure ipykernel install prompt is displayed every time you try to run a cell (Interactive)', async function () { + test('Ensure ipykernel install prompt is displayed every time you try to run a cell in an Interactive Window', async function () { if (IS_REMOTE_NATIVE_TEST) { return this.skip(); } @@ -273,7 +276,7 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { // Once ipykernel prompt has been dismissed, execution should stop due to missing dependencies. await waitForCondition( - async () => cell.document.getText().includes('Canceled') && assertVSCCellIsNotRunning(cell), + async () => assertVSCCellIsNotRunning(cell), defaultNotebookTestTimeout, 'No errors in cell' ); @@ -616,6 +619,12 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { return true; } + function verifyInstallIPyKernelInstructionsInOutput(cell: NotebookCell) { + const textToLookFor = `Run the following command to install '${ProductNames.get(Product.ipykernel)!}'`; + const err = translateCellErrorOutput(cell.outputs[0]); + assert.include(err.traceback.join(''), textToLookFor); + return true; + } type Awaited = T extends PromiseLike ? U : T; function hookupKernelSelected( promptToInstall: Awaited>, diff --git a/src/test/datascience/mockJupyterSession.ts b/src/test/datascience/mockJupyterSession.ts index f7f92913deb..1e72179e069 100644 --- a/src/test/datascience/mockJupyterSession.ts +++ b/src/test/datascience/mockJupyterSession.ts @@ -103,7 +103,7 @@ export class MockJupyterSession implements IJupyterSession { public waitForIdle(_timeout: number): Promise { if (this.pendingIdleFailure) { this.pendingIdleFailure = false; - return Promise.reject(new JupyterWaitForIdleError('Kernel is dead')); + return Promise.reject(new JupyterWaitForIdleError({} as KernelConnectionMetadata)); } return sleep(this.timedelay); } diff --git a/src/test/datascience/notebook/kernelCrashes.vscode.test.ts b/src/test/datascience/notebook/kernelCrashes.vscode.test.ts index 97cff7dfb85..071c049cc74 100644 --- a/src/test/datascience/notebook/kernelCrashes.vscode.test.ts +++ b/src/test/datascience/notebook/kernelCrashes.vscode.test.ts @@ -12,7 +12,7 @@ import { DataScience } from '../../../client/common/utils/localize'; import { IVSCodeNotebook } from '../../../client/common/application/types'; import { traceInfo } from '../../../client/common/logger'; import { IConfigurationService, IDisposable, IJupyterSettings, ReadWrite } from '../../../client/common/types'; -import { captureScreenShot, IExtensionTestApi } from '../../common'; +import { captureScreenShot, IExtensionTestApi, waitForCondition } from '../../common'; import { initialize } from '../../initialize'; import { closeNotebooksAndCleanUpAfterTests, @@ -26,7 +26,8 @@ import { waitForExecutionCompletedSuccessfully, runAllCellsInActiveNotebook, waitForKernelToGetAutoSelected, - deleteCell + deleteCell, + defaultNotebookTestTimeout } from './helper'; import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_NON_RAW_NATIVE_TEST, IS_REMOTE_NATIVE_TEST } from '../../constants'; import * as dedent from 'dedent'; @@ -35,8 +36,9 @@ import { createDeferred } from '../../../client/common/utils/async'; import { sleep } from '../../core'; import { getDisplayNameOrNameOfKernelConnection } from '../../../client/datascience/jupyter/kernels/helpers'; import { INotebookEditorProvider } from '../../../client/datascience/types'; -import { Uri, workspace } from 'vscode'; +import { Uri, window, workspace } from 'vscode'; import { getDisplayPath } from '../../../client/common/platform/fs-paths'; +import { translateCellErrorOutput } from '../../../client/datascience/notebook/helpers/helpers'; const codeToKillKernel = dedent` import IPython @@ -207,6 +209,29 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' // If execution order is 1, then we know the kernel restarted. assert.strictEqual(cell3.executionSummary?.executionOrder, 1); }); + test('Ensure we get an error displayed in cell output and prompt when user has a file named random.py next to the ipynb file', async function () { + await runAndFailWithKernelCrash(); + await insertCodeCell('print("123412341234")', { index: 2 }); + const cell3 = vscodeNotebook.activeNotebookEditor!.document.cellAt(2); + const kernel = kernelProvider.get(vscodeNotebook.activeNotebookEditor!.document)!; + + const expectedErrorMessage = DataScience.cannotRunCellKernelIsDead().format( + getDisplayNameOrNameOfKernelConnection(kernel.kernelConnectionMetadata) + ); + const restartPrompt = await hijackPrompt( + 'showErrorMessage', + { + exactMatch: expectedErrorMessage + }, + { text: DataScience.restartKernel(), clickImmediately: true }, + disposables + ); + // Confirm we get a prompt to restart the kernel, and it gets restarted. + // & also confirm the cell completes execution with an execution count of 1 (thats how we tell kernel restarted). + await Promise.all([restartPrompt.displayed, runCell(cell3), waitForExecutionCompletedSuccessfully(cell3)]); + // If execution order is 1, then we know the kernel restarted. + assert.strictEqual(cell3.executionSummary?.executionOrder, 1); + }); test('Ensure cell outupt does not have errors when execution fails due to dead kernel', async function () { await runAndFailWithKernelCrash(); await insertCodeCell('print("123412341234")', { index: 2 }); @@ -271,12 +296,8 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' assert.isOk(vscodeNotebook.activeNotebookEditor, 'No active notebook'); await waitForKernelToGetAutoSelected(); } - async function displayErrorAboutOverriddenBuiltInModules(disablePythonDaemon: boolean) { + async function displayErrorAboutOverriddenBuiltInModules() { await closeNotebooksAndCleanUpAfterTests(disposables); - - const settings = config.getSettings() as ReadWrite; - settings.disablePythonDaemon = disablePythonDaemon; - const randomFile = path.join( EXTENSION_ROOT_DIR_FOR_TESTS, 'src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/random.py' @@ -294,16 +315,25 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' disposables ); - await createAndOpenTemporaryNotebookForKernelCrash(`nb${disablePythonDaemon}.ipynb`); + await createAndOpenTemporaryNotebookForKernelCrash(`nb.ipynb`); await insertCodeCell('print("123412341234")'); await runAllCellsInActiveNotebook(); // Wait for a max of 1s for error message to be dispalyed. await Promise.race([prompt.displayed, sleep(5_000).then(() => Promise.reject('Prompt not displayed'))]); prompt.dispose(); + + // Verify we have an output in the cell that contains the same information (about overirding built in modules). + const cell = window.activeNotebookEditor!.document.cellAt(0); + await waitForCondition(async () => cell.outputs.length > 0, defaultNotebookTestTimeout, 'No output'); + const err = translateCellErrorOutput(cell.outputs[0]); + assert.include(err.traceback.join(''), 'random.py'); + assert.include( + err.traceback.join(''), + 'seems to be overriding built in modules and interfering with the startup of the kernel' + ); + assert.include(err.traceback.join(''), 'Consider renaming the file and starting the kernel again'); } - test.skip('Display error about overriding builtin modules (with Python daemon', () => - displayErrorAboutOverriddenBuiltInModules(false)); test('Display error about overriding builtin modules (without Python daemon', () => - displayErrorAboutOverriddenBuiltInModules(true)); + displayErrorAboutOverriddenBuiltInModules()); }); });