From f51fe75397e0ec9fedf79a93f57331b33085aba2 Mon Sep 17 00:00:00 2001
From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com>
Date: Thu, 9 Mar 2023 14:34:39 -0800
Subject: [PATCH] Null & Error handling in Azure core (#22259)

---
 .../src/account-provider/auths/azureAuth.ts   | 19 +++++++++++--------
 .../platform/accounts/common/accountStore.ts  | 14 ++++++++------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/extensions/azurecore/src/account-provider/auths/azureAuth.ts b/extensions/azurecore/src/account-provider/auths/azureAuth.ts
index 9a08c45929..34d703a4bb 100644
--- a/extensions/azurecore/src/account-provider/auths/azureAuth.ts
+++ b/extensions/azurecore/src/account-provider/auths/azureAuth.ts
@@ -139,11 +139,8 @@ export abstract class AzureAuth implements vscode.Disposable {
 			if (ex instanceof AzureAuthError) {
 				if (loginComplete) {
 					loginComplete.reject(ex);
-					Logger.error(ex);
-				} else {
-					void vscode.window.showErrorMessage(ex.message);
-					Logger.error(ex.originalMessageAndException);
 				}
+				Logger.error(ex.originalMessageAndException);
 			} else {
 				const message = ex.errorMessage || ex.message;
 				if (message) {
@@ -155,10 +152,11 @@ export abstract class AzureAuth implements vscode.Disposable {
 					};
 				}
 				Logger.error(ex);
-
 			}
 			return {
-				canceled: false
+				canceled: false,
+				errorCode: ex.errorCode,
+				errorMessage: ex.errorMessage || ex.message
 			};
 		}
 	}
@@ -476,7 +474,12 @@ export abstract class AzureAuth implements vscode.Disposable {
 			Logger.verbose('Fetching tenants with uri {0}', tenantUri);
 			let tenantList: string[] = [];
 			const tenantResponse = await this.makeGetRequest(tenantUri, token);
-			const tenants: Tenant[] = tenantResponse.data.value.map((tenantInfo: TenantResponse) => {
+			const data = tenantResponse.data;
+			if (data.error) {
+				Logger.error(`Error fetching tenants :${data.error.code} - ${data.error.message}`);
+				throw new Error(`${data.error.code} - ${data.error.message}`);
+			}
+			const tenants: Tenant[] = data.value.map((tenantInfo: TenantResponse) => {
 				if (tenantInfo.displayName) {
 					tenantList.push(tenantInfo.displayName);
 				} else {
@@ -501,7 +504,7 @@ export abstract class AzureAuth implements vscode.Disposable {
 			return tenants;
 		} catch (ex) {
 			Logger.error(`Error fetching tenants :${ex}`);
-			throw new Error('Error retrieving tenant information');
+			throw ex;
 		}
 	}
 
diff --git a/src/sql/platform/accounts/common/accountStore.ts b/src/sql/platform/accounts/common/accountStore.ts
index 4b1cc37c8d..fc4e6e1715 100644
--- a/src/sql/platform/accounts/common/accountStore.ts
+++ b/src/sql/platform/accounts/common/accountStore.ts
@@ -28,7 +28,7 @@ export default class AccountStore implements IAccountStore {
 			return this.readFromMemento()
 				.then(accounts => {
 					// Determine if account exists and proceed accordingly
-					const match = accounts.findIndex(account => AccountStore.findAccountByKey(account.key, newAccount.key));
+					const match = accounts.findIndex(account => AccountStore.isSameAccountKey(account.key, newAccount.key));
 					return match < 0
 						? this.addToAccountList(accounts, newAccount)
 						: this.updateAccountList(accounts, newAccount.key, matchAccount => AccountStore.mergeAccounts(newAccount, matchAccount));
@@ -100,9 +100,11 @@ export default class AccountStore implements IAccountStore {
 	}
 
 	// PRIVATE METHODS /////////////////////////////////////////////////////
-	private static findAccountByKey(key1: azdata.AccountKey, key2: azdata.AccountKey): boolean {
+	private static isSameAccountKey(key1: azdata.AccountKey | undefined, key2: azdata.AccountKey | undefined): boolean {
 		// Provider ID and Account ID must match
-		return key1.providerId === key2.providerId && key1.accountId === key2.accountId;
+		return key1 && key2
+			? key1.providerId === key2.providerId && key1.accountId === key2.accountId
+			: false;
 	}
 
 	private static mergeAccounts(source: azdata.Account, target: azdata.Account): void {
@@ -135,7 +137,7 @@ export default class AccountStore implements IAccountStore {
 
 	private addToAccountList(accounts: azdata.Account[], accountToAdd: azdata.Account): AccountListOperationResult {
 		// Check if the entry already exists
-		const match = accounts.findIndex(account => AccountStore.findAccountByKey(account.key, accountToAdd.key));
+		const match = accounts.findIndex(account => AccountStore.isSameAccountKey(account.key, accountToAdd.key));
 		if (match >= 0) {
 			// Account already exists, we won't do anything
 			return {
@@ -160,7 +162,7 @@ export default class AccountStore implements IAccountStore {
 
 	private removeFromAccountList(accounts: azdata.Account[], accountToRemove: azdata.AccountKey): AccountListOperationResult {
 		// Check if the entry exists
-		const match = accounts.findIndex(account => AccountStore.findAccountByKey(account.key, accountToRemove));
+		const match = accounts.findIndex(account => AccountStore.isSameAccountKey(account.key, accountToRemove));
 		if (match >= 0) {
 			// Account exists, remove it from the account list
 			accounts.splice(match, 1);
@@ -177,7 +179,7 @@ export default class AccountStore implements IAccountStore {
 
 	private updateAccountList(accounts: azdata.Account[], accountToUpdate: azdata.AccountKey, updateOperation: (account: azdata.Account) => void): AccountListOperationResult {
 		// Check if the entry exists
-		const match = accounts.findIndex(account => AccountStore.findAccountByKey(account.key, accountToUpdate));
+		const match = accounts.findIndex(account => AccountStore.isSameAccountKey(account.key, accountToUpdate));
 		if (match < 0) {
 			// Account doesn't exist, we won't do anything
 			return {