Skip to content

Commit

Permalink
Refactor RemoteURI storage and related
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonJayamanne
DonJayamanne committed May 29, 2023
1 parent c37e3c0 commit 00cc269
Show file tree
Hide file tree
Showing 18 changed files with 556 additions and 488 deletions.
10 changes: 0 additions & 10 deletions src/gdpr.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -967,16 +967,6 @@
"${include}": [
"${F1}"
]
}
*/
//Telemetry.SetJupyterURIToUserSpecified
/* __GDPR__
"DATASCIENCE.SET_JUPYTER_URI_USER_SPECIFIED" : {
"azure": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"Was the URI set to an Azure uri.","owner":"donjayamanne"},
"${include}": [
"${F1}"
]
}
*/
Expand Down
50 changes: 46 additions & 4 deletions src/kernels/jupyter/connection/jupyterConnection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify';
import { noop } from '../../../platform/common/utils/misc';
import { RemoteJupyterServerUriProviderError } from '../../errors/remoteJupyterServerUriProviderError';
import { BaseError } from '../../../platform/errors/types';
import { createRemoteConnectionInfo } from '../jupyterUtils';
import { createRemoteConnectionInfo, handleExpiredCertsError, handleSelfCertsError } from '../jupyterUtils';
import {
IJupyterServerUri,
IJupyterServerUriStorage,
Expand All @@ -14,6 +14,15 @@ import {
IJupyterUriProviderRegistration,
JupyterServerProviderHandle
} from '../types';
import { IDataScienceErrorHandler } from '../../errors/types';
import { IApplicationShell } from '../../../platform/common/application/types';
import { IConfigurationService } from '../../../platform/common/types';
import { Telemetry, sendTelemetryEvent } from '../../../telemetry';
import { JupyterSelfCertsExpiredError } from '../../../platform/errors/jupyterSelfCertsExpiredError';
import { JupyterInvalidPasswordError } from '../../errors/jupyterInvalidPassword';
import { RemoteJupyterServerConnectionError } from '../../../platform/errors/remoteJupyterServerConnectionError';
import { traceError } from '../../../platform/logging';
import { JupyterSelfCertsError } from '../../../platform/errors/jupyterSelfCertsError';

/**
* Creates IJupyterConnection objects for URIs and 3rd party handles/ids.
Expand All @@ -25,7 +34,11 @@ export class JupyterConnection {
private readonly jupyterPickerRegistration: IJupyterUriProviderRegistration,
@inject(IJupyterSessionManagerFactory)
private readonly jupyterSessionManagerFactory: IJupyterSessionManagerFactory,
@inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage
@inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage,
@inject(IDataScienceErrorHandler)
private readonly errorHandler: IDataScienceErrorHandler,
@inject(IApplicationShell) private readonly applicationShell: IApplicationShell,
@inject(IConfigurationService) private readonly configService: IConfigurationService
) {}

public async createConnectionInfo(serverHandle: JupyterServerProviderHandle) {
Expand All @@ -37,9 +50,10 @@ export class JupyterConnection {
return createRemoteConnectionInfo(serverHandle, serverUri);
}

public async validateRemoteUri(
public async validateJupyterServer(
serverHandle: JupyterServerProviderHandle,
serverUri?: IJupyterServerUri
serverUri?: IJupyterServerUri,
doNotDisplayUnActionableMessages?: boolean
): Promise<void> {
let sessionManager: IJupyterSessionManager | undefined = undefined;
serverUri = serverUri || (await this.getJupyterServerUri(serverHandle));
Expand All @@ -50,6 +64,34 @@ export class JupyterConnection {
sessionManager = await this.jupyterSessionManagerFactory.create(connection, false);
await Promise.all([sessionManager.getRunningKernels(), sessionManager.getKernelSpecs()]);
// We should throw an exception if any of that fails.
} catch (err) {
if (JupyterSelfCertsError.isSelfCertsError(err)) {
sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter);
const handled = await handleSelfCertsError(this.applicationShell, this.configService, err.message);
if (!handled) {
throw err;
}
} else if (JupyterSelfCertsExpiredError.isSelfCertsExpiredError(err)) {
sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter);
const handled = await handleExpiredCertsError(this.applicationShell, this.configService, err.message);
if (!handled) {
throw err;
}
} else if (err && err instanceof JupyterInvalidPasswordError) {
throw err;
} else if (serverUri && !doNotDisplayUnActionableMessages) {
await this.errorHandler.handleError(
new RemoteJupyterServerConnectionError(serverUri.baseUrl, serverHandle, err)
);
// Can't set the URI in this case.
throw err;
} else {
traceError(
`Uri verification error ${serverHandle.extensionId}, id=${serverHandle.id}, handle=${serverHandle.handle}`,
err
);
throw err;
}
} finally {
connection.dispose();
if (sessionManager) {
Expand Down
23 changes: 17 additions & 6 deletions src/kernels/jupyter/connection/jupyterConnection.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,18 @@ import { ServiceContainer } from '../../../platform/ioc/container';
import { IServiceContainer } from '../../../platform/ioc/types';
import { JupyterConnectionWaiter } from '../launcher/jupyterConnectionWaiter.node';
import { noop } from '../../../test/core';
import { IDataScienceErrorHandler } from '../../errors/types';
import { IApplicationShell } from '../../../platform/common/application/types';
use(chaiAsPromised);
suite('Jupyter Connection', async () => {
let jupyterConnection: JupyterConnection;
let registrationPicker: IJupyterUriProviderRegistration;
let sessionManagerFactory: IJupyterSessionManagerFactory;
let sessionManager: IJupyterSessionManager;
let serverUriStorage: IJupyterServerUriStorage;
let errorHandler: IDataScienceErrorHandler;
let applicationShell: IApplicationShell;
let configService: IConfigurationService;
const disposables: IDisposable[] = [];
const provider = {
extensionId: 'ext',
Expand All @@ -55,10 +60,16 @@ suite('Jupyter Connection', async () => {
sessionManagerFactory = mock<IJupyterSessionManagerFactory>();
sessionManager = mock<IJupyterSessionManager>();
serverUriStorage = mock<IJupyterServerUriStorage>();
errorHandler = mock<IDataScienceErrorHandler>();
applicationShell = mock<IApplicationShell>();
configService = mock<IConfigurationService>();
jupyterConnection = new JupyterConnection(
instance(registrationPicker),
instance(sessionManagerFactory),
instance(serverUriStorage)
instance(serverUriStorage),
instance(errorHandler),
instance(applicationShell),
instance(configService)
);

(instance(sessionManager) as any).then = undefined;
Expand All @@ -77,7 +88,7 @@ suite('Jupyter Connection', async () => {
when(sessionManager.getKernelSpecs()).thenResolve([]);
when(sessionManager.getRunningKernels()).thenResolve([]);

await jupyterConnection.validateRemoteUri(provider, server);
await jupyterConnection.validateJupyterServer(provider, server);

verify(sessionManager.getKernelSpecs()).once();
verify(sessionManager.getRunningKernels()).once();
Expand All @@ -90,7 +101,7 @@ suite('Jupyter Connection', async () => {
when(sessionManager.getRunningKernels()).thenResolve([]);
when(registrationPicker.getJupyterServerUri(provider)).thenResolve(server);

await jupyterConnection.validateRemoteUri(provider);
await jupyterConnection.validateJupyterServer(provider);

verify(sessionManager.getKernelSpecs()).once();
verify(sessionManager.getRunningKernels()).once();
Expand All @@ -103,7 +114,7 @@ suite('Jupyter Connection', async () => {
when(sessionManager.getRunningKernels()).thenResolve([]);
when(registrationPicker.getJupyterServerUri(anything())).thenReject(new Error('kaboom'));

await assert.isRejected(jupyterConnection.validateRemoteUri(provider));
await assert.isRejected(jupyterConnection.validateJupyterServer(provider));

verify(sessionManager.getKernelSpecs()).never();
verify(sessionManager.getRunningKernels()).never();
Expand All @@ -115,7 +126,7 @@ suite('Jupyter Connection', async () => {
when(sessionManager.getKernelSpecs()).thenResolve([]);
when(sessionManager.getRunningKernels()).thenReject(new Error('Kaboom kernels failure'));

await assert.isRejected(jupyterConnection.validateRemoteUri(provider, server), 'Kaboom kernels failure');
await assert.isRejected(jupyterConnection.validateJupyterServer(provider, server), 'Kaboom kernels failure');

verify(sessionManager.getKernelSpecs()).once();
verify(sessionManager.getRunningKernels()).once();
Expand All @@ -126,7 +137,7 @@ suite('Jupyter Connection', async () => {
when(sessionManager.getKernelSpecs()).thenReject(new Error('Kaboom kernelspec failure'));
when(sessionManager.getRunningKernels()).thenResolve([]);

await assert.isRejected(jupyterConnection.validateRemoteUri(provider, server), 'Kaboom kernelspec failure');
await assert.isRejected(jupyterConnection.validateJupyterServer(provider, server), 'Kaboom kernelspec failure');

verify(sessionManager.getKernelSpecs()).once();
verify(sessionManager.getRunningKernels()).once();
Expand Down
7 changes: 3 additions & 4 deletions src/kernels/jupyter/connection/jupyterPasswordConnect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,9 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {

// Try once and see if it fails with unauthorized.
try {
return await this.requestCreator.getFetchMethod()(
url,
this.addAllowUnauthorized(url, allowUnauthorized ? true : false, options)
);
const requestInit = this.addAllowUnauthorized(url, allowUnauthorized ? true : false, options);
const result = await this.requestCreator.getFetchMethod()(url, requestInit);
return result;
} catch (e) {
if (e.message.indexOf('reason: self signed certificate') >= 0) {
// Ask user to change setting and possibly try again.
Expand Down
6 changes: 3 additions & 3 deletions src/kernels/jupyter/connection/jupyterPasswordConnect.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ suite('JupyterPasswordConnect', () => {
assert(result, 'Failed to get password');
if (result) {
// eslint-disable-next-line
assert.ok((result.requestHeaders as any).Cookie, 'No cookie');
assert.ok(result.requestHeaders?.Cookie, 'No cookie');
}

// Verfiy calls
Expand Down Expand Up @@ -224,7 +224,7 @@ suite('JupyterPasswordConnect', () => {
assert(result, 'Failed to get password');
if (result) {
// eslint-disable-next-line
assert.ok((result.requestHeaders as any).Cookie, 'No cookie');
assert.ok(result.requestHeaders?.Cookie, 'No cookie');
}

// Verfiy calls
Expand Down Expand Up @@ -277,7 +277,7 @@ suite('JupyterPasswordConnect', () => {
assert(result, 'Failed to get password');
if (result) {
// eslint-disable-next-line
assert.ok((result.requestHeaders as any).Cookie, 'No cookie');
assert.ok(result.requestHeaders?.Cookie, 'No cookie');
}

// Verfiy calls
Expand Down
112 changes: 7 additions & 105 deletions src/kernels/jupyter/connection/serverSelector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,9 @@
/* eslint-disable @typescript-eslint/no-use-before-define */

import { inject, injectable } from 'inversify';
import { IApplicationShell, IWorkspaceService } from '../../../platform/common/application/types';
import { traceError, traceWarning } from '../../../platform/logging';
import { DataScience } from '../../../platform/common/utils/localize';
import { sendTelemetryEvent } from '../../../telemetry';
import { Telemetry } from '../../../telemetry';
import {
IJupyterServerUri,
IJupyterServerUriStorage,
IJupyterUriProviderRegistration,
JupyterServerProviderHandle
} from '../types';
import { IDataScienceErrorHandler } from '../../errors/types';
import { IConfigurationService, IDisposableRegistry } from '../../../platform/common/types';
import { handleExpiredCertsError, handleSelfCertsError, jupyterServerHandleToString } from '../jupyterUtils';
import { IJupyterServerUriStorage, JupyterServerProviderHandle } from '../types';
import { JupyterConnection } from './jupyterConnection';
import { JupyterSelfCertsError } from '../../../platform/errors/jupyterSelfCertsError';
import { RemoteJupyterServerConnectionError } from '../../../platform/errors/remoteJupyterServerConnectionError';
import { JupyterSelfCertsExpiredError } from '../../../platform/errors/jupyterSelfCertsExpiredError';
import { JupyterInvalidPasswordError } from '../../errors/jupyterInvalidPassword';
import { traceError } from '../../../platform/logging';

export type SelectJupyterUriCommandSource =
| 'nonUser'
Expand All @@ -33,106 +17,24 @@ export type SelectJupyterUriCommandSource =
| 'errorHandler'
| 'prompt';

export async function validateSelectJupyterURI(
jupyterConnection: JupyterConnection,
applicationShell: IApplicationShell,
configService: IConfigurationService,
isWebExtension: boolean,
serverHandle: JupyterServerProviderHandle,
serverUri: IJupyterServerUri
): Promise<string | undefined> {
// Double check this server can be connected to. Might need a password, might need a allowUnauthorized
try {
await jupyterConnection.validateRemoteUri(serverHandle, serverUri);
} catch (err) {
traceWarning('Uri verification error', err);
if (JupyterSelfCertsError.isSelfCertsError(err)) {
sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter);
const handled = await handleSelfCertsError(applicationShell, configService, err.message);
if (!handled) {
return DataScience.jupyterSelfCertFailErrorMessageOnly;
}
} else if (JupyterSelfCertsExpiredError.isSelfCertsExpiredError(err)) {
sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter);
const handled = await handleExpiredCertsError(applicationShell, configService, err.message);
if (!handled) {
return DataScience.jupyterSelfCertExpiredErrorMessageOnly;
}
} else if (err && err instanceof JupyterInvalidPasswordError) {
return DataScience.passwordFailure;
} else {
// Return the general connection error to show in the validation box
// Replace any Urls in the error message with markdown link.
const urlRegex = /(https?:\/\/[^\s]+)/g;
const errorMessage = (err.message || err.toString()).replace(urlRegex, (url: string) => `[${url}](${url})`);
return (
isWebExtension || true
? DataScience.remoteJupyterConnectionFailedWithoutServerWithErrorWeb
: DataScience.remoteJupyterConnectionFailedWithoutServerWithError
)(errorMessage);
}
}
}

