Properly expose errors in ads and tests (#8692)

* add code to expose errors outside zone

* remove unexpect error hiding

* remove uncessary code

* fix tests

* trying to catch more errros

* revert for testing

* wip

* wip

* figured out what was going on

* wip

* fix tests

* fix tests

src/sql/workbench/test/electron-browser/api/extHostAccountManagement.test.ts

@@ -101,25 +101,24 @@ suite('ExtHostAccountManagement', () => {
 	// TODO: Test for unregistering a provider
 
 	// CLEAR TESTS /////////////////////////////////////////////////////////
-	test('Clear - Success', (done) => {
+	test('Clear - Success', () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
 		extHost.$registerAccountProvider(mockAccountMetadata, mockProvider.object);
 
 		// If: I clear an account
-		extHost.$clear(0, mockAccount.key)
+		return extHost.$clear(0, mockAccount.key)
 			.then(() => {
 				// Then: The call should have been passed to the provider
 				mockProvider.verify(
 					(obj) => obj.clear(TypeMoq.It.isValue(mockAccount.key)),
 					TypeMoq.Times.once()
 				);
-			})
-			.then(() => done(), (err) => done(err));
+			});
 	});
 
-	test('Clear - Handle does not exist', (done) => {
+	test('Clear - Handle does not exist', async () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
@@ -127,38 +126,37 @@ suite('ExtHostAccountManagement', () => {
 
 		// If: I clear an account for a handle that doesn't exist
 		// Then: It should fail
-		extHost.$clear(1, mockAccount.key)
-			.then(() => done('Clear succeeded when it should have failed'))
-			.then(null, () => {
-				// The provider's clear should not have been called
-				mockProvider.verify(
-					(obj) => obj.clear(TypeMoq.It.isAny()),
-					TypeMoq.Times.never()
-				);
-			})
-			.then(() => done(), (err) => done(err));
+		try {
+			await extHost.$clear(1, mockAccount.key);
+			assert.fail('Clear succeeded when it should have failed');
+		} catch (e) {
+			// The provider's clear should not have been called
+			mockProvider.verify(
+				(obj) => obj.clear(TypeMoq.It.isAny()),
+				TypeMoq.Times.never()
+			);
+		}
 	});
 
 	// INITIALIZE TESTS ////////////////////////////////////////////////////
-	test('Initialize - Success', (done) => {
+	test('Initialize - Success', () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
 		extHost.$registerAccountProvider(mockAccountMetadata, mockProvider.object);
 
 		// If: I initialize the provider
-		extHost.$initialize(0, [mockAccount])
+		return extHost.$initialize(0, [mockAccount])
 			.then(() => {
 				// Then: The call should have been passed to the provider
 				mockProvider.verify(
 					(obj) => obj.initialize(TypeMoq.It.isValue([mockAccount])),
 					TypeMoq.Times.once()
 				);
-			})
-			.then(() => done(), (err) => done(err));
+			});
 	});
 
-	test('Initialize - Handle does not exist', (done) => {
+	test('Initialize - Handle does not exist', async () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
@@ -166,38 +164,37 @@ suite('ExtHostAccountManagement', () => {
 
 		// If: I initialize for a handle that doesn't exist
 		// Then: It should fail
-		extHost.$initialize(1, [mockAccount])
-			.then(() => done('Initialize succeeded when it should have failed'))
-			.then(null, () => {
-				// The provider's clear should not have been called
-				mockProvider.verify(
-					(obj) => obj.initialize(TypeMoq.It.isAny()),
-					TypeMoq.Times.never()
-				);
-			})
-			.then(() => done(), (err) => done(err));
+		try {
+			await extHost.$initialize(1, [mockAccount]);
+			assert.fail('Initialize succeeded when it should have failed');
+		} catch (e) {
+			// The provider's clear should not have been called
+			mockProvider.verify(
+				(obj) => obj.initialize(TypeMoq.It.isAny()),
+				TypeMoq.Times.never()
+			);
+		}
 	});
 
 	// PROMPT TESTS ////////////////////////////////////////////////////////
-	test('Prompt - Success', (done) => {
+	test('Prompt - Success', () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
 		extHost.$registerAccountProvider(mockAccountMetadata, mockProvider.object);
 
 		// If: I prompt for an account
-		extHost.$prompt(0)
+		return extHost.$prompt(0)
 			.then(() => {
 				// Then: The call should have been passed to the provider
 				mockProvider.verify(
 					(obj) => obj.prompt(),
 					TypeMoq.Times.once()
 				);
-			})
-			.then(() => done(), (err) => done(err));
+			});
 	});
 
-	test('Prompt - Handle does not exist', (done) => {
+	test('Prompt - Handle does not exist', async () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
@@ -205,38 +202,37 @@ suite('ExtHostAccountManagement', () => {
 
 		// If: I prompt with a handle that doesn't exist
 		// Then: It should fail
-		extHost.$prompt(1)
-			.then(() => done('Prompt succeeded when it should have failed'))
-			.then(null, () => {
-				// The provider's clear should not have been called
-				mockProvider.verify(
-					(obj) => obj.prompt(),
-					TypeMoq.Times.never()
-				);
-			})
-			.then(() => done(), (err) => done(err));
+		try {
+			await extHost.$prompt(1);
+			assert.fail('Prompt succeeded when it should have failed');
+		} catch (e) {
+			// The provider's clear should not have been called
+			mockProvider.verify(
+				(obj) => obj.prompt(),
+				TypeMoq.Times.never()
+			);
+		}
 	});
 
 	// REFRESH TESTS ///////////////////////////////////////////////////////
-	test('Refresh - Success', (done) => {
+	test('Refresh - Success', () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
 		extHost.$registerAccountProvider(mockAccountMetadata, mockProvider.object);
 
 		// If: I refresh an account
-		extHost.$refresh(0, mockAccount)
+		return extHost.$refresh(0, mockAccount)
 			.then(() => {
 				// Then: The call should have been passed to the provider
 				mockProvider.verify(
 					(obj) => obj.refresh(TypeMoq.It.isValue(mockAccount)),
 					TypeMoq.Times.once()
 				);
-			})
-			.then(() => done(), (err) => done(err));
+			});
 	});
 
-	test('Refresh - Handle does not exist', (done) => {
+	test('Refresh - Handle does not exist', async () => {
 		// Setup: Create ext host account management with registered account provider
 		let extHost = new ExtHostAccountManagement(threadService);
 		let mockProvider = getMockAccountProvider();
@@ -244,20 +240,20 @@ suite('ExtHostAccountManagement', () => {
 
 		// If: I refresh an account for a handle that doesn't exist
 		// Then: It should fail
-		extHost.$refresh(1, mockAccount)
-			.then(() => done('Refresh succeeded when it should have failed'))
-			.then(null, () => {
-				// The provider's clear should not have been called
-				mockProvider.verify(
-					(obj) => obj.refresh(TypeMoq.It.isAny()),
-					TypeMoq.Times.never()
-				);
-			})
-			.then(() => done(), (err) => done(err));
+		try {
+			await extHost.$refresh(1, mockAccount);
+			assert.fail('Refresh succeeded when it should have failed');
+		} catch (e) {
+			// The provider's clear should not have been called
+			mockProvider.verify(
+				(obj) => obj.refresh(TypeMoq.It.isAny()),
+				TypeMoq.Times.never()
+			);
+		}
 	});
 
 	// GETALLACCOUNTS TESTS ///////////////////////////////////////////////////////
-	test('GetAllAccounts - Success', (done) => {
+	test('GetAllAccounts - Success', () => {
 		let mockAccountProviderMetadata = {
 			id: 'azure',
 			displayName: 'Azure'
@@ -305,7 +301,7 @@ suite('ExtHostAccountManagement', () => {
 		extHost.$registerAccountProvider(mockAccountProviderMetadata, new AccountProviderStub());
 
 		// If: I get all accounts
-		extHost.$getAllAccounts()
+		return extHost.$getAllAccounts()
 			.then((accounts) => {
 				// Then: The call should have been passed to the account management service
 				mockAccountManagementService.verify(
@@ -316,25 +312,24 @@ suite('ExtHostAccountManagement', () => {
 				assert.ok(Array.isArray(accounts));
 				assert.equal(accounts.length, expectedAccounts.length);
 				assert.deepStrictEqual(accounts, expectedAccounts);
-			})
-			.then(() => done(), (err) => done(err));
+			});
 	});
 
-	test('GetAllAccounts - No account providers', (done) => {
+	test('GetAllAccounts - No account providers', async () => {
 		// Setup: Create ext host account management with no registered account providers
 		let extHost = new ExtHostAccountManagement(threadService);
 
 		// If: I get all accounts
 		// Then: It should throw
-		assert.throws(
-			() => extHost.$getAllAccounts(),
-			(error) => {
-				return error.message === 'No account providers registered.';
-			});
-		done();
+		try {
+			await extHost.$getAllAccounts();
+			assert.fail('Succedded when should have failed');
+		} catch (e) {
+			assert(e.message === 'No account providers registered.');
+		}
 	});
 
-	test('GetSecurityToken - Success', (done) => {
+	test('GetSecurityToken - Success', () => {
 		let mockAccountProviderMetadata = {
 			id: 'azure',
 			displayName: 'Azure'
@@ -365,15 +360,14 @@ suite('ExtHostAccountManagement', () => {
 		let extHost = new ExtHostAccountManagement(threadService);
 		extHost.$registerAccountProvider(mockAccountProviderMetadata, new AccountProviderStub());
 
-		extHost.$getAllAccounts()
+		return extHost.$getAllAccounts()
 			.then((accounts) => {
 				// If: I get security token it will not throw
 				return extHost.$getSecurityToken(mockAccount1, AzureResource.ResourceManagement);
-			}
-			).then(() => done(), (err) => done(new Error(err)));
+			});
 	});
 
-	test('GetSecurityToken - Account not found', (done) => {
+	test('GetSecurityToken - Account not found', async () => {
 		let mockAccountProviderMetadata = {
 			id: 'azure',
 			displayName: 'Azure'
@@ -419,16 +413,11 @@ suite('ExtHostAccountManagement', () => {
 			isStale: false
 		};
 
-		extHost.$getAllAccounts()
-			.then(accounts => {
-				return extHost.$getSecurityToken(mockAccount2, AzureResource.ResourceManagement);
-			})
-			.then((noError) => {
-				done(new Error('Expected getSecurityToken to throw'));
-			}, (err) => {
-				// Expected error caught
-				done();
-			});
+		await extHost.$getAllAccounts();
+		try {
+			await extHost.$getSecurityToken(mockAccount2, AzureResource.ResourceManagement);
+			assert.fail('Expected getSecurityToken to throw');
+		} catch (e) { }
 	});
 });
 

src/sql/workbench/test/electron-browser/api/extHostCredentialManagement.test.ts

@@ -56,7 +56,7 @@ suite('ExtHostCredentialManagement', () => {
 		assert.equal(extHost.getProviderCount(), 1);
 	});
 
-	test('Get Credential Provider - Success', (done) => {
+	test('Get Credential Provider - Success', () => {
 		// Setup: Register a mock credential provider
 		let extHost = new ExtHostCredentialManagement(threadService);
 		let mockCredentialProvider = new TestCredentialsProvider();
@@ -68,7 +68,7 @@ suite('ExtHostCredentialManagement', () => {
 		let credential = 'test_credential';
 		let expectedCredentialId = `${namespaceId}|${credentialId}`;
 		let credProvider: CredentialProvider;
-		extHost.$getCredentialProvider(namespaceId)
+		return extHost.$getCredentialProvider(namespaceId)
 			.then((provider) => {
 				// Then: There should still only be one provider registered
 				assert.equal(extHost.getProviderCount(), 1);
@@ -100,11 +100,10 @@ suite('ExtHostCredentialManagement', () => {
 			.then(() => {
 				// Then: The credential with its namespace should no longer exist
 				assert.strictEqual(mockCredentialProvider.storedCredentials[expectedCredentialId], undefined);
-			})
-			.then(() => done(), (err) => done(err));
+			});
 	});
 
-	test('Get Credential Provider - No Namespace', (done) => {
+	test('Get Credential Provider - No Namespace', async () => {
 		// Setup: Register a mock credential provider
 		let extHost = new ExtHostCredentialManagement(threadService);
 		let mockCredentialProvider = new TestCredentialsProvider();
@@ -112,20 +111,19 @@ suite('ExtHostCredentialManagement', () => {
 
 		// If: I get a credential provider with an invalid namespace ID
 		// Then: I should get an error
-		extHost.$getCredentialProvider(undefined)
-			.then(
-				() => { done('Provider was returned from undefined'); },
-				() => { /* Swallow error, this is success path */ }
-			)
-			.then(() => { return extHost.$getCredentialProvider(null); })
-			.then(
-				() => { done('Provider was returned from null'); },
-				() => { /* Swallow error, this is success path */ }
-			)
-			.then(() => { return extHost.$getCredentialProvider(''); })
-			.then(
-				() => { done('Provider was returned from \'\''); },
-				() => { done(); }
-			);
+		try {
+			await extHost.$getCredentialProvider(undefined);
+			assert.fail('Provider was returned from undefined');
+		} catch (e) { }
+
+		try {
+			await extHost.$getCredentialProvider(null);
+			assert.fail('Provider was returned from null');
+		} catch (e) { }
+
+		try {
+			await extHost.$getCredentialProvider('');
+			assert.fail('Provider was returned from \'\'');
+		} catch (e) { }
 	});
 });

src/sql/workbench/test/electron-browser/api/extHostModelView.test.ts

@@ -77,44 +77,29 @@ suite('ExtHostModelView Validation Tests', () => {
 			extHostModelView.$registerWidget(handle, widgetId, undefined, undefined);
 		});
 
-		test('The custom validation output of a component gets set when it is initialized', done => {
-			extHostModelView.$runCustomValidations(handle, inputBox.id).then(valid => {
-				try {
-					assert.equal(valid, false, 'Empty input box did not validate as false');
-					done();
-				} catch (err) {
-					done(err);
-				}
-			}, err => done(err));
+		test('The custom validation output of a component gets set when it is initialized', () => {
+			return extHostModelView.$runCustomValidations(handle, inputBox.id).then(valid => {
+				assert.equal(valid, false, 'Empty input box did not validate as false');
+			});
 		});
 
-		test('The custom validation output of a component changes if its value changes', done => {
+		test('The custom validation output of a component changes if its value changes', () => {
 			inputBox.value = validText;
-			extHostModelView.$runCustomValidations(handle, inputBox.id).then(valid => {
-				try {
-					assert.equal(valid, true, 'Valid input box did not validate as valid');
-					done();
-				} catch (err) {
-					done(err);
-				}
-			}, err => done(err));
+			return extHostModelView.$runCustomValidations(handle, inputBox.id).then(valid => {
+				assert.equal(valid, true, 'Valid input box did not validate as valid');
+			});
 		});
 
-		test('The custom validation output of a component changes after a PropertiesChanged event', done => {
+		test('The custom validation output of a component changes after a PropertiesChanged event', () => {
 			extHostModelView.$handleEvent(handle, inputBox.id, {
 				args: {
 					'value': validText
 				},
 				eventType: ComponentEventType.PropertiesChanged
 			} as IComponentEventArgs);
-			extHostModelView.$runCustomValidations(handle, inputBox.id).then(valid => {
-				try {
-					assert.equal(valid, true, 'Valid input box did not validate as valid after PropertiesChanged event');
-					done();
-				} catch (err) {
-					done(err);
-				}
-			}, err => done(err));
+			return extHostModelView.$runCustomValidations(handle, inputBox.id).then(valid => {
+				assert.equal(valid, true, 'Valid input box did not validate as valid after PropertiesChanged event');
+			});
 		});
 
 		test('The validity of a component is set by main thread validationChanged events', () => {

src/sql/workbench/test/electron-browser/api/exthostNotebook.test.ts

@@ -13,7 +13,6 @@ import { IMainContext } from 'vs/workbench/api/common/extHost.protocol';
 
 import { ExtHostNotebook } from 'sql/workbench/api/common/extHostNotebook';
 import { MainThreadNotebookShape } from 'sql/workbench/api/common/sqlExtHost.protocol';
-import * as testUtils from '../../../../base/test/common/async';
 import { INotebookManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes';
 import { mssqlProviderName } from 'sql/platform/connection/common/constants';
 
@@ -40,7 +39,10 @@ suite('ExtHostNotebook Tests', () => {
 
 	suite('getNotebookManager', () => {
 		test('Should throw if no matching provider is defined', async () => {
-			await testUtils.assertThrowsAsync(() => extHostNotebook.$getNotebookManager(-1, notebookUri));
+			try {
+				await extHostNotebook.$getNotebookManager(-1, notebookUri);
+				assert.fail('expected to throw');
+			} catch (e) { }
 		});
 		suite('with provider', () => {
 			let providerHandle: number = -1;

src/sql/workbench/test/electron-browser/modalComponents/componentBase.test.ts

@@ -64,7 +64,7 @@ suite('ComponentBase Tests', () => {
 		testContainer = new TestContainer(modelStore, 'testContainer');
 	});
 
-	test('Component validation runs external validations stored in the model store', done => {
+	test('Component validation runs external validations stored in the model store', () => {
 		assert.equal(testComponent.valid, true, 'Test component validity did not default to true');
 		let validationCalls = 0;
 		modelStore.registerValidationCallback(componentId => {
@@ -72,19 +72,14 @@ suite('ComponentBase Tests', () => {
 			return Promise.resolve(false);
 		});
 
-		testComponent.validate().then(valid => {
-			try {
-				assert.equal(validationCalls, 1, 'External validation was not called once');
-				assert.equal(valid, false, 'Validate call did not return correct value from the external validation');
-				assert.equal(testComponent.valid, false, 'Validate call did not update the component valid property');
-				done();
-			} catch (err) {
-				done(err);
-			}
-		}, err => done(err));
+		return testComponent.validate().then(valid => {
+			assert.equal(validationCalls, 1, 'External validation was not called once');
+			assert.equal(valid, false, 'Validate call did not return correct value from the external validation');
+			assert.equal(testComponent.valid, false, 'Validate call did not update the component valid property');
+		});
 	});
 
-	test('Component validation runs default component validations', done => {
+	test('Component validation runs default component validations', () => {
 		assert.equal(testComponent.valid, true, 'Test component validity did not default to true');
 		let validationCalls = 0;
 		testComponent.addValidation(() => {
@@ -92,29 +87,23 @@ suite('ComponentBase Tests', () => {
 			return false;
 		});
 
-		testComponent.validate().then(valid => {
-			try {
-				assert.equal(validationCalls, 1, 'Default validation was not called once');
-				assert.equal(valid, false, 'Validate call did not return correct value from the default validation');
-				assert.equal(testComponent.valid, false, 'Validate call did not update the component valid property');
-				done();
-			} catch (err) {
-				done(err);
-			}
-		}, err => done(err));
+		return testComponent.validate().then(valid => {
+			assert.equal(validationCalls, 1, 'Default validation was not called once');
+			assert.equal(valid, false, 'Validate call did not return correct value from the default validation');
+			assert.equal(testComponent.valid, false, 'Validate call did not update the component valid property');
+		});
 	});
 
-	test('Container validation reflects child component validity', done => {
+	test('Container validation reflects child component validity', () => {
 		assert.equal(testContainer.valid, true, 'Test container validity did not default to true');
 		testContainer.addToContainer(testComponent.descriptor, undefined);
 		testComponent.addValidation(() => false);
-		testComponent.validate().then(() => {
-			testContainer.validate().then(valid => {
+		return testComponent.validate().then(() => {
+			return testContainer.validate().then(valid => {
 				assert.equal(valid, false, 'Validate call did not return correct value for container child validation');
 				assert.equal(testContainer.valid, false, 'Validate call did not update the container valid property');
-				done();
-			}, err => done(err));
-		}, err => done(err));
+			});
+		});
 	});
 
 	test('Container child validity changes cause the parent container validity to change', done => {