Apply API recommendations for Create Env API (microsoft#21804)

Closes microsoft#21090

src/client/pythonEnvironments/creation/createEnvironment.ts

@@ -33,14 +33,12 @@ function fireStartedEvent(options?: CreateEnvironmentOptions): void {
 }
 
 function fireExitedEvent(result?: CreateEnvironmentResult, options?: CreateEnvironmentOptions, error?: Error): void {
-    onCreateEnvironmentExitedEvent.fire({
-        options,
-        workspaceFolder: result?.workspaceFolder,
-        path: result?.path,
-        action: result?.action,
-        error: error || result?.error,
-    });
     startedEventCount -= 1;
+    if (result) {
+        onCreateEnvironmentExitedEvent.fire({ options, ...result });
+    } else if (error) {
+        onCreateEnvironmentExitedEvent.fire({ options, error });
+    }
 }
 
 export function getCreationEvents(): {
@@ -195,5 +193,8 @@ export async function handleCreateEnvironmentCommand(
         }
     }
 
-    return result;
+    if (result) {
+        return Object.freeze(result);
+    }
+    return undefined;
 }

src/client/pythonEnvironments/creation/proposed.createEnvApis.ts

@@ -40,40 +40,83 @@ export interface EnvironmentWillCreateEvent {
     /**
      * Options used to create a Python environment.
      */
-    options: CreateEnvironmentOptions | undefined;
+    readonly options: CreateEnvironmentOptions | undefined;
 }
 
+export type CreateEnvironmentResult =
+    | {
+          /**
+           * Workspace folder associated with the environment.
+           */
+          readonly workspaceFolder?: WorkspaceFolder;
+
+          /**
+           * Path to the executable python in the environment
+           */
+          readonly path: string;
+
+          /**
+           * User action that resulted in exit from the create environment flow.
+           */
+          readonly action?: CreateEnvironmentUserActions;
+
+          /**
+           * Error if any occurred during environment creation.
+           */
+          readonly error?: Error;
+      }
+    | {
+          /**
+           * Workspace folder associated with the environment.
+           */
+          readonly workspaceFolder?: WorkspaceFolder;
+
+          /**
+           * Path to the executable python in the environment
+           */
+          readonly path?: string;
+
+          /**
+           * User action that resulted in exit from the create environment flow.
+           */
+          readonly action: CreateEnvironmentUserActions;
+
+          /**
+           * Error if any occurred during environment creation.
+           */
+          readonly error?: Error;
+      }
+    | {
+          /**
+           * Workspace folder associated with the environment.
+           */
+          readonly workspaceFolder?: WorkspaceFolder;
+
+          /**
+           * Path to the executable python in the environment
+           */
+          readonly path?: string;
+
+          /**
+           * User action that resulted in exit from the create environment flow.
+           */
+          readonly action?: CreateEnvironmentUserActions;
+
+          /**
+           * Error if any occurred during environment creation.
+           */
+          readonly error: Error;
+      };
+
 /**
  * Params passed on `onDidCreateEnvironment` event handler.
  */
-export interface EnvironmentDidCreateEvent extends CreateEnvironmentResult {
+export type EnvironmentDidCreateEvent = CreateEnvironmentResult & {
     /**
      * Options used to create the Python environment.
      */
-    options: CreateEnvironmentOptions | undefined;
-}
-
-export interface CreateEnvironmentResult {
-    /**
-     * Workspace folder associated with the environment.
-     */
-    workspaceFolder: WorkspaceFolder | undefined;
-
-    /**
-     * Path to the executable python in the environment
-     */
-    path: string | undefined;
-
-    /**
-     * User action that resulted in exit from the create environment flow.
-     */
-    action: CreateEnvironmentUserActions | undefined;
-
-    /**
-     * Error if any occurred during environment creation.
-     */
-    error: Error | undefined;
-}
+    readonly options: CreateEnvironmentOptions | undefined;
+};
 
 /**
  * Extensions that want to contribute their own environment creation can do that by registering an object
@@ -120,14 +163,14 @@ export interface ProposedCreateEnvironmentAPI {
      * provider (including internal providers). This will also receive any options passed in
      * or defaults used to create environment.
      */
-    onWillCreateEnvironment: Event<EnvironmentWillCreateEvent>;
+    readonly onWillCreateEnvironment: Event<EnvironmentWillCreateEvent>;
 
     /**
      * This API can be used to detect when the environment provider exits for any registered
      * provider (including internal providers). This will also receive created environment path,
      * any errors, or user actions taken from the provider.
      */
