Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private registries might returns NaN in Impact's Size percentage #9

Open
anchnk opened this issue Jul 30, 2018 · 3 comments
Open

Private registries might returns NaN in Impact's Size percentage #9

anchnk opened this issue Jul 30, 2018 · 3 comments

Comments

@anchnk
Copy link

anchnk commented Jul 30, 2018

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 anchnk changed the title Private registry might returns NaN in Impact's Size percentage Private registries might returns NaN in Impact's Size percentage Jul 30, 2018
@anchnk
Copy link
Author

anchnk commented Jul 30, 2018

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

@delfrrr
Copy link
Owner

delfrrr commented Aug 6, 2018

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

@anchnk
Copy link
Author

anchnk commented Aug 7, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants