`npm --version` takes 3 minutes to complete when run in child process and behind corporate proxy

cli
priority:medium
triaged

(Andreas) #1

What I Wanted to Do

Execute npm -v through child_process.spawn behind an enterprise proxy.
update-notifier seems to block/delay that request for 3 minutes cause it does not support proxies.
Unfortunately update-notifier is part of npm@4.4.0 :cry:

This bug was reported years ago and recently by myself, thus I want to reopen it and create awareness here:

They contain all the relevant data to address this issue.

What Happened Instead

More than 3 minutes delay for executing via child_process.spawn() behind an enterprise proxy:

  • npm whoami
  • npm owner ls
  • npm -v
  • maybe more npm commands are affected by this bug :sob:

Reproduction Steps

All scripts, version, platforms to reproduce this bug are listed in aforementionend github issues

Details

Platform Info

Windows 7 and 10

$ npm --versions
<!-- paste output here -->
$ node -p process.platform
<!-- paste output here -->

Reported by multiple setups:
1)

  • npm -v prints: 5.6.0
  • node -v prints: 8.11.1
  • npm -v prints: 6.1.0
  • node -v prints: v10.5.0

(Kat Marchán) #2

yikes.

Ugh, we should just hack this together ourselves instead of relying on update-notifier. It’s probably less effort at this point, 'cause we keep having to bloody fix it :unamused:

As in, we should probably drop update-notifier altogether.


(Brian Olore) #3

If we were to update the existing update-notifier to allow a timeout, we’d have to change a few packages:

The package update-notifier calls latest-version (with no options)
return latestVersion()(this.packageName).then(latestVersion => {

latest-version calls package-json (no options are passed from update-notifier)
module.exports = (name, options) => packageJson(name.toLowerCase(), options).then(data => data.version);

package-json calls got (but no timeout option)
return got(pkgUrl, {json: true, headers}) )

got actually makes the http call got can take a timeout - got(string, {timeout: 10000}) - See: https://github.com/sindresorhus/got#timeout


I feel like this is doable, replacing update-notifier sounds a little more daunting to me. Though, it might be easier to get got to support proxies rather than passing a timeout all the way down.


(Brian Olore) #4

heh, or maybe not as easy as I thought https://github.com/sindresorhus/got/issues/560


(Kat Marchán) #5

It’s really not a lot of work to just write our own. And we’d have full config compatibility. Specially with my npm-registry-fetch port. I think that’s my preferred solution to this. I’m kinda tired dealing with complaints about how update-notifier does things when the entire point was to not have to do that work ourselves at all.


(Brian Olore) #6

Ok, so if I were to take a shot at this, I’d remove update-notifier and use https://github.com/npm/npm-registry-fetch ?


(Kat Marchán) #7

probably pacote.manifest('npm@latest', pacoteOpts()). Look at other places in the code that use pacote. And then check the local npm’s package.json. And then throw in the various conditionals + format the output. Probably toss it in its own file in lib/, so you can just call it without all the conditional logic that’s been bloating its current file.


(Brian Olore) #8

@zkat I haven’t written any tests for this, but what do you think?

There are references to ‘update-notifier’ in several places, including config, so I was thinking keep the name/config switch for the time being. (It was introduced into config in 6.2)


(Kat Marchán) #9

I’ll take a look on Monday ;)


(Andreas) #10

Thanks for your quick feedback.

Indeed got does not plan to add proxy support by checking environment variables :frowning:

The other issue gives following workarounds to disable update-notifier meanwhile:

See https://github.com/yeoman/update-notifier#user-settings

  • Set environment variable NO_UPDATE_NOTIFIER=1
  • Use --no-update-notifier flag
  • Set "optOut": true in ~/.config/configstore/update-notifier-[your-module-name].json

BTW I agree with @zkat to remove update-notifier (and it’s dependencies) and therefor get control back of whats going on under the hood.


(Kat Marchán) #11

