Notebook Extension: First logging improvements (#13729)

* First logging improvements

* PR feedback for err output

extensions/notebook/src/common/utils.ts

@@ -262,7 +262,7 @@ export function getIgnoreSslVerificationConfigSetting(): boolean {
 		const config = vscode.workspace.getConfiguration(bdcConfigSectionName);
 		return config.get<boolean>(ignoreSslConfigName, true);
 	} catch (error) {
-		console.error(`Unexpected error retrieving ${bdcConfigSectionName}.${ignoreSslConfigName} setting : ${error}`);
+		console.error('Unexpected error retrieving ${bdcConfigSectionName}.${ignoreSslConfigName} setting : ', error);
 	}
 	return true;
 }

extensions/notebook/src/dialog/configurePython/configurePythonWizard.ts

@@ -5,6 +5,7 @@
 
 import * as nls from 'vscode-nls';
 import * as azdata from 'azdata';
+import * as vscode from 'vscode';
 import { BasePage } from './basePage';
 import { ConfigurePathPage } from './configurePathPage';
 import { PickPackagesPage } from './pickPackagesPage';
@@ -36,9 +37,9 @@ export class ConfigurePythonWizard {
 	private _setupComplete: Deferred<void>;
 	private pythonPathsPromise: Promise<PythonPathInfo[]>;
 
-	constructor(private jupyterInstallation: JupyterServerInstallation) {
+	constructor(private jupyterInstallation: JupyterServerInstallation, private readonly _outputChannel: vscode.OutputChannel) {
 		this._setupComplete = new Deferred<void>();
-		this.pythonPathsPromise = (new PythonPathLookup()).getSuggestions();
+		this.pythonPathsPromise = (new PythonPathLookup(this._outputChannel)).getSuggestions();
 	}
 
 	public get wizard(): azdata.window.Wizard {
@@ -49,7 +50,7 @@ export class ConfigurePythonWizard {
 		return this._setupComplete.promise;
 	}
 
-	public async start(kernelName?: string, rejectOnCancel?: boolean, ...args: any[]): Promise<void> {
+	public async start(kernelName?: string, rejectOnCancel?: boolean): Promise<void> {
 		this.model = <ConfigurePythonModel>{
 			kernelName: kernelName,
 			pythonPathsPromise: this.pythonPathsPromise,

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

@@ -111,6 +111,8 @@ export class AddNewPackageTab {
 				placeHolder: this.SearchPlaceholder(this.dialog.model.currentPackageType)
 			});
 			await this.setFieldsToEmpty();
+		} catch (err) {
+			console.error('Exception encountered when resetting new package page fields: ', err);
 		} finally {
 			await this.toggleNewPackagesFields(true);
 		}

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

@@ -65,7 +65,6 @@ export class InstalledPackagesTab {
 				}
 				catch (err) {
 					this.dialog.showErrorMessage(utils.getErrorMessage(err));
-
 				}
 
 			});

extensions/notebook/src/dialog/pythonPathLookup.ts

@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as glob from 'glob';
+import * as vscode from 'vscode';
 
 import * as utils from '../common/utils';
 import * as constants from '../common/constants';
@@ -15,7 +16,7 @@ export interface PythonPathInfo {
 
 export class PythonPathLookup {
 	private condaLocations: string[];
-	constructor() {
+	constructor(private readonly _outputChannel: vscode.OutputChannel) {
 		if (process.platform !== constants.winPlatform) {
 			let userFolder = process.env['HOME'];
 			this.condaLocations = [
@@ -56,6 +57,7 @@ export class PythonPathLookup {
 			let condaFiles = condaResults.reduce((first, second) => first.concat(second));
 			return condaFiles.filter(condaPath => condaPath && condaPath.length > 0);
 		} catch (err) {
+			this._outputChannel.appendLine(`Problem encountered getting Conda installations: ${err}`);
 		}
 		return [];
 	}
@@ -95,7 +97,7 @@ export class PythonPathLookup {
 				return value;
 			}
 		} catch (err) {
-			// Ignore errors here, since this python version will just be excluded.
+			this._outputChannel.appendLine(`Problem encountered getting Python path: ${err}`);
 		}
 
 		return undefined;
@@ -158,7 +160,7 @@ export class PythonPathLookup {
 				};
 			}
 		} catch (err) {
-			// Ignore errors here, since this python version will just be excluded.
+			this._outputChannel.appendLine(`Problem encountered getting Python info for path: ${err}`);
 		}
 		return undefined;
 	}

extensions/notebook/src/extension.ts

@@ -114,6 +114,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
 				cellType = selection.id;
 			}
 		} catch (err) {
+			console.error('Unexpected error adding new cell: ', err);
 			return;
 		}
 		if (cellType) {

extensions/notebook/src/intellisense/completionItemProvider.ts

@@ -57,8 +57,8 @@ export class NotebookCompletionItemProvider implements vscode.CompletionItemProv
 					return session.kernel;
 				}
 			}
-		} catch {
-			// If an exception occurs, swallow it currently
+		} catch (err) {
+			console.error('Exception encountered finding document kernel: ', err);
 			return undefined;
 		}
 		return undefined;
@@ -194,4 +194,4 @@ export interface INewIntellisenseInfo {
 	cell: nb.NotebookCell;
 	notebook: nb.NotebookDocument;
 	kernel?: nb.IKernel;
-}
+}

