search

TooTallNate / proxy-agents

Node.js HTTP Proxy Agents Monorepo
https://proxy-agents.n8.io
872 stars 229 forks source link

Proxy `auth` option no longer works #167

Closed jportner closed 1 year ago

jportner commented 1 year ago

I can't find release notes for https-proxy-agent@6.0.0, but it appears that introduced a breaking change where the auth option can no longer be used.

Before:

https://github.com/TooTallNate/proxy-agents/blob/d0d80cc0482f20495aa8595f802e1a9f3b1b3409/src/agent.ts#L105-L110

After:

https://github.com/TooTallNate/proxy-agents/blob/82243326309475be720491ddc7a03401c05a759f/packages/http-proxy-agent/src/index.ts#L92-L102

It's a major version upgrade, so breaking changes are not unexpected. However, I'm not sure if this was an intentional breaking change or not.

If it was intentional, this might be a good thing to point out in release notes and/or an upgrade guide. If it wasn't intentional, I think backwards compatibility could easily be preserved here by falling back to using the auth option if both username and password are not defined.

(Also: thank you for maintaining such an awesome group of packages! Very excited to see keep-alive support has been added!)

EDIT: I found that the https-proxy-agent directory contains a changelog for the 6.0.0 release, and this isn't mentioned.

So, maybe this should just be added to the changelog?

It might also be good to add a top-level changelog file with links to the individual changelogs, so it's easier to discover when you're navigating here from the npm website.

I'd be happy to submit a PR if you think this is a good idea.

shrkbait-hpe commented 1 year ago

I just stumbled across this too. Note both https://nodejs.org/api/url.html#urlurltohttpoptionsurl and the original url.parse create 'auth' and do not split into username and password. An else if would be great here.

TooTallNate commented 1 year ago

Ya, auth support was lost because all the packages switched over to using the URL constructor instead of url.parse(). I'd prefer not to have multiple ways of doing it.

I also don't see a need to use url.urlToHttpOptions(). http.request()/https.request() support passing in a URL instance directly.

Happy to accept PRs for links to the changelogs and/or additions to the changelogs explaining breaking changes.

shrkbait-hpe commented 1 year ago

I missed that v6 changed the constructor to support a full URL and then the extra options separately. With that I stand corrected and it all just works. Thanks for all your efforts.