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

Adding @request/promise-core dependency broke npm behind a proxy. #137

Closed
dsandor opened this issue Jul 18, 2016 · 22 comments
Closed

Adding @request/promise-core dependency broke npm behind a proxy. #137

dsandor opened this issue Jul 18, 2016 · 22 comments

Comments

@dsandor
Copy link

dsandor commented Jul 18, 2016

It looks like since commit 1cc3202 when you added "@request/promise-core" this breaks users using npm through a corporate npm proxy like sinopia or artifactory. I have tried both proxies and both fail with a 404. I think the @ is scoping the dependency to the local proxy which does not resolve it. Is it possible to reference this dependency a different way?

@analog-nico
Copy link
Member

Hi @dsandor , interesting catch!

I tried it with sinopia and verified that the described solutions actually work. In particular, you may use:

npm config set "@request:registry" https://registry.npmjs.org/

Since it is the proxies' fault that they don't work with scoped packages I wouldn't want to fix it on request-promises's side. The proxies hopefully get updated in the near future anyway. Does this work for you?

@dsandor
Copy link
Author

dsandor commented Jul 18, 2016

This does indeed work.

I am a little worried that I will soon have a nightmare situation having to configure any package that uses scoped dependencies this way. But that is more of a proxy problem than an issue with request-promise.

Thanks for your help.

@dsandor dsandor closed this as completed Jul 18, 2016
@analog-nico
Copy link
Member

analog-nico commented Jul 18, 2016

The sinopia project seems to be dead. You may try verdaccio. They fixed it.

@dsandor
Copy link
Author

dsandor commented Jul 18, 2016

I actually use Artifactory which has the same problem and is a commercial product from jfrog. So in a corporate environment where access to npmjs.org is restricted the solution does not actually work. :/ We have to fork the repo and upload it to our internal repo.

@analog-nico analog-nico reopened this Jul 18, 2016
@analog-nico
Copy link
Member

I found a section in the Artifactory docs that explicitly states that @request/promise-core should work:

Artifactory fully supports npm scope packages. The support is transparent to the user and does not require any different usage of the npm client.

Please read that section and verify that you set up your proxy correctly.

It would be great if you post here what you had to do in order to make it working. Thanks!

@todd
Copy link

todd commented Jul 18, 2016

We also use Artifactory and I can confirm that adding the following config to my project's .npmrc fixed this issue:

"@request:registry"="https://registry.npmjs.org"

@dsandor
Copy link
Author

dsandor commented Jul 18, 2016

@todd doesn't that cause your client to fetch the package from npmjs.org?

@analog-nico I am working with my admin to get Artifactory setup to rewrite the scoped modules in such a way that it will properly proxy these dependencies. Will update with info when I get it.

@todd
Copy link

todd commented Jul 18, 2016

@dsandor Yup. We mirror npmjs.org so we can also publish private packages to it. But we don't otherwise restrict access to npmjs prime.

@dsandor
Copy link
Author

dsandor commented Jul 18, 2016

So I am in a different situation where access is only allowed through Artifactory. npmjs.org is blocked.

@todd
Copy link

todd commented Jul 18, 2016

Right, I was simply confirming that the configuration change works for people that are in our situation.

@KpjComp
Copy link

KpjComp commented Jul 21, 2016

Just to let people know, I'm use Nexus Repository Manager as a Proxy & Local NPM, and it works without issues.

@dsandor
Copy link
Author

dsandor commented Jul 21, 2016

We are opening an issue with jfrog about this. It is configured per the docs but it is not working. I will let you know when there is a reported fix to either the software or our config.

@analog-nico
Copy link
Member

@dsandor Thanks for diligently working on this! And I am sorry for the inconvenience.

@crazy4groovy
Copy link

This is also an issue on tonicdev.com; not sure what power I have to fix it there (other than report a missing package)..

@analog-nico
Copy link
Member

You are right @crazy4groovy , we can only report it there. It seems they didn't implement requiring scoped packages yet.

@dsandor
Copy link
Author

dsandor commented Jul 27, 2016

This was resolved at my organization by following these steps (remember we use Artifactory):

  1. Remove trailing slash from Nginx reverse proxy-pass directive so Nginx wouldn’t re-write %2f in requests
  2. Add -Dorg.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true to Artifactory JRE so Tomcat wouldn’t return 400 when it saw %2f
  3. Create a virtual NPM repo in Artifactory and enable dependency rewriting
  4. Whitelist @request/promise-core for dependency resolution

@analog-nico
Copy link
Member

analog-nico commented Jul 28, 2016

Thanks for the info @dsandor !

I am closing this issue since its original points are resolved. However, everyone should feel free to reopen this issue if they experience this issue in other environments / for other NPM proxies.

@lorenwest
Copy link

I work behind an Artifactory proxy, and the IT folks that administer Artifactory are degrees of separation from me. Peer dependencies are fragile due to versioning - what reason are peer dependencies and scoped packages necessary?

@analog-nico
Copy link
Member

Hi @lorenwest , I am sorry to hear that you also have to face an organizational hurdle.

  • @request/promise-core became a scoped package because it is an internal package not intended to be used by anyone directly. If I had known that scoped packages are not yet fully supported even though they were introduced long ago, I probably would have created a public package. Anyhow, since scoped packages are used more and more, you will probably run into the same issue with another package sooner or later. I know how complicated internal affairs in corporations can get but wouldn't it be a good time to upgrade your proxy now? request-promise@4 doesn't have much more to offer compared to request-promise@3. So you have time to take care of it...
  • Defining request as a peer dependency was requested by multiple voices of the community. Actually, to have better control over the request version they want to use. What do you mean by "Peer dependencies are fragile due to versioning"?

@lorenwest
Copy link

Hi @analog-nico,

Thanks for the update. I will look into request-promise@3 to see if this helps during my quest to have our corporate IT update Artifactory version and configs.

As to peer dependencies being fragile, if two packages require a peer dependency of the same module, they're going to get the same version. If their package.json files require incompatible peerDependency versions for a specific package, you're out of luck. Pick one to use and throw the other one away.

I'm always leery of packages that use peerDependencies, because I may have to find another package and change a bunch of code when it randomly clashes with an unrelated package with a different version of the same peerDependency.

My understanding is peerDependencies was a poor man's solution to the real solution that the latest npm uses - it always installs dependencies as peers unless there's a version clash.

If you have to have a peerDependency for a module to work, it's usually because the module being depended on wasn't designed to work with different instances in same vm.

@analog-nico
Copy link
Member

I just published request-promise@4.1.1 which fixes this issue for good.

The number of bug reports in this issue and elsewhere piled up way too high so I renamed @request/promise-core to request-promise-core. Sorry guys, I didn't anticipate that scoped packages produce that much hassle...

@lorenwest
Copy link

Thank you! I don't have any favors to use in our IT department and was
worried I'd be stuck on request-promise@2 forever...

-Loren

On Mon, Aug 8, 2016 at 4:14 AM, Nicolai Kamenzky notifications@github.com
wrote:

I just published request-promise@4.1.1 which fixes this issue for good.

The number of bug reports in this issue and elsewhere piled up way too
high so I renamed @request/promise-core to request-promise-core. Sorry
guys, I didn't anticipate that scoped packages produce that much hassle...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#137 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWzIqaTDHmwtIJJqoPUW5PMEKTtSztEks5qdw-OgaJpZM4JOpGN
.

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

6 participants