microsoft · rchiodo · Jun 11, 2020 · Jun 11, 2020 · Jun 11, 2020 · Jun 11, 2020
diff --git a/news/2 Fixes/11749.md b/news/2 Fixes/11749.md
@@ -0,0 +1 @@
+Jupyter can fail to run a kernel if the user's environment contains non string values.
diff --git a/src/client/datascience/jupyter/kernels/helpers.ts b/src/client/datascience/jupyter/kernels/helpers.ts
@@ -8,6 +8,9 @@ import { JupyterKernelSpec } from './jupyterKernelSpec';
 // tslint:disable-next-line: no-var-requires no-require-imports
 const NamedRegexp = require('named-js-regexp') as typeof import('named-js-regexp');
 
+// tslint:disable-next-line: no-require-imports
+import cloneDeep = require('lodash/cloneDeep');
+
 // Helper functions for dealing with kernels and kernelspecs
 
 export const defaultKernelSpecName = 'python_defaultSpec_';
@@ -42,3 +45,24 @@ export function detectDefaultKernelName(name: string) {
     const regEx = NamedRegexp('python\\s*(?<version>(\\d+))', 'g');
     return regEx.exec(name.toLowerCase());
 }
+
+export function cleanEnvironment<T>(spec: T): T {
+    // tslint:disable-next-line: no-any
+    const copy = cloneDeep(spec) as { env?: any };
+
+    if (copy.env) {
+        // Scrub the environment of the spec to make sure it has allowed values (they all must be strings)
+        // See this issue here: https://github.com/microsoft/vscode-python/issues/11749
+        const keys = Object.keys(copy.env);
+        keys.forEach((k) => {
+            if (copy.env) {
+                const value = copy.env[k];
+                if (value !== null && value !== undefined) {
+                    copy.env[k] = value.toString();
+                }
+            }
+        });
+    }
+
+    return copy as T;
+}
diff --git a/src/client/datascience/jupyter/kernels/kernelService.ts b/src/client/datascience/jupyter/kernels/kernelService.ts
@@ -32,7 +32,7 @@ import {
     IKernelDependencyService,
     KernelInterpreterDependencyResponse
 } from '../../types';
-import { detectDefaultKernelName } from './helpers';
+import { cleanEnvironment, detectDefaultKernelName } from './helpers';
 import { JupyterKernelSpec } from './jupyterKernelSpec';
 import { LiveKernelModel } from './types';
 
@@ -395,14 +395,15 @@ export class KernelService {
         forceWrite?: boolean
     ) {
         const specedKernel = kernel as JupyterKernelSpec;
-        if (specedKernel.specFile && interpreter) {
-            const specModel: ReadWrite<Kernel.ISpecModel> = JSON.parse(
+        if (specedKernel.specFile) {
+            let specModel: ReadWrite<Kernel.ISpecModel> = JSON.parse(
                 await this.fileSystem.readFile(specedKernel.specFile)
             );
+            let shouldUpdate = false;
 
             // Make sure the specmodel has an interpreter or already in the metadata or we
             // may overwrite a kernel created by the user
-            if (specModel.metadata?.interpreter || forceWrite) {
+            if (interpreter && (specModel.metadata?.interpreter || forceWrite)) {
                 // Ensure we use a fully qualified path to the python interpreter in `argv`.
                 if (specModel.argv[0].toLowerCase() === 'conda') {
                     // If conda is the first word, its possible its a conda activation command.
@@ -438,7 +439,19 @@ export class KernelService {
                 // tslint:disable-next-line: no-any
                 specModel.metadata.interpreter = interpreter as any;
 
-                // Update the kernel.json with our new stuff.
+                // Indicate we need to write
+                shouldUpdate = true;
+            }
+
+            // Scrub the environment of the specmodel to make sure it has allowed values (they all must be strings)
+            // See this issue here: https://github.com/microsoft/vscode-python/issues/11749
+            if (specModel.env) {
+                specModel = cleanEnvironment(specModel);
+                shouldUpdate = true;
+            }
+
+            // Update the kernel.json with our new stuff.
+            if (shouldUpdate) {
                 await this.fileSystem.writeFile(specedKernel.specFile, JSON.stringify(specModel, undefined, 2), {
                     flag: 'w',
                     encoding: 'utf8'

diff --git a/src/client/datascience/kernel-launcher/kernelProcess.ts b/src/client/datascience/kernel-launcher/kernelProcess.ts
@@ -14,13 +14,11 @@ import { noop, swallowExceptions } from '../../common/utils/misc';
 import { PythonInterpreter } from '../../pythonEnvironments/discovery/types';
 import { captureTelemetry } from '../../telemetry';
 import { Telemetry } from '../constants';
-import { findIndexOfConnectionFile } from '../jupyter/kernels/helpers';
+import { cleanEnvironment, findIndexOfConnectionFile } from '../jupyter/kernels/helpers';
 import { IJupyterKernelSpec } from '../types';
 import { PythonKernelLauncherDaemon } from './kernelLauncherDaemon';
 import { IKernelConnection, IKernelProcess, IPythonKernelDaemon, PythonKernelDiedError } from './types';
 
-// tslint:disable-next-line: no-require-imports
-import cloneDeep = require('lodash/cloneDeep');
 import { IFileSystem } from '../../common/platform/types';
 import { KernelDaemonPool } from './kernelDaemonPool';
 
@@ -58,7 +56,7 @@ export class KernelProcess implements IKernelProcess {
         private readonly interpreter?: PythonInterpreter
     ) {
         this.originalKernelSpec = kernelSpec;
-        this._kernelSpec = cloneDeep(kernelSpec);
+        this._kernelSpec = cleanEnvironment(kernelSpec);
     }
     public async interrupt(): Promise<void> {
         if (this.kernelDaemon) {

diff --git a/src/test/datascience/jupyter/kernels/kernelService.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelService.unit.test.ts
@@ -585,7 +585,11 @@ suite('Data Science - KernelService', () => {
             const kernel = new JupyterKernelSpec(userKernelSpecModel, kernelJsonFile);
             when(jupyterInterpreterExecutionService.getKernelSpecs(anything())).thenResolve([kernel]);
             when(fs.readFile(kernelJsonFile)).thenResolve(JSON.stringify(userKernelSpecModel));
-            when(fs.writeFile(kernelJsonFile, anything())).thenResolve();
+            let contents: string | undefined;
+            when(fs.writeFile(kernelJsonFile, anything(), anything())).thenCall((_f, c) => {
+                contents = c;
+                return Promise.resolve();
+            });
             const envVariables = { MYVAR: '1' };
             when(activationHelper.getActivatedEnvironmentVariables(undefined, interpreter, true)).thenResolve(
                 envVariables
@@ -596,7 +600,9 @@ suite('Data Science - KernelService', () => {
 
             // tslint:disable-next-line: no-any
             assert.deepEqual(kernel, installedKernel as any);
-            verify(fs.writeFile(anything(), anything(), anything())).never();
+            assert.ok(contents, 'Env not updated');
+            const obj = JSON.parse(contents!);
+            assert.notOk(obj.metadata.interpreter, 'MetaData should not have been written');
         });
     });
 });
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Jupyter can fail to run a kernel if the user's environment contains non string values.