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

Investigate workspaces with invalid URIs (invalid schemes) #1159

Closed
radeksimko opened this issue Jun 24, 2022 · 4 comments · Fixed by #1163
Closed

Investigate workspaces with invalid URIs (invalid schemes) #1159

radeksimko opened this issue Jun 24, 2022 · 4 comments · Fixed by #1163
Assignees
Labels
bug Something isn't working
Milestone

Comments

@radeksimko
Copy link
Member

radeksimko commented Jun 24, 2022

Background

Generally the language server expects documents to have file:// scheme. However based on reports and available telemetry, we can tell there are users attempting to use the language server with other schemes which are not file.

Here's a list of some top URI schemes from telemetry data representing users of the VS Code extension 2.23.0 which we treat as invalid.

  • vscode-remote:// (87%)
    • vscode-remote://ssh-remote%2B
    • vscode-remote://wsl%2B
    • vscode-remote://dev-container%2B
    • vscode-remote://attached-container%2B
    • vscode-remote://k8s-container%2B
    • vscode-remote://codespaces%2B
  • vsls:/ (6.29%)
  • git:/ (2.68%)
  • file://wsl%24 (1.62%)
  • ssh:// (1.29%)
below 1%
  • vscode-vfs://
  • vscode-local:/
  • sftp://
  • gitlens://
  • docker://
  • azurestorage:/
  • gitduck:/
  • vscode-local-history:/
  • review:/
  • vscode-remote:/
  • gitlab-remote://
  • gl-review:/
  • pr:/
  • gist://
  • vsls-scc:/
  • repo:/
  • git://
  • github://
  • s3:/
  • githubpr:/

Some of these also contain just a single /, which if provided during the initialization can cause LS to crash, see hashicorp/terraform-ls#959

At this point it is unclear how exactly we end up receiving documents with URIs with the above schemes, but hashicorp/terraform-ls#959 shed some light on the azurestorage:/ one at least and we have some insight into file://wsl%24 from #1102 and some other earlier reports from WSL users.

Proposal

There is a few options:

  • prevent the client from sending these to the server (probably best option)
  • ignore them on the server side
  • provide IntelliSense for files within these workspaces (unclear how and whether people even want and expect that)
@radeksimko
Copy link
Member Author

This seems somewhat related to our filtering logic used for filtering files received as part of vscode.window.onDidChangeActiveTextEditor which is also inaccurate and duplicates the detection we already do as part of package.json

/*
Detects whether this is a Terraform file we can perform operations on
*/
export function isTerraformFile(document?: vscode.TextDocument): boolean {
if (document === undefined) {
return false;
}
if (document.isUntitled) {
// Untitled files are files which haven't been saved yet, so we don't know if they
// are terraform so we return false
return false;
}
// TODO: check for supported language IDs here instead
if (document.fileName.endsWith('tf')) {
// For the purposes of this extension, anything with the tf file
// extension is a Terraform file
return true;
}
// be safe and default to false
return false;
}

