Usage of local host registries fails after upgrade of `npm-registry-fetch` to 16.1.0

[ymeine]

EDIT: I added a comment below, correcting some of my statements. Most of what's written here remains relevant, but a few parts are wrong: the issue is not HTTP vs HTTPS, but rather local host vs remote registries. Updating the whole description would be tedious, so I'm leaving it mostly as is, and just updated a few parts.

---

Hello,

When you fixed the issue #48 I posted, I checked the new version and it worked as expected. But that was only with regards to what the issue was actually about. I hadn’t noticed some changes had broken something else (I discovered this with some delay only because I had a forced break in the middle of my task - hardware problems, brrr).

Context and investigation

So, my whole goal is to run a local, private npm registry, and for that I use Verdaccio. I run the latter on address http://127.0.0.1:4873. What actually matters here is the use of a registry with the HTTP protocol, instead of HTTPSlocal host registry instead of a remote one.

It looks like the package npm-registry-fetch made quite some changes in that area. Here’s what I found out:

• vscode processes HTTPlocal host requests differently from HTTPSremote: vscode\resources\app\node_modules.asar\agent-base\dist\index.js line 77 on version 1.84.1
• it calls the addRequest method on the socket instance: you can see it here; agent-base is a dependency of http-proxy-agent and https-proxy-agent, which are themselves dependencies of vscode
• this method is concretely defined in the custom Agent implementation of @npmcli/agent, and fails when trying to add a header when apparently they were already sent, leading to error ERR_HTTP_HEADERS_SENT
• if you debug, you’ll see the other packages involved in that stack are make-fetch-happen and npm-registry-fetch. Here’s an overview of the stack trace, without actual methods and lines:
  • package-json-upgrade\src\npm.ts
  • npm-registry-fetch\lib\index.js
  • make-fetch-happen\lib\index.js
  • make-fetch-happen\lib\fetch.js
  • make-fetch-happen\lib\remote.js
  • @npmcli\agent\lib\index.js

Differences I can see when upgrading:

• npm-registry-fetch: goes from version 14.0.5 to 16.1.0
• which depends on make-fetch-happen: goes from version ^11.0.0 to ^13.0.0
• make-fetch-happen: version 11.1.1 doesn’t use @npmcli/agent, while 13.0.0 does

Possible solutions

I fully understand that the source of the issue is unlikely to be on your side, but the fact is you are using a package version which leads to that issue. Either the latter has a bug, or there’s a more complex combination of events leading to this issue (some wrong usage/configuration somewhere).

That means there are several options (not exclusive):

• simply downgrade npm-registry-fetch (but I understand we wish to always use the latest, it makes more sense)
• confirm and open an issue on some of the 3rd-party packages involved (possibly downgrading npm-registry-fetch in the meantime, or living with the known limitation)
• optional: add an automated test for HTTPlocal host based registries

The detailed investigation above may not be necessary here, especially if you just choose to downgrade the dependency, but it may be useful for two things:

• helping with checking if the issue comes from a wrong usage, or if it’s a bug on some 3rd-party
• can be used as a base investigation reference if this turns out to be a 3rd party issue

---

Thanks again in advance for your time, and trust me, I know how hard it is to dedicate time to project development, maintenance, etc.

That’s why I don’t want to overwhelm you too much with crazy investigations, and I’m not asking or expecting you to push it further to see if this is a bug or not. If I personally had more resources these days, I would probably push it myself and bother one of the 3rd parties instead. But right now it looks too complex to me to know who’s the culprit here.

[ymeine]

So, in the meantime I thought I would be saved by switching to HTTPS, even though it requires much more setup, harder to automate. Turns out I was wrong: the culprit is not HTTP vs HTTPS, it's rather local host vs the rest. And 127.0.0.1 is a local host.

Now, my initial investigation still points out most of the relevant facts: versions upgrades, 3rd party packages involved, failing piece of code, etc. But the interpretation is slightly different.

What's for sure: the issue is completely related to the behavior of custom Agent implementations. Now, what's weird is what I experimented in debug mode:

• make sure an HTTPS request is spawned: https://127.0.0.1:4873/.../...
• at some point in the execution flow, override some internal state about whether the URL is local host or not: make it as if it's not
  • the related piece of code overrides the options.agent value, and uses different options based on whether the target is a local host or not
• the code still goes inside the condition checking if the agent is an HTTP one, even though the request clearly targets an HTTPS URL
• however, the request works and I can see the latest version of my package appear in my editor

After checking a bit more, I realized that forcing isLocalhost to false at that time actually picked a different agent implementation (as I expected), and it turns out to be the Node.js' builtin HTTP agent (_http_agent).

That means multiple things:

• a base HTTP agent can be used even if the target URL is HTTPS
• the addRequest method implementation in the custom @npmcli/agent's agent is probably wrong to try writing some headers at that stage, but I can't be sure it's always the case (maybe in some situations it could succeed in writing that header and it would be relevant)

---

As a conclusion, it's even more complex that I imagined. The downside is that I can't use HTTPS as a workaround, since the problem is rather than it's a local host, and that I cannot change it.

[pgsandstrom]

Thank you for the bug report. That is very interesting. Could you clone this repo, downgrade npm-registry-fetch to 14, and confirm that everything works? I would be willing to downgrade if it fixes your issue.

[ymeine]

I already tried it. It should have been the first thing I did, but I instead I went debugging right away, to clarify the issue.

At some point, I tested with the version you previously set, 14.0.5, and it worked. Since I have a use case set up on my side, let me try latest 14.x and even 15.x while I'm at it, so then you can pick the one you judge more relevant.

[ymeine]

• 14.0.5 (was already the latest version for 14.x): works
• 15.0.0 (the only version for 15.x): works
• 16.0.0 and 16.1.0: fails

Now, would 15.0.0 bring other issues, since it's a major version change, I don't know. 14.0.5 could be a safer choice, but the decision is up to you (considering you also updated other packages anyways and it seems fine).

[pgsandstrom]

I downgraded npm-registry-fetch to 14.0.5. Let me know if you run into any other problems.

[ymeine]

I just checked now, it all works fine! Thanks for the quick update