Fix rest of notebook unhandled promises (#16933)

* Fix rest of notebook unhandled promises

* add rule

* fix some tests

extensions/notebook/.eslintrc.json

@@ -0,0 +1,13 @@
+{
+	"parserOptions": {
+		"project": "./extensions/notebook/tsconfig.json"
+	},
+	"rules": {
+		"@typescript-eslint/no-floating-promises": [
+			"error",
+			{
+				"ignoreVoid": true
+			}
+		]
+	}
+}

extensions/notebook/src/book/bookModel.ts

@@ -48,7 +48,7 @@ export class BookModel {
 	public watchTOC(): void {
 		fs.watchFile(this.tableOfContentsPath, async (curr, prev) => {
 			if (curr.mtime > prev.mtime) {
-				this.reinitializeContents();
+				this.reinitializeContents().catch(err => console.error('Error reinitializing book contents ', err));
 			}
 		});
 	}

extensions/notebook/src/book/bookTocManager.ts

@@ -346,7 +346,7 @@ export class BookTocManager implements IBookTocManager {
 			const movedSections = await this.traverseSections(files);
 			this.newSection.sections = movedSections;
 			this._modifiedDirectory.add(path.dirname(section.book.contentPath));
-			this.cleanUp(path.dirname(section.book.contentPath));
+			await this.cleanUp(path.dirname(section.book.contentPath));
 		}
 
 		if (bookItem.book.version === BookVersion.v1) {

extensions/notebook/src/book/bookTreeView.ts

@@ -155,8 +155,8 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 
 	async createBook(): Promise<void> {
 		const dialog = new CreateBookDialog(this.bookTocManager);
-		dialog.createDialog();
 		TelemetryReporter.sendActionEvent(BookTelemetryView, NbTelemetryActions.CreateBook);
+		return dialog.createDialog();
 	}
 
 	async getSelectionQuickPick(movingElement: BookTreeItem): Promise<quickPickResults> {
@@ -237,7 +237,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 
 			if (showPreview) {
 				this.currentBook = this.books.find(book => book.bookPath === bookPath);
-				this._bookViewer.reveal(this.currentBook.bookItems[0], { expand: vscode.TreeItemCollapsibleState.Expanded, focus: true, select: true });
+				await this._bookViewer.reveal(this.currentBook.bookItems[0], { expand: vscode.TreeItemCollapsibleState.Expanded, focus: true, select: true });
 				await this.showPreviewFile(urlToOpen);
 			}
 

extensions/notebook/src/book/bookTrustManager.ts

@@ -76,7 +76,7 @@ export class BookTrustManager implements IBookTrustManager {
 		let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(constants.notebookConfigKey);
 		let storeInWorspace: boolean = this.hasWorkspaceFolders();
 
-		config.update(constants.trustedBooksConfigKey, bookPaths, storeInWorspace ? false : vscode.ConfigurationTarget.Global);
+		void config.update(constants.trustedBooksConfigKey, bookPaths, storeInWorspace ? false : vscode.ConfigurationTarget.Global);
 	}
 
 	hasWorkspaceFolders(): boolean {

extensions/notebook/src/book/githubRemoteBook.ts

@@ -23,7 +23,7 @@ export class GitHubRemoteBook extends RemoteBook {
 		this.setLocalPath();
 		this.outputChannel.appendLine(loc.msgDownloadLocation(this.localPath.fsPath));
 		this.outputChannel.appendLine(loc.msgRemoteBookDownloadProgress);
-		this.createDirectory();
+		await this.createDirectory();
 		let notebookConfig = vscode.workspace.getConfiguration(constants.notebookConfigKey);
 		let downloadTimeout = notebookConfig[constants.remoteBookDownloadTimeout];
 

extensions/notebook/src/common/utils.ts

@@ -465,7 +465,7 @@ export function getPinnedNotebooks(): IPinnedNotebook[] {
 	});
 	if (updateFormat) {
 		//Need to modify the format of how pinnedNotebooks are stored for users that used the September release version.
-		setPinnedBookPathsInConfig(pinnedBookDirectories);
+		setPinnedBookPathsInConfig(pinnedBookDirectories).catch(err => console.error('Error setting pinned notebook paths in config ', err));
 	}
 	return pinnedBookDirectories;
 }

extensions/notebook/src/dialog/managePackages/installedPackagesTab.ts

@@ -135,7 +135,7 @@ export class InstalledPackagesTab {
 			await view.initializeModel(this.installedPackagesLoader);
 
 			await this.loadInstalledPackagesInfo();
-			this.packageTypeDropdown.focus();
+			await this.packageTypeDropdown.focus();
 		});
 	}
 
@@ -252,7 +252,7 @@ export class InstalledPackagesTab {
 			return;
 		}
 
-		this.uninstallPackageButton.updateProperties({ enabled: false });
+		await this.uninstallPackageButton.updateProperties({ enabled: false });
 		let doUninstall = await this.prompter.promptSingle<boolean>(<IQuestion>{
 			type: QuestionTypes.confirm,
 			message: localize('managePackages.confirmUninstall', "Are you sure you want to uninstall the specified packages?"),
@@ -309,6 +309,6 @@ export class InstalledPackagesTab {
 			}
 		}
 
-		this.uninstallPackageButton.updateProperties({ enabled: true });
+		await this.uninstallPackageButton.updateProperties({ enabled: true });
 	}
 }

extensions/notebook/src/dialog/remoteBookDialog.ts

@@ -100,7 +100,7 @@ export class RemoteBookDialog {
 			}).component();
 
 			this.languageDropdown.onValueChanged(async () => this.checkValues());
-			this.setFieldsToEmpty();
+			await this.setFieldsToEmpty();
 
 			this.formModel = this.view.modelBuilder.formContainer()
 				.withFormItems([{
@@ -191,7 +191,7 @@ export class RemoteBookDialog {
 					if (releases) {
 						this.releaseDropdown.enabled = true;
 						await this.fillReleasesDropdown();
-						this.setFieldsToEmpty();
+						await this.setFieldsToEmpty();
 					}
 				} else {
 					throw new Error(loc.urlGithubError);
@@ -200,7 +200,7 @@ export class RemoteBookDialog {
 		}
 		catch (error) {
 			await this.fillReleasesDropdown();
-			this.setFieldsToEmpty();
+			await this.setFieldsToEmpty();
 			this.showErrorMessage(error.message);
 		}
 	}
@@ -219,7 +219,7 @@ export class RemoteBookDialog {
 			}
 		}
 		catch (error) {
-			this.setFieldsToEmpty();
+			await this.setFieldsToEmpty();
 			this.showErrorMessage(error.message);
 		}
 	}

extensions/notebook/src/extension.ts

@@ -71,7 +71,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
 
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openRemoteBook', async () => {
 		let dialog = new RemoteBookDialog(remoteBookController);
-		dialog.createDialog();
+		return dialog.createDialog();
 	}));
 
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('_notebook.command.new', async (options?: azdata.nb.NotebookShowOptions) => {

extensions/notebook/src/jupyter/jupyterController.ts

@@ -156,7 +156,7 @@ export class JupyterController {
 					+ os.EOL + '.option(\"header\", \"true\")' + os.EOL + '.csv(\'{0}\'))' + os.EOL + 'df.show(10)';
 				// TODO re-enable insert into document once APIs are finalized.
 				// editor.document.cells[0].source = [analyzeCommand.replace('{0}', hdfsPath)];
-				editor.edit(editBuilder => {
+				await editor.edit(editBuilder => {
 					editBuilder.replace(0, {
 						cell_type: 'code',
 						source: analyzeCommand.replace('{0}', hdfsPath)

extensions/notebook/src/jupyter/jupyterNotebookProvider.ts

@@ -61,7 +61,7 @@ export class JupyterNotebookProvider implements nb.NotebookProvider {
 			let sessionManager = (manager.sessionManager as JupyterSessionManager);
 			let session = sessionManager.listRunning().find(e => e.path === notebookUri.fsPath);
 			if (session) {
-				manager.sessionManager.shutdown(session.id);
+				manager.sessionManager.shutdown(session.id).then(undefined, err => console.error('Error shutting down session after notebook closed ', err));
 			}
 			if (sessionManager.listRunning().length === 0) {
 				let notebookConfig: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(constants.notebookConfigKey);

extensions/notebook/src/jupyter/jupyterServerInstallation.ts

@@ -509,7 +509,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation {
 		// If the latest version of ADS-Python is not installed, then prompt the user to upgrade
 		if (!this._upgradePrompted && isPythonInstalled && !this._usingExistingPython && utils.compareVersions(await this.getInstalledPythonVersion(this._pythonExecutable), constants.pythonVersion) < 0) {
 			this._upgradePrompted = true;
-			this.promptUserForPythonUpgrade();
+			await this.promptUserForPythonUpgrade();
 		}
 
 		let areRequiredPackagesInstalled = await this.areRequiredPackagesInstalled(kernelDisplayName);

extensions/notebook/src/jupyter/serverInstance.ts

@@ -276,7 +276,7 @@ export class PerFolderServerInstance implements IServerInstance {
 		let action = this.errorHandler.handleError();
 		if (action === ErrorAction.Shutdown) {
 			this.notify(this.options.install, localize('jupyterError', "Error sent from Jupyter: {0}", utils.getErrorMessage(error)));
-			this.stop();
+			this.stop().catch(err => console.log('Error stopping Jupyter Server ', err));
 		}
 	}
 	private handleConnectionClosed(): void {

extensions/notebook/src/test/book/bookPinManager.test.ts

@@ -146,7 +146,7 @@ describe('BookPinManagerTests', function () {
 			should(isNotebookPinnedBeforeChange).be.false('Notebook should NOT be pinned');
 
 			// mock pin book item from viewlet
-			bookPinManager.pinNotebook(books[0].bookItems[1]);
+			await bookPinManager.pinNotebook(books[0].bookItems[1]);
 
 			let isNotebookPinnedAfterChange = isBookItemPinned(notebookUri);
 			should(isNotebookPinnedAfterChange).be.true('Notebook should be pinned');
@@ -158,7 +158,7 @@ describe('BookPinManagerTests', function () {
 
 			should(isNotebookPinned).be.true('Notebook should be pinned');
 
-			bookPinManager.unpinNotebook(books[0].bookItems[0]);
+			await bookPinManager.unpinNotebook(books[0].bookItems[0]);
 			let isNotebookPinnedAfterChange = isBookItemPinned(notebookUri);
 
 			should(isNotebookPinnedAfterChange).be.false('Notebook should not be pinned after notebook is unpinned');

extensions/notebook/src/test/book/githubRemoteBook.test.ts

@@ -31,7 +31,7 @@ describe('Github Remote Book', function () {
 
 	it('Verify GitHub Remote Book is created by controller', async function (): Promise<void> {
 		let releaseURL = vscode.Uri.parse('https://api.github.com/repos/microsoft/test/releases/v1');
-		let asset : IAsset = {
+		let asset: IAsset = {
 			name: 'CU-1.0-EN.zip',
 			book: 'CU',
 			version: '1.0',
@@ -41,7 +41,13 @@ describe('Github Remote Book', function () {
 			browserDownloadUrl: vscode.Uri.parse('https://github.com/microsoft/test/releases/download/v1/CU-1.0-EN.zip'),
 		};
 		let remoteLocation = loc.onGitHub;
-		controller.setRemoteBook(releaseURL, remoteLocation, asset);
+		nock('https://github.com')
+			.persist()
+			.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
+			.replyWithFile(200, __filename);
+		// Aren't returning an actual zip so just stub this out since we don't care about actually testing that functionality currently
+		sinon.stub(GitHubRemoteBook.prototype, 'extractFiles').resolves();
+		await controller.setRemoteBook(releaseURL, remoteLocation, asset);
 		should(controller.model.remoteBook).not.null();
 		should(controller.model.remoteBook instanceof GitHubRemoteBook).be.true();
 		let book = model.remoteBook as GitHubRemoteBook;
@@ -50,7 +56,7 @@ describe('Github Remote Book', function () {
 
 	it('Verify set local path is called when creating a GitHub Remote Book', async function (): Promise<void> {
 		let releaseURL = vscode.Uri.parse('https://api.github.com/repos/microsoft/test/releases/v1');
-		let asset : IAsset = {
+		let asset: IAsset = {
 			name: 'CU-1.0-EN.zip',
 			book: 'CU',
 			version: '1.0',
@@ -60,16 +66,22 @@ describe('Github Remote Book', function () {
 			browserDownloadUrl: vscode.Uri.parse('https://github.com/microsoft/test/releases/download/v1/CU-1.0-EN.zip'),
 		};
 		let remoteLocation = loc.onGitHub;
+		nock('https://github.com')
+			.persist()
+			.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
+			.replyWithFile(200, __filename);
+		// Aren't returning an actual zip so just stub this out since we don't care about actually testing that functionality currently
 		const createCopySpy = sinon.spy(GitHubRemoteBook.prototype, 'createLocalCopy');
+		sinon.stub(GitHubRemoteBook.prototype, 'extractFiles').resolves();
 		const setPathSpy = sinon.spy(RemoteBook.prototype, 'setLocalPath');
-		controller.setRemoteBook(releaseURL, remoteLocation, asset);
-		should(createCopySpy.calledOnce).be.true();
-		should(setPathSpy.calledOnce).be.true();
+		await controller.setRemoteBook(releaseURL, remoteLocation, asset);
+		should(createCopySpy.calledOnce).be.true('createLocalCopy not called');
+		should(setPathSpy.calledOnce).be.true('setLocalPath not called');
 	});
 
 	it('Should download contents from Github', async function (): Promise<void> {
 		let releaseURL = vscode.Uri.parse('https://api.github.com/repos/microsoft/test/releases/v1');
-		let asset : IAsset = {
+		let asset: IAsset = {
 			name: 'CU-1.0-EN.zip',
 			book: 'CU',
 			version: '1.0',
@@ -79,18 +91,19 @@ describe('Github Remote Book', function () {
 			browserDownloadUrl: vscode.Uri.parse('https://github.com/microsoft/test/releases/download/v1/CU-1.0-EN.zip'),
 		};
 		let remoteLocation = loc.onGitHub;
-		controller.setRemoteBook(releaseURL, remoteLocation, asset);
+		const setExtractSpy = sinon.spy(GitHubRemoteBook.prototype, 'extractFiles');
+		nock('https://github.com')
+			.persist()
+			.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
+			.replyWithFile(200, __filename);
+		await controller.setRemoteBook(releaseURL, remoteLocation, asset);
 
 		model.remoteBook.localPath = vscode.Uri.file(os.tmpdir());
 		let setPathStub = sinon.stub(GitHubRemoteBook.prototype, 'setLocalPath');
-		setPathStub.callsFake(function() {
+		setPathStub.callsFake(function () {
 			console.log(`Downloading book in ${model.remoteBook.localPath}`);
 		});
-		const setExtractSpy = sinon.spy(GitHubRemoteBook.prototype, 'extractFiles');
-		nock('https://github.com')
-				.persist()
-				.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
-				.replyWithFile(200, __filename);
+
 		await model.remoteBook.createLocalCopy();
 		should(setExtractSpy.calledOnceWith(vscode.Uri.file(model.remoteBook.localPath.fsPath)));
 		await fs.promises.stat(model.remoteBook.localPath.fsPath);
@@ -98,18 +111,18 @@ describe('Github Remote Book', function () {
 
 	it('Should reject if unexpected error', async function (): Promise<void> {
 		nock('https://github.com')
-				.persist()
-				.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
-				.replyWithError(new Error('Unexpected Error'));
+			.persist()
+			.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
+			.replyWithError(new Error('Unexpected Error'));
 		await should(model.remoteBook.createLocalCopy()).be.rejected();
 	});
 
 	it('Should reject if response status code is not 200', async function (): Promise<void> {
 		nock('https://github.com')
-				.persist()
-				.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
-				.reply(404);
-		const createLocalCopy =  model.remoteBook.createLocalCopy();
+			.persist()
+			.get('/microsoft/test/releases/download/v1/CU-1.0-EN.zip')
+			.reply(404);
+		const createLocalCopy = model.remoteBook.createLocalCopy();
 		await should(createLocalCopy).be.rejected();
 	});
 });

extensions/notebook/src/test/common/notebookUtils.test.ts

@@ -82,7 +82,7 @@ describe('notebookUtils Tests', function (): void {
 				should(azdata.nb.notebookDocuments.find(doc => doc.fileName === notebookUri.fsPath)).not.be.undefined();
 				await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
 			} finally {
-				tryDeleteFile(notebookPath);
+				await tryDeleteFile(notebookPath);
 			}
 		});
 

extensions/notebook/src/test/managePackages/managePackagesDialogModel.test.ts

@@ -55,7 +55,7 @@ describe('Manage Packages', () => {
 		await should(model.installPackages([])).rejected();
 		await should(model.uninstallPackages([])).rejected();
 		should.equal(await model.getLocations(), undefined, 'Get Locations should be undefined before provider is set');
-		should(model.getPackageOverview('package')).rejected();
+		await should(model.getPackageOverview('package')).rejected();
 
 		// Change provider and then retest functions which throw without valid provider
 		model.changeProvider(provider.providerId);
@@ -63,7 +63,7 @@ describe('Manage Packages', () => {
 		await should(model.installPackages([])).resolved();
 		await should(model.uninstallPackages([])).resolved();
 		should.deepEqual(await model.getLocations(), await provider.getLocations(), 'Get Locations should be valid after provider is set');
-		should(model.getPackageOverview('p1')).resolved();
+		await should(model.getPackageOverview('p1')).resolved();
 		model.changeLocation('location1');
 
 	});

extensions/notebook/src/test/model/jupyterController.test.ts

@@ -78,7 +78,7 @@ describe('Jupyter Controller', function () {
 		await controller.activate();
 		should(controller.notebookProvider.standardKernels).deepEqual([], 'Notebook provider standard kernels should return empty array');
 		should(controller.notebookProvider.providerId).equal('jupyter', 'Notebook provider should be jupyter');
-		should(controller.notebookProvider.getNotebookManager(undefined)).be.rejected();
+		await should(controller.notebookProvider.getNotebookManager(undefined)).be.rejected();
 		should(controller.notebookProvider.notebookManagerCount).equal(0);
 		controller.notebookProvider.handleNotebookClosed(undefined);
 	});

extensions/notebook/src/test/model/kernel.test.ts

@@ -146,7 +146,7 @@ describe('Jupyter Future', function (): void {
 			})
 		});
 		should(handler).not.be.undefined();
-		verifyRelayMessage('shell', handler, () => msg);
+		await verifyRelayMessage('shell', handler, () => msg);
 
 	});
 
@@ -162,7 +162,7 @@ describe('Jupyter Future', function (): void {
 			})
 		});
 		should(handler).not.be.undefined();
-		verifyRelayMessage('stdin', handler, () => msg);
+		await verifyRelayMessage('stdin', handler, () => msg);
 	});
 
 	it('should relay IOPub message', async function (): Promise<void> {
@@ -177,11 +177,11 @@ describe('Jupyter Future', function (): void {
 			})
 		});
 		should(handler).not.be.undefined();
-		verifyRelayMessage('iopub', handler, () => msg);
+		await verifyRelayMessage('iopub', handler, () => msg);
 	});
 
-	function verifyRelayMessage(channel: nb.Channel | KernelMessage.Channel, handler: (msg: KernelMessage.IMessage) => void | PromiseLike<void>, getMessage: () => nb.IMessage): void {
-		handler({
+	async function verifyRelayMessage(channel: nb.Channel | KernelMessage.Channel, handler: (msg: KernelMessage.IMessage) => void | PromiseLike<void>, getMessage: () => nb.IMessage): Promise<void> {
+		await handler({
 			channel: <any>channel,
 			content: { value: 'test' },
 			metadata: { value: 'test' },

extensions/notebook/src/test/model/sessionManager.test.ts

@@ -65,7 +65,7 @@ describe('Jupyter Session Manager', function (): void {
 		sessionManager.ready.then(() => {
 			should(sessionManager.isReady).be.true();
 			done();
-		});
+		}).catch(err => done(err));
 	});
 
 	it('should passthrough the ready calls', function (done): void {
@@ -75,7 +75,7 @@ describe('Jupyter Session Manager', function (): void {
 
 		// When I wait on the ready method before completing
 		sessionManager.setJupyterSessionManager(mockJupyterManager.object);
-		sessionManager.ready.then(() => done());
+		sessionManager.ready.then(() => done()).catch(err => done(err));
 
 		// Then session manager should eventually resolve
 		deferred.resolve();