From f746c983ac4d941dddfa3e0ec27bd27184bee2e5 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 9 Nov 2021 09:47:46 -0800 Subject: [PATCH 01/10] misc --- .../datascience/jupyter/kernelDoctor/analysis.ts | 0 .../datascience/jupyter/kernelDoctor/types.ts | 13 +++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 src/client/datascience/jupyter/kernelDoctor/analysis.ts create mode 100644 src/client/datascience/jupyter/kernelDoctor/types.ts diff --git a/src/client/datascience/jupyter/kernelDoctor/analysis.ts b/src/client/datascience/jupyter/kernelDoctor/analysis.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/client/datascience/jupyter/kernelDoctor/types.ts b/src/client/datascience/jupyter/kernelDoctor/types.ts new file mode 100644 index 00000000000..8eb8f02d3c2 --- /dev/null +++ b/src/client/datascience/jupyter/kernelDoctor/types.ts @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export enum KernelFailureReason { + /** + * Creating files such as os.py and having this in the root directory or some place where Python would load it, + * would result in the `os` module being overwritten with the users `os.py` file. + * Novice python users tend to do this very often, and this causes the python runtime to crash. + * + * We identify this based on the error message dumped in the stderr stream. + */ + overridingBuiltinModules = 'overridingBuiltinModules' +} From 62df52bdb27db15d6ec5628e740ede2b97bbf009 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 9 Nov 2021 15:02:05 -0800 Subject: [PATCH 02/10] fix formatting --- src/client/common/errors/errorUtils.ts | 367 +++++++++++++++++- src/client/common/errors/errors.ts | 123 +----- src/client/datascience/errors/errorHandler.ts | 97 ++++- .../jupyter/kernelDoctor/analysis.ts | 0 .../datascience/jupyter/kernelDoctor/types.ts | 13 - .../datascience/jupyter/kernels/kernel.ts | 4 +- .../kernel-launcher/kernelProcess.ts | 7 +- src/client/datascience/types.ts | 5 + .../datascience/errorHandler.unit.test.ts | 11 +- 9 files changed, 487 insertions(+), 140 deletions(-) delete mode 100644 src/client/datascience/jupyter/kernelDoctor/analysis.ts delete mode 100644 src/client/datascience/jupyter/kernelDoctor/types.ts diff --git a/src/client/common/errors/errorUtils.ts b/src/client/common/errors/errorUtils.ts index 0bfa99ca179..65f39f54a84 100644 --- a/src/client/common/errors/errorUtils.ts +++ b/src/client/common/errors/errorUtils.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { WorkspaceFolder } from 'vscode'; + // eslint-disable-next-line @typescript-eslint/no-extraneous-class export class ErrorUtils { public static outputHasModuleNotInstalledError(moduleName: string, content?: string): boolean { @@ -23,7 +25,7 @@ export function getTelemetrySafeErrorMessageFromPythonTraceback(traceback: strin // Look for something like `NameError: name 'XYZ' is not defined` in the last line. const pythonErrorMessageRegExp = /\S+Error: /g; // Suffix with `:`, in case we pass the value `NameError` back into this function. - const reversedLines = `${traceback}: ` + const reversedLines = `${traceback.trim()}: ` .split('\n') .filter((item) => item.trim().length) .reverse(); @@ -37,6 +39,43 @@ export function getTelemetrySafeErrorMessageFromPythonTraceback(traceback: strin return parts.length && parts[0].endsWith('Error') ? parts[0] : undefined; } +/** + * Given a python traceback, attempt to get the Python error message. + * Note: Python error messages are at the bottom of the traceback. + */ +export function getErrorMessageFromPythonTraceback(traceback: string = '') { + const lines = getLastTwoLinesFromPythonTracebackWithErrorMessage(traceback); + return lines ? lines[1] : undefined; +} + +/** + * Given a python traceback, attempt to get the last two lines. + * THe last line will contain the Python error message. + * Note: Python error messages are at the bottom of the traceback. + */ +export function getLastTwoLinesFromPythonTracebackWithErrorMessage( + traceback: string = '' +): undefined | [string, string] { + if (!traceback) { + return; + } + const reversedLines = traceback + .trim() + .split('\n') + .map((item) => item.trim()) + .filter((item) => item.trim().length) + .reverse(); + if (reversedLines.length === 0) { + return; + } + // If last line doesn't contain the error, return the traceback as is. + if (!reversedLines[0].includes('Error')) { + return; + } else { + return [reversedLines.length > 1 ? reversedLines[1] : '', reversedLines[0]]; + } +} + export function getLastFrameFromPythonTraceback( traceback: string ): { fileName: string; folderName: string; packageName: string } | undefined { @@ -70,3 +109,329 @@ export function getLastFrameFromPythonTraceback( } return { fileName: reversedParts[0], folderName: reversedParts[1], packageName }; } + +export enum KernelFailureReason { + /** + * Errors specific to importing of `win32api` module. + */ + importWin32apiError = 'importWin32apiError', + /** + * Errors specific to the zmq module. + */ + zmqModuleError = 'zmqModuleError', + /** + * Errors specific to older versions of IPython. + */ + oldIPythonError = 'oldIPythonError', + /** + * Errors specific to older versions of IPyKernel. + */ + oldIPyKernelError = 'oldIPyKernelError', + /** + * Creating files such as os.py and having this in the root directory or some place where Python would load it, + * would result in the `os` module being overwritten with the users `os.py` file. + * Novice python users tend to do this very often, and this causes the python runtime to crash. + * + * We identify this based on the error message dumped in the stderr stream. + */ + overridingBuiltinModules = 'overridingBuiltinModules', + /** + * Some module or type is not found. + * This is a common error when using the `import` statement. + * Again, possible a module has been overwritten with a user file. + * Or the installed package doesn't contain the necessary types. + * Samples include: + * 1. import win32api + * ImportError: No module named 'win32api' + * 2. 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) + */ + importError = 'importError', + /** + * Some missing dependency is not installed. + * Error messages are of the form `ModuleNotFoundError: No module named 'zmq'` + */ + moduleNotFoundError = 'moduleNotFound', + /** + * Failure to load some dll. + * Error messages are of the following form + * import win32api + * ImportError: DLL load failed: The specified procedure could not be found. + */ + dllLoadFailure = 'dllLoadFailure' +} +export type KernelFailure = { + /** + * Classifications of the errors that is safe for telemetry (no pii). + */ + telemetrySafeTags: string[]; +} & ( + | { + reason: KernelFailureReason.overridingBuiltinModules; + /** + * The module that has been overwritten. + */ + moduleName: string; + /** + * Fully qualified path to the module. + */ + fileName: string; + } + | { + reason: KernelFailureReason.importError; + /** + * What is being imported from the module. + */ + name?: string; + moduleName: string; + fileName?: string; + } + | { + reason: KernelFailureReason.moduleNotFoundError; + /** + * Name of the missing module. + */ + moduleName: string; + } + | { + reason: KernelFailureReason.dllLoadFailure; + /** + * Name of the module that couldn't be loaded. + */ + moduleName?: string; + } + | { + reason: KernelFailureReason.dllLoadFailure; + /** + * Name of the module that couldn't be loaded. + */ + moduleName?: string; + } + | { + reason: KernelFailureReason.importWin32apiError; + } + | { + reason: KernelFailureReason.zmqModuleError; + } + | { + reason: KernelFailureReason.oldIPyKernelError; + } + | { + reason: KernelFailureReason.oldIPythonError; + } +); + +export function analyseKernelErrors( + stdErrOrStackTrace: string, + workspaceFolders: readonly WorkspaceFolder[] = [], + pythonSysPrefix: string = '' +): KernelFailure | undefined { + const lastTwolinesOfError = getLastTwoLinesFromPythonTracebackWithErrorMessage(stdErrOrStackTrace); + const stdErr = stdErrOrStackTrace.toLowerCase(); + + if (stdErr.includes("ImportError: No module named 'win32api'".toLowerCase())) { + // force re-installing ipykernel worked. + /* + File "C:\Users\\miniconda3\envs\env_zipline\lib\contextlib.py", line 59, in enter + return next(self.gen) + File "C:\Users\\miniconda3\envs\env_zipline\lib\site-packages\jupyter_client\connect.py", line 100, in secure_write + win32_restrict_file_to_user(fname) + File "C:\Users\\miniconda3\envs\env_zipline\lib\site-packages\jupyter_client\connect.py", line 53, in win32_restrict_file_to_user + import win32api + ImportError: No module named 'win32api' + */ + return { + reason: KernelFailureReason.importWin32apiError, + telemetrySafeTags: ['win32api'] + }; + } + if (stdErr.includes('ImportError: DLL load failed'.toLowerCase()) && stdErr.includes('win32api')) { + // Possibly a conda issue on windows + /* + win32_restrict_file_to_user + import win32api + ImportError: DLL load failed: 找不到指定的程序。 + */ + return { + reason: KernelFailureReason.importWin32apiError, + telemetrySafeTags: ['dll.load.failed', 'win32api'] + }; + } + if (stdErr.includes('ImportError: DLL load failed'.toLowerCase())) { + /* + import xyz + ImportError: DLL load failed: .... + */ + const moduleName = + lastTwolinesOfError && lastTwolinesOfError[0].toLowerCase().startsWith('import') + ? lastTwolinesOfError[0].substring('import'.length).trim() + : undefined; + return { + reason: KernelFailureReason.dllLoadFailure, + moduleName, + telemetrySafeTags: ['dll.load.failed'] + }; + } + if (stdErr.includes("AssertionError: Couldn't find Class NSProcessInfo".toLowerCase())) { + // Conda environment with IPython 5.8.0 fails with this message. + // Updating to latest version of ipython fixed it (conda update ipython). + // Possible we might have to update other packages as well (when using `conda update ipython` plenty of other related pacakges got updated, such as zeromq, nbclient, jedi) + /* + Error: Kernel died with exit code 1. Traceback (most recent call last): + File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 90, in nope + "Because Reasons" + File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 60, in beginActivityWithOptions + NSProcessInfo = C('NSProcessInfo') + File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 38, in C + assert ret is not None, "Couldn't find Class %s" % classname + AssertionError: Couldn't find Class NSProcessInfo + */ + return { + reason: KernelFailureReason.oldIPythonError, + telemetrySafeTags: ['oldipython'] + }; + } + if ( + stdErr.includes('NotImplementedError'.toLowerCase()) && + stdErr.includes('asyncio'.toLowerCase()) && + stdErr.includes('events.py'.toLowerCase()) + ) { + /* + "C:\Users\\AppData\Roaming\Python\Python38\site-packages\zmq\eventloop\zmqstream.py", line 127, in __init__ + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: self._init_io_state() + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: File "C:\Users\\AppData\Roaming\Python\Python38\site-packages\zmq\eventloop\zmqstream.py", line 546, in _init_io_state + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: self.io_loop.add_handler(self.socket, self._handle_events, self.io_loop.READ) + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: File "C:\Users\\AppData\Roaming\Python\Python38\site-packages\tornado\platform\asyncio.py", line 99, in add_handler + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: self.asyncio_loop.add_reader(fd, self._handle_events, fd, IOLoop.READ) + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: File "C:\Users\\AppData\Local\Programs\Python\Python38-32\lib\asyncio\events.py", line 501, in add_reader + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: raise NotImplementedError + Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: NotImplementedError + */ + return { + reason: KernelFailureReason.oldIPyKernelError, + telemetrySafeTags: ['oldipykernel'] + }; + } + + { + // zmq errors. + const tags: string[] = []; + if ( + stdErr.includes('ImportError: cannot import name'.toLowerCase()) && + stdErr.includes('from partially initialized module'.toLowerCase()) && + stdErr.includes('zmq.backend.cython'.toLowerCase()) + ) { + // force re-installing ipykernel worked. + tags.push('zmq.backend.cython'); + } + if ( + stdErr.includes('zmq'.toLowerCase()) && + stdErr.includes('cython'.toLowerCase()) && + stdErr.includes('__init__.py'.toLowerCase()) + ) { + // force re-installing ipykernel worked. + /* + File "C:\Users\\AppData\Roaming\Python\Python38\site-packages\zmq\backend\cython\__init__.py", line 6, in + from . import (constants, error, message, context, + 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) + */ + tags.push('zmq.cython'); + } + // ZMQ general errors + if (stdErr.includes('zmq.error.ZMQError')) { + tags.push('zmq.error'); + } + + if (tags.length) { + return { + reason: KernelFailureReason.zmqModuleError, + telemetrySafeTags: tags + }; + } + } + + if (lastTwolinesOfError && lastTwolinesOfError[1].toLowerCase().startsWith('importerror')) { + const info = extractModuleAndFileFromImportError(lastTwolinesOfError[1]); + if (info) { + // First check if we're overriding any built in modules. + const error = isBuiltInModuleOverwritten(info.moduleName, info.fileName, workspaceFolders, pythonSysPrefix); + if (error) { + return error; + } + + return { + reason: KernelFailureReason.importError, + moduleName: info.moduleName, + fileName: info.fileName, + telemetrySafeTags: ['import.error'] + }; + } + } + if (lastTwolinesOfError && lastTwolinesOfError[1].toLowerCase().startsWith('ModuleNotFoundError'.toLowerCase())) { + const info = extractModuleAndFileFromImportError(lastTwolinesOfError[1]); + if (info) { + return { + reason: KernelFailureReason.moduleNotFoundError, + moduleName: info.moduleName, + telemetrySafeTags: ['module.notfound.error'] + }; + } + } +} + +function extractModuleAndFileFromImportError(errorLine: string) { + errorLine = errorLine.replace(/"/g, "'"); + const matches = errorLine.match(/'[^\\']*(\\'[^\\']*)*'/g); + const fileMatches = errorLine.match(/\((.*?)\)/g); + let moduleName: string | undefined; + let fileName: string | undefined; + if (matches && matches[matches.length - 1].length > 2) { + moduleName = matches[matches.length - 1]; + moduleName = moduleName.substring(1, moduleName.length - 1); + } + if (fileMatches && fileMatches[fileMatches.length - 1].length > 2) { + fileName = fileMatches[fileMatches.length - 1]; + fileName = fileName.substring(1, fileName.length - 1); + } + + return moduleName ? { moduleName, fileName } : undefined; +} +/** + * When we override the built in modules errors are of the form + * `ImportError: cannot import name 'Random' from 'random' (/home/don/samples/pySamples/crap/kernel_crash/no_start/random.py)` + */ +function isBuiltInModuleOverwritten( + moduleName: string, + fileName: string | undefined, + workspaceFolders: readonly WorkspaceFolder[], + pythonSysPrefix?: string +): KernelFailure | undefined { + // If we cannot tell whether this belongs to python environment, + // then we cannot tell if it overwrites any built in modules. + if (!pythonSysPrefix) { + return; + } + // If we cannot tell whether this is part of a worksapce folder, + // then we cannot tell if the user created this. + if (workspaceFolders.length === 0) { + return; + } + if (!fileName) { + return; + } + + if (fileName.toLowerCase().startsWith(pythonSysPrefix)) { + // This is a python file, not created by the user. + return; + } + + if (!workspaceFolders.some((folder) => fileName?.toLowerCase().startsWith(folder.uri.fsPath.toLowerCase()))) { + return; + } + + return { + reason: KernelFailureReason.overridingBuiltinModules, + fileName, + moduleName, + telemetrySafeTags: ['import.error', 'override.modules'] + }; +} diff --git a/src/client/common/errors/errors.ts b/src/client/common/errors/errors.ts index 35aa70fad17..fe45ca284e8 100644 --- a/src/client/common/errors/errors.ts +++ b/src/client/common/errors/errors.ts @@ -1,16 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -const taggers = [ - tagWithWin32Error, - tagWithZmqError, - tagWithDllLoadError, - tagWithOldIPyKernel, - tagWithOldIPython, - tagWithChildProcessExited, - tagWithKernelRestarterFailed -]; -export function getErrorTags(stdErrOrStackTrace: string) { +import { analyseKernelErrors } from './errorUtils'; + +const taggers = [tagWithChildProcessExited, tagWithKernelRestarterFailed]; +export function getErrorTags(stdErrOrStackTrace: string | string[]) { const tags: string[] = []; // This parameter might be either a string or a string array @@ -18,38 +12,13 @@ export function getErrorTags(stdErrOrStackTrace: string) { ? stdErrOrStackTrace[0].toLowerCase() : stdErrOrStackTrace.toLowerCase(); taggers.forEach((tagger) => tagger(stdErrOrStackTraceLowered, tags)); - + const error = analyseKernelErrors(stdErrOrStackTraceLowered); + if (error?.telemetrySafeTags.length) { + tags.push(...error?.telemetrySafeTags); + } return Array.from(new Set(tags)).join(','); } -function tagWithWin32Error(stdErr: string, tags: string[] = []) { - if (stdErr.includes("ImportError: No module named 'win32api'".toLowerCase())) { - // force re-installing ipykernel worked. - /* - File "C:\Users\\miniconda3\envs\env_zipline\lib\contextlib.py", line 59, in enter - return next(self.gen) - File "C:\Users\\miniconda3\envs\env_zipline\lib\site-packages\jupyter_client\connect.py", line 100, in secure_write - win32_restrict_file_to_user(fname) - File "C:\Users\\miniconda3\envs\env_zipline\lib\site-packages\jupyter_client\connect.py", line 53, in win32_restrict_file_to_user - import win32api - ImportError: No module named 'win32api' - */ - tags.push('win32api'); - } - if (stdErr.includes("ImportError: No module named 'win32api'".toLowerCase())) { - // force re-installing ipykernel worked. - /* - File "C:\Users\\miniconda3\envs\env_zipline\lib\contextlib.py", line 59, in enter - return next(self.gen) - File "C:\Users\\miniconda3\envs\env_zipline\lib\site-packages\jupyter_client\connect.py", line 100, in secure_write - win32_restrict_file_to_user(fname) - File "C:\Users\\miniconda3\envs\env_zipline\lib\site-packages\jupyter_client\connect.py", line 53, in win32_restrict_file_to_user - import win32api - ImportError: No module named 'win32api' - */ - tags.push('win32api'); - } -} function tagWithChildProcessExited(stdErrOrStackTrace: string, tags: string[] = []) { // StackTrace = at ChildProcess.exithandler child_process.js:312:12\n\tat ChildProcess.emit events.js:315:20\n\tat maybeClose :1021:16\n\tat Process.ChildProcess._handle.onexit :286:5" // StackTrace = at ChildProcess.exithandler child_process.js:312:12\n\tat ChildProcess.emit events.js:315:20\n\tat ChildProcess.EventEmitter.emit domain.js:483:12\n\tat maybeClose :1021:16\n\tat Socket :443:11\n\tat Socket.emit events.js:315:20\n\tat Socket.EventEmitter.emit domain.js:483:12\n\tat Pipe net.js:674:12" @@ -69,79 +38,3 @@ function tagWithKernelRestarterFailed(stdErrOrStackTrace: string, tags: string[] tags.push('KernelRestarter.failed'); } } -function tagWithZmqError(stdErr: string, tags: string[] = []) { - if ( - stdErr.includes('ImportError: cannot import name'.toLowerCase()) && - stdErr.includes('from partially initialized module'.toLowerCase()) && - stdErr.includes('zmq.backend.cython'.toLowerCase()) - ) { - // force re-installing ipykernel worked. - tags.push('zmq.backend.cython'); - } - if ( - stdErr.includes('zmq'.toLowerCase()) && - stdErr.includes('cython'.toLowerCase()) && - stdErr.includes('__init__.py'.toLowerCase()) - ) { - // force re-installing ipykernel worked. - /* - File "C:\Users\\AppData\Roaming\Python\Python38\site-packages\zmq\backend\cython\__init__.py", line 6, in - from . import (constants, error, message, context, - 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) - */ - tags.push('zmq.cython'); - } - // ZMQ general errors - if (stdErr.includes('zmq.error.ZMQError')) { - tags.push('zmq.error'); - } -} -function tagWithDllLoadError(stdErr: string, tags: string[] = []) { - if (stdErr.includes('ImportError: DLL load failed'.toLowerCase())) { - // Possibly a conda issue on windows - /* - win32_restrict_file_to_user - import win32api - ImportError: DLL load failed: 找不到指定的程序。 - */ - tags.push('dll.load.failed'); - } -} -function tagWithOldIPython(stdErr: string, tags: string[] = []) { - if (stdErr.includes("AssertionError: Couldn't find Class NSProcessInfo".toLowerCase())) { - // Conda environment with IPython 5.8.0 fails with this message. - // Updating to latest version of ipython fixed it (conda update ipython). - // Possible we might have to update other packages as well (when using `conda update ipython` plenty of other related pacakges got updated, such as zeromq, nbclient, jedi) - /* - Error: Kernel died with exit code 1. Traceback (most recent call last): - File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 90, in nope - "Because Reasons" - File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 60, in beginActivityWithOptions - NSProcessInfo = C('NSProcessInfo') - File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 38, in C - assert ret is not None, "Couldn't find Class %s" % classname - AssertionError: Couldn't find Class NSProcessInfo - */ - tags.push('oldipython'); - } -} -function tagWithOldIPyKernel(stdErr: string, tags: string[] = []) { - if ( - stdErr.includes('NotImplementedError'.toLowerCase()) && - stdErr.includes('asyncio'.toLowerCase()) && - stdErr.includes('events.py'.toLowerCase()) - ) { - /* - "C:\Users\\AppData\Roaming\Python\Python38\site-packages\zmq\eventloop\zmqstream.py", line 127, in __init__ - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: self._init_io_state() - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: File "C:\Users\\AppData\Roaming\Python\Python38\site-packages\zmq\eventloop\zmqstream.py", line 546, in _init_io_state - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: self.io_loop.add_handler(self.socket, self._handle_events, self.io_loop.READ) - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: File "C:\Users\\AppData\Roaming\Python\Python38\site-packages\tornado\platform\asyncio.py", line 99, in add_handler - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: self.asyncio_loop.add_reader(fd, self._handle_events, fd, IOLoop.READ) - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: File "C:\Users\\AppData\Local\Programs\Python\Python38-32\lib\asyncio\events.py", line 501, in add_reader - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: raise NotImplementedError - Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: NotImplementedError - */ - tags.push('oldipykernel'); - } -} diff --git a/src/client/datascience/errors/errorHandler.ts b/src/client/datascience/errors/errorHandler.ts index e8475c7a14d..2af2c244a29 100644 --- a/src/client/datascience/errors/errorHandler.ts +++ b/src/client/datascience/errors/errorHandler.ts @@ -2,8 +2,8 @@ // Licensed under the MIT License. import type * as nbformat from '@jupyterlab/nbformat'; import { inject, injectable } from 'inversify'; -import { IApplicationShell } from '../../common/application/types'; -import { WrappedError } from '../../common/errors/types'; +import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; +import { BaseError, WrappedError } from '../../common/errors/types'; import { traceError, traceWarning } from '../../common/logger'; import { DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -20,12 +20,15 @@ import { KernelDiedError } from './kernelDiedError'; import { KernelPortNotUsedTimeoutError } from './kernelPortNotUsedTimeoutError'; import { KernelProcessExitedError } from './kernelProcessExitedError'; import { PythonKernelDiedError } from './pythonKernelDiedError'; +import { analyseKernelErrors, getErrorMessageFromPythonTraceback } from '../../common/errors/errorUtils'; +import { KernelConnectionMetadata } from '../jupyter/kernels/types'; @injectable() export class DataScienceErrorHandler implements IDataScienceErrorHandler { constructor( @inject(IApplicationShell) private applicationShell: IApplicationShell, - @inject(IJupyterInterpreterDependencyManager) protected dependencyManager: IJupyterInterpreterDependencyManager + @inject(IJupyterInterpreterDependencyManager) protected dependencyManager: IJupyterInterpreterDependencyManager, + @inject(IWorkspaceService) protected workspace: IWorkspaceService ) {} public async handleError(err: Error, purpose?: 'start' | 'restart' | 'interrupt'): Promise { const errorPrefix = getErrorMessagePrefix(purpose); @@ -49,7 +52,15 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { ) { this.applicationShell.showErrorMessage(err.message).then(noop, noop); } else if (err instanceof KernelDiedError || err instanceof KernelProcessExitedError) { - this.applicationShell.showErrorMessage(getCombinedErrorMessage(errorPrefix, err.stdErr)).then(noop, noop); + if (purpose === 'restart' || purpose === 'start') { + const analysis = analyseKernelErrors(err.stdErr); + console.error(analysis); + } + this.applicationShell + .showErrorMessage( + getCombinedErrorMessage(errorPrefix, getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr) + ) + .then(noop, noop); } else if (err instanceof PythonKernelDiedError) { this.applicationShell .showErrorMessage(getCombinedErrorMessage(errorPrefix, err.errorMessage)) @@ -62,8 +73,84 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { } traceError('DataScience Error', err); } -} + public async handleKernelStartRestartError( + err: Error, + purpose: 'start' | 'restart', + kernelConnection: KernelConnectionMetadata + ): Promise { + await this.handleErrorImplementation(err, purpose, (error: BaseError) => { + const analysis = analyseKernelErrors( + error.stdErr || '', + this.workspace.workspaceFolders, + kernelConnection.interpreter?.sysPrefix + ); + if (analysis) { + console.log(analysis); + } else { + const errorPrefix = getErrorMessagePrefix(purpose); + void this.applicationShell + .showErrorMessage( + getCombinedErrorMessage( + errorPrefix, + getErrorMessageFromPythonTraceback(error.stdErr) || error.stdErr + ) + ) + .then(noop, noop); + } + }); + } + private async handleErrorImplementation( + err: Error, + purpose?: 'start' | 'restart' | 'interrupt', + handler?: (error: BaseError) => void + ): Promise { + const errorPrefix = getErrorMessagePrefix(purpose); + // Unwrap the errors. + err = WrappedError.unwrap(err); + if (err instanceof JupyterInstallError) { + await this.dependencyManager.installMissingDependencies(err); + } else if (err instanceof JupyterSelfCertsError) { + // Don't show the message for self cert errors + noop(); + } else if (err instanceof IpyKernelNotInstalledError) { + // Don't show the message, as user decided not to install IPyKernel. + noop(); + } else if (err instanceof VscCancellationError || err instanceof CancellationError) { + // Don't show the message for cancellation errors + traceWarning(`Cancelled by user`, err); + } else if ( + err instanceof KernelConnectionTimeoutError || + err instanceof KernelConnectionTimeoutError || + err instanceof KernelPortNotUsedTimeoutError + ) { + this.applicationShell.showErrorMessage(err.message).then(noop, noop); + } else if (err instanceof KernelDiedError || err instanceof KernelProcessExitedError) { + if ((purpose === 'restart' || purpose === 'start') && handler) { + handler(err); + } else { + this.applicationShell + .showErrorMessage( + getCombinedErrorMessage( + errorPrefix, + getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr + ) + ) + .then(noop, noop); + } + } else if (err instanceof PythonKernelDiedError) { + this.applicationShell + .showErrorMessage(getCombinedErrorMessage(errorPrefix, err.errorMessage)) + .then(noop, noop); + } else { + // Some errors have localized and/or formatted error messages. + this.applicationShell + .showErrorMessage(getCombinedErrorMessage(errorPrefix, err.message || err.toString())) + .then(noop, noop); + } + traceError('DataScience Error', err); + } +} function getCombinedErrorMessage(prefix?: string, message?: string) { const errorMessage = [prefix || '', message || ''] .map((line) => line.trim()) diff --git a/src/client/datascience/jupyter/kernelDoctor/analysis.ts b/src/client/datascience/jupyter/kernelDoctor/analysis.ts deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/client/datascience/jupyter/kernelDoctor/types.ts b/src/client/datascience/jupyter/kernelDoctor/types.ts deleted file mode 100644 index 8eb8f02d3c2..00000000000 --- a/src/client/datascience/jupyter/kernelDoctor/types.ts +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -export enum KernelFailureReason { - /** - * Creating files such as os.py and having this in the root directory or some place where Python would load it, - * would result in the `os` module being overwritten with the users `os.py` file. - * Novice python users tend to do this very often, and this causes the python runtime to crash. - * - * We identify this based on the error message dumped in the stderr stream. - */ - overridingBuiltinModules = 'overridingBuiltinModules' -} diff --git a/src/client/datascience/jupyter/kernels/kernel.ts b/src/client/datascience/jupyter/kernels/kernel.ts index c0e518710da..9fd363185f2 100644 --- a/src/client/datascience/jupyter/kernels/kernel.ts +++ b/src/client/datascience/jupyter/kernels/kernel.ts @@ -382,7 +382,9 @@ export class Kernel implements IKernel { sendTelemetryEvent(Telemetry.KernelStartFailedAndUIDisabled); } else { // eslint-disable-next-line @typescript-eslint/no-explicit-any - this.errorHandler.handleError(ex, 'start').ignoreErrors(); // Just a notification, so don't await this + this.errorHandler + .handleKernelStartRestartError(ex, 'start', this.kernelConnectionMetadata) + .ignoreErrors(); // Just a notification, so don't await this } traceError(`failed to start INotebook in kernel, UI Disabled = ${options?.disableUI}`, ex); this.startCancellation.cancel(); diff --git a/src/client/datascience/kernel-launcher/kernelProcess.ts b/src/client/datascience/kernel-launcher/kernelProcess.ts index 94d496497c0..7d574288c04 100644 --- a/src/client/datascience/kernel-launcher/kernelProcess.ts +++ b/src/client/datascience/kernel-launcher/kernelProcess.ts @@ -9,7 +9,10 @@ import * as tmp from 'tmp'; import { CancellationToken, Event, EventEmitter } from 'vscode'; import { IPythonExtensionChecker } from '../../api/types'; import { createPromiseFromCancellation } from '../../common/cancellation'; -import { getTelemetrySafeErrorMessageFromPythonTraceback } from '../../common/errors/errorUtils'; +import { + getErrorMessageFromPythonTraceback, + getTelemetrySafeErrorMessageFromPythonTraceback +} from '../../common/errors/errorUtils'; import { traceDecorators, traceError, traceInfo, traceVerbose, traceWarning } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; import { IProcessServiceFactory, IPythonExecutionFactory, ObservableExecutionResult } from '../../common/process/types'; @@ -229,7 +232,7 @@ export class KernelProcess implements IKernelProcess { } // If we have the python error message in std outputs, display that. const errorMessage = - getTelemetrySafeErrorMessageFromPythonTraceback(stderrProc || stderr) || + getErrorMessageFromPythonTraceback(stderrProc || stderr) || (stderrProc || stderr).substring(0, 100); throw new KernelDiedError( localize.DataScience.kernelDied().format(errorMessage), diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index fc215ae8ed8..06ac6af15b0 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -387,6 +387,11 @@ export interface IDataScienceErrorHandler { * Thus based on the context the error message would be different. */ handleError(err: Error, context?: 'start' | 'restart' | 'interrupt'): Promise; + handleKernelStartRestartError( + err: Error, + context: 'start' | 'restart', + kernelConnection: KernelConnectionMetadata + ): Promise; } /** diff --git a/src/test/datascience/errorHandler.unit.test.ts b/src/test/datascience/errorHandler.unit.test.ts index 8d514462720..3e934888e89 100644 --- a/src/test/datascience/errorHandler.unit.test.ts +++ b/src/test/datascience/errorHandler.unit.test.ts @@ -3,7 +3,7 @@ 'use strict'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; -import { IApplicationShell } from '../../client/common/application/types'; +import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import * as localize from '../../client/common/utils/localize'; import { DataScienceErrorHandler } from '../../client/datascience/errors/errorHandler'; import { JupyterInstallError } from '../../client/datascience/errors/jupyterInstallError'; @@ -14,12 +14,17 @@ suite('DataScience Error Handler Unit Tests', () => { let applicationShell: typemoq.IMock; let dataScienceErrorHandler: DataScienceErrorHandler; let dependencyManager: IJupyterInterpreterDependencyManager; - + let worksapceService: IWorkspaceService; setup(() => { applicationShell = typemoq.Mock.ofType(); + worksapceService = mock(); dependencyManager = mock(); when(dependencyManager.installMissingDependencies(anything())).thenResolve(); - dataScienceErrorHandler = new DataScienceErrorHandler(applicationShell.object, instance(dependencyManager)); + dataScienceErrorHandler = new DataScienceErrorHandler( + applicationShell.object, + instance(dependencyManager), + instance(worksapceService) + ); }); const message = 'Test error message.'; From fc6808b90f30d490270bb1d01c3808f55900f36f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Nov 2021 16:40:07 -0800 Subject: [PATCH 03/10] Fixes --- package.nls.json | 1 + .../kernel_launcher.py | 9 +- src/client/common/cancellation.ts | 17 ++- src/client/common/errors/errorUtils.ts | 142 +++++++++--------- src/client/common/errors/types.ts | 1 + src/client/common/platform/fs-paths.ts | 24 ++- src/client/common/utils/localize.ts | 5 + src/client/datascience/baseJupyterSession.ts | 17 ++- src/client/datascience/errors/errorHandler.ts | 134 +++++++---------- .../errors/sessionDisposedError.ts | 11 ++ .../interpreter/jupyterInterpreterService.ts | 41 ++--- .../jupyter/jupyterNotebookProvider.ts | 4 +- .../datascience/jupyter/jupyterSession.ts | 7 +- .../jupyter/jupyterSessionManager.ts | 5 +- .../jupyter/kernels/cellExecution.ts | 27 +++- .../datascience/jupyter/kernels/kernel.ts | 12 +- .../jupyter/kernels/kernelCommandListener.ts | 4 +- .../kernels/kernelDependencyService.ts | 13 +- .../jupyter/liveshare/hostJupyterServer.ts | 5 +- .../datascience/jupyter/notebookStarter.ts | 7 +- .../kernel-launcher/kernelLauncher.ts | 25 ++- .../kernel-launcher/kernelProcess.ts | 3 +- .../intellisense/jupyterCompletionProvider.ts | 19 ++- .../liveshare/hostRawNotebookProvider.ts | 13 +- .../raw-kernel/rawJupyterSession.ts | 37 ++++- src/client/datascience/types.ts | 9 +- .../datascience/errorHandler.unit.test.ts | 11 +- 27 files changed, 356 insertions(+), 247 deletions(-) create mode 100644 src/client/datascience/errors/sessionDisposedError.ts diff --git a/package.nls.json b/package.nls.json index b02d95d4ae0..12efe13143c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -441,6 +441,7 @@ "DataScience.failedToInterruptKernel": "Failed to interrupt the Kernel.", "DataScience.kernelDied": "The kernel died. View Jupyter [log](command:jupyter.viewOutput) for further details. \nError: {0}...", "DataScience.kernelDiedWithoutError": "The kernel '{0}' died. View Jupyter [log](command:jupyter.viewOutput) for further details.", + "DataScience.fileSeemsToBeInterferingWithKernelStartup": "The file '{0}' seems to be overriding built in modules and interfering with the startup of the kernel. Consider renaming the file and starting the kernel again.", "DataScience.cannotRunCellKernelIsDead": "Cannot run cells, as the kernel '{0}' is dead.", "DataScience.waitingForJupyterSessionToBeIdle": "Waiting for Jupyter Session to be idle", "DataScience.gettingListOfKernelsForLocalConnection": "Fetching Kernels", diff --git a/pythonFiles/vscode_datascience_helpers/kernel_launcher.py b/pythonFiles/vscode_datascience_helpers/kernel_launcher.py index 342f28d46b5..a94505740c2 100644 --- a/pythonFiles/vscode_datascience_helpers/kernel_launcher.py +++ b/pythonFiles/vscode_datascience_helpers/kernel_launcher.py @@ -11,9 +11,6 @@ # See comment at the point of our use of Popen from subprocess import Popen, PIPE # nosec -from ipython_genutils.encoding import getdefaultencoding -from ipython_genutils.py3compat import cast_bytes_py2 - def launch_kernel( cmd, @@ -79,7 +76,6 @@ def launch_kernel( env = env if (env is not None) else os.environ.copy() - encoding = getdefaultencoding(prefer_stream=False) kwargs = kw.copy() main_args = dict( stdin=_stdin, @@ -92,10 +88,7 @@ def launch_kernel( # Spawn a kernel. if sys.platform == "win32": - # Popen on Python 2 on Windows cannot handle unicode args or cwd - cmd = [cast_bytes_py2(c, encoding) for c in cmd] if cwd: - cwd = cast_bytes_py2(cwd, sys.getfilesystemencoding() or "ascii") kwargs["cwd"] = cwd from .win_interrupt import create_interrupt_event @@ -103,7 +96,7 @@ def launch_kernel( # Create a Win32 event for interrupting the kernel # and store it in an environment variable. interrupt_event = create_interrupt_event() - env["JPY_INTERRUPT_EVENT"] = str(interrupt_event) + env[" "] = str(interrupt_event) # deprecated old env name: env["IPY_INTERRUPT_EVENT"] = env["JPY_INTERRUPT_EVENT"] diff --git a/src/client/common/cancellation.ts b/src/client/common/cancellation.ts index 3aebe21a161..e09b89dc5e2 100644 --- a/src/client/common/cancellation.ts +++ b/src/client/common/cancellation.ts @@ -24,11 +24,18 @@ export class CancellationError extends BaseError { * @param {({ defaultValue: T; token: CancellationToken; cancelAction: 'reject' | 'resolve' })} options * @returns {Promise} */ -export function createPromiseFromCancellation(options: { - defaultValue: T; - token?: CancellationToken; - cancelAction: 'reject' | 'resolve'; -}): Promise { +export function createPromiseFromCancellation( + options: + | { + defaultValue: T; + token?: CancellationToken; + cancelAction: 'resolve'; + } + | { + token?: CancellationToken; + cancelAction: 'reject'; + } +): Promise { return new Promise((resolve, reject) => { // Never resolve. if (!options.token) { diff --git a/src/client/common/errors/errorUtils.ts b/src/client/common/errors/errorUtils.ts index 65f39f54a84..35634aee23a 100644 --- a/src/client/common/errors/errorUtils.ts +++ b/src/client/common/errors/errorUtils.ts @@ -114,19 +114,19 @@ export enum KernelFailureReason { /** * Errors specific to importing of `win32api` module. */ - importWin32apiError = 'importWin32apiError', + importWin32apiFailure = 'importWin32apiFailure', /** * Errors specific to the zmq module. */ - zmqModuleError = 'zmqModuleError', + zmqModuleFailure = 'zmqModuleFailure', /** * Errors specific to older versions of IPython. */ - oldIPythonError = 'oldIPythonError', + oldIPythonFailure = 'oldIPythonFailure', /** * Errors specific to older versions of IPyKernel. */ - oldIPyKernelError = 'oldIPyKernelError', + oldIPyKernelFailure = 'oldIPyKernelFailure', /** * Creating files such as os.py and having this in the root directory or some place where Python would load it, * would result in the `os` module being overwritten with the users `os.py` file. @@ -145,12 +145,12 @@ export enum KernelFailureReason { * ImportError: No module named 'win32api' * 2. 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) */ - importError = 'importError', + importFailure = 'importFailure', /** * Some missing dependency is not installed. * Error messages are of the form `ModuleNotFoundError: No module named 'zmq'` */ - moduleNotFoundError = 'moduleNotFound', + moduleNotFoundFailure = 'moduleNotFound', /** * Failure to load some dll. * Error messages are of the following form @@ -159,66 +159,70 @@ export enum KernelFailureReason { */ dllLoadFailure = 'dllLoadFailure' } -export type KernelFailure = { +type BaseFailure = { + reason: Reason; /** * Classifications of the errors that is safe for telemetry (no pii). */ telemetrySafeTags: string[]; -} & ( - | { - reason: KernelFailureReason.overridingBuiltinModules; - /** - * The module that has been overwritten. - */ - moduleName: string; - /** - * Fully qualified path to the module. - */ - fileName: string; - } - | { - reason: KernelFailureReason.importError; - /** - * What is being imported from the module. - */ - name?: string; - moduleName: string; - fileName?: string; - } - | { - reason: KernelFailureReason.moduleNotFoundError; - /** - * Name of the missing module. - */ - moduleName: string; - } - | { - reason: KernelFailureReason.dllLoadFailure; - /** - * Name of the module that couldn't be loaded. - */ - moduleName?: string; - } - | { - reason: KernelFailureReason.dllLoadFailure; - /** - * Name of the module that couldn't be loaded. - */ - moduleName?: string; - } - | { - reason: KernelFailureReason.importWin32apiError; - } - | { - reason: KernelFailureReason.zmqModuleError; - } - | { - reason: KernelFailureReason.oldIPyKernelError; - } - | { - reason: KernelFailureReason.oldIPythonError; - } -); +} & MoreInfo; +export type OverridingBuiltInModulesFailure = BaseFailure< + KernelFailureReason.overridingBuiltinModules, + { + /** + * The module that has been overwritten. + */ + moduleName: string; + /** + * Fully qualified path to the module. + */ + fileName: string; + } +>; +export type ImportErrorFailure = BaseFailure< + KernelFailureReason.importFailure, + { + /** + * What is being imported from the module. + */ + name?: string; + moduleName: string; + fileName?: string; + } +>; +export type ModuleNotFoundFailure = BaseFailure< + KernelFailureReason.moduleNotFoundFailure, + { + /** + * Name of the missing module. + */ + moduleName: string; + } +>; +export type DllLoadFailure = BaseFailure< + KernelFailureReason.dllLoadFailure, + { + /** + * Name of the module that couldn't be loaded. + */ + moduleName?: string; + } +>; +export type ImportWin32ApiFailure = BaseFailure; +export type ZmqModuleFailure = BaseFailure; +export type OldIPyKernelFailure = BaseFailure; +export type OldIPythonFailure = BaseFailure; + +export type KernelFailure = + | OverridingBuiltInModulesFailure + | ImportErrorFailure + | ModuleNotFoundFailure + | DllLoadFailure + | ImportWin32ApiFailure + | ImportWin32ApiFailure + | ZmqModuleFailure + | OldIPyKernelFailure + | OldIPythonFailure; export function analyseKernelErrors( stdErrOrStackTrace: string, @@ -240,7 +244,7 @@ export function analyseKernelErrors( ImportError: No module named 'win32api' */ return { - reason: KernelFailureReason.importWin32apiError, + reason: KernelFailureReason.importWin32apiFailure, telemetrySafeTags: ['win32api'] }; } @@ -252,7 +256,7 @@ export function analyseKernelErrors( ImportError: DLL load failed: 找不到指定的程序。 */ return { - reason: KernelFailureReason.importWin32apiError, + reason: KernelFailureReason.importWin32apiFailure, telemetrySafeTags: ['dll.load.failed', 'win32api'] }; } @@ -286,7 +290,7 @@ export function analyseKernelErrors( AssertionError: Couldn't find Class NSProcessInfo */ return { - reason: KernelFailureReason.oldIPythonError, + reason: KernelFailureReason.oldIPythonFailure, telemetrySafeTags: ['oldipython'] }; } @@ -307,7 +311,7 @@ export function analyseKernelErrors( Info 2020-08-10 12:14:11: Python Daemon (pid: 16976): write to stderr: NotImplementedError */ return { - reason: KernelFailureReason.oldIPyKernelError, + reason: KernelFailureReason.oldIPyKernelFailure, telemetrySafeTags: ['oldipykernel'] }; } @@ -343,7 +347,7 @@ export function analyseKernelErrors( if (tags.length) { return { - reason: KernelFailureReason.zmqModuleError, + reason: KernelFailureReason.zmqModuleFailure, telemetrySafeTags: tags }; } @@ -359,7 +363,7 @@ export function analyseKernelErrors( } return { - reason: KernelFailureReason.importError, + reason: KernelFailureReason.importFailure, moduleName: info.moduleName, fileName: info.fileName, telemetrySafeTags: ['import.error'] @@ -370,7 +374,7 @@ export function analyseKernelErrors( const info = extractModuleAndFileFromImportError(lastTwolinesOfError[1]); if (info) { return { - reason: KernelFailureReason.moduleNotFoundError, + reason: KernelFailureReason.moduleNotFoundFailure, moduleName: info.moduleName, telemetrySafeTags: ['module.notfound.error'] }; diff --git a/src/client/common/errors/types.ts b/src/client/common/errors/types.ts index b062051175e..d7bcfbfc686 100644 --- a/src/client/common/errors/types.ts +++ b/src/client/common/errors/types.ts @@ -70,6 +70,7 @@ export type ErrorCategory = | 'notinstalled' | 'kernelspecnotfound' // Left for historical purposes, not used anymore. | 'unsupportedKernelSpec' // Left for historical purposes, not used anymore. + | 'sessionDisposed' | 'unknown'; // If there are errors, then the are added to the telementry properties. diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index fdf189e4cf5..8890a9d4082 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import * as nodepath from 'path'; -import { Uri } from 'vscode'; +import { Uri, WorkspaceFolder } from 'vscode'; import { getOSType, OSType } from '../utils/platform'; import { IExecutables, IFileSystemPaths, IFileSystemPathUtils } from './types'; // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports @@ -145,7 +145,25 @@ export class FileSystemPathUtils implements IFileSystemPathUtils { } const homePath = untildify('~'); -export function getDisplayPath(filename?: string | Uri) { +export function getDisplayPath( + filename?: string | Uri, + workspaceFolder: readonly WorkspaceFolder[] | WorkspaceFolder[] = [] +) { + const relativeToHome = getDisplayPathImpl(filename); + const relativeToWorkspaceFolders = workspaceFolder.map((folder) => getDisplayPathImpl(filename, folder.uri.fsPath)); + // Pick the shortest path for display purposes. + // As those are most likely relative to some workspace folder. + let bestDisplayPath = relativeToHome; + [relativeToHome, ...relativeToWorkspaceFolders].forEach((relativePath) => { + if (relativePath.length < bestDisplayPath.length) { + bestDisplayPath = relativePath; + } + }); + + return bestDisplayPath; +} + +function getDisplayPathImpl(filename?: string | Uri, cwd?: string): string { let file = ''; if (typeof filename === 'string') { file = filename; @@ -158,6 +176,8 @@ export function getDisplayPath(filename?: string | Uri) { } if (!file) { return ''; + } else if (cwd && file.startsWith(cwd)) { + return `.${nodepath.sep}${nodepath.relative(cwd, file)}`; } else if (file.startsWith(homePath)) { return `~${nodepath.sep}${nodepath.relative(homePath, file)}`; } else { diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 7dceb5ec633..eab1f869821 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -981,6 +981,11 @@ export namespace DataScience { export const kernelCategoryForPyEnv = localize('jupyter.kernel.category.pyenv', 'PyEnv Env'); export const kernelCategoryForGlobal = localize('jupyter.kernel.category.global', 'Global Env'); export const kernelCategoryForVirtual = localize('jupyter.kernel.category.virtual', 'Virtual Env'); + + export const fileSeemsToBeInterferingWithKernelStartup = localize( + 'DataScience.fileSeemsToBeInterferingWithKernelStartup', + "The file '{0}' seems to be overriding built in modules and interfering with the startup of the kernel. Consider renaming the file and starting the kernel again.." + ); } // Skip using vscode-nls and instead just compute our strings based on key values. Key values diff --git a/src/client/datascience/baseJupyterSession.ts b/src/client/datascience/baseJupyterSession.ts index a3dd6cf9190..c913ac38bfe 100644 --- a/src/client/datascience/baseJupyterSession.ts +++ b/src/client/datascience/baseJupyterSession.ts @@ -23,6 +23,7 @@ import { KernelConnectionMetadata } from './jupyter/kernels/types'; import { suppressShutdownErrors } from './raw-kernel/rawKernel'; import { IJupyterSession, ISessionWithSocket, KernelSocketInformation } from './types'; import { KernelInterruptTimeoutError } from './errors/kernelInterruptTimeoutError'; +import { SessionDisposedError } from './errors/sessionDisposedError'; /** * Exception raised when starting a Jupyter Session fails. @@ -198,7 +199,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { } this.shutdownSession(oldSession, undefined, false).ignoreErrors(); } else { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } } @@ -208,7 +209,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { metadata?: JSONObject ): Kernel.IShellFuture { if (!this.session?.kernel) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } return this.session.kernel.requestExecute(content, disposeOnDone, metadata); } @@ -218,7 +219,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { disposeOnDone?: boolean ): Kernel.IControlFuture { if (!this.session?.kernel) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } return this.session.kernel.requestDebug(content, disposeOnDone); } @@ -227,7 +228,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { content: KernelMessage.IInspectRequestMsg['content'] ): Promise { if (!this.session?.kernel) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } return this.session.kernel.requestInspect(content); } @@ -236,7 +237,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { content: KernelMessage.ICompleteRequestMsg['content'] ): Promise { if (!this.session?.kernel) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } return this.session.kernel.requestComplete(content); } @@ -255,7 +256,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { if (this.session && this.session.kernel) { this.session.kernel.registerCommTarget(targetName, callback); } else { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } } @@ -266,7 +267,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { if (this.session?.kernel) { return this.session.kernel.registerMessageHook(msgId, hook); } else { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } } public removeMessageHook( @@ -276,7 +277,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { if (this.session?.kernel) { return this.session.kernel.removeMessageHook(msgId, hook); } else { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } } diff --git a/src/client/datascience/errors/errorHandler.ts b/src/client/datascience/errors/errorHandler.ts index 2af2c244a29..7bc08e079e0 100644 --- a/src/client/datascience/errors/errorHandler.ts +++ b/src/client/datascience/errors/errorHandler.ts @@ -2,10 +2,10 @@ // Licensed under the MIT License. import type * as nbformat from '@jupyterlab/nbformat'; import { inject, injectable } from 'inversify'; -import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; +import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types'; import { BaseError, WrappedError } from '../../common/errors/types'; import { traceError, traceWarning } from '../../common/logger'; -import { DataScience } from '../../common/utils/localize'; +import { Common, DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { IpyKernelNotInstalledError } from './ipyKernelNotInstalledError'; import { JupyterInstallError } from './jupyterInstallError'; @@ -20,90 +20,68 @@ import { KernelDiedError } from './kernelDiedError'; import { KernelPortNotUsedTimeoutError } from './kernelPortNotUsedTimeoutError'; import { KernelProcessExitedError } from './kernelProcessExitedError'; import { PythonKernelDiedError } from './pythonKernelDiedError'; -import { analyseKernelErrors, getErrorMessageFromPythonTraceback } from '../../common/errors/errorUtils'; +import { + analyseKernelErrors, + getErrorMessageFromPythonTraceback, + KernelFailureReason +} from '../../common/errors/errorUtils'; import { KernelConnectionMetadata } from '../jupyter/kernels/types'; +import { getDisplayPath } from '../../common/platform/fs-paths'; +import { IBrowserService } from '../../common/types'; @injectable() export class DataScienceErrorHandler implements IDataScienceErrorHandler { constructor( @inject(IApplicationShell) private applicationShell: IApplicationShell, @inject(IJupyterInterpreterDependencyManager) protected dependencyManager: IJupyterInterpreterDependencyManager, - @inject(IWorkspaceService) protected workspace: IWorkspaceService + @inject(IWorkspaceService) protected workspace: IWorkspaceService, + @inject(IBrowserService) protected browser: IBrowserService, + @inject(ICommandManager) protected commandManager: ICommandManager ) {} - public async handleError(err: Error, purpose?: 'start' | 'restart' | 'interrupt'): Promise { - const errorPrefix = getErrorMessagePrefix(purpose); - // Unwrap the errors. - err = WrappedError.unwrap(err); - if (err instanceof JupyterInstallError) { - await this.dependencyManager.installMissingDependencies(err); - } else if (err instanceof JupyterSelfCertsError) { - // Don't show the message for self cert errors - noop(); - } else if (err instanceof IpyKernelNotInstalledError) { - // Don't show the message, as user decided not to install IPyKernel. - noop(); - } else if (err instanceof VscCancellationError || err instanceof CancellationError) { - // Don't show the message for cancellation errors - traceWarning(`Cancelled by user`, err); - } else if ( - err instanceof KernelConnectionTimeoutError || - err instanceof KernelConnectionTimeoutError || - err instanceof KernelPortNotUsedTimeoutError - ) { - this.applicationShell.showErrorMessage(err.message).then(noop, noop); - } else if (err instanceof KernelDiedError || err instanceof KernelProcessExitedError) { - if (purpose === 'restart' || purpose === 'start') { - const analysis = analyseKernelErrors(err.stdErr); - console.error(analysis); - } - this.applicationShell - .showErrorMessage( - getCombinedErrorMessage(errorPrefix, getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr) - ) - .then(noop, noop); - } else if (err instanceof PythonKernelDiedError) { - this.applicationShell - .showErrorMessage(getCombinedErrorMessage(errorPrefix, err.errorMessage)) - .then(noop, noop); - } else { - // Some errors have localized and/or formatted error messages. - this.applicationShell - .showErrorMessage(getCombinedErrorMessage(errorPrefix, err.message || err.toString())) - .then(noop, noop); - } + public async handleError(err: Error): Promise { + await this.handleErrorImplementation(err); traceError('DataScience Error', err); } - public async handleKernelStartRestartError( + public async handleKernelError( err: Error, - purpose: 'start' | 'restart', + purpose: 'start' | 'restart' | 'interrupt' | 'execution', kernelConnection: KernelConnectionMetadata ): Promise { - await this.handleErrorImplementation(err, purpose, (error: BaseError) => { - const analysis = analyseKernelErrors( + await this.handleErrorImplementation(err, purpose, async (error: BaseError, defaultErrorMessage: string) => { + const failureInfo = analyseKernelErrors( error.stdErr || '', this.workspace.workspaceFolders, kernelConnection.interpreter?.sysPrefix ); - if (analysis) { - console.log(analysis); - } else { - const errorPrefix = getErrorMessagePrefix(purpose); - void this.applicationShell - .showErrorMessage( - getCombinedErrorMessage( - errorPrefix, - getErrorMessageFromPythonTraceback(error.stdErr) || error.stdErr + switch (failureInfo?.reason) { + case KernelFailureReason.overridingBuiltinModules: { + await this.applicationShell + .showErrorMessage( + DataScience.fileSeemsToBeInterferingWithKernelStartup().format( + getDisplayPath(failureInfo.fileName, this.workspace.workspaceFolders || []) + ), + DataScience.showJupyterLogs(), + Common.learnMore() ) - ) - .then(noop, noop); + .then((selection) => { + if (selection === Common.learnMore()) { + this.browser.launch('https://aka.ms/kernelFailuresOverridingBuiltInModules'); + } else if (selection === DataScience.showJupyterLogs()) { + void this.commandManager.executeCommand('jupyter.viewOutput'); + } + }); + break; + } + default: + await this.applicationShell.showErrorMessage(defaultErrorMessage); } }); } private async handleErrorImplementation( err: Error, - purpose?: 'start' | 'restart' | 'interrupt', - handler?: (error: BaseError) => void + purpose?: 'start' | 'restart' | 'interrupt' | 'execution', + handler?: (error: BaseError, defaultErrorMessage: string) => Promise ): Promise { const errorPrefix = getErrorMessagePrefix(purpose); // Unwrap the errors. @@ -119,29 +97,23 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { } else if (err instanceof VscCancellationError || err instanceof CancellationError) { // Don't show the message for cancellation errors traceWarning(`Cancelled by user`, err); + } else if (err instanceof KernelConnectionTimeoutError || err instanceof KernelPortNotUsedTimeoutError) { + this.applicationShell.showErrorMessage(err.message).then(noop, noop); } else if ( - err instanceof KernelConnectionTimeoutError || - err instanceof KernelConnectionTimeoutError || - err instanceof KernelPortNotUsedTimeoutError + err instanceof KernelDiedError || + err instanceof KernelProcessExitedError || + err instanceof PythonKernelDiedError ) { - this.applicationShell.showErrorMessage(err.message).then(noop, noop); - } else if (err instanceof KernelDiedError || err instanceof KernelProcessExitedError) { + const defaultErrorMessage = getCombinedErrorMessage( + errorPrefix, + // 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 + ); if ((purpose === 'restart' || purpose === 'start') && handler) { - handler(err); + await handler(err, defaultErrorMessage); } else { - this.applicationShell - .showErrorMessage( - getCombinedErrorMessage( - errorPrefix, - getErrorMessageFromPythonTraceback(err.stdErr) || err.stdErr - ) - ) - .then(noop, noop); + this.applicationShell.showErrorMessage(defaultErrorMessage).then(noop, noop); } - } else if (err instanceof PythonKernelDiedError) { - this.applicationShell - .showErrorMessage(getCombinedErrorMessage(errorPrefix, err.errorMessage)) - .then(noop, noop); } else { // Some errors have localized and/or formatted error messages. this.applicationShell @@ -161,7 +133,7 @@ function getCombinedErrorMessage(prefix?: string, message?: string) { } return errorMessage; } -function getErrorMessagePrefix(purpose?: 'start' | 'restart' | 'interrupt') { +function getErrorMessagePrefix(purpose?: 'start' | 'restart' | 'interrupt' | 'execution') { switch (purpose) { case 'restart': return DataScience.failedToRestartKernel(); diff --git a/src/client/datascience/errors/sessionDisposedError.ts b/src/client/datascience/errors/sessionDisposedError.ts new file mode 100644 index 00000000000..9b511ca1a98 --- /dev/null +++ b/src/client/datascience/errors/sessionDisposedError.ts @@ -0,0 +1,11 @@ +// 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 SessionDisposedError extends BaseError { + constructor() { + super('sessionDisposed', DataScience.sessionDisposed()); + } +} diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts index ecea5b09a19..6bdfa0c9e6d 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts @@ -82,15 +82,17 @@ export class JupyterInterpreterService { * @memberof JupyterInterpreterService */ public async selectInterpreter(token?: CancellationToken): Promise { - const resolveToUndefinedWhenCancelled = createPromiseFromCancellation({ - cancelAction: 'resolve', - defaultValue: undefined, - token - }); - const interpreter = await Promise.race([ - this.jupyterInterpreterSelector.selectInterpreter(), - resolveToUndefinedWhenCancelled - ]); + const promises = [this.jupyterInterpreterSelector.selectInterpreter()]; + if (token) { + promises.push( + createPromiseFromCancellation({ + cancelAction: 'resolve', + defaultValue: undefined, + token + }) + ); + } + const interpreter = await Promise.race(promises); if (!interpreter) { sendTelemetryEvent(Telemetry.SelectJupyterInterpreter, undefined, { result: 'notSelected' }); return; @@ -176,17 +178,18 @@ export class JupyterInterpreterService { token?: CancellationToken ): Promise { try { - const resolveToUndefinedWhenCancelled = createPromiseFromCancellation({ - cancelAction: 'resolve', - defaultValue: undefined, - token - }); - + const promises = [this.interpreterService.getInterpreterDetails(pythonPath, undefined)]; + if (token) { + promises.push( + createPromiseFromCancellation({ + cancelAction: 'resolve', + defaultValue: undefined, + token + }) + ); + } // First see if we can get interpreter details - const interpreter = await Promise.race([ - this.interpreterService.getInterpreterDetails(pythonPath, undefined), - resolveToUndefinedWhenCancelled - ]); + const interpreter = await Promise.race(promises); if (interpreter) { // Then check that dependencies are installed if (await this.interpreterConfiguration.areDependenciesInstalled(interpreter, token)) { diff --git a/src/client/datascience/jupyter/jupyterNotebookProvider.ts b/src/client/datascience/jupyter/jupyterNotebookProvider.ts index 7f4046b28d3..4f888b5f3e0 100644 --- a/src/client/datascience/jupyter/jupyterNotebookProvider.ts +++ b/src/client/datascience/jupyter/jupyterNotebookProvider.ts @@ -4,7 +4,7 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import * as localize from '../../common/utils/localize'; +import { SessionDisposedError } from '../errors/sessionDisposedError'; import { ConnectNotebookProviderOptions, NotebookCreationOptions, @@ -44,6 +44,6 @@ export class JupyterNotebookProvider implements IJupyterNotebookProvider { } // We want createNotebook to always return a notebook promise, so if we don't have a server // here throw our generic server disposed message that we use in server creatio n - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } } diff --git a/src/client/datascience/jupyter/jupyterSession.ts b/src/client/datascience/jupyter/jupyterSession.ts index 296ce9f6c45..2bffe2592d2 100644 --- a/src/client/datascience/jupyter/jupyterSession.ts +++ b/src/client/datascience/jupyter/jupyterSession.ts @@ -29,6 +29,7 @@ import { JupyterWebSockets } from './jupyterWebSocket'; import { getNameOfKernelConnection } from './kernels/helpers'; import { JupyterKernelService } from './kernels/jupyterKernelService'; import { KernelConnectionMetadata } from './kernels/types'; +import { SessionDisposedError } from '../errors/sessionDisposedError'; const jvscIdentifier = '-jvsc-'; function getRemoteIPynbSuffix(): string { @@ -82,10 +83,6 @@ export class JupyterSession extends BaseJupyterSession { } public async connect(cancelToken?: CancellationToken, disableUI?: boolean): Promise { - if (!this.connInfo) { - throw new Error(localize.DataScience.sessionDisposed()); - } - // Start a new session this.setSession(await this.createNewKernelSession(cancelToken, disableUI)); @@ -155,7 +152,7 @@ export class JupyterSession extends BaseJupyterSession { ): Promise { // We need all of the above to create a restart session if (!session || !this.contentsManager || !this.sessionManager) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } let result: ISessionWithSocket | undefined; let tryCount = 0; diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts index 564248b35e6..6fe1ee6e5e4 100644 --- a/src/client/datascience/jupyter/jupyterSessionManager.ts +++ b/src/client/datascience/jupyter/jupyterSessionManager.ts @@ -27,6 +27,7 @@ import { } from '../../common/types'; import { sleep } from '../../common/utils/async'; import * as localize from '../../common/utils/localize'; +import { SessionDisposedError } from '../errors/sessionDisposedError'; import { IJupyterConnection, IJupyterKernel, @@ -183,7 +184,7 @@ export class JupyterSessionManager implements IJupyterSessionManager { !this.serverSettings || !this.specsManager ) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } // Create a new session and attempt to connect to it const session = new JupyterSession( @@ -213,7 +214,7 @@ export class JupyterSessionManager implements IJupyterSessionManager { public async getKernelSpecs(): Promise { if (!this.connInfo || !this.sessionManager || !this.contentsManager) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } try { // Fetch the list the session manager already knows about. Refreshing may not work. diff --git a/src/client/datascience/jupyter/kernels/cellExecution.ts b/src/client/datascience/jupyter/kernels/cellExecution.ts index d49b4b25e3b..68d1856e085 100644 --- a/src/client/datascience/jupyter/kernels/cellExecution.ts +++ b/src/client/datascience/jupyter/kernels/cellExecution.ts @@ -53,6 +53,7 @@ import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; import { disposeAllDisposables } from '../../../common/helpers'; import { CellHashProviderFactory } from '../../editor-integration/cellHashProviderFactory'; import { InteractiveWindowView } from '../../notebook/constants'; +import { BaseError } from '../../../common/errors/types'; // Helper interface for the set_next_input execute reply payload interface ISetNextInputPayload { @@ -147,6 +148,7 @@ export class CellExecution implements IDisposable { private readonly disposables: IDisposable[] = []; private readonly prompts = new Set(); private _preExecuteEmitter = new EventEmitter(); + private session?: IJupyterSession; private constructor( public readonly cell: NotebookCell, private readonly errorHandler: IDataScienceErrorHandler, @@ -220,6 +222,7 @@ export class CellExecution implements IDisposable { ); } public async start(session: IJupyterSession) { + this.session = session; if (this.cancelHandled) { traceCellMessage(this.cell, 'Not starting as it was cancelled'); return; @@ -302,15 +305,32 @@ export class CellExecution implements IDisposable { this.lastUsedStreamOutput = undefined; } private completedWithErrors(error: Partial) { + traceWarning(`Cell completed with errors`, error); traceCellMessage(this.cell, 'Completed with errors'); this.sendPerceivedCellExecute(); traceCellMessage(this.cell, 'Update with error state & output'); - this.execution?.appendOutput([translateErrorOutput(createErrorOutput(error))]).then(noop, noop); + // No need to append errors related to failures in Kernel execution in output. + // We will display messages for those. + if (!error || !(error instanceof BaseError)) { + this.execution?.appendOutput([translateErrorOutput(createErrorOutput(error))]).then(noop, noop); + } this.endCellTask('failed'); this._completed = true; - this.errorHandler.handleError((error as unknown) as Error).ignoreErrors(); + + // If the kernel is dead, then no point handling errors. + // We have other code that deals with kernels dying. + // We're only concerned with failures in execution while kernel is still running. + let handleError = true; + if (this.session?.disposed || this.session?.status === 'terminating' || this.session?.status === 'dead') { + handleError = false; + } + if (handleError) { + this.errorHandler + .handleKernelError((error as unknown) as Error, 'execution', this.kernelConnection) + .ignoreErrors(); + } traceCellMessage(this.cell, 'Completed with errors, & resolving'); this._result.resolve(NotebookCellRunState.Error); } @@ -488,7 +508,7 @@ export class CellExecution implements IDisposable { ); } catch (ex) { traceError(`Cell execution failed without request, for cell Index ${this.cell.index}`, ex); - return this.completedWithErrors(new Error('Session cannot generate requests')); + return this.completedWithErrors(ex); } // Listen to messages and update our cell execution state appropriately // Keep track of our clear state @@ -525,6 +545,7 @@ export class CellExecution implements IDisposable { this.completedSuccessfully(); traceCellMessage(this.cell, 'Executed successfully in executeCell'); } catch (ex) { + traceError('Error in waiting for cell to complete', ex); // @jupyterlab/services throws a `Canceled` error when the kernel is interrupted. // Such an error must be ignored. if (ex && ex instanceof Error && ex.message.includes('Canceled')) { diff --git a/src/client/datascience/jupyter/kernels/kernel.ts b/src/client/datascience/jupyter/kernels/kernel.ts index 9fd363185f2..4774228d574 100644 --- a/src/client/datascience/jupyter/kernels/kernel.ts +++ b/src/client/datascience/jupyter/kernels/kernel.ts @@ -234,6 +234,7 @@ export class Kernel implements IKernel { return this.disposingPromise; } this._ignoreNotebookDisposedErrors = true; + this.startCancellation.cancel(); const disposeImpl = async () => { traceInfo(`Dispose kernel ${(this.resourceUri || this.notebookDocument.uri).toString()}`); this.restarting = undefined; @@ -380,11 +381,14 @@ export class Kernel implements IKernel { ); if (options?.disableUI) { sendTelemetryEvent(Telemetry.KernelStartFailedAndUIDisabled); + } else if (this._disposing) { + // If the kernel was killed for any reason, then no point displaying + // errors about startup failures. + traceWarning(`Ignoring kernel startup failure as kernel was disposed`, ex); } else { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - this.errorHandler - .handleKernelStartRestartError(ex, 'start', this.kernelConnectionMetadata) - .ignoreErrors(); // Just a notification, so don't await this + void this.errorHandler + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .handleKernelError(ex as any, 'start', this.kernelConnectionMetadata); // Just a notification, so don't await this } traceError(`failed to start INotebook in kernel, UI Disabled = ${options?.disableUI}`, ex); this.startCancellation.cancel(); diff --git a/src/client/datascience/jupyter/kernels/kernelCommandListener.ts b/src/client/datascience/jupyter/kernels/kernelCommandListener.ts index 4edca1acd0f..548e64743ad 100644 --- a/src/client/datascience/jupyter/kernels/kernelCommandListener.ts +++ b/src/client/datascience/jupyter/kernels/kernelCommandListener.ts @@ -133,7 +133,7 @@ export class KernelCommandListener implements IDataScienceCommandListener { } } catch (err) { traceError('Failed to interrupt kernel', err); - void this.errorHandler.handleError(err, errorContext); + void this.errorHandler.handleKernelError(err, errorContext, kernel.kernelConnectionMetadata); } finally { this.kernelInterruptedDontAskToRestart = false; status.dispose(); @@ -212,7 +212,7 @@ export class KernelCommandListener implements IDataScienceCommandListener { exc ); traceError('Failed to restart the kernel', exc); - void this.errorHandler.handleError(exc, 'restart'); + void this.errorHandler.handleKernelError(exc, 'restart', kernel.kernelConnectionMetadata); } finally { status.dispose(); } diff --git a/src/client/datascience/jupyter/kernels/kernelDependencyService.ts b/src/client/datascience/jupyter/kernels/kernelDependencyService.ts index a729fcb33f3..83e799f3714 100644 --- a/src/client/datascience/jupyter/kernels/kernelDependencyService.ts +++ b/src/client/datascience/jupyter/kernels/kernelDependencyService.ts @@ -63,6 +63,9 @@ export class KernelDependencyService implements IKernelDependencyService { if (await this.areDependenciesInstalled(interpreter, token)) { return; } + if (token?.isCancellationRequested) { + return; + } // Cache the install run let promise = this.installPromises.get(interpreter.path); @@ -74,14 +77,20 @@ export class KernelDependencyService implements IKernelDependencyService { // Get the result of the question try { const result = await promise; + if (token?.isCancellationRequested) { + return; + } await this.handleKernelDependencyResponse(result, interpreter, resource); } finally { // Don't need to cache anymore this.installPromises.delete(interpreter.path); } } - public areDependenciesInstalled(interpreter: PythonEnvironment, _token?: CancellationToken): Promise { - return this.installer.isInstalled(Product.ipykernel, interpreter).then((installed) => installed === true); + public areDependenciesInstalled(interpreter: PythonEnvironment, token?: CancellationToken): Promise { + return Promise.race([ + this.installer.isInstalled(Product.ipykernel, interpreter).then((installed) => installed === true), + createPromiseFromCancellation({ token, defaultValue: false, cancelAction: 'resolve' }) + ]); } private async handleKernelDependencyResponse( diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index bfc522421a0..9338cedd589 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -38,6 +38,7 @@ import { sendKernelTelemetryEvent } from '../../telemetry/telemetry'; import { StopWatch } from '../../../common/utils/stopWatch'; import { JupyterSession } from '../jupyterSession'; import { JupyterSessionManager } from '../jupyterSessionManager'; +import { SessionDisposedError } from '../../errors/sessionDisposedError'; /* eslint-disable @typescript-eslint/no-explicit-any */ @injectable() @@ -201,7 +202,7 @@ export class HostJupyterServer implements INotebookServer { cancelToken?: CancellationToken ): Promise { if (!this.sessionManager || this.isDisposed) { - throw new Error(localize.DataScience.sessionDisposed()); + throw new SessionDisposedError(); } const stopWatch = new StopWatch(); // Create a notebook and return it. @@ -292,7 +293,7 @@ export class HostJupyterServer implements INotebookServer { } // Default is just say session was disposed - return new Error(localize.DataScience.sessionDisposed()); + return new SessionDisposedError(); } protected trackDisposable(notebook: Promise) { notebook diff --git a/src/client/datascience/jupyter/notebookStarter.ts b/src/client/datascience/jupyter/notebookStarter.ts index ee0d30fd41b..855937b3b87 100644 --- a/src/client/datascience/jupyter/notebookStarter.ts +++ b/src/client/datascience/jupyter/notebookStarter.ts @@ -130,15 +130,14 @@ export class NotebookStarter implements Disposable { } const connection = await Promise.race([ starter.waitForConnection(), - createPromiseFromCancellation({ + createPromiseFromCancellation({ cancelAction: 'reject', - defaultValue: new CancellationError(), token: cancelToken }) ]); - if (connection instanceof CancellationError) { - throw connection; + if (!connection) { + throw new CancellationError(); } // Fire off telemetry for the process being talkable diff --git a/src/client/datascience/kernel-launcher/kernelLauncher.ts b/src/client/datascience/kernel-launcher/kernelLauncher.ts index 6014a0e4a5e..e786bf1348e 100644 --- a/src/client/datascience/kernel-launcher/kernelLauncher.ts +++ b/src/client/datascience/kernel-launcher/kernelLauncher.ts @@ -23,7 +23,7 @@ import { KernelDaemonPool } from './kernelDaemonPool'; import { KernelEnvironmentVariablesService } from './kernelEnvVarsService'; import { KernelProcess } from './kernelProcess'; import { IKernelConnection, IKernelLauncher, IKernelProcess } from './types'; -import { CancellationError } from '../../common/cancellation'; +import { CancellationError, createPromiseFromCancellation } from '../../common/cancellation'; import { sendKernelTelemetryWhenDone } from '../telemetry/telemetry'; import { sendTelemetryEvent } from '../../telemetry'; import { getTelemetrySafeErrorMessageFromPythonTraceback } from '../../common/errors/errorUtils'; @@ -110,6 +110,9 @@ export class KernelLauncher implements IKernelLauncher { cancelToken, disableUI ); + if (cancelToken?.isCancellationRequested) { + throw new CancellationError(); + } } // Should be available now, wait with a timeout @@ -127,6 +130,9 @@ export class KernelLauncher implements IKernelLauncher { cancelToken?: CancellationToken ): Promise { const connection = await this.getKernelConnection(kernelConnectionMetadata); + if (cancelToken?.isCancellationRequested) { + throw new CancellationError(); + } const kernelProcess = new KernelProcess( this.processExecutionFactory, this.daemonPool, @@ -138,9 +144,21 @@ export class KernelLauncher implements IKernelLauncher { this.kernelEnvVarsService, this.pythonExecFactory ); - await kernelProcess.launch(workingDirectory, timeout, cancelToken); - kernelProcess.exited( + try { + await Promise.race([ + kernelProcess.launch(workingDirectory, timeout, cancelToken), + createPromiseFromCancellation({ token: cancelToken, cancelAction: 'reject' }) + ]); + } catch (ex) { + void kernelProcess.dispose(); + if (ex instanceof CancellationError || cancelToken?.isCancellationRequested) { + throw new CancellationError(); + } + throw ex; + } + + const disposable = kernelProcess.exited( ({ exitCode, reason }) => { sendTelemetryEvent(Telemetry.RawKernelSessionKernelProcessExited, undefined, { exitCode, @@ -151,6 +169,7 @@ export class KernelLauncher implements IKernelLauncher { KernelLauncher._usedPorts.delete(connection.iopub_port); KernelLauncher._usedPorts.delete(connection.shell_port); KernelLauncher._usedPorts.delete(connection.stdin_port); + disposable.dispose(); }, this, this.disposables diff --git a/src/client/datascience/kernel-launcher/kernelProcess.ts b/src/client/datascience/kernel-launcher/kernelProcess.ts index 7d574288c04..746587e5b9f 100644 --- a/src/client/datascience/kernel-launcher/kernelProcess.ts +++ b/src/client/datascience/kernel-launcher/kernelProcess.ts @@ -213,8 +213,7 @@ export class KernelProcess implements IKernelProcess { deferred.promise, createPromiseFromCancellation({ token: cancelToken, - cancelAction: 'reject', - defaultValue: undefined + cancelAction: 'reject' }) ]); } catch (e) { diff --git a/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts b/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts index fd6faad747e..3ec0a40ccef 100644 --- a/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts +++ b/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import type { KernelMessage } from '@jupyterlab/services'; import { inject, injectable } from 'inversify'; import { CancellationToken, @@ -127,13 +128,17 @@ export class JupyterCompletionProvider implements CompletionItemProvider { metadata: {} }; } - const result = await Promise.race([ - session.requestComplete({ - code: cellCode, - cursor_pos: offsetInCode - }), - createPromiseFromCancellation({ defaultValue: undefined, cancelAction: 'resolve', token: cancelToken }) - ]); + const promises: Promise[] = cancelToken + ? [createPromiseFromCancellation({ defaultValue: undefined, cancelAction: 'resolve', token: cancelToken })] + : []; + const result = await Promise.race( + promises.concat( + session.requestComplete({ + code: cellCode, + cursor_pos: offsetInCode + }) + ) + ); traceInfoIfCI( `Got jupyter notebook completions. Is cancel? ${cancelToken?.isCancellationRequested}: ${ result ? JSON.stringify(result) : 'empty' diff --git a/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts b/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts index 25c295c9bd5..84177c21d7f 100644 --- a/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts +++ b/src/client/datascience/raw-kernel/liveshare/hostRawNotebookProvider.ts @@ -12,6 +12,7 @@ import { traceError, traceInfo, traceVerbose } from '../../../common/logger'; import { IAsyncDisposableRegistry, IConfigurationService, + IDisposable, IDisposableRegistry, IOutputChannel, Resource @@ -40,6 +41,7 @@ import { STANDARD_OUTPUT_CHANNEL } from '../../../common/constants'; import { getDisplayPath } from '../../../common/platform/fs-paths'; import { JupyterNotebook } from '../../jupyter/jupyterNotebook'; import * as uuid from 'uuid/v4'; +import { disposeAllDisposables } from '../../../common/helpers'; // eslint-disable-next-line @typescript-eslint/no-require-imports /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -104,7 +106,7 @@ export class HostRawNotebookProvider implements IRawNotebookProvider { traceInfo(`Creating raw notebook for ${getDisplayPath(document.uri)}`); const notebookPromise = createDeferred(); this.trackDisposable(notebookPromise.promise); - let progressDisposable: vscode.Disposable | undefined; + const disposables: IDisposable[] = []; let rawSession: RawJupyterSession | undefined; traceInfo(`Getting preferred kernel for ${getDisplayPath(document.uri)}`); @@ -124,12 +126,15 @@ export class HostRawNotebookProvider implements IRawNotebookProvider { // We need to locate kernelspec and possible interpreter for this launch based on resource and notebook metadata const displayName = getDisplayNameOrNameOfKernelConnection(kernelConnection); - progressDisposable = !disableUI + const progressDisposable = !disableUI ? this.progressReporter.createProgressIndicator( localize.DataScience.connectingToKernel().format(displayName) ) : undefined; - + if (progressDisposable) { + disposables.push(progressDisposable); + cancelToken?.onCancellationRequested(() => progressDisposable?.dispose(), this, disposables); + } traceInfo(`Computing working directory ${getDisplayPath(document.uri)}`); const workingDirectory = await computeWorkingDirectory(resource, this.workspaceService); const launchTimeout = this.configService.getSettings(resource).jupyterLaunchTimeout; @@ -176,7 +181,7 @@ export class HostRawNotebookProvider implements IRawNotebookProvider { // This original promise must be rejected as it is cached (check `setNotebook`). notebookPromise.reject(ex); } finally { - progressDisposable?.dispose(); // NOSONAR + disposeAllDisposables(disposables); } return notebookPromise.promise; diff --git a/src/client/datascience/raw-kernel/rawJupyterSession.ts b/src/client/datascience/raw-kernel/rawJupyterSession.ts index 1752a6de62c..40ce4d82030 100644 --- a/src/client/datascience/raw-kernel/rawJupyterSession.ts +++ b/src/client/datascience/raw-kernel/rawJupyterSession.ts @@ -4,7 +4,7 @@ import type { Kernel, KernelMessage } from '@jupyterlab/services'; import type { Slot } from '@lumino/signaling'; import { CancellationToken } from 'vscode-jsonrpc'; -import { CancellationError } from '../../common/cancellation'; +import { CancellationError, createPromiseFromCancellation } from '../../common/cancellation'; import { getTelemetrySafeErrorMessageFromPythonTraceback } from '../../common/errors/errorUtils'; import { traceError, traceInfo, traceInfoIfCI, traceWarning } from '../../common/logger'; import { IDisposable, IOutputChannel, Resource } from '../../common/types'; @@ -82,7 +82,9 @@ 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(cancelToken, disableUI); - + if (cancelToken?.isCancellationRequested) { + return; + } // Only connect our session if we didn't cancel or timeout sendKernelTelemetryEvent(this.resource, Telemetry.RawKernelSessionStartSuccess); sendKernelTelemetryEvent(this.resource, Telemetry.RawKernelSessionStart, stopWatch.elapsedTime); @@ -268,8 +270,20 @@ export class RawJupyterSession extends BaseJupyterSession { // Create our raw session, it will own the process lifetime const result = new RawSession(process, this.resource); - // Wait for it to be ready - await result.waitForReady(); + try { + // Wait for it to be ready + await Promise.race([ + result.waitForReady(), + createPromiseFromCancellation({ cancelAction: 'reject', token: cancelToken }) + ]); + } catch (ex) { + void process.dispose(); + void result.dispose(); + if (ex instanceof CancellationError || cancelToken?.isCancellationRequested) { + throw new CancellationError(); + } + throw ex; + } // Attempt to get kernel to respond to requests (this is what jupyter does today). // Kinda warms up the kernel communiocation & ensure things are in the right state. @@ -286,10 +300,17 @@ export class RawJupyterSession extends BaseJupyterSession { gotIoPubMessage = createDeferred(); const iopubHandler = () => gotIoPubMessage.resolve(true); result.iopubMessage.connect(iopubHandler); - await Promise.race([ - Promise.all([result.kernel.requestKernelInfo(), gotIoPubMessage.promise]), - sleep(Math.min(this.launchTimeout, 10)) - ]); + try { + await Promise.race([ + Promise.all([result.kernel.requestKernelInfo(), gotIoPubMessage.promise]), + sleep(Math.min(this.launchTimeout, 10)), + createPromiseFromCancellation({ cancelAction: 'reject', token: cancelToken }) + ]); + } catch (ex) { + void process.dispose(); + void result.dispose(); + throw ex; + } result.iopubMessage.disconnect(iopubHandler); if (gotIoPubMessage.completed) { diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 06ac6af15b0..e954746b321 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -386,10 +386,13 @@ export interface IDataScienceErrorHandler { * The value of `context` is used to determine the context of the error message, whether it applies to starting or interrupting kernels or the like. * Thus based on the context the error message would be different. */ - handleError(err: Error, context?: 'start' | 'restart' | 'interrupt'): Promise; - handleKernelStartRestartError( + handleError(err: Error): Promise; + /** + * Handles errors specific to kernels. + */ + handleKernelError( err: Error, - context: 'start' | 'restart', + context: 'start' | 'restart' | 'interrupt' | 'execution', kernelConnection: KernelConnectionMetadata ): Promise; } diff --git a/src/test/datascience/errorHandler.unit.test.ts b/src/test/datascience/errorHandler.unit.test.ts index 3e934888e89..9b420dfdc0b 100644 --- a/src/test/datascience/errorHandler.unit.test.ts +++ b/src/test/datascience/errorHandler.unit.test.ts @@ -3,7 +3,8 @@ 'use strict'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; -import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; +import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; +import { IBrowserService } from '../../client/common/types'; import * as localize from '../../client/common/utils/localize'; import { DataScienceErrorHandler } from '../../client/datascience/errors/errorHandler'; import { JupyterInstallError } from '../../client/datascience/errors/jupyterInstallError'; @@ -15,15 +16,21 @@ suite('DataScience Error Handler Unit Tests', () => { let dataScienceErrorHandler: DataScienceErrorHandler; let dependencyManager: IJupyterInterpreterDependencyManager; let worksapceService: IWorkspaceService; + let browser: IBrowserService; + let commandManager: ICommandManager; setup(() => { applicationShell = typemoq.Mock.ofType(); worksapceService = mock(); dependencyManager = mock(); + browser = mock(); + commandManager = mock(); when(dependencyManager.installMissingDependencies(anything())).thenResolve(); dataScienceErrorHandler = new DataScienceErrorHandler( applicationShell.object, instance(dependencyManager), - instance(worksapceService) + instance(worksapceService), + instance(browser), + instance(commandManager) ); }); const message = 'Test error message.'; From cb6995146c37a49c8258930b0ae93459d39b00d4 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Nov 2021 16:53:23 -0800 Subject: [PATCH 04/10] oops --- .../intellisense/jupyterCompletionProvider.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts b/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts index 3ec0a40ccef..fd6faad747e 100644 --- a/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts +++ b/src/client/datascience/notebook/intellisense/jupyterCompletionProvider.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import type { KernelMessage } from '@jupyterlab/services'; import { inject, injectable } from 'inversify'; import { CancellationToken, @@ -128,17 +127,13 @@ export class JupyterCompletionProvider implements CompletionItemProvider { metadata: {} }; } - const promises: Promise[] = cancelToken - ? [createPromiseFromCancellation({ defaultValue: undefined, cancelAction: 'resolve', token: cancelToken })] - : []; - const result = await Promise.race( - promises.concat( - session.requestComplete({ - code: cellCode, - cursor_pos: offsetInCode - }) - ) - ); + const result = await Promise.race([ + session.requestComplete({ + code: cellCode, + cursor_pos: offsetInCode + }), + createPromiseFromCancellation({ defaultValue: undefined, cancelAction: 'resolve', token: cancelToken }) + ]); traceInfoIfCI( `Got jupyter notebook completions. Is cancel? ${cancelToken?.isCancellationRequested}: ${ result ? JSON.stringify(result) : 'empty' From f1b38306aba2e2a1ea88d3a9aa83a1b70a831476 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Nov 2021 16:55:52 -0800 Subject: [PATCH 05/10] fix formatting --- .../interpreter/jupyterInterpreterService.ts | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts index 6bdfa0c9e6d..b450c2c86b6 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts @@ -82,17 +82,15 @@ export class JupyterInterpreterService { * @memberof JupyterInterpreterService */ public async selectInterpreter(token?: CancellationToken): Promise { - const promises = [this.jupyterInterpreterSelector.selectInterpreter()]; - if (token) { - promises.push( - createPromiseFromCancellation({ - cancelAction: 'resolve', - defaultValue: undefined, - token - }) - ); - } - const interpreter = await Promise.race(promises); + const resolveToUndefinedWhenCancelled = createPromiseFromCancellation({ + cancelAction: 'resolve', + defaultValue: undefined, + token + }); + const interpreter = await Promise.race([ + this.jupyterInterpreterSelector.selectInterpreter(), + resolveToUndefinedWhenCancelled + ]); if (!interpreter) { sendTelemetryEvent(Telemetry.SelectJupyterInterpreter, undefined, { result: 'notSelected' }); return; @@ -178,18 +176,16 @@ export class JupyterInterpreterService { token?: CancellationToken ): Promise { try { - const promises = [this.interpreterService.getInterpreterDetails(pythonPath, undefined)]; - if (token) { - promises.push( - createPromiseFromCancellation({ - cancelAction: 'resolve', - defaultValue: undefined, - token - }) - ); - } + const resolveToUndefinedWhenCancelled = createPromiseFromCancellation({ + cancelAction: 'resolve', + defaultValue: undefined, + token + }); // First see if we can get interpreter details - const interpreter = await Promise.race(promises); + const interpreter = await Promise.race([ + this.interpreterService.getInterpreterDetails(pythonPath, undefined), + resolveToUndefinedWhenCancelled + ]); if (interpreter) { // Then check that dependencies are installed if (await this.interpreterConfiguration.areDependenciesInstalled(interpreter, token)) { From f044abced45ca95ad5706af8df385c49a3965601 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 11 Nov 2021 09:22:55 -0800 Subject: [PATCH 06/10] oops --- pythonFiles/vscode_datascience_helpers/kernel_launcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/vscode_datascience_helpers/kernel_launcher.py b/pythonFiles/vscode_datascience_helpers/kernel_launcher.py index a94505740c2..15f05d60414 100644 --- a/pythonFiles/vscode_datascience_helpers/kernel_launcher.py +++ b/pythonFiles/vscode_datascience_helpers/kernel_launcher.py @@ -96,7 +96,7 @@ def launch_kernel( # Create a Win32 event for interrupting the kernel # and store it in an environment variable. interrupt_event = create_interrupt_event() - env[" "] = str(interrupt_event) + env["JPY_INTERRUPT_EVENT"] = str(interrupt_event) # deprecated old env name: env["IPY_INTERRUPT_EVENT"] = env["JPY_INTERRUPT_EVENT"] From 49c4129177ac10dd7657fdc75ffed8f55a0ac89d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 11 Nov 2021 09:26:17 -0800 Subject: [PATCH 07/10] fix formatting --- src/client/datascience/errors/errorHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/errors/errorHandler.ts b/src/client/datascience/errors/errorHandler.ts index 7bc08e079e0..46382aefebb 100644 --- a/src/client/datascience/errors/errorHandler.ts +++ b/src/client/datascience/errors/errorHandler.ts @@ -39,8 +39,8 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { @inject(ICommandManager) protected commandManager: ICommandManager ) {} public async handleError(err: Error): Promise { - await this.handleErrorImplementation(err); traceError('DataScience Error', err); + await this.handleErrorImplementation(err); } public async handleKernelError( From 699bb3c7e18f627f3ed0bbe4256aa8be6d1399a2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 11 Nov 2021 09:27:38 -0800 Subject: [PATCH 08/10] fix formatting --- src/client/datascience/jupyter/kernels/cellExecution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/jupyter/kernels/cellExecution.ts b/src/client/datascience/jupyter/kernels/cellExecution.ts index 68d1856e085..fff7ee49dc3 100644 --- a/src/client/datascience/jupyter/kernels/cellExecution.ts +++ b/src/client/datascience/jupyter/kernels/cellExecution.ts @@ -312,7 +312,7 @@ export class CellExecution implements IDisposable { traceCellMessage(this.cell, 'Update with error state & output'); // No need to append errors related to failures in Kernel execution in output. // We will display messages for those. - if (!error || !(error instanceof BaseError)) { + if (!(error instanceof BaseError)) { this.execution?.appendOutput([translateErrorOutput(createErrorOutput(error))]).then(noop, noop); } From 6660e651d6ab1d4b908e0125edbe5293de8308a0 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 11 Nov 2021 12:41:28 -0800 Subject: [PATCH 09/10] Fix tests --- .vscode/launch.json | 4 +- .../common/process/pythonExecutionFactory.ts | 1 + src/client/datascience/errors/errorHandler.ts | 7 +- src/test/datascience/.vscode/settings.json | 4 +- .../notebook/kernelCrashes.vscode.test.ts | 67 ++++++++++++++++++- .../overrideBuiltinModule/.gitignore | 1 + .../overrideBuiltinModule/random.py | 0 7 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/.gitignore create mode 100644 src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/random.py diff --git a/.vscode/launch.json b/.vscode/launch.json index fce164c0d89..4b5a3eade83 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -193,7 +193,7 @@ "env": { "VSC_JUPYTER_CI_TEST_GREP": "", // Leave as `VSCode Notebook` to run only Notebook tests. "VSC_JUPYTER_CI_TEST_INVERT_GREP": "", // Initialize this to invert the grep (exclude tests with value defined in grep). - "CI_PYTHON_PATH": "/home/don/samples/pySamples/crap/.venv/bin/python", // Update with path to real python interpereter used for testing. + "CI_PYTHON_PATH": "", // Update with path to real python interpereter used for testing. "VSC_FORCE_REAL_JUPYTER": "true", // Enalbe tests that require Jupyter. "VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST": "", // Initialize this to run tests again Julia & other kernels. "VSC_JUPYTER_RUN_NB_TEST": "true", // Initialize this to run notebook tests (must be using VSC Insiders). @@ -209,7 +209,7 @@ "stopOnEntry": false, "sourceMaps": true, "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"], - // "preLaunchTask": "Compile", + "preLaunchTask": "Compile", "skipFiles": ["/**"], "presentation": { "group": "2_tests", diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 868641f2a5f..4e6e03ac0fa 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -91,6 +91,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { (interpreter?.version && interpreter.version.major < 3) || this.config.getSettings().disablePythonDaemon ) { + traceInfo(`Not using daemon support for ${pythonPath}`); return activatedProcPromise; } diff --git a/src/client/datascience/errors/errorHandler.ts b/src/client/datascience/errors/errorHandler.ts index 46382aefebb..29921375afa 100644 --- a/src/client/datascience/errors/errorHandler.ts +++ b/src/client/datascience/errors/errorHandler.ts @@ -58,17 +58,14 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { case KernelFailureReason.overridingBuiltinModules: { await this.applicationShell .showErrorMessage( - DataScience.fileSeemsToBeInterferingWithKernelStartup().format( + `${DataScience.fileSeemsToBeInterferingWithKernelStartup().format( getDisplayPath(failureInfo.fileName, this.workspace.workspaceFolders || []) - ), - DataScience.showJupyterLogs(), + )} \n${DataScience.viewJupyterLogForFurtherInfo()}`, Common.learnMore() ) .then((selection) => { if (selection === Common.learnMore()) { this.browser.launch('https://aka.ms/kernelFailuresOverridingBuiltInModules'); - } else if (selection === DataScience.showJupyterLogs()) { - void this.commandManager.executeCommand('jupyter.viewOutput'); } }); break; diff --git a/src/test/datascience/.vscode/settings.json b/src/test/datascience/.vscode/settings.json index 9496201af6f..e00c6274980 100644 --- a/src/test/datascience/.vscode/settings.json +++ b/src/test/datascience/.vscode/settings.json @@ -11,7 +11,7 @@ "python.linting.mypyEnabled": false, "python.linting.banditEnabled": false, "python.formatting.provider": "yapf", - "python.pythonPath": "python", + "python.pythonPath": "/home/don/samples/pySamples/crap/.venv/bin/python", "jupyter.experiments.optInto": ["NativeNotebookEditor"], // Do not set this to "Microsoft", else it will result in LS being downloaded on CI // and that slows down tests significantly. We have other tests on CI for testing @@ -30,5 +30,5 @@ "jupyter.magicCommandsAsComments": false, // Python experiments have to be on for insiders to work "python.experiments.enabled": true, - "python.defaultInterpreterPath": "python" + "python.defaultInterpreterPath": "/home/don/samples/pySamples/crap/.venv/bin/python" } diff --git a/src/test/datascience/notebook/kernelCrashes.vscode.test.ts b/src/test/datascience/notebook/kernelCrashes.vscode.test.ts index 123a265d779..608842b3eec 100644 --- a/src/test/datascience/notebook/kernelCrashes.vscode.test.ts +++ b/src/test/datascience/notebook/kernelCrashes.vscode.test.ts @@ -4,12 +4,14 @@ 'use strict'; /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ +import * as path from 'path'; +import * as fs from 'fs-extra'; import { assert } from 'chai'; import * as sinon from 'sinon'; import { DataScience } from '../../../client/common/utils/localize'; import { IVSCodeNotebook } from '../../../client/common/application/types'; import { traceInfo } from '../../../client/common/logger'; -import { IDisposable } from '../../../client/common/types'; +import { IConfigurationService, IDisposable, IJupyterSettings, ReadWrite } from '../../../client/common/types'; import { captureScreenShot, IExtensionTestApi } from '../../common'; import { initialize } from '../../initialize'; import { @@ -23,14 +25,18 @@ import { createEmptyPythonNotebook, workAroundVSCodeNotebookStartPages, waitForExecutionCompletedSuccessfully, - runAllCellsInActiveNotebook + runAllCellsInActiveNotebook, + waitForKernelToGetAutoSelected } from './helper'; -import { IS_NON_RAW_NATIVE_TEST, IS_REMOTE_NATIVE_TEST } from '../../constants'; +import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_NON_RAW_NATIVE_TEST, IS_REMOTE_NATIVE_TEST } from '../../constants'; import * as dedent from 'dedent'; import { IKernelProvider } from '../../../client/datascience/jupyter/kernels/types'; 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 { getDisplayPath } from '../../../client/common/platform/fs-paths'; const codeToKillKernel = dedent` import IPython @@ -44,6 +50,7 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' const disposables: IDisposable[] = []; let vscodeNotebook: IVSCodeNotebook; let kernelProvider: IKernelProvider; + let config: IConfigurationService; this.timeout(120_000); suiteSetup(async function () { @@ -55,6 +62,7 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' return this.skip(); } kernelProvider = api.serviceContainer.get(IKernelProvider); + config = api.serviceContainer.get(IConfigurationService); await workAroundVSCodeNotebookStartPages(); await startJupyterServer(); await prewarmNotebooks(); @@ -85,9 +93,12 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' if (this.currentTest?.isFailed()) { await captureScreenShot(this.currentTest?.title); } + const settings = config.getSettings() as ReadWrite; + settings.disablePythonDaemon = false; // Added temporarily to identify why tests are failing. process.env.VSC_JUPYTER_LOG_KERNEL_OUTPUT = undefined; await closeNotebooksAndCleanUpAfterTests(disposables); + sinon.restore(); traceInfo(`Ended Test (completed) ${this.currentTest?.title}`); }); suiteTeardown(() => closeNotebooksAndCleanUpAfterTests(disposables)); @@ -247,5 +258,55 @@ suite('DataScience - VSCode Notebook Kernel Error Handling - (Execution) (slow)' assert.strictEqual(restartPrompt.getDisplayCount(), 1, 'Should only have one restart prompt'); }); + async function createAndOpenTemporaryNotebookForKernelCrash(nbFileName: string) { + const { serviceContainer } = await initialize(); + const editorProvider = serviceContainer.get(INotebookEditorProvider); + const vscodeNotebook = serviceContainer.get(IVSCodeNotebook); + const nbFile = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + `src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/${nbFileName}` + ); + fs.ensureDirSync(path.dirname(nbFile)); + fs.writeFileSync(nbFile, ''); + disposables.push({ dispose: () => fs.unlinkSync(nbFile) }); + // Open a python notebook and use this for all tests in this test suite. + await editorProvider.open(Uri.file(nbFile)); + assert.isOk(vscodeNotebook.activeNotebookEditor, 'No active notebook'); + await waitForKernelToGetAutoSelected(); + } + async function displayErrorAboutOverriddenBuiltInModules(disablePythonDaemon: boolean) { + 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' + ); + const expectedErrorMessage = `${DataScience.fileSeemsToBeInterferingWithKernelStartup().format( + getDisplayPath(randomFile, workspace.workspaceFolders || []) + )} \n${DataScience.viewJupyterLogForFurtherInfo()}`; + + const prompt = await hijackPrompt( + 'showErrorMessage', + { + exactMatch: expectedErrorMessage + }, + { dismissPrompt: true }, + disposables + ); + + await createAndOpenTemporaryNotebookForKernelCrash(`nb${disablePythonDaemon}.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(); + } + test('Display error about overriding builtin modules (with Python daemon', () => + displayErrorAboutOverriddenBuiltInModules(false)); + test('Display error about overriding builtin modules (without Python daemon', () => + displayErrorAboutOverriddenBuiltInModules(true)); }); }); diff --git a/src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/.gitignore b/src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/.gitignore new file mode 100644 index 00000000000..fa656082978 --- /dev/null +++ b/src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/.gitignore @@ -0,0 +1 @@ +*.ipynb diff --git a/src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/random.py b/src/test/datascience/notebook/kernelFailures/overrideBuiltinModule/random.py new file mode 100644 index 00000000000..e69de29bb2d From 489bdd4641e4a497786ae9dce5d8887181f2d3c0 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 11 Nov 2021 14:23:33 -0800 Subject: [PATCH 10/10] fix formatting --- src/client/common/errors/errorUtils.ts | 2 +- src/client/common/errors/errors.ts | 4 ++-- src/client/datascience/errors/errorHandler.ts | 4 ++-- src/test/datascience/.vscode/settings.json | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/common/errors/errorUtils.ts b/src/client/common/errors/errorUtils.ts index 35634aee23a..21b5688b603 100644 --- a/src/client/common/errors/errorUtils.ts +++ b/src/client/common/errors/errorUtils.ts @@ -224,7 +224,7 @@ export type KernelFailure = | OldIPyKernelFailure | OldIPythonFailure; -export function analyseKernelErrors( +export function analyzeKernelErrors( stdErrOrStackTrace: string, workspaceFolders: readonly WorkspaceFolder[] = [], pythonSysPrefix: string = '' diff --git a/src/client/common/errors/errors.ts b/src/client/common/errors/errors.ts index fe45ca284e8..97c05cde1d7 100644 --- a/src/client/common/errors/errors.ts +++ b/src/client/common/errors/errors.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { analyseKernelErrors } from './errorUtils'; +import { analyzeKernelErrors } from './errorUtils'; const taggers = [tagWithChildProcessExited, tagWithKernelRestarterFailed]; export function getErrorTags(stdErrOrStackTrace: string | string[]) { @@ -12,7 +12,7 @@ export function getErrorTags(stdErrOrStackTrace: string | string[]) { ? stdErrOrStackTrace[0].toLowerCase() : stdErrOrStackTrace.toLowerCase(); taggers.forEach((tagger) => tagger(stdErrOrStackTraceLowered, tags)); - const error = analyseKernelErrors(stdErrOrStackTraceLowered); + const error = analyzeKernelErrors(stdErrOrStackTraceLowered); if (error?.telemetrySafeTags.length) { tags.push(...error?.telemetrySafeTags); } diff --git a/src/client/datascience/errors/errorHandler.ts b/src/client/datascience/errors/errorHandler.ts index 29921375afa..1857a11ad6c 100644 --- a/src/client/datascience/errors/errorHandler.ts +++ b/src/client/datascience/errors/errorHandler.ts @@ -21,7 +21,7 @@ import { KernelPortNotUsedTimeoutError } from './kernelPortNotUsedTimeoutError'; import { KernelProcessExitedError } from './kernelProcessExitedError'; import { PythonKernelDiedError } from './pythonKernelDiedError'; import { - analyseKernelErrors, + analyzeKernelErrors, getErrorMessageFromPythonTraceback, KernelFailureReason } from '../../common/errors/errorUtils'; @@ -49,7 +49,7 @@ export class DataScienceErrorHandler implements IDataScienceErrorHandler { kernelConnection: KernelConnectionMetadata ): Promise { await this.handleErrorImplementation(err, purpose, async (error: BaseError, defaultErrorMessage: string) => { - const failureInfo = analyseKernelErrors( + const failureInfo = analyzeKernelErrors( error.stdErr || '', this.workspace.workspaceFolders, kernelConnection.interpreter?.sysPrefix diff --git a/src/test/datascience/.vscode/settings.json b/src/test/datascience/.vscode/settings.json index e00c6274980..9496201af6f 100644 --- a/src/test/datascience/.vscode/settings.json +++ b/src/test/datascience/.vscode/settings.json @@ -11,7 +11,7 @@ "python.linting.mypyEnabled": false, "python.linting.banditEnabled": false, "python.formatting.provider": "yapf", - "python.pythonPath": "/home/don/samples/pySamples/crap/.venv/bin/python", + "python.pythonPath": "python", "jupyter.experiments.optInto": ["NativeNotebookEditor"], // Do not set this to "Microsoft", else it will result in LS being downloaded on CI // and that slows down tests significantly. We have other tests on CI for testing @@ -30,5 +30,5 @@ "jupyter.magicCommandsAsComments": false, // Python experiments have to be on for insiders to work "python.experiments.enabled": true, - "python.defaultInterpreterPath": "/home/don/samples/pySamples/crap/.venv/bin/python" + "python.defaultInterpreterPath": "python" }