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

Drop support of (old) LS without multiple folders #816

Closed
radeksimko opened this issue Oct 18, 2021 · 1 comment · Fixed by #845
Closed

Drop support of (old) LS without multiple folders #816

radeksimko opened this issue Oct 18, 2021 · 1 comment · Fixed by #845
Assignees
Labels
enhancement New feature or request

Comments

@radeksimko
Copy link
Member

radeksimko commented Oct 18, 2021

Problem Statement

Users have reported bugs in #805 (and probably some other older issues too) which are most likely caused by the fact that we maintain a map of client/server instances where reading or writing is not thread-safe.

const clients: Map<string, TerraformLanguageClient> = new Map();

This can sometimes lead to client instance being read before being written. The problem is in most cases masked by the fact that the LS is initialized quickly, such that reading happens after, but this is rather happy coincidence than a guarantee.

Expected User Experience

Most users will see no UX changes.

A small minority of users which have not upgraded to LS 0.19.0 yet would be already affected by a different incompatibility we introduced earlier when upgrading vscode-languageclient to 7.0.0 in #780 and presented with the following error anyway

Screenshot 2021-10-18 at 14 26 44

Proposal

Refactor ClientHandler such that a single client is only ever used and no situation occurs where we would attempt to access uninitialized client instance.

An error will be shown to the user whenever LS capability workspace.workspaceFolders.supported is set to false just to more explicitly communicate requirements, just in case we happen drop support in the future by accident or someone attempts to use the extension with a different LS which doesn't have that capability.

@radeksimko radeksimko added the enhancement New feature or request label Oct 18, 2021
@jpogran jpogran linked a pull request Nov 4, 2021 that will close this issue
@jpogran jpogran linked a pull request Nov 10, 2021 that will close this issue
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
2 participants