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(node): Fix proxy resolution logic running from node context #30

Merged
merged 2 commits into from
Jan 15, 2021

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Jan 15, 2021

Because this code brings in vscode-proxy-agent, and agent-base, we hit the same issue here microsoft/vscode#93167 - which manifests in Onivim in issues like: onivim/oni2#2981

This works around the issue by only using the 2-argument overload for https/http.request`.

@bryphe bryphe merged commit 723edbe into master Jan 15, 2021
@bryphe bryphe deleted the fix/node/proxy-resolver branch January 15, 2021 17:39
bryphe added a commit to onivim/oni2 that referenced this pull request Jan 15, 2021
…uest call (#2995)

__Issue:__ The terraform extension was failing with an error:

> Error: Activating 'hashicorp terraform' failed: The "listener" argument must be of type function. Received an instance of Object

__Defect:__ The extension was crashing here:
```
	https.request(url, options, res => {
			if (res.statusCode === 301 || res.statusCode === 302) { // follow redirects
				return resolve(httpsRequest(res.headers.location, options, encoding));
			}
```
when trying to resolve download locations for terraform releases. This was not specific to this plugin, but actually could crash in any 3-param call to `https.request`. (There are two overloads - a `https.request(options, cb)`, and `https.request(url, options.cb)`.

Because of the `agent-base` dependency that is brought in the node environment, it overwrites the `https.request` handler such that only the 2-param overload is accepted. More info here: microsoft/vscode#93167

__Fix:__ Update the `proxyResolver` code to only use the 2-param overload in our node environment, which the extension host is running in - onivim/vscode-exthost#30 . Add some test cases to ensure that both overloads can make requests

With this fix, as the language server now gets installed, get some basic language features (diagnostics, some completion, and outline) for terraform files:
![image](https://user-images.githubusercontent.com/13532591/104756239-6b867800-5710-11eb-8aff-cfd082ce0f1b.png)

Fixes #2981 
Related #1058 

Note that there is still a pending issue relating to the syntax highlights for terraform: #2220
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.

1 participant