Can you please make a PR for this so we can start reviewing it? Off the top of my head, this still needs a few things:

  1. A timeout to prevent it from staying up for too long – I think spawning off a standalone process for this is overkill, but maybe @iarna thinks it’s worth it? fwiw, update-notifier does use a separate, detached process, so it’s probably fine either way.
  2. Don’t make this block anything. Fire it off and let it to its job on its own, and don’t worry about it completing.
  3. Ignore all errors. If it doesn’t work, it shouldn’t say anything. Maybe write that it got an error to some file in ~/.npm.
  4. This needs some kind of mechanism for limiting how often it does these checks. update-notifier does this by using configstore to write a lastUpdateCheck date and then compares it to opts.updateCheckInterval.

(Rebecca Turner) #12

A detached process is mandatory to not hang in certain network conditions. We already do this for the metrics agent.


(Brian Olore) #13

:+1: https://github.com/npm/cli/pull/61


(Adrian Wilkins) #14

Found my way here eventually because I’m also having an issue with update-notifier.

Don’t know why, but multiple update checks are spawning on my EC2 instance … which is in a highly locked down subnet, it has no hope of completing any network connection to an update server.

These processes are eating as much CPU time as they can get, which on a burst-optimized instance rapidly eats all the way through the available CPU credit and makes the server grind to a halt, making it difficult to even shell into it and diagnose the problem.

The server is running an app using pm2 - it seems to be more pronounced when the app enters a fail/restart loop but still most of the CPU time is being eaten by update-notifier. Not sure if pm2 is calling it or what. Will be poking into it some more.

[centos@prsqat2app2-n-0733f74f161d8d48f ~]$ ps -ef | grep npm
centos   28960     1  2 10:51 ?        00:00:14 /usr/bin/node /usr/lib/node_modules/npm/node_modules/update-notifier/check.js {"pkg":{"name":"npm","version":"5.3.0"}}
centos   29109     1  6 10:59 ?        00:00:13 /usr/bin/node /usr/lib/node_modules/npm/node_modules/update-notifier/check.js {"pkg":{"name":"npm","version":"5.3.0"}}

Ok, it’s nothing to do with pm2, this is an instance of node that’s a direct child of systemd, so I’m guessing it’s a scheduled task installed with the default packages (this is 8.4.0, yes, I know).


(Brian Olore) #15

@awilkins ouch! Thanks for sharing.

You probably already found this, but until we get this squared away you can skip the update check with the --no-update-notifier flag


(Andreas) #16

Just wanted to share that how and who should handle http proxy environment variables is a kinda heated discussion and still not settled yet (for years):

  • nodejs/node/issues/8381
  • nodejs/node/issues/15620

@awilkins
You have 3 ways to disable update-notifier, they are documented at their Readme on github in #user-settings.


(Andreas) #17

@zkat
@olore
I just got notified, that my post #1 and #10 of this issue are being marked as spam and thus hidden.
Well I’m a bit confused why?
#1 is a description of a real bug and I took the time to investigate it and communicate it officially.
#10 Is a list of possible workarounds which I used successfully to avoid this bug

May I ask you whats wrong with them?
Personally I’m convinced that other devs, sysadmins, etc. can profit allot from the workarounds and all my findings.

As I understood it my bug ticket was accepted at your side, so I’m a bit confused why this censonship happened now :confused:


(Brian Olore) #18

Hey @AndyOGo. It is marked as “flagged by the community and is temporarily hidden”. I can click to view it.

I don’t have access to see who or why it was flagged. It might have been a mistake or a troll. Those messages certainly should not have been flagged.

@zkat can you take a look into this?


(Andreas) #19

@olore Thanks for your quick answer.
I contacted your support and they fixed super quick:)


(Rebecca Turner) #20

Yeah, sorrry about that. Hopefully I have things configured properly to not catch folks inappropriately like that again. (It didn’t like how many links you had to one website. But that website was github, which is gonna be common. I whitelisted links to github and npm itself.)