From 4f8ced1f6bc9da67f51324756326c73e794ac87a Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Mon, 2 Dec 2019 15:54:33 -0800 Subject: [PATCH] More dangling promise cleanup (#8518) * More dangling promise cleanup * return void * Function to async * Fix a couple missed promises --- src/sql/workbench/browser/actions.ts | 4 +- .../contrib/tasks/browser/tasksView.ts | 2 +- .../contrib/tasks/common/tasksAction.ts | 4 +- .../browser/accountManagementService.ts | 5 +- .../backup/browser/backupUiService.ts | 40 ++++++------- .../common/capabilitiesServiceImpl.ts | 5 +- .../browser/connectionDialogService.ts | 60 +++++++++---------- .../connectionManagementService.test.ts | 51 ++++++++-------- 8 files changed, 81 insertions(+), 90 deletions(-) diff --git a/src/sql/workbench/browser/actions.ts b/src/sql/workbench/browser/actions.ts index d2c17b4dc8..2a5e8dcd0d 100644 --- a/src/sql/workbench/browser/actions.ts +++ b/src/sql/workbench/browser/actions.ts @@ -84,7 +84,7 @@ export class ConfigureDashboardAction extends Task { }); } - runTask(accessor: ServicesAccessor): Promise { - return accessor.get(IOpenerService).open(URI.parse(ConfigureDashboardAction.configHelpUri)).then(); + async runTask(accessor: ServicesAccessor): Promise { + accessor.get(IOpenerService).open(URI.parse(ConfigureDashboardAction.configHelpUri)); } } diff --git a/src/sql/workbench/contrib/tasks/browser/tasksView.ts b/src/sql/workbench/contrib/tasks/browser/tasksView.ts index f345121c2d..c5da9603be 100644 --- a/src/sql/workbench/contrib/tasks/browser/tasksView.ts +++ b/src/sql/workbench/contrib/tasks/browser/tasksView.ts @@ -97,7 +97,7 @@ export class TaskHistoryView extends Disposable { } private updateTask(task: TaskNode): void { - this._tree.refresh(task); + this._tree.refresh(task).catch(err => errors.onUnexpectedError(err)); } public refreshTree(): void { diff --git a/src/sql/workbench/contrib/tasks/common/tasksAction.ts b/src/sql/workbench/contrib/tasks/common/tasksAction.ts index 2a3898ceb6..15ad481016 100644 --- a/src/sql/workbench/contrib/tasks/common/tasksAction.ts +++ b/src/sql/workbench/contrib/tasks/common/tasksAction.ts @@ -57,12 +57,12 @@ export class ScriptAction extends Action { super(id, label); } - public run(element: TaskNode): Promise { + public async run(element: TaskNode): Promise { if (element instanceof TaskNode) { if (element.script && element.script !== '') { this._queryEditorService.newSqlEditor(element.script); } } - return Promise.resolve(true); + return true; } } diff --git a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts index ae0552f66d..2725bf8c7e 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts @@ -22,6 +22,7 @@ import { IOpenerService } from 'vs/platform/opener/common/opener'; import { URI } from 'vs/base/common/uri'; import { firstIndex } from 'vs/base/common/arrays'; import { values } from 'vs/base/common/collections'; +import { onUnexpectedError } from 'vs/base/common/errors'; export class AccountManagementService implements IAccountManagementService { // CONSTANTS /////////////////////////////////////////////////////////// @@ -314,8 +315,8 @@ export class AccountManagementService implements IAccountManagementService { * Copy the user code to the clipboard and open a browser to the verification URI */ public copyUserCodeAndOpenBrowser(userCode: string, uri: string): void { - this._clipboardService.writeText(userCode); - this._openerService.open(URI.parse(uri)); + this._clipboardService.writeText(userCode).catch(err => onUnexpectedError(err)); + this._openerService.open(URI.parse(uri)).catch(err => onUnexpectedError(err)); } // SERVICE MANAGEMENT METHODS ////////////////////////////////////////// diff --git a/src/sql/workbench/services/backup/browser/backupUiService.ts b/src/sql/workbench/services/backup/browser/backupUiService.ts index ff54132668..6c972702c8 100644 --- a/src/sql/workbench/services/backup/browser/backupUiService.ts +++ b/src/sql/workbench/services/backup/browser/backupUiService.ts @@ -59,7 +59,7 @@ export class BackupUiService implements IBackupUiService { } } - public showBackupDialog(connection: IConnectionProfile): Promise { + public async showBackupDialog(connection: IConnectionProfile): Promise { let self = this; self._connectionUri = ConnectionUtils.generateUri(connection); self._currentProvider = connection.providerName; @@ -79,31 +79,27 @@ export class BackupUiService implements IBackupUiService { } let backupOptions = this.getOptions(this._currentProvider); - return new Promise((resolve) => { - let uri = this._connectionManagementService.getConnectionUri(connection) - + ProviderConnectionInfo.idSeparator - + ConnectionUtils.ConnectionUriBackupIdAttributeName - + ProviderConnectionInfo.nameValueSeparator - + BackupUiService._connectionUniqueId; + let uri = this._connectionManagementService.getConnectionUri(connection) + + ProviderConnectionInfo.idSeparator + + ConnectionUtils.ConnectionUriBackupIdAttributeName + + ProviderConnectionInfo.nameValueSeparator + + BackupUiService._connectionUniqueId; - this._connectionUri = uri; + this._connectionUri = uri; - BackupUiService._connectionUniqueId++; + BackupUiService._connectionUniqueId++; - // Create connection if needed - if (!this._connectionManagementService.isConnected(uri)) { - this._connectionManagementService.connect(connection, uri).then(() => { - this._onShowBackupEvent.fire({ connection: connection, ownerUri: uri }); - }); - } + if (backupOptions) { + (backupDialog as OptionsDialog).open(backupOptions, self._optionValues); + } else { + (backupDialog as BackupDialog).open(connection); + } - if (backupOptions) { - (backupDialog as OptionsDialog).open(backupOptions, self._optionValues); - } else { - (backupDialog as BackupDialog).open(connection); - } - resolve(void 0); - }); + // Create connection if needed + if (!this._connectionManagementService.isConnected(uri)) { + await this._connectionManagementService.connect(connection, uri); + this._onShowBackupEvent.fire({ connection: connection, ownerUri: uri }); + } } public onShowBackupDialog() { diff --git a/src/sql/workbench/services/capabilities/common/capabilitiesServiceImpl.ts b/src/sql/workbench/services/capabilities/common/capabilitiesServiceImpl.ts index 1a79f82ae4..2577803cf7 100644 --- a/src/sql/workbench/services/capabilities/common/capabilitiesServiceImpl.ts +++ b/src/sql/workbench/services/capabilities/common/capabilitiesServiceImpl.ts @@ -18,6 +18,7 @@ import { IConnectionProviderRegistry, Extensions as ConnectionExtensions } from import { ICapabilitiesService, ProviderFeatures, clientCapabilities, ConnectionProviderProperties } from 'sql/platform/capabilities/common/capabilitiesService'; import { find } from 'vs/base/common/arrays'; import { entries } from 'sql/base/common/collections'; +import { onUnexpectedError } from 'vs/base/common/errors'; const connectionRegistry = Registry.as(ConnectionExtensions.ConnectionProviderContributions); @@ -71,7 +72,7 @@ export class CapabilitiesService extends Disposable implements ICapabilitiesServ extensionService.whenInstalledExtensionsRegistered().then(() => { this.cleanupProviders(); - }); + }).catch(err => onUnexpectedError(err)); _storageService.onWillSaveState(() => this.shutdown()); @@ -85,7 +86,7 @@ export class CapabilitiesService extends Disposable implements ICapabilitiesServ let id = extension.contributes[connectionProvider].providerId; delete this.capabilities.connectionProviderCache[id]; } - }); + }).catch(err => onUnexpectedError(err)); })); } diff --git a/src/sql/workbench/services/connection/browser/connectionDialogService.ts b/src/sql/workbench/services/connection/browser/connectionDialogService.ts index 7971c14acc..6a9837839e 100644 --- a/src/sql/workbench/services/connection/browser/connectionDialogService.ts +++ b/src/sql/workbench/services/connection/browser/connectionDialogService.ts @@ -30,6 +30,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur import { CmsConnectionController } from 'sql/workbench/services/connection/browser/cmsConnectionController'; import { entries } from 'sql/base/common/collections'; import { find } from 'vs/base/common/arrays'; +import { onUnexpectedError } from 'vs/base/common/errors'; export interface IConnectionValidateResult { isValid: boolean; @@ -181,12 +182,12 @@ export class ConnectionDialogService implements IConnectionDialogService { profile.savePassword = true; } - this.handleDefaultOnConnect(params, profile); + this.handleDefaultOnConnect(params, profile).catch(err => onUnexpectedError(err)); } else { profile.serverName = trim(profile.serverName); - this._connectionManagementService.addSavedPassword(profile).then(connectionWithPassword => { - this.handleDefaultOnConnect(params, connectionWithPassword); - }); + this._connectionManagementService.addSavedPassword(profile).then(async (connectionWithPassword) => { + await this.handleDefaultOnConnect(params, connectionWithPassword); + }).catch(err => onUnexpectedError(err)); } } } @@ -219,14 +220,14 @@ export class ConnectionDialogService implements IConnectionDialogService { } } - private handleDefaultOnConnect(params: INewConnectionParams, connection: IConnectionProfile): Thenable { + private async handleDefaultOnConnect(params: INewConnectionParams, connection: IConnectionProfile): Promise { if (this.ignoreNextConnect) { this._connectionDialog.resetConnection(); this._connectionDialog.close(); this.ignoreNextConnect = false; this._connecting = false; this._dialogDeferredPromise.resolve(connection); - return Promise.resolve(); + return; } let fromEditor = params && params.connectionType === ConnectionType.editor; let isTemporaryConnection = params && params.connectionType === ConnectionType.temporary; @@ -242,7 +243,8 @@ export class ConnectionDialogService implements IConnectionDialogService { showFirewallRuleOnError: true }; - return this._connectionManagementService.connectAndSaveProfile(connection, uri, options, params && params.input).then(connectionResult => { + try { + const connectionResult = await this._connectionManagementService.connectAndSaveProfile(connection, uri, options, params && params.input); this._connecting = false; if (connectionResult && connectionResult.connected) { this._connectionDialog.close(); @@ -255,11 +257,11 @@ export class ConnectionDialogService implements IConnectionDialogService { this._connectionDialog.resetConnection(); this.showErrorDialog(Severity.Error, this._connectionErrorTitle, connectionResult.errorMessage, connectionResult.callStack); } - }).catch(err => { + } catch (err) { this._connecting = false; this._connectionDialog.resetConnection(); this.showErrorDialog(Severity.Error, this._connectionErrorTitle, err); - }); + } } private get uiController(): IConnectionComponentController { @@ -339,7 +341,7 @@ export class ConnectionDialogService implements IConnectionDialogService { this._model = this.createModel(connectionWithPassword); this.uiController.fillInConnectionInputs(this._model); - }); + }).catch(err => onUnexpectedError(err)); this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[connectionInfo.providerName]); } @@ -381,12 +383,9 @@ export class ConnectionDialogService implements IConnectionDialogService { return newProfile; } - private showDialogWithModel(): Promise { - return new Promise((resolve, reject) => { - this.updateModelServerCapabilities(this._inputModel); - this.doShowDialog(this._params); - resolve(null); - }); + private async showDialogWithModel(): Promise { + this.updateModelServerCapabilities(this._inputModel); + await this.doShowDialog(this._params); } public openDialogAndWait( @@ -438,7 +437,7 @@ export class ConnectionDialogService implements IConnectionDialogService { }); } - private doShowDialog(params: INewConnectionParams): Promise { + private async doShowDialog(params: INewConnectionParams): Promise { if (!this._connectionDialog) { this._connectionDialog = this._instantiationService.createInstance(ConnectionDialogWidget, this._providerDisplayNames, this._providerNameToDisplayNameMap[this._model.providerName], this._providerNameToDisplayNameMap); this._connectionDialog.onCancel(() => { @@ -457,12 +456,10 @@ export class ConnectionDialogService implements IConnectionDialogService { this._connectionDialog.newConnectionParams = params; this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[this._currentProviderType]); - return new Promise(() => { - const recentConnections: ConnectionProfile[] = this._connectionManagementService.getRecentConnections(params.providers); - this._connectionDialog.open(recentConnections.length > 0); - this.uiController.focusOnOpen(); - recentConnections.forEach(conn => conn.dispose()); - }); + const recentConnections: ConnectionProfile[] = this._connectionManagementService.getRecentConnections(params.providers); + await this._connectionDialog.open(recentConnections.length > 0); + this.uiController.focusOnOpen(); + recentConnections.forEach(conn => conn.dispose()); } private showErrorDialog(severity: Severity, headerTitle: string, message: string, messageDetails?: string): void { @@ -477,16 +474,15 @@ export class ConnectionDialogService implements IConnectionDialogService { localize('kerberosHelpLink', "Help configuring Kerberos is available at {0}", helpLink), localize('kerberosKinit', "If you have previously connected you may need to re-run kinit.") ].join('\r\n'); - actions.push(new Action('Kinit', 'Run kinit', null, true, () => { + actions.push(new Action('Kinit', 'Run kinit', null, true, async () => { this._connectionDialog.close(); - this._clipboardService.writeText('kinit\r'); - this._commandService.executeCommand('workbench.action.terminal.focus').then(resolve => { - // setTimeout to allow for terminal Instance to load. - setTimeout(() => { - return this._commandService.executeCommand('workbench.action.terminal.paste'); - }, 10); - }).then(resolve => null, reject => null); - return null; + await this._clipboardService.writeText('kinit\r'); + await this._commandService.executeCommand('workbench.action.terminal.focus'); + // setTimeout to allow for terminal Instance to load. + setTimeout(() => { + return this._commandService.executeCommand('workbench.action.terminal.paste'); + }, 10); + return; })); } diff --git a/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts b/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts index bdf574757b..aa75956463 100644 --- a/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts +++ b/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts @@ -214,35 +214,32 @@ suite('SQL ConnectionManagementService tests', () => { } - function connect(uri: string, options?: IConnectionCompletionOptions, fromDialog?: boolean, connection?: IConnectionProfile, error?: string, errorCode?: number, errorCallStack?: string): Promise { + async function connect(uri: string, options?: IConnectionCompletionOptions, fromDialog?: boolean, connection?: IConnectionProfile, error?: string, errorCode?: number, errorCallStack?: string): Promise { let connectionToUse = connection ? connection : connectionProfile; - return new Promise((resolve, reject) => { - let id = connectionToUse.getOptionsKey(); - let defaultUri = 'connection:' + (id ? id : connectionToUse.serverName + ':' + connectionToUse.databaseName); - connectionManagementService.onConnectionRequestSent(() => { - let info: azdata.ConnectionInfoSummary = { - connectionId: error ? undefined : 'id', - connectionSummary: { - databaseName: connectionToUse.databaseName, - serverName: connectionToUse.serverName, - userName: connectionToUse.userName - }, - errorMessage: error, - errorNumber: errorCode, - messages: errorCallStack, - ownerUri: uri ? uri : defaultUri, - serverInfo: undefined - }; - connectionManagementService.onConnectionComplete(0, info); - }); - connectionManagementService.cancelConnectionForUri(uri).then(() => { - if (fromDialog) { - resolve(connectionManagementService.connectAndSaveProfile(connectionToUse, uri, options)); - } else { - resolve(connectionManagementService.connect(connectionToUse, uri, options)); - } - }); + let id = connectionToUse.getOptionsKey(); + let defaultUri = 'connection:' + (id ? id : connectionToUse.serverName + ':' + connectionToUse.databaseName); + connectionManagementService.onConnectionRequestSent(() => { + let info: azdata.ConnectionInfoSummary = { + connectionId: error ? undefined : 'id', + connectionSummary: { + databaseName: connectionToUse.databaseName, + serverName: connectionToUse.serverName, + userName: connectionToUse.userName + }, + errorMessage: error, + errorNumber: errorCode, + messages: errorCallStack, + ownerUri: uri ? uri : defaultUri, + serverInfo: undefined + }; + connectionManagementService.onConnectionComplete(0, info); }); + await connectionManagementService.cancelConnectionForUri(uri); + if (fromDialog) { + return connectionManagementService.connectAndSaveProfile(connectionToUse, uri, options); + } else { + return connectionManagementService.connect(connectionToUse, uri, options); + } } test('showConnectionDialog should open the dialog with default type given no parameters', done => {