Skip to content

Gracefully handle errors where the SSH configuration file is corrupt or malformed#4644

Merged
alexr00 merged 3 commits intomicrosoft:mainfrom
eastwood:ssh-config-errors
Apr 11, 2023
Merged

Gracefully handle errors where the SSH configuration file is corrupt or malformed#4644
alexr00 merged 3 commits intomicrosoft:mainfrom
eastwood:ssh-config-errors

Conversation

@eastwood
Copy link
Contributor

@eastwood eastwood commented Mar 20, 2023 •
edited
Loading

Developer notes:

  • Bug is around some presumptious FP composition which doesn't consider errors thrown from outside environment.
  • Have filtered out issues where the config resolution is erroneous.
  • Would like to write tests, but the particular code path is not accessible by tests. The singleton appears to be making it difficult and debugging tests hasn't been entirely easy with the singletons.
  • Will have a go at injecting the config file dependency so that we test the scenario

@eastwood
Copy link
Contributor Author

eastwood commented Mar 28, 2023

@microsoft-github-policy-service agree

@eastwood eastwood marked this pull request as ready for review March 28, 2023 06:44
@alexr00 alexr00 added this to the April 2023 milestone Mar 28, 2023
alexr00
alexr00 approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I will merge after the March release.

@eastwood
Copy link
Contributor Author

eastwood commented Mar 28, 2023

You're welcome :) I performed some QA on this locally, but had a bit of difficulty hooking into the tests in protocol.test.ts due to the singletons. Would you be able to confirm that with a malformed .ssh/config file such as:

Host:

The error logs are what you expect?

This is my first contribution to a vscode extension, so hoping that I used the idiomatic logging mechanism. Criticism is appreciated :)

@alexr00
Copy link
Member

alexr00 commented Mar 28, 2023

For this extension, the Logger that you used is the best way to do it

@alexr00 alexr00 enabled auto-merge (squash) April 11, 2023 10:51
@alexr00 alexr00 merged commit e54342f into microsoft:main Apr 11, 2023
@eastwood eastwood deleted the ssh-config-errors branch April 19, 2023 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aeschli aeschli aeschli approved these changes

@alexr00 alexr00 alexr00 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

April 2023

Development

Successfully merging this pull request may close these issues.

3 participants

@eastwood @alexr00 @aeschli