Fix deployment wizard to not close when cancelling out of password prompt (#14083)

extensions/arc/src/common/cacheManager.ts

@@ -1,83 +0,0 @@
-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the Source EULA. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-
-import { Deferred } from './promise';
-
-const enum Status {
-	notStarted,
-	inProgress,
-	done
-}
-
-interface State<T> {
-	entry?: T,
-	error?: Error,
-	status: Status,
-	id: number,
-	pendingOperation: Deferred<void>
-}
-
-/**
- * An implementation of Cache Manager which ensures that only one call to populate cache miss is pending at a given time.
- * All remaining calls for retrieval are awaited until the one in progress finishes and then all awaited calls are resolved with the value
- * from the cache.
- */
-export class CacheManager<K, T> {
-	private _cache = new Map<K, State<T>>();
-	private _id = 0;
-
-	public async getCacheEntry(key: K, retrieveEntry: (key: K) => Promise<T>): Promise<T> {
-		const cacheHit: State<T> | undefined = this._cache.get(key);
-		// each branch either throws or returns the password.
-		if (cacheHit === undefined) {
-			// populate a new state entry and add it to the cache
-			const state: State<T> = {
-				status: Status.notStarted,
-				id: this._id++,
-				pendingOperation: new Deferred<void>()
-			};
-			this._cache.set(key, state);
-			// now that we have the state entry initialized, retry to fetch the cacheEntry
-			let returnValue: T = await this.getCacheEntry(key, retrieveEntry);
-			await state.pendingOperation;
-			return returnValue!;
-		} else {
-			switch (cacheHit.status) {
-				case Status.notStarted: {
-					cacheHit.status = Status.inProgress;
-					// retrieve and populate the missed cache hit.
-					try {
-						cacheHit.entry = await retrieveEntry(key);
-					} catch (error) {
-						cacheHit.error = error;
-					} finally {
-						cacheHit.status = Status.done;
-						// we do not reject here even in error case because we do not want our awaits on pendingOperation to throw
-						// We track our own error state and when all done we throw if an error had happened. This results
-						// in the rejection of the promised returned by this method.
-						cacheHit.pendingOperation.resolve();
-					}
-					return await this.getCacheEntry(key, retrieveEntry);
-				}
-
-				case Status.inProgress: {
-					await cacheHit.pendingOperation;
-					return await this.getCacheEntry(key, retrieveEntry);
-				}
-
-				case Status.done: {
-					if (cacheHit.error !== undefined) {
-						await cacheHit.pendingOperation;
-						throw cacheHit.error;
-					}
-					else {
-						await cacheHit.pendingOperation;
-						return cacheHit.entry!;
-					}
-				}
-			}
-		}
-	}
-}

extensions/arc/src/localizedConstants.ts

@@ -218,7 +218,7 @@ export function couldNotFindAzureResource(name: string): string { return localiz
 export function passwordResetFailed(error: any): string { return localize('arc.passwordResetFailed', "Failed to reset password. {0}", getErrorMessage(error)); }
 export function errorConnectingToController(error: any): string { return localize('arc.errorConnectingToController', "Error connecting to controller. {0}", getErrorMessage(error, true)); }
 export function passwordAcquisitionFailed(error: any): string { return localize('arc.passwordAcquisitionFailed', "Failed to acquire password. {0}", getErrorMessage(error)); }
-export const loginFailed = localize('arc.loginFailed', "Error logging into controller, try again.");
+export const loginFailed = localize('arc.loginFailed', "Error logging into controller - wrong username or password");
 export function errorVerifyingPassword(error: any): string { return localize('arc.errorVerifyingPassword', "Error encountered while verifying password. {0}", getErrorMessage(error)); }
 export const noControllersConnected = localize('noControllersConnected', "No Azure Arc controllers are currently connected. Please run the command: 'Connect to Existing Azure Arc Controller' and then try again");
 export const variableValueFetchForUnsupportedVariable = (variableName: string) => localize('getVariableValue.unknownVariableName', "Attempt to get variable value for unknown variable:{0}", variableName);

extensions/arc/src/providers/arcControllersOptionsSourceProvider.ts

@@ -7,7 +7,6 @@ import * as arc from 'arc';
 import * as azdata from 'azdata';
 import * as rd from 'resource-deployment';
 import { getControllerPassword, getRegisteredDataControllers, reacquireControllerPassword } from '../common/api';
-import { CacheManager } from '../common/cacheManager';
 import { throwUnless } from '../common/utils';
 import * as loc from '../localizedConstants';
 import { AzureArcTreeDataProvider } from '../ui/tree/azureArcTreeDataProvider';
@@ -16,11 +15,10 @@ import { AzureArcTreeDataProvider } from '../ui/tree/azureArcTreeDataProvider';
  * Class that provides options sources for an Arc Data Controller
  */
 export class ArcControllersOptionsSourceProvider implements rd.IOptionsSourceProvider {
-	private _cacheManager = new CacheManager<string, string>();
 	readonly id = 'arc.controllers';
 	constructor(private _treeProvider: AzureArcTreeDataProvider) { }
 
-	async getOptions(): Promise<string[] | azdata.CategoryValue[]> {
+	public async getOptions(): Promise<string[] | azdata.CategoryValue[]> {
 		const controllers = await getRegisteredDataControllers(this._treeProvider);
 		throwUnless(controllers !== undefined && controllers.length !== 0, loc.noControllersConnected);
 		return controllers.map(ci => {
@@ -28,8 +26,7 @@ export class ArcControllersOptionsSourceProvider implements rd.IOptionsSourcePro
 		});
 	}
 
-	private async retrieveVariable(key: string): Promise<string> {
+	public async getVariableValue(variableName: string, controllerLabel: string): Promise<string> {
-		const [variableName, controllerLabel] = JSON.parse(key);
 		const controller = (await getRegisteredDataControllers(this._treeProvider)).find(ci => ci.label === controllerLabel);
 		throwUnless(controller !== undefined, loc.noControllerInfoFound(controllerLabel));
 		switch (variableName) {
@@ -42,12 +39,6 @@ export class ArcControllersOptionsSourceProvider implements rd.IOptionsSourcePro
 		}
 	}
 
-	getVariableValue(variableName: string, controllerLabel: string): Promise<string> {
-		// capture 'this' in an arrow function object
-		const retrieveVariable = (key: string) => this.retrieveVariable(key);
-		return this._cacheManager.getCacheEntry(JSON.stringify([variableName, controllerLabel]), retrieveVariable);
-	}
-
 	private async getPassword(controller: arc.DataController): Promise<string> {
 		let password = await getControllerPassword(this._treeProvider, controller.info);
 		if (!password) {
@@ -57,7 +48,7 @@ export class ArcControllersOptionsSourceProvider implements rd.IOptionsSourcePro
 		return password;
 	}
 
-	getIsPassword(variableName: string): boolean {
+	public getIsPassword(variableName: string): boolean {
 		switch (variableName) {
 			case 'endpoint': return false;
 			case 'username': return false;

extensions/resource-deployment/src/common/utils.ts

@@ -3,6 +3,7 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 import * as path from 'path';
+import { ErrorType, ErrorWithType } from 'resource-deployment';
 import { ToolsInstallPath } from '../constants';
 import { ITool, NoteBookEnvironmentVariablePrefix } from '../interfaces';
 
@@ -12,6 +13,10 @@ export function getErrorMessage(error: any): string {
 		: typeof error === 'string' ? error : `${JSON.stringify(error, undefined, '\t')}`;
 }
 
+export function isUserCancelledError(err: any): boolean {
+	return err instanceof Error && 'type' in err && (<ErrorWithType>err).type === ErrorType.userCancelled;
+}
+
 export function getDateTimeString(): string {
 	return new Date().toISOString().slice(0, 19).replace(/[^0-9]/g, ''); // Take the date time information and only leaving the numbers
 }

extensions/resource-deployment/src/ui/modelViewUtils.ts

@@ -10,7 +10,7 @@ import * as path from 'path';
 import { IOptionsSourceProvider } from 'resource-deployment';
 import * as vscode from 'vscode';
 import * as nls from 'vscode-nls';
-import { getDateTimeString, getErrorMessage, throwUnless } from '../common/utils';
+import { getDateTimeString, getErrorMessage, isUserCancelledError, throwUnless } from '../common/utils';
 import { AzureAccountFieldInfo, AzureLocationsFieldInfo, ComponentCSSStyles, DialogInfoBase, FieldInfo, FieldType, FilePickerFieldInfo, instanceOfDynamicEnablementInfo, IOptionsSource, KubeClusterContextFieldInfo, LabelPosition, NoteBookEnvironmentVariablePrefix, OptionsInfo, OptionsType, PageInfoBase, RowInfo, SectionInfo, TextCSSStyles } from '../interfaces';
 import * as loc from '../localizedConstants';
 import { apiService } from '../services/apiService';
@@ -667,12 +667,16 @@ async function configureOptionsSourceSubfields(context: FieldContext, optionsSou
 			try {
 				return await optionsSourceProvider.getVariableValue!(variableKey, value);
 			} catch (e) {
+				if (!isUserCancelledError(e)) {
+					// User cancelled is a normal scenario so we shouldn't disable anything in that case
+					// so that the user can retry if they want to
 					disableControlButtons(context.container);
 					context.container.message = {
 						text: getErrorMessage(e),
 						description: '',
 						level: azdata.window.MessageLevel.Error
 					};
+				}
 				throw e;
 			}
 		},

extensions/resource-deployment/src/ui/notebookWizard/notebookWizardModel.ts

@@ -12,7 +12,7 @@ import { DeploymentType, NotebookWizardDeploymentProvider, NotebookWizardInfo }
 import { IPlatformService } from '../../services/platformService';
 import { NotebookWizardAutoSummaryPage } from './notebookWizardAutoSummaryPage';
 import { NotebookWizardPage } from './notebookWizardPage';
-import { ErrorType, ErrorWithType } from 'resource-deployment';
+import { isUserCancelledError } from '../../common/utils';
 
 export class NotebookWizardModel extends ResourceTypeModel {
 	private _inputComponents: InputComponents = {};
@@ -68,11 +68,13 @@ export class NotebookWizardModel extends ResourceTypeModel {
 			try {
 				notebook = await this.prepareNotebookAndEnvironment();
 			} catch (e) {
-				const isUserCancelled = e instanceof Error && 'type' in e && (<ErrorWithType>e).type === ErrorType.userCancelled;
+				// If there was a user prompt while preparing the Notebook environment (such as prompting for password) and the user
-				// user cancellation is a normal scenario, we just bail out of the wizard without actually opening the notebook, so rethrow for any other case
+				// cancelled out of that then we shouldn't display an error since that's a normal case but should still keep the Wizard
-				if (!isUserCancelled) {
+				// open so they can make any changes they want and try again without needing to re-enter the information again.
-					throw e;
+				if (isUserCancelledError(e)) {
+					return false;
 				}
+				throw e;
 			}
 			if (notebook) { // open the notebook if it was successfully prepared
 				await this.openNotebook(notebook);