"contributes": {
"languages": [
{
"id": "terraform",
"aliases": [
"Terraform",
"terraform"
],
"extensions": [
".tf"
],
"configuration": "./language-configuration.json"
},
{
"id": "terraform-vars",
"extensions": [
".tfvars"
],
"configuration": "./language-configuration.json"
},

I guess the broader question is whether there's any single place where we could limit what files and URI schemes do we actually care about, such that we can just do it once and avoid having to check it in all these different places downstream.

documentSelector: documentSelector,
synchronize: {
fileEvents: [
vscode.workspace.createFileSystemWatcher('**/*.tf'),
vscode.workspace.createFileSystemWatcher('**/*.tfvars'),
],
},

@radeksimko
Copy link
Member Author

radeksimko commented Jun 27, 2022

The official Go extension seems to account for some common edge cases:

https://github.com/golang/vscode-go/blob/8dfd39349da7a523b4ed0f781c9d10c753be76bf/src/commands/startLanguageServer.ts#L115-L139

We could also provide some more helpful log messages or notifications for the common cases (like WSL, VSLS or SSH).

The Go extension continues to send some of the "unsupported" workspace URIs to the server for some reason (which are then silently ignored by gopls). I'd argue it's better to just not send what's not supported at all, but I'm open to other opinions.

@jpogran
Copy link
Contributor

jpogran commented Jun 29, 2022

VS Code determines what gets put there based on the type of extension:

Workspace Extensions: These extensions are run on the same machine as where the workspace is located. When in a local workspace, Workspace Extensions run on the local machine. When in a remote workspace or when using Codespaces, Workspace Extensions run on the remote machine / environment. Workspace Extensions can access files in the workspace to provide rich, multi-file language services, debugger support, or perform complex operations on multiple files in the workspace (either directly or by invoking scripts/tools). While Workspace Extensions do not focus on modifying the UI, they can contribute explorers, views, and other UI elements as well.

You can either rely on VS Code to choose, or specify the kind yourself via extensionkind. Back when I was looking at #1102 I incorrectly thought we could specify:

"extensionKind": ["workspace", "ui"] — Indicates the extension prefers to run as a workspace extension, but does not have any hard requirements on accessing workspace contents. When using VS Code, the extension will run in VS Code's workspace extension host if it exists in remote workspace, otherwise will run in VS Code's local extension host if it exists locally. When using VS Code for the Web with Codespaces it will run in the remote extension host always (as no local extension host is available).

I thought it would stop it attempting to load on the local machine and force it to load on the remote machine where appropriate. However as we continue to see in this ticket, the extension is left in the local instance and it's up to the user to 'see' that it needs to be installed on the remote instance. The key appears to be 'prefers' and 'no hard requirements'. We 'need' instead of 'prefer' and have 'hard requirements', so we should be more restrictive.

To force VS Code to automatically install our extension in both place and activate the remote one when in a remote contenxt, we need to restrict the extensionKind to workspace:

"extensionKind": ["workspace"] — Indicates the extension requires access to workspace contents and therefore needs to run where the workspace is located. That can be on the local machine or on the remote machine or Codespace. Most extensions fall into this category.

Setting this value is easy, but testing this is easier said that done. We can install from VSIX in both locations, but this does not test VS Code's automatic detection installation mechanism, since we are doing this manually. If you install from VSIX on your local instance, then open a WSL instance it will pull from the Marketplace to install, which has the wrong extensionKind.

To fully test this, you need to follow https://code.visualstudio.com/api/advanced-topics/remote-extensions#incorrect-execution-location, which says that we set remote.extensionKind inside our client settings and it will override what the extension manifest has and enable us to test this. The key is that this need to be in the machine setting scope, not your user setting scope:

2022-06-29 (1)

Once a WSL VS Code instance is opened, installing the Terraform Extension through the Marketplace puts it both locally and in the remote instance. Then when interacting with the Terraform files, the proper paths are used:

image

So, the final fix should be:

"extensionKind": [ "workspace" ]

You might then question, what about our plans to support the Web host (github.dev or vscode.dev)? Shouldn't setting to workspace mean that we won't be able to run in the Web host? According to https://code.visualstudio.com/api/advanced-topics/extension-host#preferred-extension-location:

If an extension is web-only, it will always run on the web extension host, regardless of the extensionKind setting.

According to https://code.visualstudio.com/api/extension-guides/web-extensions, when we do enable this extension to be web only, it will show up as a web extension and work there.

@jpogran jpogran linked a pull request Jun 29, 2022 that will close this issue
@jpogran jpogran self-assigned this Jun 29, 2022
@radeksimko radeksimko added this to the v2.24.0 milestone Jul 8, 2022
@github-actions
Copy link

github-actions bot commented Aug 8, 2022

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 Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants