fix: no_proxy issue

[Avishagp]

Fix or fully support NO_PROXY setting.
Supporting extended configuration of NO_PROXY patterns like using wildcards …
Addresses: #383

[tomas]

Thanks for the PR. What issue does it fix, specifically?

Also, is it really necessary to add a dependency for this? Seems like a well-crafted regex would do. :)

[nejch]

Thanks for working on this @Avishagp!

@tomas if it helps with the context, I outlined some of the no_proxy behavior that clients would usually observe here:
#382 (comment)

@Avishagp I would maybe also ensure/test that no_proxy=* resolves to false as that is also a pattern.
FWIW I'm just an outsider but having a lib/proxy.js for should_proxy_to and get_env_var makes sense to me. Something similar was done in request it seems https://github.com/request/request/tree/master/lib.

[Avishagp]

@nejch @tomas
I've made the following changes:

• Got rid of the export for should_proxy_to and created proxy.js instead, that exports should_proxy_to and get_env_var
• Added a test for no_proxy=* resolved to false

I'd really appreciate your review to get this issue resolved!

[Avishagp]

Hi @tomas , is there anything else I should do to get this fix approved?

[Avishagp]

Hi @tomas, I've changed everything you asked for, is there anything else I should do to get this merged?

[tomas]

Sorry, busy days lately (newborn daughter). I'll take a look at it later today.

[Avishagp]

Hi @tomas, congrats! hope all is well. I've fixed some broken tests in this PR and believe it's fully ready now

[nejch]

Congrats from my side as well!

Hi @tomas, congrats! hope all is well. I've fixed some broken tests in this PR and believe it's fully ready now

Would also be great to see a release after this gets merged if possible! :)

[Avishagp]

@tomas sorry to ping again knowing you're probably swamped, just want to make sure this doesn't get forgotten :)

[tomas]

👀 :)

[Avishagp]

Hi @tomas sorry to ping again, it's been another month so thought I should make sure this doesn't get forgotten. Thank you and hope all is well with you and your family!

[Avishagp]

Hi @tomas this is getting more urgent for the users, any chance you could approve this? 🙏

[tomas]

Hi @Avishagp and sorry for the extremely long wait.

I just refactored your solution into another branch and PR and just merged it (#416). Initially I wasn't convinced of having a separate proxy.js lying around, but I ended up moving all of the helper functions into a separate utils.js module that not only lets us test should_proxy_to separately but also other utility functions too.

Anyway, thanks again for the PR and the amazing patience. Baby Mara is happy about this issue finally being solved. :)