Skip to content

Ask for local app fallback when Timed out while waiting for handshake #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 5, 2022
Merged

Ask for local app fallback when Timed out while waiting for handshake #20

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6b5362
Ask for local app fallback when `Timed out while waiting for handshake`
jeanp413 Aug 31, 2022
9251450
:lipstick:
jeanp413 Aug 31, 2022
6989ca9
:lipstick:
jeanp413 Sep 1, 2022
6436613
:lipstick:
jeanp413 Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/experiments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class ExperimentalSettings {
this.extensionVersion = new semver.SemVer(extensionVersion);
}

async get<T>(key: string, userId?: string): Promise<T | undefined> {
async get<T>(key: string, userId?: string, custom?: { [key: string]: string }): Promise<T | undefined> {
const config = vscode.workspace.getConfiguration('gitpod');
const values = config.inspect<T>(key.substring('gitpod.'.length));
if (!values || !EXPERTIMENTAL_SETTINGS.includes(key)) {
Expand All @@ -48,22 +48,22 @@ export class ExperimentalSettings {
return values.globalValue;
}

const user = userId ? new configcatcommon.User(userId) : undefined;
const user = userId ? new configcatcommon.User(userId, undefined, undefined, custom) : undefined;
const configcatKey = key.replace(/\./g, '_'); // '.' are not allowed in configcat
const experimentValue = (await this.configcatClient.getValueAsync(configcatKey, undefined, user)) as T | undefined;

return experimentValue ?? values.defaultValue;
}

async inspect<T>(key: string, userId?: string): Promise<{ key: string; defaultValue?: T; globalValue?: T; experimentValue?: T } | undefined> {
async inspect<T>(key: string, userId?: string, custom?: { [key: string]: string }): Promise<{ key: string; defaultValue?: T; globalValue?: T; experimentValue?: T } | undefined> {
const config = vscode.workspace.getConfiguration('gitpod');
const values = config.inspect<T>(key.substring('gitpod.'.length));
if (!values || !EXPERTIMENTAL_SETTINGS.includes(key)) {
this.logger.error(`Cannot inspect invalid experimental setting '${key}'`);
return values;
}

const user = userId ? new configcatcommon.User(userId) : undefined;
const user = userId ? new configcatcommon.User(userId, undefined, undefined, custom) : undefined;
const configcatKey = key.replace(/\./g, '_'); // '.' are not allowed in configcat
const experimentValue = (await this.configcatClient.getValueAsync(configcatKey, undefined, user)) as T | undefined;

Expand Down
29 changes: 22 additions & 7 deletions src/remoteConnector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ export default class RemoteConnector extends Disposable {
}).connect({
host: sshDestInfo.hostName,
username: sshDestInfo.user,
readyTimeout: 40000,
Copy link
Member

@akosyakov akosyakov Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it seconds? 40? I don't mind making it higher like 1-2 minutes. WDYT? cc @mustard-mh @iQQBot

Copy link
Member Author

@jeanp413 jeanp413 Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes seconds, we can increase it more but that would mean their network it's just too slow and the experience would be subpar (I guess that's up to the user to decide so ¯\_(ツ)_/¯), if it takes 1 minute to do the check it will take another minute to connect again using remote-ssh extension 😬

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to show a process bar when connecting and show "fallback to/try with local-app" action after 20 seconds? 40s or 1-2minutes are too long to wait

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to show a process bar when connecting

yeah possible but this connection does nothing it's just a quick test to check if port is not blocked so not sure if we should expose it to the user

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to show a process bar when connecting and show "fallback to/try with local-app" action after 20 seconds? 40s or 1-2minutes are too long to wait

I mean generally it should be very quick, so maybe it is not bad to show progress bar if you really have problems, then we need to add some messaging Verifying SSH connection...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this test should be very quick, if handshake take 40s, It's hard to imagine what the latency would be once you've successfully connected

authHandler(_methodsLeft, _partialSuccess, _callback) {
return {
type: 'password',
Expand Down Expand Up @@ -786,7 +787,7 @@ export default class RemoteConnector extends Disposable {

// Only use experiment for SaaS
const forceUseLocalApp = getServiceURL(params.gitpodHost) === 'https://gitpod.io'
? (await this.experiments.get<boolean>('gitpod.remote.useLocalApp', session.account.id))!
? (await this.experiments.get<boolean>('gitpod.remote.useLocalApp', session.account.id, { gitpodHost: params.gitpodHost }))!
: vscode.workspace.getConfiguration('gitpod').get<boolean>('remote.useLocalApp')!;
const userOverride = isUserOverrideSetting('gitpod.remote.useLocalApp');
let sshDestination: string | undefined;
Expand All @@ -806,8 +807,22 @@ export default class RemoteConnector extends Disposable {
this.telemetry.sendRawTelemetryEvent('vscode_desktop_ssh', { kind: 'gateway', status: 'failed', reason: e.toString(), ...params, gitpodVersion: gitpodVersion.raw, userOverride });
if (e instanceof NoSSHGatewayError) {
this.logger.error('No SSH gateway:', e);
vscode.window.showWarningMessage(`${e.host} does not support [direct SSH access](https://github.com/gitpod-io/gitpod/blob/main/install/installer/docs/workspace-ssh-access.md), connecting via the deprecated SSH tunnel over WebSocket.`);
// Do nothing and continue execution
const ok = 'Ok';
const action = await vscode.window.showWarningMessage(`${e.host} does not support [direct SSH access](https://github.com/gitpod-io/gitpod/blob/main/install/installer/docs/workspace-ssh-access.md), connecting via the deprecated SSH tunnel over WebSocket.`, ok);
if (action === ok) {
// Do nothing and continue execution
} else {
return;
}
} else if (e instanceof SSHError && e.message === 'Timed out while waiting for handshake') {
this.logger.error('SSH test connection error:', e);
const ok = 'Ok';
const action = await vscode.window.showWarningMessage(`Timed out while waiting for SSH handshake, it's possible SSH connections on port 22 are blocked or your network is too slow, connecting via the deprecated SSH tunnel over WebSocket.`, ok);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @akosyakov if you want to tweak the message

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks good to me

cc @loujaybee

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from @filiptronicek #20 (comment)

if (action === ok) {
// Do nothing and continue execution
} else {
return;
}
} else if (e instanceof NoRunningInstanceError) {
this.logger.error('No Running instance:', e);
vscode.window.showErrorMessage(`Failed to connect to ${e.workspaceId} Gitpod workspace: workspace not running`);
Expand Down Expand Up @@ -934,17 +949,17 @@ export default class RemoteConnector extends Disposable {
const action = await vscode.window.showInformationMessage(`Could not install local extensions on remote workspace, please enable Settings Sync with Gitpod.`, addSyncProvider, config);
if (action === addSyncProvider) {
vscode.commands.executeCommand('gitpod.syncProvider.add');
} else if(action === config) {
vscode.commands.executeCommand('workbench.action.openSettings',`@ext:${this.context.extension.id} sync extensions`);
} else if (action === config) {
vscode.commands.executeCommand('workbench.action.openSettings', `@ext:${this.context.extension.id} sync extensions`);
}
} else if (e instanceof NoSettingsSyncSession) {
const enableSettingsSync = 'Enable Settings Sync';
const config = 'Configure';
const action = await vscode.window.showInformationMessage(`Could not install local extensions on remote workspace, please enable Settings Sync.`, enableSettingsSync, config);
if (action === enableSettingsSync) {
vscode.commands.executeCommand('workbench.userDataSync.actions.turnOn');
} else if(action === config) {
vscode.commands.executeCommand('workbench.action.openSettings',`@ext:${this.context.extension.id} sync extensions`);
} else if (action === config) {
vscode.commands.executeCommand('workbench.action.openSettings', `@ext:${this.context.extension.id} sync extensions`);
}
} else {
this.logger.error('Error while fetching settings sync extension data:', e);
Expand Down