-    onDidCreateEnvironment: Event<EnvironmentDidCreateEvent>;
+    readonly onDidCreateEnvironment: Event<EnvironmentDidCreateEvent>;
 
     /**
      * This API will show a QuickPick to select an environment provider from available list of

src/client/pythonEnvironments/creation/provider/condaCreationProvider.ts

@@ -4,7 +4,7 @@
 import { CancellationToken, ProgressLocation, WorkspaceFolder } from 'vscode';
 import * as path from 'path';
 import { Commands, PVSC_EXTENSION_ID } from '../../../common/constants';
-import { traceError, traceLog } from '../../../logging';
+import { traceError, traceInfo, traceLog } from '../../../logging';
 import { CreateEnvironmentProgress } from '../types';
 import { pickWorkspaceFolder } from '../common/workspaceSelection';
 import { execObservable } from '../../../common/process/rawProcessApis';
@@ -77,7 +77,7 @@ async function createCondaEnv(
     args: string[],
     progress: CreateEnvironmentProgress,
     token?: CancellationToken,
-): Promise<string | undefined> {
+): Promise<string> {
     progress.report({
         message: CreateEnv.Conda.creating,
     });
@@ -174,6 +174,7 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
                 traceError('Workspace was not selected or found for creating conda environment.');
                 return MultiStepAction.Cancel;
             }
+            traceInfo(`Selected workspace ${workspace.uri.fsPath} for creating conda environment.`);
             return MultiStepAction.Continue;
         },
         undefined,
@@ -196,6 +197,7 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
                 traceError('Python version was not selected for creating conda environment.');
                 return MultiStepAction.Cancel;
             }
+            traceInfo(`Selected Python version ${version} for creating conda environment.`);
             return MultiStepAction.Continue;
         },
         undefined,
@@ -217,8 +219,6 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
             progress: CreateEnvironmentProgress,
             token: CancellationToken,
         ): Promise<CreateEnvironmentResult | undefined> => {
-            let hasError = false;
-
             progress.report({
                 message: CreateEnv.statusStarting,
             });
@@ -237,17 +237,20 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
                         progress,
                         token,
                     );
+
+                    if (envPath) {
+                        return { path: envPath, workspaceFolder: workspace };
+                    }
+
+                    throw new Error('Failed to create conda environment. See Output > Python for more info.');
+                } else {
+                    throw new Error('A workspace is needed to create conda environment');
                 }
             } catch (ex) {
                 traceError(ex);
-                hasError = true;
+                showErrorMessageWithLogs(CreateEnv.Conda.errorCreatingEnvironment);
                 throw ex;
-            } finally {
-                if (hasError) {
-                    showErrorMessageWithLogs(CreateEnv.Conda.errorCreatingEnvironment);
-                }
             }
-            return { path: envPath, workspaceFolder: workspace, action: undefined, error: undefined };
         },
     );
 }

src/client/pythonEnvironments/creation/provider/venvCreationProvider.ts

@@ -8,7 +8,7 @@ import { createVenvScript } from '../../../common/process/internal/scripts';
 import { execObservable } from '../../../common/process/rawProcessApis';
 import { createDeferred } from '../../../common/utils/async';
 import { Common, CreateEnv } from '../../../common/utils/localize';
-import { traceError, traceLog, traceVerbose } from '../../../logging';
+import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging';
 import { CreateEnvironmentProgress } from '../types';
 import { pickWorkspaceFolder } from '../common/workspaceSelection';
 import { IInterpreterQuickPick } from '../../../interpreter/configuration/types';
@@ -144,6 +144,7 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
                     traceError('Workspace was not selected or found for creating virtual environment.');
                     return MultiStepAction.Cancel;
                 }
+                traceInfo(`Selected workspace ${workspace.uri.fsPath} for creating virtual environment.`);
                 return MultiStepAction.Continue;
             },
             undefined,
@@ -183,6 +184,7 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
                     traceError('Virtual env creation requires an interpreter.');
                     return MultiStepAction.Cancel;
                 }
+                traceInfo(`Selected interpreter ${interpreter} for creating virtual environment.`);
                 return MultiStepAction.Continue;
             },
             undefined,
@@ -237,8 +239,6 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
                 progress: CreateEnvironmentProgress,
                 token: CancellationToken,
             ): Promise<CreateEnvironmentResult | undefined> => {
-                let hasError = false;
-
                 progress.report({
                     message: CreateEnv.statusStarting,
                 });
@@ -247,18 +247,19 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
                 try {
                     if (interpreter && workspace) {
                         envPath = await createVenv(workspace, interpreter, args, progress, token);
+                        if (envPath) {
+                            return { path: envPath, workspaceFolder: workspace };
+                        }
+                        throw new Error('Failed to create virtual environment. See Output > Python for more info.');
                     }
+                    throw new Error(
+                        'Failed to create virtual environment. Either interpreter or workspace is undefined.',
+                    );
                 } catch (ex) {
                     traceError(ex);
-                    hasError = true;
+                    showErrorMessageWithLogs(CreateEnv.Venv.errorCreatingEnvironment);
                     throw ex;
-                } finally {
-                    if (hasError) {
-                        showErrorMessageWithLogs(CreateEnv.Venv.errorCreatingEnvironment);
-                    }
                 }
-
-                return { path: envPath, workspaceFolder: workspace, action: undefined, error: undefined };
             },
         );
     }

src/test/pythonEnvironments/creation/provider/condaCreationProvider.unit.test.ts

@@ -134,8 +134,6 @@ suite('Conda Creation provider tests', () => {
         assert.deepStrictEqual(await promise, {
             path: 'new_environment',
             workspaceFolder: workspace1,
-            action: undefined,
-            error: undefined,
         });
         assert.isTrue(showErrorMessageWithLogsStub.notCalled);
     });

src/test/pythonEnvironments/creation/provider/venvCreationProvider.unit.test.ts

@@ -158,8 +158,6 @@ suite('venv Creation provider tests', () => {
         assert.deepStrictEqual(actual, {
             path: 'new_environment',
             workspaceFolder: workspace1,
-            action: undefined,
-            error: undefined,
         });
         interpreterQuickPick.verifyAll();
         progressMock.verifyAll();