Some promise cleanup (#8092)

* Some promise cleanup

* Handle more promise issues

* Remove changes that aren't needed anymore

* Use log service

* another one

* Be more explicit

src/sql/platform/query/common/gridDataProvider.ts

@@ -23,7 +23,7 @@ export interface IGridDataProvider {
 	 * @param resultId The result id of the result to copy from
 	 * @param includeHeaders [Optional]: Should column headers be included in the copy selection
 	 */
-	copyResults(selection: Slick.Range[], includeHeaders?: boolean): void;
+	copyResults(selection: Slick.Range[], includeHeaders?: boolean): Promise<void>;
 
 	/**
 	 * Gets the EOL terminator to use for this data type.

src/sql/platform/query/common/queryManagement.ts

@@ -178,6 +178,7 @@ export class QueryManagementService implements IQueryManagementService {
 				displayActualQueryPlan: runOptions.displayActualQueryPlan
 			});
 		}
+		// tslint:disable-next-line:no-floating-promises
 		TelemetryUtils.addTelemetry(this._telemetryService, this.logService, eventName, data);
 	}
 

src/sql/platform/query/common/queryModelService.ts

@@ -152,9 +152,9 @@ export class QueryModelService implements IQueryModelService {
 		}
 	}
 
-	public copyResults(uri: string, selection: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): void {
+	public async copyResults(uri: string, selection: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): Promise<void> {
 		if (this._queryInfoMap.has(uri)) {
-			this._queryInfoMap.get(uri)!.queryRunner.copyResults(selection, batchId, resultId, includeHeaders);
+			return this._queryInfoMap.get(uri)!.queryRunner.copyResults(selection, batchId, resultId, includeHeaders);
 		}
 	}
 
@@ -187,29 +187,29 @@ export class QueryModelService implements IQueryModelService {
 	/**
 	 * Run a query for the given URI with the given text selection
 	 */
-	public runQuery(uri: string, selection: azdata.ISelectionData, queryInput: QueryInput, runOptions?: azdata.ExecutionPlanOptions): void {
+	public async runQuery(uri: string, selection: azdata.ISelectionData, queryInput: QueryInput, runOptions?: azdata.ExecutionPlanOptions): Promise<void> {
-		this.doRunQuery(uri, selection, queryInput, false, runOptions);
+		return this.doRunQuery(uri, selection, queryInput, false, runOptions);
 	}
 
 	/**
 	 * Run the current SQL statement for the given URI
 	 */
-	public runQueryStatement(uri: string, selection: azdata.ISelectionData, queryInput: QueryInput): void {
+	public async runQueryStatement(uri: string, selection: azdata.ISelectionData, queryInput: QueryInput): Promise<void> {
-		this.doRunQuery(uri, selection, queryInput, true);
+		return this.doRunQuery(uri, selection, queryInput, true);
 	}
 
 	/**
 	 * Run the current SQL statement for the given URI
 	 */
-	public runQueryString(uri: string, selection: string, queryInput: QueryInput): void {
+	public async runQueryString(uri: string, selection: string, queryInput: QueryInput): Promise<void> {
-		this.doRunQuery(uri, selection, queryInput, true);
+		return this.doRunQuery(uri, selection, queryInput, true);
 	}
 
 	/**
 	 * Run Query implementation
 	 */
-	private doRunQuery(uri: string, selection: azdata.ISelectionData | string, queryInput: QueryInput,
+	private async doRunQuery(uri: string, selection: azdata.ISelectionData | string, queryInput: QueryInput,
-		runCurrentStatement: boolean, runOptions?: azdata.ExecutionPlanOptions): void {
+		runCurrentStatement: boolean, runOptions?: azdata.ExecutionPlanOptions): Promise<void> {
 		// Reuse existing query runner if it exists
 		let queryRunner: QueryRunner | undefined;
 		let info: QueryInfo;
@@ -243,11 +243,11 @@ export class QueryModelService implements IQueryModelService {
 			} else {
 				info.selectionSnippet = selection.substring(0, selectionSnippetMaxLen - 3) + '...';
 			}
-			queryRunner.runQuery(selection, runOptions);
+			return queryRunner.runQuery(selection, runOptions);
 		} else if (runCurrentStatement) {
-			queryRunner.runQueryStatement(selection);
+			return queryRunner.runQueryStatement(selection);
 		} else {
-			queryRunner.runQuery(selection, runOptions);
+			return queryRunner.runQuery(selection, runOptions);
 		}
 	}
 
@@ -403,11 +403,11 @@ export class QueryModelService implements IQueryModelService {
 
 	}
 
-	public disposeQuery(ownerUri: string): void {
+	public async disposeQuery(ownerUri: string): Promise<void> {
 		// Get existing query runner
 		let queryRunner = this.internalGetQueryRunner(ownerUri);
 		if (queryRunner) {
-			queryRunner.disposeQuery();
+			await queryRunner.disposeQuery();
 		}
 		// remove our info map
 		if (this._queryInfoMap.has(ownerUri)) {
@@ -416,7 +416,7 @@ export class QueryModelService implements IQueryModelService {
 	}
 
 	// EDIT DATA METHODS /////////////////////////////////////////////////////
-	initializeEdit(ownerUri: string, schemaName: string, objectName: string, objectType: string, rowLimit: number, queryString: string): void {
+	async initializeEdit(ownerUri: string, schemaName: string, objectName: string, objectType: string, rowLimit: number, queryString: string): Promise<void> {
 		// Reuse existing query runner if it exists
 		let queryRunner: QueryRunner;
 		let info: QueryInfo;
@@ -522,7 +522,7 @@ export class QueryModelService implements IQueryModelService {
 			}
 		}
 
-		queryRunner.initializeEdit(ownerUri, schemaName, objectName, objectType, rowLimit, queryString);
+		return queryRunner.initializeEdit(ownerUri, schemaName, objectName, objectType, rowLimit, queryString);
 	}
 
 	public cancelInitializeEdit(input: QueryRunner | string): void {

src/sql/platform/query/common/queryRunner.ts

@@ -26,6 +26,7 @@ import { URI } from 'vs/base/common/uri';
 import { mssqlProviderName } from 'sql/platform/connection/common/constants';
 import { IGridDataProvider, getResultsString } from 'sql/platform/query/common/gridDataProvider';
 import { getErrorMessage } from 'vs/base/common/errors';
+import { ILogService } from 'vs/platform/log/common/log';
 
 export interface IEditSessionReadyEvent {
 	ownerUri: string;
@@ -103,7 +104,8 @@ export default class QueryRunner extends Disposable {
 		@INotificationService private _notificationService: INotificationService,
 		@IConfigurationService private _configurationService: IConfigurationService,
 		@IInstantiationService private instantiationService: IInstantiationService,
-		@ITextResourcePropertiesService private _textResourcePropertiesService: ITextResourcePropertiesService
+		@ITextResourcePropertiesService private _textResourcePropertiesService: ITextResourcePropertiesService,
+		@ILogService private _logService: ILogService
 	) {
 		super();
 	}
@@ -347,11 +349,12 @@ export default class QueryRunner extends Disposable {
 			let hasShowPlan = !!result.resultSetSummary.columnInfo.find(e => e.columnName === 'Microsoft SQL Server 2005 XML Showplan');
 			if (hasShowPlan) {
 				this._isQueryPlan = true;
+
 				this.getQueryRows(0, 1, result.resultSetSummary.batchId, result.resultSetSummary.id).then(e => {
 					if (e.resultSubset.rows) {
 						this._planXml.resolve(e.resultSubset.rows[0][0].displayValue);
 					}
-				});
+				}).catch((e) => this._logService.error(e));
 			}
 			// we will just ignore the set if we already have it
 			// ideally this should never happen
@@ -374,6 +377,7 @@ export default class QueryRunner extends Disposable {
 			if (hasShowPlan) {
 				this._isQueryPlan = true;
 				this.getQueryRows(0, 1, result.resultSetSummary.batchId, result.resultSetSummary.id).then(e => {
+
 					if (e.resultSubset.rows) {
 						let planXmlString = e.resultSubset.rows[0][0].displayValue;
 						this._planXml.resolve(e.resultSubset.rows[0][0].displayValue);
@@ -386,7 +390,7 @@ export default class QueryRunner extends Disposable {
 							});
 						}
 					}
-				});
+				}).catch((e) => this._logService.error(e));
 			}
 			if (batchSet) {
 				// Store the result set in the batch and emit that a result set has completed
@@ -525,10 +529,9 @@ export default class QueryRunner extends Disposable {
 	/**
 	 * Disposes the Query from the service client
 	 */
-	public disposeQuery(): void {
+	public async disposeQuery(): Promise<void> {
-		this._queryManagementService.disposeQuery(this.uri).then(() => {
+		await this._queryManagementService.disposeQuery(this.uri);
 		this.dispose();
-		});
 	}
 
 	public dispose() {
@@ -547,9 +550,9 @@ export default class QueryRunner extends Disposable {
 	 * @param resultId The result id of the result to copy from
 	 * @param includeHeaders [Optional]: Should column headers be included in the copy selection
 	 */
-	copyResults(selection: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): void {
+	async copyResults(selection: Slick.Range[], batchId: number, resultId: number, includeHeaders?: boolean): Promise<void> {
 		let provider = this.getGridDataProvider(batchId, resultId);
-		provider.copyResults(selection, includeHeaders);
+		return provider.copyResults(selection, includeHeaders);
 	}
 
 
@@ -618,14 +621,14 @@ export class QueryGridDataProvider implements IGridDataProvider {
 		return this.queryRunner.getQueryRows(rowStart, numberOfRows, this.batchId, this.resultSetId);
 	}
 
-	copyResults(selection: Slick.Range[], includeHeaders?: boolean): void {
+	copyResults(selection: Slick.Range[], includeHeaders?: boolean): Promise<void> {
-		this.copyResultsAsync(selection, includeHeaders);
+		return this.copyResultsAsync(selection, includeHeaders);
 	}
 
 	private async copyResultsAsync(selection: Slick.Range[], includeHeaders?: boolean): Promise<void> {
 		try {
 			let results = await getResultsString(this, selection, includeHeaders);
-			this._clipboardService.writeText(results);
+			await this._clipboardService.writeText(results);
 		} catch (error) {
 			this._notificationService.error(nls.localize('copyFailed', "Copy failed with error {0}", getErrorMessage(error)));
 		}

src/sql/workbench/parts/notebook/browser/outputs/gridOutput.component.ts

@@ -217,8 +217,8 @@ class DataResourceDataProvider implements IGridDataProvider {
 		return Promise.resolve(resultSubset);
 	}
 
-	copyResults(selection: Slick.Range[], includeHeaders?: boolean): void {
+	async copyResults(selection: Slick.Range[], includeHeaders?: boolean): Promise<void> {
-		this.copyResultsAsync(selection, includeHeaders);
+		return this.copyResultsAsync(selection, includeHeaders);
 	}
 
 	private async copyResultsAsync(selection: Slick.Range[], includeHeaders?: boolean): Promise<void> {