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.

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

Conversation

jeanp413
Copy link
Member

@jeanp413 jeanp413 commented Aug 31, 2022

Description

Ask for local app fallback when Timed out while waiting for handshake
Add gitpodHost custom attribute to configcat

Related Issue(s)

Fixes gitpod-io/gitpod#12475

How to test

  1. Block ssh connections and check a notification is shown, Clicking Ok will fallback to use local-app

To block port 22 you can follow https://www.digitalocean.com/community/tutorials/how-to-set-up-a-firewall-with-ufw-on-ubuntu-18-04 if you are on linux

} 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.

@@ -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

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

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

@akosyakov
Copy link
Member

Block ssh connections

@jeanp413 Any guidance how to do it? Maybe we should document it in https://www.notion.so/gitpod/VS-Code-Knowledge-sharing-FAQ-s-23145c3d43dd4b6db25b9f02486de3e0

Co-authored-by: Filip Troníček <[email protected]>
@jeanp413 jeanp413 force-pushed the jp/handle-handshake-timeout branch from 8f6ef93 to 9251450 Compare August 31, 2022 17:53
@jeanp413
Copy link
Member Author

Block ssh connections

@jeanp413 Any guidance how to do it? Maybe we should document it in https://www.notion.so/gitpod/VS-Code-Knowledge-sharing-FAQ-s-23145c3d43dd4b6db25b9f02486de3e0

Added a link in the description for linux, don't know if it works for mac

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

I think code-wise it looks good beside: https://github.com/gitpod-io/gitpod-vscode-desktop/pull/20/files#r960355213

we should always continue execution does not matter how a user closed the warning with cancel or OK

I have not tried it. @mustard-mh @filiptronicek could you help with testing 🙏

@mustard-mh
Copy link
Contributor

Tested by change source code

  • NoSSHGatewayError - change sshHostKeyEndPoint requests URL
  • Timeout - change readyTimeout to 1 ms

All good, as close notification will also fallback to local-app [1]

@jeanp413 jeanp413 merged commit 4025455 into master Sep 5, 2022
@jeanp413 jeanp413 deleted the jp/handle-handshake-timeout branch September 5, 2022 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare to migrate SH users to SSH Gateway in VS Code Desktop
5 participants