Upgrade 'https-proxy-agent' dependency to v5

[denver-HJS]

Hi all,

I'm attempting to use the latest got HTTP client library in my project, and it appears that there's some interference between the https-proxy-agent library, which the newrelic node APM agent depends on, and the native node APIs that got makes use of.

Due to this, I'm getting this error for any HTTPS request I make:
GotError: connect ECONNREFUSED 127.0.0.1:443

Please refer to these issue threads for more detail:
observed problem: sindresorhus/got#951

the solution:

This should be fixed now in node-agent-base v5: https://github.com/TooTallNate/node-agent-base/releases/tag/5.0.0

(The associated PR: TooTallNate/node-agent-base#36)

[denver-HJS]

FWIW this looks to be related or even the same issue as #316

[astormnewrelic]

@denver-HJS Thanks for letting us know -- bug reports are always appreciated. 👍

1. Do you have some sample code from one of your got based requests that fails?

2. Do you know for certain that this poor interaction between the agent and got is due to https-proxy-agent? Or is this speculation based on seeing similar problems in their repository?

Asking the second question because we're working on #316 right now. Once we release that fix it might solve your problem -- but if there's a specific problem you know about with https-proxy-agent that separate we've love to know the details. Otherwise we'll likely get the fix for #316 in place first that then revisit this one that fix is in place.

[denver-HJS]

Thanks for the reply @astormnewrelic.

1. I will try to get a simple repository put up that demonstrates the behavior. However, I'm not sure how to do that without exposing my company's NewRelic app key. Are there any suggestions you might have, since I imagine that's commonplace?
2. Admittedly I don't have much evidence other than following along with the issue threads linked above. This particular statement gets to the crux of the matter:

https-proxy-agent overrides http.request. Their function takes only two arguments instead of three. Got uses Node.js 10 API, which is three arguments.

My interpretation is that upgrading https-proxy-agent will pull in the latest node-agent-base version it depends on, so I think the fix for #316 and my issue are one and the same.

[astormnewrelic]

However, I'm not sure how to do that without exposing my company's NewRelic app key. Are there any suggestions you might have, since I imagine that's commonplace?

Using an environment variable in place of your host key and then accessing its value via process.env is the best way to do that how we usually do that. This will let another user use your sample by setting their own key. Also, the node agent already knows how to read the NEW_RELIC_LICENSE_KEY env variable. Does that get you where you need to be?

[denver-HJS]

@astormnewrelic that did get me where I need to be, thanks for the help.

I wasn't able to reproduce the issue when isolating down to only [email protected] + [email protected]

Oddly enough, I can still get back to the GotError: connect ECONNREFUSED 127.0.0.1:443 error in my real project, but I'm happy to close this issue and see if the fix for #316 also patches up what I'm seeing.

for reference here's the repo with the isolated packages: https://github.com/denver-HJS/newrelic-issue-reproducing/tree/NR-agent-328

[astormnewrelic]

@denver-HJS Presuming you meant [email protected] and not [email protected]? If not, um, let us know where you got a 4.6.2 release, because that definitely doesn't exist in our timeline. ;)

It sounds like things are working for you again and you can make web requests with got. That's great. we probably wouldn't have gotten this fix in without yoru report, so thank you for bringing it to our attention.

Per your comments we'll to close this issue out. Don't hesitate to reopen or open a new one if you run into trouble (usual caveats about official support channels being the best way to get official support, etc. etc.)

[denver-HJS]

Yes good catch, I fat fingered the version numbers there 🙂 I've updated the comment, so it won't confuse others.

Thanks again for the quick replies!