Add error logging when fetching account provider accounts (#13082)

* Add error logging when fetching account provider accounts

* fix test

* fix tests #2
This commit is contained in:
Charles Gagnon
2020-10-26 16:06:55 -07:00
committed by GitHub
parent 791dee1457
commit c2bd11fa9e
3 changed files with 27 additions and 19 deletions

View File

@@ -8,6 +8,7 @@ import { Event, Emitter } from 'vs/base/common/event';
import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces'; import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces';
import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes'; import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes';
import { coalesce } from 'vs/base/common/arrays'; import { coalesce } from 'vs/base/common/arrays';
import { ILogService } from 'vs/platform/log/common/log';
/** /**
* View model for account dialog * View model for account dialog
@@ -23,7 +24,7 @@ export class AccountViewModel {
private _updateAccountListEmitter: Emitter<UpdateAccountListEventParams>; private _updateAccountListEmitter: Emitter<UpdateAccountListEventParams>;
public get updateAccountListEvent(): Event<UpdateAccountListEventParams> { return this._updateAccountListEmitter.event; } public get updateAccountListEvent(): Event<UpdateAccountListEventParams> { return this._updateAccountListEmitter.event; }
constructor(@IAccountManagementService private _accountManagementService: IAccountManagementService) { constructor(@IAccountManagementService private _accountManagementService: IAccountManagementService, @ILogService private _logService: ILogService) {
// Create our event emitters // Create our event emitters
this._addProviderEmitter = new Emitter<AccountProviderAddedEventParams>(); this._addProviderEmitter = new Emitter<AccountProviderAddedEventParams>();
this._removeProviderEmitter = new Emitter<azdata.AccountProviderMetadata>(); this._removeProviderEmitter = new Emitter<azdata.AccountProviderMetadata>();
@@ -41,24 +42,29 @@ export class AccountViewModel {
* and fires an event after each provider/accounts has been loaded. * and fires an event after each provider/accounts has been loaded.
* *
*/ */
public initialize(): Promise<AccountProviderAddedEventParams[]> { public async initialize(): Promise<AccountProviderAddedEventParams[]> {
// Load a baseline of the account provider metadata and accounts // Load a baseline of the account provider metadata and accounts
// 1) Get all the providers from the account management service // 1) Get all the providers from the account management service
// 2) For each provider, get the accounts // 2) For each provider, get the accounts
// 3) Build parameters to add a provider and return it // 3) Build parameters to add a provider and return it
return this._accountManagementService.getAccountProviderMetadata() try {
.then(providers => { const metadata = await this._accountManagementService.getAccountProviderMetadata();
const promises = providers.map(provider => { const accounts = await Promise.all(metadata.map(async providerMetadata => {
return this._accountManagementService.getAccountsForProvider(provider.id) try {
.then(accounts => <AccountProviderAddedEventParams>{ const accounts = await this._accountManagementService.getAccountsForProvider(providerMetadata.id);
addedProvider: provider, return <AccountProviderAddedEventParams>{
addedProvider: providerMetadata,
initialAccounts: accounts initialAccounts: accounts
}, () => undefined); };
}); } catch (err) {
return Promise.all(promises).then(accounts => coalesce(accounts)); this._logService.warn(`Error getting accounts for provider ${providerMetadata.id} : ${err}`);
}, () => { return undefined;
/* Swallow failures and just pretend we don't have any providers */ }
}));
return coalesce(accounts);
} catch (err) {
this._logService.warn(`Error getting account provider metadata : ${err}`);
return []; return [];
}); }
} }
} }

View File

@@ -11,6 +11,7 @@ import { AccountViewModel } from 'sql/platform/accounts/common/accountViewModel'
import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes'; import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes';
import { TestAccountManagementService } from 'sql/platform/accounts/test/common/testAccountManagementService'; import { TestAccountManagementService } from 'sql/platform/accounts/test/common/testAccountManagementService';
import { EventVerifierSingle } from 'sql/base/test/common/event'; import { EventVerifierSingle } from 'sql/base/test/common/event';
import { NullLogService } from 'vs/platform/log/common/log';
// SUITE STATE ///////////////////////////////////////////////////////////// // SUITE STATE /////////////////////////////////////////////////////////////
let mockAddProviderEmitter: Emitter<AccountProviderAddedEventParams>; let mockAddProviderEmitter: Emitter<AccountProviderAddedEventParams>;
@@ -63,7 +64,7 @@ suite('Account Management Dialog ViewModel Tests', () => {
test('Construction - Events are properly defined', () => { test('Construction - Events are properly defined', () => {
// If: I create an account viewmodel // If: I create an account viewmodel
let mockAccountManagementService = getMockAccountManagementService(false, false); let mockAccountManagementService = getMockAccountManagementService(false, false);
let vm = new AccountViewModel(mockAccountManagementService.object); let vm = new AccountViewModel(mockAccountManagementService.object, new NullLogService());
// Then: // Then:
// ... All the events for the view models should be properly initialized // ... All the events for the view models should be properly initialized
@@ -195,7 +196,7 @@ function getViewModel(
evRemove: EventVerifierSingle<azdata.AccountProviderMetadata>, evRemove: EventVerifierSingle<azdata.AccountProviderMetadata>,
evUpdate: EventVerifierSingle<UpdateAccountListEventParams> evUpdate: EventVerifierSingle<UpdateAccountListEventParams>
): AccountViewModel { ): AccountViewModel {
let vm = new AccountViewModel(ams); let vm = new AccountViewModel(ams, new NullLogService());
vm.addProviderEvent(evAdd.eventHandler); vm.addProviderEvent(evAdd.eventHandler);
vm.removeProviderEvent(evRemove.eventHandler); vm.removeProviderEvent(evRemove.eventHandler);
vm.updateAccountListEvent(evUpdate.eventHandler); vm.updateAccountListEvent(evUpdate.eventHandler);

View File

@@ -14,6 +14,7 @@ import { TestErrorMessageService } from 'sql/platform/errorMessage/test/common/t
import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService'; import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
import { AccountListRenderer } from 'sql/workbench/services/accountManagement/browser/accountListRenderer'; import { AccountListRenderer } from 'sql/workbench/services/accountManagement/browser/accountListRenderer';
import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
import { NullLogService } from 'vs/platform/log/common/log';
// TESTS /////////////////////////////////////////////////////////////////// // TESTS ///////////////////////////////////////////////////////////////////
suite('Account Management Dialog Controller Tests', () => { suite('Account Management Dialog Controller Tests', () => {
@@ -70,7 +71,7 @@ suite('Account Management Dialog Controller Tests', () => {
function createInstantiationService(addAccountFailureEmitter?: Emitter<string>): InstantiationService { function createInstantiationService(addAccountFailureEmitter?: Emitter<string>): InstantiationService {
// Create a mock account dialog view model // 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 mockAccountViewModel = TypeMoq.Mock.ofInstance(accountViewModel);
let mockEvent = new Emitter<any>(); let mockEvent = new Emitter<any>();
mockAccountViewModel.setup(x => x.addProviderEvent).returns(() => mockEvent.event); mockAccountViewModel.setup(x => x.addProviderEvent).returns(() => mockEvent.event);