/**
* Provides the UI for picking a remote server. Multiplexes to one of two implementations based on the 'showOnlyOneTypeOfKernel' experiment.
*/
@injectable()
export class JupyterServerSelector {
constructor(
@inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage,
@inject(IDataScienceErrorHandler)
private readonly errorHandler: IDataScienceErrorHandler,
@inject(IApplicationShell) private readonly applicationShell: IApplicationShell,
@inject(IConfigurationService) private readonly configService: IConfigurationService,
@inject(JupyterConnection) private readonly jupyterConnection: JupyterConnection,
@inject(IWorkspaceService) readonly workspaceService: IWorkspaceService,
@inject(IDisposableRegistry) readonly disposableRegistry: IDisposableRegistry,
@inject(IJupyterUriProviderRegistration)
private readonly jupyterPickerRegistration: IJupyterUriProviderRegistration
@inject(JupyterConnection) private readonly jupyterConnection: JupyterConnection
) {}

public async addJupyterServer(serverHandle: JupyterServerProviderHandle): Promise<void> {
const serverHandleId = jupyterServerHandleToString(serverHandle);
// Double check this server can be connected to. Might need a password, might need a allowUnauthorized
let serverUri: undefined | IJupyterServerUri;
try {
serverUri = await this.jupyterPickerRegistration.getJupyterServerUri(serverHandle);
await this.jupyterConnection.validateRemoteUri(serverHandle);
// Double check this server can be connected to. Might need a password, might need a allowUnauthorized
await this.jupyterConnection.validateJupyterServer(serverHandle);
} catch (err) {
if (JupyterSelfCertsError.isSelfCertsError(err)) {
sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter);
const handled = await handleSelfCertsError(this.applicationShell, this.configService, err.message);
if (!handled) {
return;
}
} else if (JupyterSelfCertsExpiredError.isSelfCertsExpiredError(err)) {
sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter);
const handled = await handleExpiredCertsError(this.applicationShell, this.configService, err.message);
if (!handled) {
return;
}
} else if (err && err instanceof JupyterInvalidPasswordError) {
return;
} else if (serverUri) {
await this.errorHandler.handleError(
new RemoteJupyterServerConnectionError(serverUri.baseUrl, serverHandle, err)
);
// Can't set the URI in this case.
return;
} else {
traceError(
`Uri verification error ${serverHandle.extensionId}, id=${serverHandle.id}, handle=${serverHandle.handle}`,
err
);
}
traceError(`Error in validating the Remote Uri ${serverHandle.id}.${serverHandle.handle}`, err);
return;
}

await this.serverUriStorage.add(serverHandle);

// Indicate setting a jupyter URI to a remote setting. Check if an azure remote or not
sendTelemetryEvent(Telemetry.SetJupyterURIToUserSpecified, undefined, {
azure: serverHandleId.toLowerCase().includes('azure')
});
}
}
Loading

0 comments on commit 00cc269

Please sign in to comment.