Skip to content
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

fix(ec2): give warning when OpenSSH client is outdated. #6018

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Nov 14, 2024

Problem

See #6015

Solution

  • Don't try to make connection, and instead throw early error to let user know what the problem is.
  • Also increase logging surrounding this failure.

Logs + Error:
(Ignore version number)
image

Notes

Believe test failures are unrelated as they are currently on master. pending fix.

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

This comment was marked as resolved.

}

function parseSshVersion(output: string): string | undefined {
const match = output.match(/OpenSSH_(\d+)\.(\d+)/)
Copy link
Contributor

@justinmk3 justinmk3 Nov 14, 2024

Choose a reason for hiding this comment

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

Instead of all this, could we we just triy to connect to the remote and surface any error messages (plus ssh version) on failure? We already do that for the "terminal" feature, right? So for vscode, we could do that as a pre-step, to really check that things are working.

An extra benefit is that this gives a much better experience if there is an ssh issue, compared to checking vscode-remote's ssh window.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error is coming from the connect script, i.e. the child process that launches the new instance of VSCode so I am not sure how to surface it. Especially since the child process is successfully opening VSCode, but then the newly opened VSCode instance throws an error. We would have to route that error back to this instance of VSCode somehow.

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.

2 participants