Skip to content

Commit

Permalink
fix(extensions/#2981): Listener function error with 3-param https req…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
bryphe authored Jan 15, 2021
1 parent 7eff3fe commit 21e2a19
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
- #2990 - Signature Help: Fix blocking `esc` key press back to normal mode
- #2991 - OSX: Fix shortcut keys double-triggering events
- #2993 - CodeLens: Handle null command id & label icons
- #2995 - Extensions: Fix bug with 3-param http/https request (fixes #2981)

### Performance

Expand Down
2 changes: 1 addition & 1 deletion node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"test": "jest"
},
"dependencies": {
"@onivim/vscode-exthost": "1.52.1000",
"@onivim/vscode-exthost": "1.52.1001",
"follow-redirects": "1.13.0",
"fs-extra": "^8.1.0",
"sudo-prompt": "^9.0.0",
Expand Down
8 changes: 4 additions & 4 deletions node/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
# yarn lockfile v1


"@onivim/[email protected].1000":
version "1.52.1000"
resolved "https://registry.yarnpkg.com/@onivim/vscode-exthost/-/vscode-exthost-1.52.1000.tgz#f69523da388b9595662ff12c600fe5fe0c2150ae"
integrity sha512-MuW8QfmHPj9q5D37lHdavUZG5cuCanTF8hZ1k/sCGbWKBCVCHPOtQ74pxRFa91t+JCXDTpzXdiW6DBLh+fIjDA==
"@onivim/[email protected].1001":
version "1.52.1001"
resolved "https://registry.yarnpkg.com/@onivim/vscode-exthost/-/vscode-exthost-1.52.1001.tgz#22139888543b604a2babb94c200693290fca4220"
integrity sha512-SbvMucGhnAnz7pwkcuv8sjgOLB8bdldYMm9d/c8Vhgm3yNwtDefsRcvmXyiXidCHRBoKk/ZRvO+9Ua7OiAMV/Q==
dependencies:
graceful-fs "4.2.3"
iconv-lite-umd "0.6.8"
Expand Down
63 changes: 63 additions & 0 deletions test/Exthost/HttpRequestTest.re
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
open TestFramework;

open Exthost;

let waitForMessage = (~name, f, context) => {
context
|> Test.waitForMessage(
~name,
fun
| Msg.MessageService(ShowMessage({message, severity, _})) =>
f(severity, message)
| _ => false,
);
};

describe("HttpRequestTest", ({test, _}) => {
test("Execute a (url, cb) request in extension host", _ => {
Test.startWithExtensions(["oni-http-request"])
|> Test.waitForExtensionActivation("oni-http-request")
|> Test.withClient(
Request.Commands.executeContributedCommand(
~arguments=[],
~command="http.request2",
),
)
|> waitForMessage(
~name="wait for request result (2-param)", (severity, message) =>
switch (severity) {
| Info => true
| Error =>
prerr_endline("Got error: " ++ message);
false;
| _ => assert(false)
}
)
|> Test.terminate
|> Test.waitForProcessClosed
});

// Regression test for #2981 - ensure 3-param requests work correctly
test("#2981: Execute a (url, options, cb) request in extension host", _ => {
Test.startWithExtensions(["oni-http-request"])
|> Test.waitForExtensionActivation("oni-http-request")
|> Test.withClient(
Request.Commands.executeContributedCommand(
~arguments=[],
~command="http.request3",
),
)
|> waitForMessage(
~name="wait for request result (3-param)", (severity, message) =>
switch (severity) {
| Info => true
| Error =>
prerr_endline("Got error: " ++ message);
false;
| _ => assert(false)
}
)
|> Test.terminate
|> Test.waitForProcessClosed
});
});
63 changes: 63 additions & 0 deletions test/collateral/extensions/oni-http-request/extension.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// The module 'vscode' contains the VS Code extensibility API
// Import the module and reference it with the alias vscode in your code below
const vscode = require('vscode');
const https = require('https');

// this method is called when your extension is activated
// your extension is activated the very first time the command is executed

/**
* @param {vscode.ExtensionContext} context
*/
function activate(context) {
context.subscriptions.push(
// Execute a 2-parameter request against an https object
vscode.commands.registerCommand('http.request2', (_args) => {
const req = https.request("https://httpbin.org/json", (res) => {
if (res.statusCode == 200) {
vscode.window.showInformationMessage('success');
} else {
vscode.window.showErrorMessage('Unexpected status code: ' + res.statusCode)
}
});

req.on('error', (e) => {
vscode.window.showErrorMessage('Error: ' + e)
});

req.end();

})
);

context.subscriptions.push(
// Execute a 3-parameter request against an https object
// This exercises the same failure that was causing #2981 -
// the [agent-base] NPM package overriding the https.request,
// only handling 2-param arguments.
vscode.commands.registerCommand('http.request3', (_args) => {
const req = https.request("https://httpbin.org/json", { agent: false }, (res) => {
if (res.statusCode == 200) {
vscode.window.showInformationMessage('success');
} else {
vscode.window.showErrorMessage('Unexpected status code: ' + res.statusCode)
}
});

req.on('error', (e) => {
vscode.window.showErrorMessage('Error: ' + e)
});

req.end();

})
);
}

// this method is called when your extension is deactivated
function deactivate() {}

module.exports = {
activate,
deactivate
}
17 changes: 17 additions & 0 deletions test/collateral/extensions/oni-http-request/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "oni-http-request",
"description": "HTTP Request extension for Onivim tests",
"version": "0.0.1",
"repository": "https://github.com/Microsoft/vscode-extension-samples/helloworld-minimal-sample",
"engines": {
"vscode": "^1.25.0"
},
"activationEvents": ["*"],
"main": "./extension.js",
"scripts": {
"postinstall": "node ./node_modules/vscode/bin/install"
},
"devDependencies": {
"vscode": "^1.1.22"
}
}

0 comments on commit 21e2a19

Please sign in to comment.