TooTallNate/node-agent-base

Don't patch core methods

Closed this issue · 8 comments

xPaw commented

It's just causing problems in unrelated code even when your dependency is not used.

sindresorhus/got#951
sindresorhus/got#941
TooTallNate/proxy-agents#88

I'm not using https-proxy-agent directly, but the mere fact it's included by web-push (which we do use) breaks our app.

dashmug commented

I have the same problem. It breaks my app even if my app does not directly use node-https-proxy-agent.

  • 👍1
TooTallNate commented

Thanks for the heads up. Please see #36 for my proposed solution.

ouraios commented

@TooTallNate When do you think you'll be able to fix this issue ? I saw that you had a working pull request, what's missing ?

TooTallNate commented

@ouraios Nothing is missing, I was just leaving the PR open for a bit to give anyone a chance to chime in if there is a problem with the fix.

dguo commented

@TooTallNate, thanks for the fix. Do you know when you'll be able to publish it to npm?

ouraios commented

I agree with @dguo , @TooTallNate can you do a minor version release, so every package using agent-base will automatically upgrade to this fixed version ?

  • 👍5
fengkx commented

I think socks-proxy-agent need to upgrade the agent-base dependency too.

  • 👍2
hildoer commented

For those of plagued by this in a rats nest of upstream dependencies that include this library with the existence of the core patch, I have simply done this in a postinstall hook. Undoubtedly it breaks something in the upstream dependencies that are dependent on a polluted HTTPS core module, but my code isn't executing that code.

find node_modules/ | grep 'agent-base/patch-core' | xargs truncate -s 0

Basically, after npm install this clears out every instance of agent-base/patch-core by any upstream dependency.

It is a hack to fix a hack, but it has us working again as all of our automated tests are passing again without losing the functionality of the upstream dependencies we actually use.

  • 👍1