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

fix(cli-framework/utils): catch connected socket. #3434

Closed
wants to merge 2 commits into from

Conversation

rdlabo
Copy link
Contributor

@rdlabo rdlabo commented Jul 30, 2018

when don't get sock.on('connect') Event, if (sock.connecting === true) get connected status.
It worked well in my environment.

Fixes #3410

@rdlabo rdlabo requested a review from imhoffd as a code owner July 30, 2018 05:22
@rdlabo rdlabo changed the title fix(cli-framework/utils): catch connected socket. #3410 fix(cli-framework/utils): catch connected socket. Jul 30, 2018
@imhoffd
Copy link
Contributor

imhoffd commented Jul 30, 2018

But sock.connecting is set to true whenever sock.connect() is called (https://nodejs.org/api/net.html#net_socket_connecting). This is the same as not attempting connections at all, isn't it?

@imhoffd
Copy link
Contributor

imhoffd commented Jul 30, 2018

This can lead to the ERR_CONNECTION_REFUSED error in Chrome because the HTTP server isn't accepting connections. I believe @amanitequeen ran into this when testing: #3410 (comment)

@rdlabo
Copy link
Contributor Author

rdlabo commented Jul 31, 2018

@dwieeb thanks for review. and as you pointed out, I confirm sock.connecting is set to true whenever error connect.

I make another pull request. It worked well in my environment(localhost). Please review this pull request.
#3444

@rdlabo rdlabo closed this Jul 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants