Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

https urls get "ECONNREFUSED 127.0.0.1:443" (https-proxy-agent breaks got without even using it) #951

Closed
2 tasks done
xPaw opened this issue Dec 1, 2019 · 13 comments
Closed
2 tasks done

Comments

@xPaw
Copy link

xPaw commented Dec 1, 2019

Describe the bug

  • Node.js version: v12.13.0
  • OS & version: Windows 10 1909
  • Got v10

Actual behavior

(node:17240) UnhandledPromiseRejectionWarning: GotError: connect ECONNREFUSED 127.0.0.1:443
at onError (D:\GitHub\thelounge\node_modules\got\dist\source\request-as-event-emitter.js:124:29)
at handleRequest (D:\GitHub\thelounge\node_modules\got\dist\source\request-as-event-emitter.js:153:17)
at processTicksAndRejections (internal/process/task_queues.js:93:5)                                                                                        at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1128:14)                                     

For some reason it's resolving any https url to 127.0.0.1. Even changing the port in the uri still tries to connect to 443. http appears to work fine.

[email protected] works fine.

Code to reproduce

require("https-proxy-agent");
const got = require("got");

got("https://google.com:1337", { // or just "https://google.com"
	timeout: 5000,
})
	.on("response", (a) => {
		console.log(a);
	})
	.on("error", (e) => {
		console.log(e);
	});

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@xPaw
Copy link
Author

xPaw commented Dec 1, 2019

Tried the alpha/beta versions.

10.0.0-alpha.3.2 works, 10.0.0-beta.1 tries to connect to 127.0.0.1.

EDIT:
Oddly enough this isn't happening in a stand alone file. Only happens within our app.

I was adding the example code above after requires in this file: https://github.com/thelounge/thelounge/blob/master/src/plugins/irc-events/link.js

EDIT2: Looks like adding require("web-push") breaks it... And more specifically require("https-proxy-agent"); which is used by web-push.

What's silly is that we don't use https-proxy-agent on got, so I'm not sure why it breaks.

@xPaw xPaw changed the title https urls get "ECONNREFUSED 127.0.0.1:443" https urls get "ECONNREFUSED 127.0.0.1:443" (https-proxy-agent breaks got without even using it) Dec 1, 2019
@sindresorhus
Copy link
Owner

See #876 (comment). This is a not a problem with Got. Please open an issue on https-proxy-agent.

@xPaw
Copy link
Author

xPaw commented Dec 1, 2019

Please note that I'm not passing https-proxy-agent into got but it still breaks.

EDIT: Dug deeper into it, specifically this crap breaks it: https://github.com/TooTallNate/node-agent-base/blob/1aaf316cc3f7e3bad0616d42b483da61416406fd/patch-core.js

@szmarczak
Copy link
Collaborator

szmarczak commented Dec 1, 2019

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.

@dashmug
Copy link

dashmug commented Dec 2, 2019

I have the same problem. I'm using got in a project using serverless.

My app does not use got directly but serverless uses it. 😢

@sdomagala
Copy link

Hi @szmarczak,
I don't think this ticket should be closed, every project with serverless is going to break after this release.
For now I've just downgraded to got@9 but it would be great to upgrade.

Cheers

@liteCarma
Copy link

Which proxy agent will work? I couldn't find it, had to go back to version 9

@dashmug
Copy link

dashmug commented Dec 10, 2019

@sdomagala The problem is caused by something not related to got. If the cause is in serverless, you should file a bug issue there.

@sdomagala
Copy link

sdomagala commented Dec 10, 2019

I wonder why got picks up dependencies from other packages, seems like an issue with the package itself. I understand that breaking change can be something different in API but dependencies of other packages interfering with this one look like a regression to me.

@dashmug
Copy link

dashmug commented Dec 10, 2019

@sdomagala It's because node-agent-base is patching a core NodeJS method. There's nothing got could have done to prevent this.

@sdomagala
Copy link

I assume you're referring to this patch function mentioned couple comments up: https://github.com/TooTallNate/node-agent-base/blob/1aaf316cc3f7e3bad0616d42b483da61416406fd/patch-core.js . You could just check if patchMarker is set and use given API accordingly. So, you could fix this regression in got repo or report it somewhere else, that's up to you, but there is a way.

@szmarczak
Copy link
Collaborator

@sdomagala Node.js core is not a dependency. Patching it is not a good idea. @dashmug is right. There's a way to fix that issue, but it's not Got fault. Got 10 is using the Node.js 10 API (Node 8 is being dropped).

that's up to you

That's up to you, as Got does nothing wrong at this point.

Repository owner locked as resolved and limited conversation to collaborators Dec 10, 2019
@sindresorhus
Copy link
Owner

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

yrambler2001 added a commit to UpLab/mailjet-apiv3-nodejs that referenced this issue Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants