windows: Fix node_runtime to support proxy by keep system env

[huacnlee]

Release Notes:

• Fixed node runtime install npm on Windows by using system proxy.

---

NPM is installed in some regions (such as mainland China) and accessing npmjs.org must go through a proxy.

[2024-05-14T22:31:02+08:00 ERROR project] failed to start language server "json-language-server": failed to execute npm info subcommand:
stdout: "{\n  \"error\": {\n    \"code\": \"EAI_FAIL\",\n    \"summary\": \"request to https://registry.npmjs.org/vscode-json-languageserver failed, reason: getaddrinfo EAI_FAIL registry.npmjs.org\",\n    
\"detail\": \"This is a problem related to network connectivity.\\nIn most cases you are behind a proxy or have bad network settings.\\n\\nIf you are behind a proxy, please make sure that the\\n'proxy' config is set properly.  See: 'npm help config'\"\n  }\n}\n"

However, when NPM install was executed in node_runtime before, the env_clear action was performed. This will lose the http_proxy environment variable set by the operating system.

I have done a lot of testing on Windows in the past few days:

• Set http_proxy, https_proxy envs directly by use envs method for command.
• Set --proxy argument.

But it still not work, the localhost proxy server 127.0.1 can't connect.

[2024-05-14T22:34:47+08:00 ERROR project] failed to start language server "json-language-server": failed to execute npm info subcommand:
stdout: "{\n  \"error\": {\n    \"code\": \"UNKNOWN\",\n    \"summary\": \"request to https://registry.npmjs.org/vscode-json-languageserver failed, reason: connect UNKNOWN 127.0.0.1:7897 - Local (undefined:undefined)\",\n    \"detail\": \"\"\n  }\n}\n"
stderr: "npm ERR! code UNKNOWN\nnpm ERR! syscall connect\nnpm ERR! errno UNKNOWN\nnpm ERR! request to https://registry.npmjs.org/vscode-json-languageserver failed, reason: connect UNKNOWN 127.0.0.1:7897 - Local (undefined:undefined)\n\nnpm ERR! A complete log of this run can be found in:\nnpm ERR!     C:\\Users\\jason\\AppData\\Local\\Zed\\node\\node-v18.15.0-win-x64\\cache\\_logs\\2024-05-14T14_34_39_920Z-debug-0.log\n"

In this log, 127.0.0.1:7897 is my local proxy server, it is readed from http_proxy env.

I haven't found out why this happens on Windows. I have executed the same command on CMD, and it all works.

The above issue not happened on macOS, it works fine.

Today, I have tried to remove the env_clear, then the proxy works. I don't why, why I set envs not work (I am sure the env and value is correct), but it actually works now.

So, I give up, to just removed env_clear let it just work.

The Node runtime already uses absolute paths and additionally sets PATH, cache, --prefix, so there seems to be no need to clear the system ENVs?

[huacnlee]

After update, now install NPM is works now.

install-with-proxx1.mp4

[SomeoneToIgnore]

The context of the clearing change was to isolate the npm/node running entirely from side effects, as at that time we've had issue with node.

But then, #6742 came and fixed all those issues, but some of the code remained since (including dummy configs settings just below).

By removing the env_clear call, we allow arbitrary env variables that were inherited by Zed process, to be used in the node process too, potentially creating more issues due to various side effects out of odd env set-ups of people.
Also see #11758 that indicates that many env vars might be cached in an incorrect state before rebooting.

So, I'm not entirely sure that this is the right way to proceed with the proxy fix.
Could we instead forward the proxy-related env vars only somehow without the env_clear?

[SomeoneToIgnore]

I would also try a newer npm version (or even pnpm), because Zed seems to use an outdated one: if that works with either of the proxy settings + a tiny Rust wrapper around with the env_clear, we might want to switch to that version first, and then use the proxy settings?

Another thing worth investigating is whether the configs below (--userconfig and --globalconfig) are to blame for such proxy ignorant behavior: what if we remove them and try the proxy configs?

[huacnlee]

So, I'm not entirely sure that this is the right way to proceed with the proxy fix.

Could we instead forward the proxy-related env vars only somehow without the env_clear?

From a technical point of view, setting env directly and retaining env should be the same, because all it needs is the value of http_proxy.

I have tried to set the env variables of http_proxy with the env function, and also tried to restore several possible proxy environment variables (all_proxy, http_proxy, https_proxy). Strangely, this never works. 😂

[SomeoneToIgnore]

Yes, this is very odd and sad to see, but npm is somewhat notorious for treating its argument oddly.
I propose to

• remove extra dummy configs from the arguments and see if this way any of the proxy-related vars will work
• try a newer npm version (same, with and without the dummy configs)

but would love to avoid dragging in entire users' envs into npm.

[huacnlee]

I have done a test with your proposal

• Remove --userconfig and --globalconfig is no effect
• And I have tried upgrade to use node-v20.9.0, but the problem is same.

[SomeoneToIgnore]

Sorry, for the rest of this week I'm relatively occupied and cannot dive into that, but I think we need to find a way of passing the proxy only, without any extra env vars.
I will try to do that at some point and would not want to proceed with this approach before it's clear what is not working with the alternative one.

If @mikayla-maki thinks it's better to move on with Windows and enable LSP servers for it now, let's continue though — it seems quite important to be held too.

[SomeoneToIgnore]

Related: #11852

Passing those into npm here seems to be the way I'd expected it to be implemented.

[huacnlee]

This PR can be put aside for now.

I'll see if there are any other ideas.

In addition, the method of #11852 may have some different solutions, but I don't know if I will encounter the above problems when executing npm in the end.

[SomeoneToIgnore]

Seems that I've overlooked that it's already supported in that PR: #11852 (comment)

Let's keep on testing and discussing it there.

[huacnlee]

Ref Rust issue rust-lang/rust#114737

[huacnlee]

@SomeoneToIgnore @JunkuiZhang

Today I read the issue of rust-lang/rust#114737 and do a test.

In Rust env_clear will remove all env vars, but on Windows, the SYSTEMROOT is special, maybe some library need this env value.

For example, in Go:

https://pkg.go.dev/os/exec#Cmd

---

And I do a quick testing by add SYSTEMROOT, the proxy is works now. 🎉

[JunkuiZhang]

Great! And I found that when proxy is set to socks5://... npm will not work, but with http://127.0.0.1:... npm actually do work.

I could push the changes to #11852 , but since it's your efforts that makes this happen, I'd like add a Coauthor info which requires your github email. Or you could publish a new PR with the changes.

[huacnlee]

You can just cherry-pick this commit

huacnlee@204ca92