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

Agent: handle socket errors while connecting to the proxy #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Agent: handle socket errors while connecting to the proxy #11

wants to merge 1 commit into from

Conversation

someone--else
Copy link

Passing client socket to the request on error allows it to be handled (vs aborting the process because of the unhandled exception)

…ed while connecting to the proxy are handled properly
@alchen
Copy link

alchen commented Feb 3, 2016

I was just checking out socksv5 and met with an uncaught exception, and this has helped. Thank you.

It'd be great If this can be merged, or someone can provide a pointer to another solution.

@wsz87
Copy link

wsz87 commented Mar 18, 2016

I agree It should be merged. It works for me too.

@mscdex
Copy link
Owner

mscdex commented Mar 20, 2016

I'm not sure this is the right approach. This PR seems to be passing a socket as "valid" whenever an error occurs. Is the point to allow the Agent implementation to be able to clean up the socket immediately after the error occurs? It seems like there may still be the possibility of the Agent to pass the socket out as valid for a short period of time?

Also while on the topic, I'm thinking of ditching the copy of node's internal http.Agent code for the async agent.createConnection() functionality which will arrive in node v6.0.0 and will simplify this code greatly.

@stroncium
Copy link

seems to be related #19

@Armalon
Copy link

Armalon commented Sep 28, 2016

Not sure if it's the right code to be merged with master, but it worked for me just fine. I was glad to be able to catch errors instead of application being dead.
Any ideas how to catch "Authentication failed" error using another method?

@GabrielLomba
Copy link

GabrielLomba commented Oct 9, 2019

I also encountered this error and made a change in the library that reemitted the error in the request object:

client.on('error', err=>req.emit('error', err));

This way, it allows Authentication Failed and other types of errors to be handled outside of the library.

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.

7 participants