extensions/notebook/src/jupyter/jupyterController.ts

@@ -234,7 +234,7 @@ export class JupyterController {
 	}
 
 	public doConfigurePython(jupyterInstaller: JupyterServerInstallation): void {
-		let pythonWizard = new ConfigurePythonWizard(jupyterInstaller);
+		let pythonWizard = new ConfigurePythonWizard(jupyterInstaller, this.appContext.outputChannel);
 		pythonWizard.start().catch((err: any) => {
 			vscode.window.showErrorMessage(utils.getErrorMessage(err));
 		});

extensions/notebook/src/jupyter/jupyterServerInstallation.ts

@@ -435,7 +435,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation {
 		let isPythonInstalled = JupyterServerInstallation.isPythonInstalled();
 		let areRequiredPackagesInstalled = await this.areRequiredPackagesInstalled(kernelDisplayName);
 		if (!isPythonInstalled || !areRequiredPackagesInstalled) {
-			let pythonWizard = new ConfigurePythonWizard(this);
+			let pythonWizard = new ConfigurePythonWizard(this, this.outputChannel);
 			await pythonWizard.start(kernelDisplayName, true);
 			return pythonWizard.setupComplete.then(() => {
 				this._kernelSetupCache.set(kernelDisplayName, true);

extensions/notebook/src/jupyter/jupyterSessionManager.ts

@@ -184,7 +184,7 @@ export class JupyterSession implements nb.ISession {
 		skipSettingEnvironmentVars?: boolean,
 		private _pythonEnvVarPath?: string) {
 		this.setEnvironmentVars(skipSettingEnvironmentVars).catch(error => {
-			console.error(`Unexpected exception setting Jupyter Session variables : ${error}`);
+			console.error('Unexpected exception setting Jupyter Session variables : ', error);
 			// We don't want callers to hang forever waiting - it's better to continue on even if we weren't
 			// able to set environment variables
 			this._messagesComplete.resolve();
@@ -240,7 +240,7 @@ export class JupyterSession implements nb.ISession {
 				await this._installation.promptForPythonInstall(kernelInfo.display_name);
 			} catch (err) {
 				// Have to swallow the error here to prevent hangs when changing back to the old kernel.
-				console.error(err.toString());
+				console.error('Exception encountered prompting for Python install', err);
 				return this._kernel;
 			}
 		}

extensions/notebook/src/jupyter/serverInstance.ts

@@ -279,7 +279,7 @@ export class PerFolderServerInstance implements IServerInstance {
 	}
 
 	private handleConnectionError(error: Error): void {
-		let action = this.errorHandler.handleError(error);
+		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();
@@ -365,7 +365,7 @@ export class PerFolderServerInstance implements IServerInstance {
 class ErrorHandler {
 	private numErrors: number = 0;
 
-	public handleError(error: Error): ErrorAction {
+	public handleError(): ErrorAction {
 		this.numErrors++;
 		return this.numErrors > 3 ? ErrorAction.Shutdown : ErrorAction.Continue;
 	}

extensions/notebook/src/test/python/configurePython.test.ts

@@ -13,13 +13,16 @@ import { PickPackagesPage } from '../../dialog/configurePython/pickPackagesPage'
 import { python3DisplayName, allKernelsName } from '../../common/constants';
 import { TestContext, createViewContext, TestButton } from '../common';
 import { EventEmitter } from 'vscode';
+import { MockOutputChannel } from '../common/stubs';
 
 describe('Configure Python Wizard', function () {
 	let testWizard: ConfigurePythonWizard;
 	let viewContext: TestContext;
 	let testInstallation: JupyterServerInstallation;
+	let mockOutputChannel: TypeMoq.IMock<MockOutputChannel>;
 
 	beforeEach(() => {
+		mockOutputChannel = TypeMoq.Mock.ofType(MockOutputChannel);
 		let mockInstall = TypeMoq.Mock.ofType(JupyterServerInstallation);
 		mockInstall.setup(i => i.getInstalledPipPackages(TypeMoq.It.isAnyString())).returns(() => Promise.resolve([]));
 		mockInstall.setup(i => i.getRequiredPackagesForKernel(TypeMoq.It.isAnyString())).returns(() => [{ name: 'TestPkg', version: '1.0.0'}]);
@@ -41,21 +44,21 @@ describe('Configure Python Wizard', function () {
 	});
 
 	it('Start wizard test', async () => {
-		let wizard = new ConfigurePythonWizard(testInstallation);
+		let wizard = new ConfigurePythonWizard(testInstallation, mockOutputChannel.object);
 		await wizard.start();
 		await wizard.close();
 		await should(wizard.setupComplete).be.resolved();
 	});
 
 	it('Reject setup on cancel test', async () => {
-		let wizard = new ConfigurePythonWizard(testInstallation);
+		let wizard = new ConfigurePythonWizard(testInstallation, mockOutputChannel.object);
 		await wizard.start(undefined, true);
 		await wizard.close();
 		await should(wizard.setupComplete).be.rejected();
 	});
 
 	it('Error message test', async () => {
-		let wizard = new ConfigurePythonWizard(testInstallation);
+		let wizard = new ConfigurePythonWizard(testInstallation, mockOutputChannel.object);
 		await wizard.start();
 
 		should(wizard.wizard.message).be.undefined();