Add diagnostic to validate ComSpec

Author: Kartik Raj

src/client/application/diagnostics/checks/pythonInterpreter.ts

@@ -28,6 +28,10 @@ import { EventName } from '../../../telemetry/constants';
 import { IExtensionSingleActivationService } from '../../../activation/types';
 import { cache } from '../../../common/utils/decorators';
 import { noop } from '../../../common/utils/misc';
+import { IPythonExecutionFactory } from '../../../common/process/types';
+import { getOSType, OSType } from '../../../common/utils/platform';
+import { IFileSystem } from '../../../common/platform/types';
+import { traceError } from '../../../logging';
 
 const messages = {
     [DiagnosticCodes.NoPythonInterpretersDiagnostic]: l10n.t(
@@ -36,6 +40,9 @@ const messages = {
     [DiagnosticCodes.InvalidPythonInterpreterDiagnostic]: l10n.t(
         'An Invalid Python interpreter is selected{0}, please try changing it to enable features such as IntelliSense, linting, and debugging. See output for more details regarding why the interpreter is invalid.',
     ),
+    [DiagnosticCodes.InvalidComspecDiagnostic]: l10n.t(
+        "The environment variable 'Comspec' seems to be set to an invalid value. Please correct it to carry valid path to Command Prompt to enable features such as IntelliSense, linting, and debugging. See instructions which might help.",
+    ),
 };
 
 export class InvalidPythonInterpreterDiagnostic extends BaseDiagnostic {
@@ -61,6 +68,12 @@ export class InvalidPythonInterpreterDiagnostic extends BaseDiagnostic {
     }
 }
 
+export class DefaultShellDiagnostic extends BaseDiagnostic {
+    constructor(code: DiagnosticCodes.InvalidComspecDiagnostic, resource: Resource, scope = DiagnosticScope.Global) {
+        super(code, messages[code], DiagnosticSeverity.Error, scope, resource, undefined, 'always');
+    }
+}
+
 export const InvalidPythonInterpreterServiceId = 'InvalidPythonInterpreterServiceId';
 
 @injectable()
@@ -73,7 +86,11 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
         @inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry,
     ) {
         super(
-            [DiagnosticCodes.NoPythonInterpretersDiagnostic, DiagnosticCodes.InvalidPythonInterpreterDiagnostic],
+            [
+                DiagnosticCodes.NoPythonInterpretersDiagnostic,
+                DiagnosticCodes.InvalidPythonInterpreterDiagnostic,
+                DiagnosticCodes.InvalidComspecDiagnostic,
+            ],
             serviceContainer,
             disposableRegistry,
             false,
@@ -103,6 +120,13 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
     public async _manualDiagnose(resource: Resource): Promise<IDiagnostic[]> {
         const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
+        const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
+        if (!currentInterpreter) {
+            const diagnostics = await this.diagnoseDefaultShell(resource);
+            if (diagnostics.length) {
+                return diagnostics;
+            }
+        }
         const hasInterpreters = await interpreterService.hasInterpreters();
         const interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
         const isInterpreterSetToDefault = interpreterPathService.get(resource) === 'python';
@@ -118,7 +142,6 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
             ];
         }
 
-        const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
         if (!currentInterpreter) {
             return [
                 new InvalidPythonInterpreterDiagnostic(
@@ -163,6 +186,17 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
 
     private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] {
         const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
+        if (diagnostic.code === DiagnosticCodes.InvalidComspecDiagnostic) {
+            return [
+                {
+                    prompt: Common.instructions,
+                    command: commandFactory.createCommand(diagnostic, {
+                        type: 'launch',
+                        options: 'https://aka.ms/AAk3djo',
+                    }),
+                },
+            ];
+        }
         const prompts = [
             {
                 prompt: Common.selectPythonInterpreter,
@@ -183,6 +217,32 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
         }
         return prompts;
     }
+
+    private async diagnoseDefaultShell(resource: Resource): Promise<IDiagnostic[]> {
+        if (getOSType() !== OSType.Windows) {
+            return [];
+        }
+        const executionFactory = this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory);
+        const executionService = await executionFactory.create({ resource });
+        try {
+            await executionService.getExecutablePath({ throwOnError: true });
+        } catch (ex) {
+            if ((ex as Error).message?.includes('4058')) {
+                // ENOENT (-4058) error is thrown by Node when the default shell is invalid.
+                if (await this.isComspecInvalid()) {
+                    traceError('ComSpec is set to an invalid value', process.env.ComSpec);
+                    return [new DefaultShellDiagnostic(DiagnosticCodes.InvalidComspecDiagnostic, resource)];
+                }
+            }
+        }
+        return [];
+    }
+
+    private async isComspecInvalid() {
+        const comSpec = process.env.ComSpec ?? '';
+        const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
+        return fs.fileExists(comSpec);
+    }
 }
 
 function getOnCloseHandler(diagnostic: IDiagnostic): IDiagnosticMessageOnCloseHandler | undefined {

src/client/application/diagnostics/constants.ts

@@ -12,6 +12,7 @@ export enum DiagnosticCodes {
     InvalidPythonPathInDebuggerLaunchDiagnostic = 'InvalidPythonPathInDebuggerLaunchDiagnostic',
     EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic',
     InvalidPythonInterpreterDiagnostic = 'InvalidPythonInterpreterDiagnostic',
+    InvalidComspecDiagnostic = 'InvalidComspecDiagnostic',
     LSNotSupportedDiagnostic = 'LSNotSupportedDiagnostic',
     PythonPathDeprecatedDiagnostic = 'PythonPathDeprecatedDiagnostic',
     JustMyCodeDiagnostic = 'JustMyCodeDiagnostic',

src/client/common/process/pythonEnvironment.ts

@@ -47,7 +47,7 @@ class PythonEnvironment implements IPythonEnvironment {
         return this.cachedInterpreterInformation;
     }
 
-    public async getExecutablePath(): Promise<string | undefined> {
+    public async getExecutablePath(options?: { throwOnError?: boolean }): Promise<string | undefined> {
         // If we've passed the python file, then return the file.
         // This is because on mac if using the interpreter /usr/bin/python2.7 we can get a different value for the path
         if (await this.deps.isValidExecutable(this.pythonPath)) {
@@ -59,7 +59,7 @@ class PythonEnvironment implements IPythonEnvironment {
             return result;
         }
         const python = this.getExecutionInfo();
-        const promise = getExecutablePath(python, this.deps.shellExec);
+        const promise = getExecutablePath(python, this.deps.shellExec, options);
         cachedExecutablePath.set(this.pythonPath, promise);
         return promise;
     }

src/client/common/process/pythonExecutionFactory.ts

@@ -145,7 +145,7 @@ function createPythonService(procService: IProcessService, env: IPythonEnvironme
     const procs = createPythonProcessService(procService, env);
     return {
         getInterpreterInformation: () => env.getInterpreterInformation(),
-        getExecutablePath: () => env.getExecutablePath(),
+        getExecutablePath: (options?: { throwOnError?: boolean }) => env.getExecutablePath(options),
         isModuleInstalled: (m) => env.isModuleInstalled(m),
         getModuleVersion: (m) => env.getModuleVersion(m),
         getExecutionInfo: (a) => env.getExecutionInfo(a),

src/client/common/process/types.ts

@@ -84,7 +84,7 @@ export const IPythonExecutionService = Symbol('IPythonExecutionService');
 
 export interface IPythonExecutionService {
     getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
-    getExecutablePath(): Promise<string | undefined>;
+    getExecutablePath(options?: { throwOnError?: boolean }): Promise<string | undefined>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
     getModuleVersion(moduleName: string): Promise<string | undefined>;
     getExecutionInfo(pythonArgs?: string[]): PythonExecInfo;
@@ -100,7 +100,7 @@ export interface IPythonExecutionService {
 export interface IPythonEnvironment {
     getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
     getExecutionObservableInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
-    getExecutablePath(): Promise<string | undefined>;
+    getExecutablePath(options?: { throwOnError?: boolean }): Promise<string | undefined>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
     getModuleVersion(moduleName: string): Promise<string | undefined>;
     getExecutionInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;

src/client/common/utils/localize.ts

@@ -49,6 +49,7 @@ export namespace Diagnostics {
 
 export namespace Common {
     export const allow = l10n.t('Allow');
+    export const instructions = l10n.t('Instructions');
     export const close = l10n.t('Close');
     export const bannerLabelYes = l10n.t('Yes');
     export const bannerLabelNo = l10n.t('No');

src/client/pythonEnvironments/info/executable.ts

@@ -17,7 +17,7 @@ import { copyPythonExecInfo, PythonExecInfo } from '../exec';
 export async function getExecutablePath(
     python: PythonExecInfo,
     shellExec: ShellExecFunc,
-    timeout?: number,
+    options?: { throwOnError?: boolean },
 ): Promise<string | undefined> {
     try {
         const [args, parse] = getExecutable();
@@ -28,14 +28,17 @@ export async function getExecutablePath(
             (p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
             '',
         );
-        const result = await shellExec(quoted, { timeout: timeout ?? 15000 });
+        const result = await shellExec(quoted, { timeout: 15000 });
         const executable = parse(result.stdout.trim());
         if (executable === '') {
             throw new Error(`${quoted} resulted in empty stdout`);
         }
         return executable;
     } catch (ex) {
         traceError(ex);
+        if (options?.throwOnError) {
+            throw ex;
+        }
         return undefined;
     }
 }