debt - introduce Promises.withAsyncBody and adopt in a few places

src/vs/base/common/async.ts

@@ -1309,6 +1309,26 @@ export namespace Promises {
 
 		return result as unknown as T[]; // cast is needed and protected by the `throw` above
 	}
+
+	/**
+	 * A helper to create a new `Promise<T>` with a body that is a promise
+	 * itself. By default, an error that raises from the async body will
+	 * end up as a unhandled rejection, so this utility properly awaits the
+	 * body and rejects the promise as a normal promise does without async
+	 * body.
+	 *
+	 * This method should only be used in rare cases where otherwise `async`
+	 * cannot be used (e.g. when callbacks are involved that require this).
+	 */
+	export function withAsyncBody<T, E = Error>(bodyFn: (resolve: (value: T) => unknown, reject: (error: E) => unknown) => Promise<unknown>): Promise<T> {
+		return new Promise<T>(async (resolve, reject) => {
+			try {
+				await bodyFn(resolve, reject);
+			} catch (error) {
+				reject(error);
+			}
+		});
+	}
 }
 
 //#endregion

src/vs/base/test/common/async.test.ts

@@ -940,6 +940,44 @@ suite('Async', () => {
 		});
 	});
 
+	suite('Promises.withAsyncBody', () => {
+		test('basics', async () => {
+
+			const p1 = async.Promises.withAsyncBody(async (resolve, reject) => {
+				resolve(1);
+			});
+
+			const p2 = async.Promises.withAsyncBody(async (resolve, reject) => {
+				reject(new Error('error'));
+			});
+
+			const p3 = async.Promises.withAsyncBody(async (resolve, reject) => {
+				throw new Error('error');
+			});
+
+			const r1 = await p1;
+			assert.strictEqual(r1, 1);
+
+			let e2: Error | undefined = undefined;
+			try {
+				await p2;
+			} catch (error) {
+				e2 = error;
+			}
+
+			assert.ok(e2 instanceof Error);
+
+			let e3: Error | undefined = undefined;
+			try {
+				await p3;
+			} catch (error) {
+				e3 = error;
+			}
+
+			assert.ok(e3 instanceof Error);
+		});
+	});
+
 	suite('ThrottledWorker', () => {
 
 		function assertArrayEquals(actual: unknown[], expected: unknown[]) {

src/vs/platform/checksum/node/checksumService.ts

@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { createHash } from 'crypto';
+import { Promises } from 'vs/base/common/async';
 import { listenStream } from 'vs/base/common/stream';
 import { URI } from 'vs/base/common/uri';
 import { IChecksumService } from 'vs/platform/checksum/common/checksumService';
@@ -16,7 +17,7 @@ export class ChecksumService implements IChecksumService {
 	constructor(@IFileService private readonly fileService: IFileService) { }
 
 	checksum(resource: URI): Promise<string> {
-		return new Promise<string>(async (resolve, reject) => {
+		return Promises.withAsyncBody<string>(async (resolve, reject) => {
 			const hash = createHash('md5');
 			const stream = (await this.fileService.readFileStream(resource)).value;
 

src/vs/platform/environment/node/shellEnv.ts

@@ -15,6 +15,7 @@ import { getSystemShell } from 'vs/base/node/shell';
 import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
 import { isLaunchedFromCli } from 'vs/platform/environment/node/argvHelper';
 import { ILogService } from 'vs/platform/log/common/log';
+import { Promises } from 'vs/base/common/async';
 
 /**
  * The maximum of time we accept to wait on resolving the shell
@@ -69,7 +70,7 @@ export async function resolveShellEnv(logService: ILogService, args: NativeParse
 		// subsequent calls since this operation can be
 		// expensive (spawns a process).
 		if (!unixShellEnvPromise) {
-			unixShellEnvPromise = new Promise(async (resolve, reject) => {
+			unixShellEnvPromise = Promises.withAsyncBody<NodeJS.ProcessEnv, string>(async (resolve, reject) => {
 				const cts = new CancellationTokenSource();
 
 				// Give up resolving shell env after some time
@@ -99,7 +100,7 @@ export async function resolveShellEnv(logService: ILogService, args: NativeParse
 }
 
 async function doResolveUnixShellEnv(logService: ILogService, token: CancellationToken): Promise<typeof process.env> {
-	return new Promise<typeof process.env>(async (resolve, reject) => {
+	return Promises.withAsyncBody<typeof process.env, Error>(async (resolve, reject) => {
 		const runAsNode = process.env['ELECTRON_RUN_AS_NODE'];
 		logService.trace('getUnixShellEnvironment#runAsNode', runAsNode);
 
@@ -120,7 +121,7 @@ async function doResolveUnixShellEnv(logService: ILogService, token: Cancellatio
 		logService.trace('getUnixShellEnvironment#shell', systemShellUnix);
 
 		if (token.isCancellationRequested) {
-			return reject(canceled);
+			return reject(canceled());
 		}
 
 		// handle popular non-POSIX shells
@@ -147,7 +148,7 @@ async function doResolveUnixShellEnv(logService: ILogService, token: Cancellatio
 		token.onCancellationRequested(() => {
 			child.kill();
 
-			return reject(canceled);
+			return reject(canceled());
 		});
 
 		child.on('error', err => {

src/vs/platform/files/browser/indexedDBFileSystemProvider.ts

@@ -503,7 +503,7 @@ class IndexedDBFileSystemProvider extends Disposable implements IIndexedDBFileSy
 	}
 
 	private deleteKeys(keys: string[]): Promise<void> {
-		return new Promise(async (c, e) => {
+		return new Promise((c, e) => {
 			if (keys.length === 0) {
 				return c();
 			}
@@ -519,7 +519,7 @@ class IndexedDBFileSystemProvider extends Disposable implements IIndexedDBFileSy
 	}
 
 	reset(): Promise<void> {
-		return new Promise(async (c, e) => {
+		return new Promise((c, e) => {
 			const transaction = this.database.transaction([this.store], 'readwrite');
 			transaction.oncomplete = () => c();
 			transaction.onerror = () => e(transaction.error);

src/vs/platform/files/common/fileService.ts

@@ -1222,8 +1222,7 @@ export class FileService extends Disposable implements IFileService {
 			stream = streamOrBufferedStream;
 		}
 
-		return new Promise(async (resolve, reject) => {
-
+		return new Promise((resolve, reject) => {
 			listenStream(stream, {
 				onData: async chunk => {
 

src/vs/platform/storage/browser/storageService.ts

@@ -250,7 +250,7 @@ export class IndexedDBStorageDatabase extends Disposable implements IIndexedDBSt
 	}
 
 	getItems(): Promise<Map<string, string>> {
-		return new Promise<Map<string, string>>(async resolve => {
+		return Promises.withAsyncBody<Map<string, string>>(async resolve => {
 			const items = new Map<string, string>();
 
 			// Open a IndexedDB Cursor to iterate over key/values
@@ -323,7 +323,7 @@ export class IndexedDBStorageDatabase extends Disposable implements IIndexedDBSt
 		}
 
 		// Update `ItemTable` with inserts and/or deletes
-		return new Promise<boolean>(async (resolve, reject) => {
+		return Promises.withAsyncBody<boolean, DOMException | null>(async (resolve, reject) => {
 			const db = await this.whenConnected;
 
 			const transaction = db.transaction(IndexedDBStorageDatabase.STORAGE_OBJECT_STORE, 'readwrite');
@@ -359,7 +359,7 @@ export class IndexedDBStorageDatabase extends Disposable implements IIndexedDBSt
 	}
 
 	clear(): Promise<void> {
-		return new Promise<void>(async (resolve, reject) => {
+		return Promises.withAsyncBody<void, DOMException | null>(async (resolve, reject) => {
 			const db = await this.whenConnected;
 
 			const transaction = db.transaction(IndexedDBStorageDatabase.STORAGE_OBJECT_STORE, 'readwrite');

src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts

@@ -186,7 +186,7 @@ export class BulkEditPane extends ViewPane {
 
 		this._currentInput = input;
 
-		return new Promise<ResourceEdit[] | undefined>(async resolve => {
+		return new Promise<ResourceEdit[] | undefined>(resolve => {
 
 			token.onCancellationRequested(() => resolve(undefined));
 

src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts

@@ -546,7 +546,7 @@ export class UserDataSyncWorkbenchService extends Disposable implements IUserDat
 			return this.authenticationProviders[0];
 		}
 
-		return new Promise<UserDataSyncAccount | IAuthenticationProvider | undefined>(async (c, e) => {
+		return new Promise<UserDataSyncAccount | IAuthenticationProvider | undefined>(c => {
 			let result: UserDataSyncAccount | IAuthenticationProvider | undefined;
 			const disposables: DisposableStore = new DisposableStore();
 			const quickPick = this.quickInputService.createQuickPick<AccountQuickPickItem>();

test/automation/src/playwrightDriver.ts

@@ -187,35 +187,33 @@ interface Options {
 	readonly headless?: boolean;
 }
 
-export function connect(options: Options = {}): Promise<{ client: IDisposable, driver: IDriver }> {
-	return new Promise(async (c) => {
-		const browser = await playwright[options.browser ?? 'chromium'].launch({ headless: options.headless ?? false });
-		const context = await browser.newContext();
-		try {
-			await context.tracing.start({ screenshots: true, snapshots: true });
-		} catch (error) {
-			console.warn(`Failed to start playwright tracing.`); // do not fail the build when this fails
+export async function connect(options: Options = {}): Promise<{ client: IDisposable, driver: IDriver }> {
+	const browser = await playwright[options.browser ?? 'chromium'].launch({ headless: options.headless ?? false });
+	const context = await browser.newContext();
+	try {
+		await context.tracing.start({ screenshots: true, snapshots: true });
+	} catch (error) {
+		console.warn(`Failed to start playwright tracing.`); // do not fail the build when this fails
+	}
+	const page = await context.newPage();
+	await page.setViewportSize({ width, height });
+	page.on('pageerror', async error => console.error(`Playwright ERROR: page error: ${error}`));
+	page.on('crash', page => console.error('Playwright ERROR: page crash'));
+	page.on('response', async response => {
+		if (response.status() >= 400) {
+			console.error(`Playwright ERROR: HTTP status ${response.status()} for ${response.url()}`);
 		}
-		const page = await context.newPage();
-		await page.setViewportSize({ width, height });
-		page.on('pageerror', async error => console.error(`Playwright ERROR: page error: ${error}`));
-		page.on('crash', page => console.error('Playwright ERROR: page crash'));
-		page.on('response', async response => {
-			if (response.status() >= 400) {
-				console.error(`Playwright ERROR: HTTP status ${response.status()} for ${response.url()}`);
-			}
-		});
-		const payloadParam = `[["enableProposedApi",""],["skipWelcome","true"]]`;
-		await page.goto(`${endpoint}&folder=vscode-remote://localhost:9888${URI.file(workspacePath!).path}&payload=${payloadParam}`);
-		const result = {
-			client: {
-				dispose: () => {
-					browser.close();
-					teardown();
-				}
-			},
-			driver: buildDriver(browser, context, page)
-		};
-		c(result);
 	});
+	const payloadParam = `[["enableProposedApi",""],["skipWelcome","true"]]`;
+	await page.goto(`${endpoint}&folder=vscode-remote://localhost:9888${URI.file(workspacePath!).path}&payload=${payloadParam}`);
+
+	return {
+		client: {
+			dispose: () => {
+				browser.close();
+				teardown();
+			}
+		},
+		driver: buildDriver(browser, context, page)
+	};
 }