From c2bd11fa9ee66dcb9c985e4b5d771e04a2a8523a Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Mon, 26 Oct 2020 16:06:55 -0700 Subject: [PATCH] Add error logging when fetching account provider accounts (#13082) * Add error logging when fetching account provider accounts * fix test * fix tests #2 --- .../accounts/common/accountViewModel.ts | 38 +++++++++++-------- .../test/common/accountViewModel.test.ts | 5 ++- .../browser/accountDialogController.test.ts | 3 +- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/sql/platform/accounts/common/accountViewModel.ts b/src/sql/platform/accounts/common/accountViewModel.ts index 8633e795ec..96375be584 100644 --- a/src/sql/platform/accounts/common/accountViewModel.ts +++ b/src/sql/platform/accounts/common/accountViewModel.ts @@ -8,6 +8,7 @@ import { Event, Emitter } from 'vs/base/common/event'; import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces'; import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes'; import { coalesce } from 'vs/base/common/arrays'; +import { ILogService } from 'vs/platform/log/common/log'; /** * View model for account dialog @@ -23,7 +24,7 @@ export class AccountViewModel { private _updateAccountListEmitter: Emitter; public get updateAccountListEvent(): Event { return this._updateAccountListEmitter.event; } - constructor(@IAccountManagementService private _accountManagementService: IAccountManagementService) { + constructor(@IAccountManagementService private _accountManagementService: IAccountManagementService, @ILogService private _logService: ILogService) { // Create our event emitters this._addProviderEmitter = new Emitter(); this._removeProviderEmitter = new Emitter(); @@ -41,24 +42,29 @@ export class AccountViewModel { * and fires an event after each provider/accounts has been loaded. * */ - public initialize(): Promise { + public async initialize(): Promise { // Load a baseline of the account provider metadata and accounts // 1) Get all the providers from the account management service // 2) For each provider, get the accounts // 3) Build parameters to add a provider and return it - return this._accountManagementService.getAccountProviderMetadata() - .then(providers => { - const promises = providers.map(provider => { - return this._accountManagementService.getAccountsForProvider(provider.id) - .then(accounts => { - addedProvider: provider, - initialAccounts: accounts - }, () => undefined); - }); - return Promise.all(promises).then(accounts => coalesce(accounts)); - }, () => { - /* Swallow failures and just pretend we don't have any providers */ - return []; - }); + try { + const metadata = await this._accountManagementService.getAccountProviderMetadata(); + const accounts = await Promise.all(metadata.map(async providerMetadata => { + try { + const accounts = await this._accountManagementService.getAccountsForProvider(providerMetadata.id); + return { + addedProvider: providerMetadata, + initialAccounts: accounts + }; + } catch (err) { + this._logService.warn(`Error getting accounts for provider ${providerMetadata.id} : ${err}`); + return undefined; + } + })); + return coalesce(accounts); + } catch (err) { + this._logService.warn(`Error getting account provider metadata : ${err}`); + return []; + } } } diff --git a/src/sql/platform/accounts/test/common/accountViewModel.test.ts b/src/sql/platform/accounts/test/common/accountViewModel.test.ts index 33f8c8faa7..97703392ae 100644 --- a/src/sql/platform/accounts/test/common/accountViewModel.test.ts +++ b/src/sql/platform/accounts/test/common/accountViewModel.test.ts @@ -11,6 +11,7 @@ import { AccountViewModel } from 'sql/platform/accounts/common/accountViewModel' import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes'; import { TestAccountManagementService } from 'sql/platform/accounts/test/common/testAccountManagementService'; import { EventVerifierSingle } from 'sql/base/test/common/event'; +import { NullLogService } from 'vs/platform/log/common/log'; // SUITE STATE ///////////////////////////////////////////////////////////// let mockAddProviderEmitter: Emitter; @@ -63,7 +64,7 @@ suite('Account Management Dialog ViewModel Tests', () => { test('Construction - Events are properly defined', () => { // If: I create an account viewmodel let mockAccountManagementService = getMockAccountManagementService(false, false); - let vm = new AccountViewModel(mockAccountManagementService.object); + let vm = new AccountViewModel(mockAccountManagementService.object, new NullLogService()); // Then: // ... All the events for the view models should be properly initialized @@ -195,7 +196,7 @@ function getViewModel( evRemove: EventVerifierSingle, evUpdate: EventVerifierSingle ): AccountViewModel { - let vm = new AccountViewModel(ams); + let vm = new AccountViewModel(ams, new NullLogService()); vm.addProviderEvent(evAdd.eventHandler); vm.removeProviderEvent(evRemove.eventHandler); vm.updateAccountListEvent(evUpdate.eventHandler); diff --git a/src/sql/workbench/contrib/accounts/test/browser/accountDialogController.test.ts b/src/sql/workbench/contrib/accounts/test/browser/accountDialogController.test.ts index 6e5448f5fc..b9e5d58107 100644 --- a/src/sql/workbench/contrib/accounts/test/browser/accountDialogController.test.ts +++ b/src/sql/workbench/contrib/accounts/test/browser/accountDialogController.test.ts @@ -14,6 +14,7 @@ import { TestErrorMessageService } from 'sql/platform/errorMessage/test/common/t import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService'; import { AccountListRenderer } from 'sql/workbench/services/accountManagement/browser/accountListRenderer'; import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; +import { NullLogService } from 'vs/platform/log/common/log'; // TESTS /////////////////////////////////////////////////////////////////// suite('Account Management Dialog Controller Tests', () => { @@ -70,7 +71,7 @@ suite('Account Management Dialog Controller Tests', () => { function createInstantiationService(addAccountFailureEmitter?: Emitter): InstantiationService { // Create a mock account dialog view model - let accountViewModel = new AccountViewModel(new TestAccountManagementService()); + let accountViewModel = new AccountViewModel(new TestAccountManagementService(), new NullLogService()); let mockAccountViewModel = TypeMoq.Mock.ofInstance(accountViewModel); let mockEvent = new Emitter(); mockAccountViewModel.setup(x => x.addProviderEvent).returns(() => mockEvent.event);