npm-consider icon indicating copy to clipboard operation
npm-consider copied to clipboard

Private registries might returns NaN in Impact's Size percentage

Open anchnk opened this issue 7 years ago • 3 comments

I gave a try to npm-consider today in my company environment.

We use Nexus from Sonatype as a private registry that mirrors npm's one and his hosting company's private modules.

For some reason (which might be a misconfiguration of the Nexus instance on our side), npm-consider can't fetch the package's dependencies size (everything is at 0).

When selecting Impact from the prompt, it gives me the following output:

? What is next? Impact
Packages  5    +4.31%
Size      0 B  +NaN%

After a quick read of the codebase, I can see that this is the line causing it to be NaN (divide by 0):

https://github.com/delfrrr/npm-consider/blob/master/lib/showImpact.js#L102

Do you think a PR that adds a check on currentPackageStats.size and setting everything to 0 or (option b) adding a message saying that something wrong happened while trying to fetch package size is appropriate ?

I am willing to work on this, just need your opinion before.

Regards

anchnk avatar Jul 30 '18 14:07 anchnk

To add a bit more context it seems that the HTTP HEAD method isn't supported by our instance. It returns a 400 Bad Request status code.

fetch does have that response in the promise resolver hence setting the size to null

See https://github.com/delfrrr/npm-consider/blob/master/lib/getPackageDetails.js#L107

anchnk avatar Jul 30 '18 15:07 anchnk

Sorry for delay!

On this problem:

  • if your instance is not supporting HTTP HEAD it's a problem; the tools relies on HTTP HEAD; is it possible for you to enable HEAD on your Nexus server?
  • if you can add proper error reporting for this case, please do; NaN definitely does not looks good

delfrrr avatar Aug 06 '18 05:08 delfrrr

No worries for the delay :)

  1. Absolutely I don't see why HEAD requests are disabled on our instance. I will reach our operational team
  2. I will take some time to submit a little patch that should fix this today.

anchnk avatar Aug 07 '18 08:08 anchnk