Show modal error when remote jupyter connect fails (#8476)

news/3 Code Health/8474.md

@@ -0,0 +1 @@
+Display error message when failing to connect to the remote Jupyter server.

package.nls.json

@@ -278,6 +278,8 @@
     "DataScience.jupyterNotebookConnectFailed": "Failed to connect to Jupyter notebook. \r\n{0}\r\n{1}",
     "DataScience.jupyterNotebookRemoteConnectFailed": "Failed to connect to remote Jupyter notebook.\r\nCheck that the Jupyter Server URI setting has a valid running server specified.\r\n{0}\r\n{1}",
     "DataScience.jupyterNotebookRemoteConnectSelfCertsFailed": "Failed to connect to remote Jupyter notebook.\r\nSpecified server is using self signed certs. Enable Allow Unauthorized Remote Connection setting to connect anyways\r\n{0}\r\n{1}",
+    "DataScience.jupyterRemoteConnectFailedModalMessage": "Failed to connect to the remote Jupyter Server. View Jupyter log for further details.",
+    "DataScience.changeJupyterRemoteConnection": "Change Jupyter Server connection.",
     "DataScience.notebookVersionFormat": "Jupyter Notebook Version: {0}",
     "DataScience.jupyterKernelSpecNotFound": "Cannot create a Jupyter kernel spec and none are available for use",
     "DataScience.jupyterKernelSpecModuleNotFound": "'Kernelspec' module not installed in the selected interpreter ({0}).\n Please re-install or update 'jupyter'.",

src/client/common/utils/localize.ts

@@ -521,6 +521,14 @@ export namespace DataScience {
         'DataScience.jupyterNotebookRemoteConnectFailed',
         'Failed to connect to remote Jupyter notebook.\r\nCheck that the Jupyter Server URI setting has a valid running server specified.\r\n{0}\r\n{1}'
     );
+    export const jupyterRemoteConnectFailedModalMessage = localize(
+        'DataScience.jupyterRemoteConnectFailedModalMessage',
+        'Failed to connect to the remote Jupyter Server. View Jupyter log for further details.'
+    );
+    export const changeJupyterRemoteConnection = localize(
+        'DataScience.changeJupyterRemoteConnection',
+        'Change Jupyter Server connection.'
+    );
     export const jupyterNotebookRemoteConnectSelfCertsFailed = localize(
         'DataScience.jupyterNotebookRemoteConnectSelfCertsFailed',
         'Failed to connect to remote Jupyter notebook.\r\nSpecified server is using self signed certs. Enable Allow Unauthorized Remote Connection setting to connect anyways\r\n{0}\r\n{1}'

src/client/datascience/interactive-common/notebookServerProvider.ts

@@ -39,7 +39,16 @@ export class NotebookServerProvider implements IJupyterServerProvider {
         @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage,
         @inject(JupyterServerSelector) private serverSelector: JupyterServerSelector,
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry
-    ) {}
+    ) {
+        serverUriStorage.onDidChangeUri(
+            () => {
+                // Possible user selected another Server.
+                this.serverPromise.remote = undefined;
+            },
+            this,
+            this.disposables
+        );
+    }
     public async getOrCreateServer(options: GetServerOptions): Promise<INotebookServer | undefined> {
         const serverOptions = await this.getNotebookServerOptions(options.resource, options.localJupyter === true);
 

src/client/datascience/jupyter/serverSelector.ts

@@ -37,7 +37,8 @@ export type SelectJupyterUriCommandSource =
     | 'toolbar'
     | 'commandPalette'
     | 'nativeNotebookStatusBar'
-    | 'nativeNotebookToolbar';
+    | 'nativeNotebookToolbar'
+    | 'prompt';
 @injectable()
 export class JupyterServerSelector {
     private readonly localLabel = `$(zap) ${DataScience.jupyterSelectURILocalLabel()}`;

src/client/datascience/notebook/notebookControllerManager.ts

@@ -57,6 +57,8 @@ import { IInterpreterService } from '../../interpreter/contracts';
 import { KernelFilterService } from './kernelFilter/kernelFilterService';
 import { getDisplayPath } from '../../common/platform/fs-paths';
 import { DisplayOptions } from '../displayOptions';
+import { JupyterServerSelector } from '../jupyter/serverSelector';
+import { DataScience } from '../../common/utils/localize';
 
 /**
  * This class tracks notebook documents that are open and the provides NotebookControllers for
@@ -77,6 +79,7 @@ export class NotebookControllerManager implements INotebookControllerManager, IE
     private selectedControllers = new Map<string, VSCodeNotebookController>();
     private readonly allKernelConnections = new Set<KernelConnectionMetadata>();
     private _controllersLoaded?: boolean;
+    private failedToFetchRemoteKernels?: boolean;
     public get onNotebookControllerSelectionChanged() {
         return this._onNotebookControllerSelectionChanged.event;
     }
@@ -119,6 +122,7 @@ export class NotebookControllerManager implements INotebookControllerManager, IE
         @inject(IApplicationShell) private readonly appShell: IApplicationShell,
         @inject(KernelFilterService) private readonly kernelFilter: KernelFilterService,
         @inject(IBrowserService) private readonly browser: IBrowserService,
+        @inject(JupyterServerSelector) private readonly jupyterServerSelector: JupyterServerSelector,
         @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage
     ) {
         this._onNotebookControllerSelected = new EventEmitter<{
@@ -453,7 +457,32 @@ export class NotebookControllerManager implements INotebookControllerManager, IE
         // Prep so that we can track the selected controller for this document
         traceInfoIfCI(`Clear controller mapping for ${getDisplayPath(document.uri)}`);
         const loadControllersPromise = this.loadNotebookControllers();
-
+        if (!this.isLocalLaunch) {
+            void loadControllersPromise.finally(() => {
+                if (this.isLocalLaunch) {
+                    return;
+                }
+                if (this.failedToFetchRemoteKernels) {
+                    void this.appShell
+                        .showErrorMessage(
+                            DataScience.jupyterRemoteConnectFailedModalMessage(),
+                            { modal: true },
+                            DataScience.changeJupyterRemoteConnection(),
+                            DataScience.showJupyterLogs()
+                        )
+                        .then((selection) => {
+                            switch (selection) {
+                                case DataScience.changeJupyterRemoteConnection():
+                                    void this.jupyterServerSelector.selectJupyterURI(true, 'prompt');
+                                    break;
+                                case DataScience.showJupyterLogs():
+                                    void this.commandManager.executeCommand('jupyter.viewOutput');
+                                    break;
+                            }
+                        });
+                }
+            });
+        }
         if (
             isPythonNotebook(getNotebookMetadata(document)) &&
             this.extensionChecker.isPythonExtensionInstalled &&
@@ -724,11 +753,11 @@ export class NotebookControllerManager implements INotebookControllerManager, IE
                 token
             });
 
-            return this.remoteKernelFinder.listKernels(undefined, connection, token).catch((ex) => {
-                traceError('Failed to get remote kernel connections', ex);
-                return [] as KernelConnectionMetadata[];
-            });
+            const kernels = await this.remoteKernelFinder.listKernels(undefined, connection, token);
+            this.failedToFetchRemoteKernels = false;
+            return kernels;
         } catch (ex) {
+            this.failedToFetchRemoteKernels = true;
             traceError('Failed to get remote kernel connections', ex);
             return [] as KernelConnectionMetadata[];
         } finally {

src/client/telemetry/index.ts

@@ -870,7 +870,13 @@ export interface IEventNamePropertyMapping {
          * nativeNotebookStatusBar - Invoked from Native notebook statusbar.
          * nativeNotebookToolbar - Invoked from Native notebook toolbar.
          */
-        commandSource: 'nonUser' | 'commandPalette' | 'toolbar' | 'nativeNotebookStatusBar' | 'nativeNotebookToolbar';
+        commandSource:
+            | 'nonUser'
+            | 'commandPalette'
+            | 'toolbar'
+            | 'nativeNotebookStatusBar'
+            | 'nativeNotebookToolbar'
+            | 'prompt';
     };
     [Telemetry.SetJupyterURIToLocal]: never | undefined;
     [Telemetry.SetJupyterURIToUserSpecified]: {

src/test/datascience/interactive-common/notebookServerProvider.unit.test.ts

@@ -4,7 +4,7 @@ import { expect } from 'chai';
 import { SemVer } from 'semver';
 import { anything, instance, mock, verify, when } from 'ts-mockito';
 import * as typemoq from 'typemoq';
-import { CancellationTokenSource, Disposable } from 'vscode';
+import { CancellationTokenSource, Disposable, EventEmitter } from 'vscode';
 import { disposeAllDisposables } from '../../../client/common/helpers';
 import { IConfigurationService, IWatchableJupyterSettings } from '../../../client/common/types';
 import { DisplayOptions } from '../../../client/datascience/displayOptions';
@@ -52,6 +52,9 @@ suite('DataScience - NotebookServerProvider', () => {
         const serverStorage = mock(JupyterServerUriStorage);
         when(serverStorage.getUri()).thenResolve('local');
         const serverSelector = mock(JupyterServerSelector);
+        const eventEmitter = new EventEmitter<void>();
+        disposables.push(eventEmitter);
+        when(serverStorage.onDidChangeUri).thenReturn(eventEmitter.event);
         when((jupyterExecution as any).then).thenReturn(undefined);
         when((serverSelector as any).then).thenReturn(undefined);
         when((serverStorage as any).then).thenReturn(undefined);

src/test/datascience/notebook/kernelSelection.vscode.test.ts

@@ -31,7 +31,8 @@ import {
     waitForKernelToChange,
     waitForKernelToGetAutoSelected,
     waitForOutputs,
-    waitForTextOutput
+    waitForTextOutput,
+    defaultNotebookTestTimeout
 } from './helper';
 
 /* eslint-disable no-invalid-this, , , @typescript-eslint/no-explicit-any */
@@ -61,7 +62,7 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () {
     const venvNoKernelSearchString = '.venvnokernel';
     const venvKernelSearchString = '.venvkernel';
     const venvNoRegSearchString = '.venvnoreg';
-    let activeIntepreterSearchString = '';
+    let activeInterpreterSearchString = '';
     let vscodeNotebook: IVSCodeNotebook;
     this.timeout(120_000); // Slow test, we need to uninstall/install ipykernel.
     /*
@@ -110,7 +111,7 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () {
         venvKernelPythonPath = interpreter2.path;
         venvNoRegPythonPath = interpreter3.path;
         venvNoKernelDisplayName = interpreter1.displayName || '.venvnokernel';
-        activeIntepreterSearchString =
+        activeInterpreterSearchString =
             activeInterpreter.displayName === interpreter1.displayName
                 ? venvNoKernelSearchString
                 : activeInterpreter.displayName === interpreter2.displayName
@@ -165,21 +166,30 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () {
         const cell = vscodeNotebook.activeNotebookEditor?.document.cellAt(0)!;
         await Promise.all([runAllCellsInActiveNotebook(), waitForExecutionCompletedSuccessfully(cell)]);
 
-        // Confirm the executable printed as a result of code in cell `import sys;sys.executable`
-        const output = getTextOutputValue(cell.outputs[0]);
-        if (
-            !output.includes(activeIntepreterSearchString) &&
-            !output.includes(getNormalizedInterpreterPath(activeInterpreterPath)) &&
-            !output.includes(activeInterpreterPath)
-        ) {
-            assert.fail(
-                output,
-                `Expected ${getNormalizedInterpreterPath(activeInterpreterPath)} or ${activeInterpreterPath}`,
-                `Interpreter does not match for ${activeIntepreterSearchString}: expected ${getNormalizedInterpreterPath(
-                    activeInterpreterPath
-                )} or ${activeInterpreterPath}, but go ${output}`
-            );
-        }
+        await waitForCondition(
+            async () => {
+                // Confirm the executable printed as a result of code in cell `import sys;sys.executable`
+                const output = getTextOutputValue(cell.outputs[0]);
+                if (
+                    !output.includes(activeInterpreterSearchString) &&
+                    !output.includes(getNormalizedInterpreterPath(activeInterpreterPath)) &&
+                    !output.includes(activeInterpreterPath)
+                ) {
+                    assert.fail(
+                        output,
+                        `Expected ${getNormalizedInterpreterPath(activeInterpreterPath)} or ${activeInterpreterPath}`,
+                        `Interpreter does not match for ${activeInterpreterSearchString}: expected ${getNormalizedInterpreterPath(
+                            activeInterpreterPath
+                        )} or ${activeInterpreterPath}, but go ${output}`
+                    );
+                }
+                return true;
+            },
+            defaultNotebookTestTimeout,
+            `Interpreter does not match for ${activeInterpreterSearchString}: expected ${getNormalizedInterpreterPath(
+                activeInterpreterPath
+            )} or ${activeInterpreterPath}, but go ${getTextOutputValue(cell.outputs[0])}`
+        );
     });
     test('Ensure kernel is auto selected and interpreter is as expected', async function () {
         if (IS_REMOTE_NATIVE_TEST) {
@@ -265,7 +275,7 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () {
         const outputText = getTextOutputValue(cell.outputs[0]).trim();
 
         // venvkernel might be the active one (if this test is run more than once)
-        if (activeIntepreterSearchString !== venvKernelSearchString) {
+        if (activeInterpreterSearchString !== venvKernelSearchString) {
             assert.equal(outputText.toLowerCase().indexOf(venvKernelSearchString), -1);
         }