Don't patch core methods #35
Comments
|
I have the same problem. It breaks my app even if my app does not directly use |
Instead of patching over `https.request()` and `https.get()` manually to add the `secureEndpoint` boolean, utilize the stack trace of the current call logic to determine whether or not the `https.js` core module is invoking `new ClientRequest()`. In theory, this is still fragile logic, but should be more future-proof and cleaner than patching over a core module, which is never really desirable. Fixes #29. Closes #30. Closes #35.
|
Thanks for the heads up. Please see #36 for my proposed solution. |
|
@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 ? |
|
@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. |
* Remove `https` core module patching logic Instead of patching over `https.request()` and `https.get()` manually to add the `secureEndpoint` boolean, utilize the stack trace of the current call logic to determine whether or not the `https.js` core module is invoking `new ClientRequest()`. In theory, this is still fragile logic, but should be more future-proof and cleaner than patching over a core module, which is never really desirable. Fixes #29. Closes #30. Closes #35. * Remove Node 10-specific test case * Prettier
|
@TooTallNate, thanks for the fix. Do you know when you'll be able to publish it to npm? |
|
I agree with @dguo , @TooTallNate can you do a minor version release, so every package using |
|
I think |
|
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.
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. |
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-agentdirectly, but the mere fact it's included byweb-push(which we do use) breaks our app.The text was updated successfully, but these errors were encountered: